[llvm] [PseudoProbe] Extend to skip instrumenting probe into the dests of invoke (PR #79919)

via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 12 19:39:26 PST 2024


================
@@ -173,21 +173,98 @@ SampleProfileProber::SampleProfileProber(Function &Func,
   BlockProbeIds.clear();
   CallProbeIds.clear();
   LastProbeId = (uint32_t)PseudoProbeReservedId::Last;
-  computeProbeIdForBlocks();
-  computeProbeIdForCallsites();
-  computeCFGHash();
+
+  DenseSet<BasicBlock *> BlocksToIgnoreProbe;
+  DenseSet<BasicBlock *> BlocksToIgnoreCall;
+  computeBlocksToIgnore(BlocksToIgnoreProbe, BlocksToIgnoreCall);
+
+  computeProbeIdForBlocks(BlocksToIgnoreProbe);
+  computeProbeIdForCallsites(BlocksToIgnoreCall);
+  computeCFGHash(BlocksToIgnoreProbe);
+}
+
+// Two purposes to compute the blocks to ignore:
+// 1. Reduce the IR size.
+// 2. Make the instrumentation(checksum mismatch) stable. e.g. the frondend may
+// generate unstable IR while optimizing nounwind attribute, some versions are
+// optimized with the call-to-invoke conversion, while other versions do not.
+// This discrepancy in probe ID could cause profile mismatching issues.
+// Note that those ignored blocks are either cold blocks or new split blocks
+// whose original blocks are instrumented, so it shouldn't degrade the profile
+// quailty.
+void SampleProfileProber::computeBlocksToIgnore(
+    DenseSet<BasicBlock *> &BlocksToIgnoreProbe,
+    DenseSet<BasicBlock *> &BlocksToIgnoreCall) {
+  // Ignore the cold EH blocks.
+  computeEHOnlyBlocks(*F, BlocksToIgnoreCall);
+  findUnreachableBlocks(BlocksToIgnoreCall);
+
+  BlocksToIgnoreProbe.insert(BlocksToIgnoreCall.begin(),
+                             BlocksToIgnoreCall.end());
+  findNewSplitBlocks(BlocksToIgnoreProbe);
+}
+
+void SampleProfileProber::findUnreachableBlocks(
+    DenseSet<BasicBlock *> &BlocksToIgnore) {
+  for (auto &BB : *F) {
+    if (&BB != &F->getEntryBlock() && pred_size(&BB) == 0)
+      BlocksToIgnore.insert(&BB);
+  }
+}
+
+// Basic block can be split into multiple blocks, e.g. due to the
+// call-to-invoke. If they are hotness-wise equal, we can optimize to only
+// instrument the leading block, ignore the other new split blocks.
----------------
WenleiHe wrote:

We need to be intentional as to what we want to handle, and also make it very clear in the comment. 

>  If they are hotness-wise equal, we can optimize to only instrument the leading block, ignore the other new split blocks. 

There are a lot more blocks that share the same hotness (control-equivalent blocks), and we are not skipping all these blocks. 

If we want to ignore all blocks that is mergeable into its predecessor, doing it this way feels a bit hacky. 

If we want to target very specific cases around EH, make it narrow and explicit, then it's probably okay.

https://github.com/llvm/llvm-project/pull/79919


More information about the llvm-commits mailing list