[PATCH] D156697: [BOLT] Split missed macro-fusion counting around removeConditionalTailCalls

[Amir Ayupov via Phabricator via llvm-commits]

Amir created this revision.
Amir added a reviewer: bolt.
Herald added a reviewer: rafauler.
Herald added subscribers: treapster, ayermolo.
Herald added a reviewer: maksfb.
Herald added a project: All.
Amir requested review of this revision.
Herald added subscribers: llvm-commits, yota9.
Herald added a project: LLVM.

D156389 <https://reviews.llvm.org/D156389> fixes conditional tail calls instrumentation by moving Offset annotation
from the conditional jump left in place of CTC to the newly constructed tail
call. However, this breaks missed macro-op fusion opportunities counting which
relies on offset information for conditional jumps.

Work around that by collecting the blocks with missed macro-fusion before
`removeConditionalTailCalls` but computing the count after it.

Depends On:
D156389 <https://reviews.llvm.org/D156389>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156697

Files:
  bolt/include/bolt/Core/BinaryFunction.h
  bolt/lib/Core/BinaryFunction.cpp


Index: bolt/lib/Core/BinaryFunction.cpp
===================================================================
--- bolt/lib/Core/BinaryFunction.cpp
+++ bolt/lib/Core/BinaryFunction.cpp
@@ -2174,6 +2174,8 @@
 }
 
 void BinaryFunction::postProcessCFG() {
+  SmallBlocksSet MissedMacroFusionBlocks = findMissedMacroOpFusionBlocks();
+
   if (isSimple() && !BasicBlocks.empty()) {
     // Convert conditional tail call branches to conditional branches that jump
     // to a tail call.
@@ -2185,7 +2187,7 @@
     postProcessBranches();
   }
 
-  calculateMacroOpFusionStats();
+  calculateMacroOpFusionStats(MissedMacroFusionBlocks);
 
   // The final cleanup of intermediate structures.
   clearList(IgnoredBranches);
@@ -2203,9 +2205,10 @@
          "invalid CFG detected after post-processing");
 }
 
-void BinaryFunction::calculateMacroOpFusionStats() {
+BinaryFunction::SmallBlocksSet BinaryFunction::findMissedMacroOpFusionBlocks() {
+  SmallBlocksSet Blocks;
   if (!getBinaryContext().isX86())
-    return;
+    return Blocks;
   for (const BinaryBasicBlock &BB : blocks()) {
     auto II = BB.getMacroOpFusionPair();
     if (II == BB.end())
@@ -2222,8 +2225,14 @@
                       << " in function " << *this << "; executed "
                       << BB.getKnownExecutionCount() << " times.\n");
     ++BC.MissedMacroFusionPairs;
-    BC.MissedMacroFusionExecCount += BB.getKnownExecutionCount();
+    Blocks.insert(&BB);
   }
+  return Blocks;
+}
+
+void BinaryFunction::calculateMacroOpFusionStats(SmallBlocksSet &Blocks) {
+  for (const BinaryBasicBlock *BB : Blocks)
+    BC.MissedMacroFusionExecCount += BB->getKnownExecutionCount();
 }
 
 void BinaryFunction::removeTagsFromProfile() {
Index: bolt/include/bolt/Core/BinaryFunction.h
===================================================================
--- bolt/include/bolt/Core/BinaryFunction.h
+++ bolt/include/bolt/Core/BinaryFunction.h
@@ -835,9 +835,13 @@
   /// them.
   void calculateLoopInfo();
 
+  /// Find blocks with missed macro-fusion opportunities.
+  using SmallBlocksSet = SmallPtrSet<const BinaryBasicBlock *, 1>;
+  SmallBlocksSet findMissedMacroOpFusionBlocks();
+
   /// Calculate missed macro-fusion opportunities and update BinaryContext
   /// stats.
-  void calculateMacroOpFusionStats();
+  void calculateMacroOpFusionStats(SmallBlocksSet &Blocks);
 
   /// Returns if loop detection has been run for this function.
   bool hasLoopInfo() const { return BLI != nullptr; }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156697.545663.patch
Type: text/x-patch
Size: 2462 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230731/543e0130/attachment.bin>