This is an archive of the discontinued LLVM Phabricator instance.

[CSSPGO] Introducing dangling pseudo probes.
ClosedPublic

Authored by hoy on Feb 3 2021, 10:34 AM.

Details

Summary

Dangling probes are the probes associated to an empty block. This usually happens when all real instructions are optimized away from the block. There is a problem with dangling probes during the offline counts processing. The way the sample profiler works is that samples collected on the first physical instruction following a probe will be counted towards the probe. This logically equals to treating the instruction next to a probe as if it is from the same block of the probe. In the dangling probe case, the real instruction following a dangling probe actually starts a new block, and samples collected on the new block may cause issues when counted towards the empty block.

To mitigate this issue, we first try to move around a dangling probe inside its owning block. If there are still native instructions preceding the probe in the same block, we can then use them as a place holder to collect samples for the probe. A pass is added to walk each block backwards looking for probes not followed by any real instruction and moving them before the first real instruction. This is done right before the object emission.

If we are unlucky to find such in-block preceding instructions for a probe, the solution we are taking is to tag such probe as dangling so that the samples reported for them will not be trusted by the compiler. We leave it up to the counts inference algorithm to get such probes a reasonable count. The number UINT64_MAX is used to mark sample count as collected for a dangling probe.

Diff Detail

Event Timeline

hoy created this revision.Feb 3 2021, 10:34 AM
hoy requested review of this revision.Feb 3 2021, 10:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2021, 10:34 AM
hoy edited the summary of this revision. (Show Details)Feb 3 2021, 10:41 AM
hoy updated this revision to Diff 321167.Feb 3 2021, 11:02 AM

Updating D95962: [CSSPGO] Introducing dangling pseudo probes.

hoy updated this revision to Diff 322421.Feb 9 2021, 9:16 AM

Rebasing.

hoy updated this revision to Diff 322836.Feb 10 2021, 3:08 PM

Adding a test for SimplifyCFG case.

hoy updated this revision to Diff 323419.Feb 12 2021, 11:29 AM

Rebasing.

hoy added a comment.Feb 12 2021, 11:41 AM

@wmi Would be great if you can take a look at this change. This is the last change we are making into LLVM-12, for which the cutoff date is coming soon. Thanks!

hoy updated this revision to Diff 324377.Feb 17 2021, 11:49 AM

Rebasing.

wmi added a comment.Feb 24 2021, 4:31 PM

Sorry I missed this patch. I will look at it soon.

wmi added a comment.Feb 24 2021, 11:32 PM

Thanks for the detailed description. That really helps for better understanding the patch.

The patch includes multiple essential changes. Is it possible to split them? Like pseudo move, unblocking optimizations, and deduplicating are relatively standalone changes and it is better to make their impact isolated from each other.

hoy added a comment.Feb 25 2021, 9:03 AM
In D95962#2586842, @wmi wrote:

Thanks for the detailed description. That really helps for better understanding the patch.

The patch includes multiple essential changes. Is it possible to split them? Like pseudo move, unblocking optimizations, and deduplicating are relatively standalone changes and it is better to make their impact isolated from each other.

Thanks for the suggestion. Split into three patches.

hoy updated this revision to Diff 326415.Feb 25 2021, 9:04 AM

Splitting patches.

hoy updated this revision to Diff 327322.Mar 1 2021, 4:41 PM
hoy edited the summary of this revision. (Show Details)

Fixing an issue when merging dangling probes.

wmi added inline comments.Mar 2 2021, 8:08 PM
llvm/include/llvm/CodeGen/MachineInstr.h
1805

Add an assertion message.

1810

Add an assertion message.

1812–1814

Nit: AttrOperand.setImm(AttrOperand.getImm() | (uint32_t)Attr);

llvm/lib/CodeGen/PseudoProbeInserter.cpp
87

Can it stop when MII equals to FirstInstr? Otherwise will the moved pseudo probe be removed and inserted before FirstInstr again?

110

It set all the pseudo probes in MBB to dangling. For DirectCall/IndirectCall types of pseudo probe, they are used to represent inline stack. Do they need to be set to dangling?

hoy updated this revision to Diff 327677.Mar 2 2021, 10:39 PM
hoy marked 2 inline comments as done.

Addressing Wei's feedbacks.

llvm/include/llvm/CodeGen/MachineInstr.h
1812–1814

Sounds good.

llvm/lib/CodeGen/PseudoProbeInserter.cpp
87

Yes, it can stop. Note that FirstInstr is not a probe but a real instruction. When MII equals to it, the break below will be run.

110

DirectCalls/IndirectCalls are real call instructions and can be sampled correctly. If there are calls in the block, other types of probes will not be considered dangling.

wmi added inline comments.Mar 2 2021, 10:56 PM
llvm/lib/CodeGen/PseudoProbeInserter.cpp
87

Ah, I see.

110

If the call is inlined, will the DirectCall be left in the block?

hoy added inline comments.Mar 2 2021, 11:00 PM
llvm/lib/CodeGen/PseudoProbeInserter.cpp
110

If the call is inlined, the direct call is gone. And the inline stack will be attached to inlined probes in form of !dbg. If the inlined probes end up in a block with no other real instructions, they will be tagged dangling.

wmi accepted this revision.Mar 2 2021, 11:12 PM

LGTM.

llvm/lib/CodeGen/PseudoProbeInserter.cpp
110

Ok, thanks.

This revision is now accepted and ready to land.Mar 2 2021, 11:12 PM
This revision was automatically updated to reflect the committed changes.