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

via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 30 13:16:12 PST 2024


================
@@ -173,21 +173,63 @@ SampleProfileProber::SampleProfileProber(Function &Func,
   BlockProbeIds.clear();
   CallProbeIds.clear();
   LastProbeId = (uint32_t)PseudoProbeReservedId::Last;
-  computeProbeIdForBlocks();
+
+  DenseSet<BasicBlock *> BlocksToIgnore;
+  // Ignore the cold EH blocks. This will reduce IR size as
+  // well as the binary size while retaining the profile quality.
+  computeEHOnlyBlocks(*F, BlocksToIgnore);
+  // While optimizing nounwind attribute, the frondend may generate unstable IR,
+  // e.g. 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. To make the probe ID consistent, we can ignore all the
+  // EH flows. Specifically, we can ignore the normal dest block which
+  // originating from the same block as the call/invoke block and the unwind
+  // dest block(computed in computeEHOnlyBlocks), which is a cold block. It
+  // doesn't affect the profile quality.
+  findInvokeNormalDests(BlocksToIgnore);
+
+  computeProbeIdForBlocks(BlocksToIgnore);
   computeProbeIdForCallsites();
-  computeCFGHash();
+  computeCFGHash(BlocksToIgnore);
+}
+
+void SampleProfileProber::findInvokeNormalDests(
+    DenseSet<BasicBlock *> &InvokeNormalDests) {
+  for (auto &BB : *F) {
+    auto *TI = BB.getTerminator();
+    if (auto *II = dyn_cast<InvokeInst>(TI))
+      InvokeNormalDests.insert(II->getNormalDest());
+  }
 }
 
 // Compute Hash value for the CFG: the lower 32 bits are CRC32 of the index
 // value of each BB in the CFG. The higher 32 bits record the number of edges
 // preceded by the number of indirect calls.
 // This is derived from FuncPGOInstrumentation<Edge, BBInfo>::computeCFGHash().
-void SampleProfileProber::computeCFGHash() {
+void SampleProfileProber::computeCFGHash(
+    const DenseSet<BasicBlock *> &BlocksToIgnore) {
   std::vector<uint8_t> Indexes;
   JamCRC JC;
   for (auto &BB : *F) {
-    for (BasicBlock *Succ : successors(&BB)) {
+    if (BlocksToIgnore.contains(&BB))
+      continue;
+    // To keep the CFG Hash consistent before and after the call-to-invoke
+    // conversion, we need to compute the hash using the original call BB's
+    // successors for the invoke BB. As the current invoke BB's
+    // successors(normal dest and unwind dest) are ignored, we keep searching to
+    // find the leaf normal dest, the leaf's successors are the original call's
+    // successors.
+    auto *BBPtr = &BB;
+    auto *TI = BBPtr->getTerminator();
+    while (auto *II = dyn_cast<InvokeInst>(TI)) {
+      BBPtr = II->getNormalDest();
+      TI = BBPtr->getTerminator();
+    }
+
+    for (BasicBlock *Succ : successors(BBPtr)) {
       auto Index = getBlockId(Succ);
+      assert(Index &&
+             "Ignored block(zero ID) should not be used for hash computation");
----------------
WenleiHe wrote:

nit: the assertion message isn't very accurate -- if it should not be used, there could be an additional check, so it doesn't explain why we assert. It's more like we expect predecessors of ignored blocks to be filtered out already. 

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


More information about the llvm-commits mailing list