CFGPrinter: fix accidentally quadratic behavior

[nhaehnle]

Initialize a ModuleStateTracker at most once per BasicBlock instead of once per Instruction. When the CFG info is provided, it is initialized once per function.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Nicolai Hähnle (nhaehnle)

Changes

Initialize a ModuleStateTracker at most once per BasicBlock instead of once per Instruction. When the CFG info is provided, it is initialized once per function.

---

Full diff: https://github.com/llvm/llvm-project/pull/125396.diff

2 Files Affected:

• (modified) llvm/include/llvm/Analysis/CFGPrinter.h (+11-20)
• (modified) llvm/lib/Analysis/CFGPrinter.cpp (+52)

diff --git a/llvm/include/llvm/Analysis/CFGPrinter.h b/llvm/include/llvm/Analysis/CFGPrinter.h
index cd785331d1f146..b844e3f11c4a50 100644
--- a/llvm/include/llvm/Analysis/CFGPrinter.h
+++ b/llvm/include/llvm/Analysis/CFGPrinter.h
@@ -31,6 +31,8 @@
 #include "llvm/Support/FormatVariadic.h"
 
 namespace llvm {
+class ModuleSlotTracker;
+
 template <class GraphType> struct GraphTraits;
 class CFGViewerPass : public PassInfoMixin<CFGViewerPass> {
 public:
@@ -61,6 +63,7 @@ class DOTFuncInfo {
   const Function *F;
   const BlockFrequencyInfo *BFI;
   const BranchProbabilityInfo *BPI;
+  std::unique_ptr<ModuleSlotTracker> MSTStorage;
   uint64_t MaxFreq;
   bool ShowHeat;
   bool EdgeWeights;
@@ -68,14 +71,10 @@ class DOTFuncInfo {
 
 public:
   DOTFuncInfo(const Function *F) : DOTFuncInfo(F, nullptr, nullptr, 0) {}
+  ~DOTFuncInfo();
 
   DOTFuncInfo(const Function *F, const BlockFrequencyInfo *BFI,
-              const BranchProbabilityInfo *BPI, uint64_t MaxFreq)
-      : F(F), BFI(BFI), BPI(BPI), MaxFreq(MaxFreq) {
-    ShowHeat = false;
-    EdgeWeights = !!BPI; // Print EdgeWeights when BPI is available.
-    RawWeights = !!BFI;  // Print RawWeights when BFI is available.
-  }
+              const BranchProbabilityInfo *BPI, uint64_t MaxFreq);
 
   const BlockFrequencyInfo *getBFI() const { return BFI; }
 
@@ -83,6 +82,8 @@ class DOTFuncInfo {
 
   const Function *getFunction() const { return this->F; }
 
+  ModuleSlotTracker *getModuleSlotTracker();
+
   uint64_t getMaxFreq() const { return MaxFreq; }
 
   uint64_t getFreq(const BasicBlock *BB) const {
@@ -203,22 +204,12 @@ struct DOTGraphTraits<DOTFuncInfo *> : public DefaultDOTGraphTraits {
     return SimpleNodeLabelString(Node);
   }
 
-  static void printBasicBlock(raw_string_ostream &OS, const BasicBlock &Node) {
-    // Prepend label name
-    Node.printAsOperand(OS, false);
-    OS << ":\n";
-    for (const Instruction &Inst : Node)
-      OS << Inst << "\n";
-  }
-
   static std::string getCompleteNodeLabel(
       const BasicBlock *Node, DOTFuncInfo *,
       function_ref<void(raw_string_ostream &, const BasicBlock &)>
-          HandleBasicBlock = printBasicBlock,
-      function_ref<void(std::string &, unsigned &, unsigned)>
-          HandleComment = eraseComment) {
-    return CompleteNodeLabelString(Node, HandleBasicBlock, HandleComment);
-  }
+          HandleBasicBlock = {},
+      function_ref<void(std::string &, unsigned &, unsigned)> HandleComment =
+          eraseComment);
 
   std::string getNodeLabel(const BasicBlock *Node, DOTFuncInfo *CFGInfo) {
 
@@ -337,6 +328,6 @@ struct DOTGraphTraits<DOTFuncInfo *> : public DefaultDOTGraphTraits {
   bool isNodeHidden(const BasicBlock *Node, const DOTFuncInfo *CFGInfo);
   void computeDeoptOrUnreachablePaths(const Function *F);
 };
-} // End llvm namespace
+} // namespace llvm
 
 #endif
diff --git a/llvm/lib/Analysis/CFGPrinter.cpp b/llvm/lib/Analysis/CFGPrinter.cpp
index af18fb6626e3bf..e6902f22f75949 100644
--- a/llvm/lib/Analysis/CFGPrinter.cpp
+++ b/llvm/lib/Analysis/CFGPrinter.cpp
@@ -19,6 +19,7 @@
 
 #include "llvm/Analysis/CFGPrinter.h"
 #include "llvm/ADT/PostOrderIterator.h"
+#include "llvm/IR/ModuleSlotTracker.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/GraphWriter.h"
@@ -90,6 +91,22 @@ static void viewCFG(Function &F, const BlockFrequencyInfo *BFI,
   ViewGraph(&CFGInfo, "cfg." + F.getName(), CFGOnly);
 }
 
+DOTFuncInfo::DOTFuncInfo(const Function *F, const BlockFrequencyInfo *BFI,
+                         const BranchProbabilityInfo *BPI, uint64_t MaxFreq)
+    : F(F), BFI(BFI), BPI(BPI), MaxFreq(MaxFreq) {
+  ShowHeat = false;
+  EdgeWeights = !!BPI; // Print EdgeWeights when BPI is available.
+  RawWeights = !!BFI;  // Print RawWeights when BFI is available.
+}
+
+DOTFuncInfo::~DOTFuncInfo() = default;
+
+ModuleSlotTracker *DOTFuncInfo::getModuleSlotTracker() {
+  if (!MSTStorage)
+    MSTStorage = std::make_unique<ModuleSlotTracker>(F->getParent());
+  return &*MSTStorage;
+}
+
 PreservedAnalyses CFGViewerPass::run(Function &F, FunctionAnalysisManager &AM) {
   if (!CFGFuncName.empty() && !F.getName().contains(CFGFuncName))
     return PreservedAnalyses::all();
@@ -208,3 +225,38 @@ bool DOTGraphTraits<DOTFuncInfo *>::isNodeHidden(const BasicBlock *Node,
   }
   return false;
 }
+
+std::string DOTGraphTraits<DOTFuncInfo *>::getCompleteNodeLabel(
+    const BasicBlock *Node, DOTFuncInfo *CFGInfo,
+    function_ref<void(raw_string_ostream &, const BasicBlock &)>
+        HandleBasicBlock,
+    function_ref<void(std::string &, unsigned &, unsigned)> HandleComment) {
+  if (HandleBasicBlock)
+    return CompleteNodeLabelString(Node, HandleBasicBlock, HandleComment);
+
+  // Default basic block printing
+  std::optional<ModuleSlotTracker> MSTStorage;
+  ModuleSlotTracker *MST = nullptr;
+
+  if (CFGInfo) {
+    MST = CFGInfo->getModuleSlotTracker();
+  } else {
+    MSTStorage.emplace(Node->getModule());
+    MST = &*MSTStorage;
+  }
+
+  return CompleteNodeLabelString(
+      Node,
+      function_ref<void(raw_string_ostream &, const BasicBlock &)>(
+          [MST](raw_string_ostream &OS, const BasicBlock &Node) -> void {
+            // Prepend label name
+            Node.printAsOperand(OS, false);
+            OS << ":\n";
+
+            for (const Instruction &Inst : Node) {
+              Inst.print(OS, *MST, /* IsForDebug */ false);
+              OS << '\n';
+            }
+          }),
+      HandleComment);
+}

[nikic]

Should pass MST here as well?

[nhaehnle]

Good point, yes.

[nhaehnle]

Pass MST to printAsOperand.

Note that the previous build failure was an issue with BOLT, looked unrelated.

[nikic]

Pass MST to printAsOperand.

I think you forgot to push the changes?

[cdevadas]

Pass MST to printAsOperand.

I think you forgot to push the changes?

Looks like he pushed it be99f44. But I don't see it either in the PR page. It still indicates "processing update" at the top of the page.

[nhaehnle]

Yeah, GitHub was taking a long time earlier and it seems some job related to this PR was dropped. I pushed it again and now it took.

[nikic]

LGTM