[PassManager] Add pretty stack frames #96078

nikic · 2024-06-19T14:17:20Z

In NewPM pass managers, add a "pretty stack frame" that tells you which pass crashed while running which function.

For example opt -O3 -passes-ep-peephole=trigger-crash-function test.ll will print something like this:

Stack dump:
0.	Program arguments: build/bin/opt -S -O3 -passes-ep-peephole=trigger-crash-function test.ll
1.	Running pass "function<eager-inv>(mem2reg,instcombine<max-iterations=1;no-use-loop-info;no-verify-fixpoint>,trigger-crash-function,simplifycfg<bonus-inst-threshold=1;no-forward-switch-cond;switch-range-to-icmp;no-switch-to-lookup;keep-loops;no-hoist-common-insts;no-sink-common-insts;speculate-blocks;simplify-cond-branch>)" on module "test.ll"
2.	Running pass "trigger-crash-function" on function "fshl_concat_i8_i8"

While the crashing pass is usually evident from the stack trace, this also shows which function triggered the crash, as well as the pipeline string for the pass (including options).

Similar functionality existed in the LegacyPM.

I'm not sure whether the way I did this is the best way to do it -- took me a while to figure out the correct combination of C++ template magic to make this work.

llvmbot · 2024-06-19T14:17:55Z

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-analysis

Author: Nikita Popov (nikic)

Changes

In NewPM pass managers, add a "pretty stack frame" that tells you which pass crashed while running which function:

Stack dump:
0.	Program arguments: build/bin/opt -S -passes=instcombine llvm/test/Transforms/InstCombine/vec_shuffle.ll
1.	Running pass "ModuleToFunctionPassAdaptor" on module "llvm/test/Transforms/InstCombine/vec_shuffle.ll"
2.	Running pass "InstCombinePass" on function "test16a"

While the crashing pass is usually evident from the stack trace, knowing which function caused it is quite handy.

Similar functionality existed in the LegacyPM.

I'm not sure whether the way I did this is the best way to do it -- took me a while to figure out the correct combination of C++ template magic to make this work.

I would have liked to print the pipeline string instead of pass name (so we also get pass options), but the necessary information for that doesn't seem to be available.

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

7 Files Affected:

(modified) llvm/include/llvm/Analysis/CGSCCPassManager.h (+4)
(modified) llvm/include/llvm/CodeGen/MachinePassManager.h (+5)
(modified) llvm/include/llvm/IR/PassManager.h (+22-3)
(modified) llvm/lib/Analysis/CGSCCPassManager.cpp (+5)
(modified) llvm/lib/CodeGen/MachinePassManager.cpp (+7)
(modified) llvm/lib/IR/PassManager.cpp (+13)
(modified) llvm/unittests/IR/PassManagerTest.cpp (+6)

diff --git a/llvm/include/llvm/Analysis/CGSCCPassManager.h b/llvm/include/llvm/Analysis/CGSCCPassManager.h
index b19d53621ac86..4d79493461b4e 100644
--- a/llvm/include/llvm/Analysis/CGSCCPassManager.h
+++ b/llvm/include/llvm/Analysis/CGSCCPassManager.h
@@ -131,6 +131,10 @@ PassManager<LazyCallGraph::SCC, CGSCCAnalysisManager, LazyCallGraph &,
             CGSCCUpdateResult &>::run(LazyCallGraph::SCC &InitialC,
                                       CGSCCAnalysisManager &AM,
                                       LazyCallGraph &G, CGSCCUpdateResult &UR);
+template <>
+void PassManager<LazyCallGraph::SCC, CGSCCAnalysisManager, LazyCallGraph &,
+                 CGSCCUpdateResult &>::StackTraceEntry::print(raw_ostream &OS)
+    const;
 extern template class PassManager<LazyCallGraph::SCC, CGSCCAnalysisManager,
                                   LazyCallGraph &, CGSCCUpdateResult &>;
 
diff --git a/llvm/include/llvm/CodeGen/MachinePassManager.h b/llvm/include/llvm/CodeGen/MachinePassManager.h
index 7d15664fbe754..780f8308dfcb6 100644
--- a/llvm/include/llvm/CodeGen/MachinePassManager.h
+++ b/llvm/include/llvm/CodeGen/MachinePassManager.h
@@ -238,6 +238,11 @@ template <>
 PreservedAnalyses
 PassManager<MachineFunction>::run(MachineFunction &,
                                   AnalysisManager<MachineFunction> &);
+
+template <>
+void PassManager<MachineFunction>::StackTraceEntry::print(
+    raw_ostream &OS) const;
+
 extern template class PassManager<MachineFunction>;
 
 /// Convenience typedef for a pass manager over functions.
diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h
index d701481202f8d..5efaa3f66ac47 100644
--- a/llvm/include/llvm/IR/PassManager.h
+++ b/llvm/include/llvm/IR/PassManager.h
@@ -48,6 +48,7 @@
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/PassManagerInternal.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/TypeName.h"
 #include <cassert>
@@ -171,6 +172,20 @@ template <typename IRUnitT,
           typename... ExtraArgTs>
 class PassManager : public PassInfoMixin<
                         PassManager<IRUnitT, AnalysisManagerT, ExtraArgTs...>> {
+  using PassConceptT =
+      detail::PassConcept<IRUnitT, AnalysisManagerT, ExtraArgTs...>;
+
+  class StackTraceEntry : public PrettyStackTraceEntry {
+    PassConceptT &Pass;
+    IRUnitT &IR;
+
+  public:
+    explicit StackTraceEntry(PassConceptT &Pass, IRUnitT &IR)
+      : Pass(Pass), IR(IR) {}
+
+    void print(raw_ostream &OS) const override;
+  };
+
 public:
   /// Construct a pass manager.
   explicit PassManager() = default;
@@ -221,6 +236,7 @@ class PassManager : public PassInfoMixin<
       if (!PI.runBeforePass<IRUnitT>(*Pass, IR))
         continue;
 
+      StackTraceEntry Entry(*Pass, IR);
       PreservedAnalyses PassPA = Pass->run(IR, AM, ExtraArgs...);
 
       // Update the analysis manager as each pass runs and potentially
@@ -271,17 +287,20 @@ class PassManager : public PassInfoMixin<
   static bool isRequired() { return true; }
 
 protected:
-  using PassConceptT =
-      detail::PassConcept<IRUnitT, AnalysisManagerT, ExtraArgTs...>;
-
   std::vector<std::unique_ptr<PassConceptT>> Passes;
 };
 
+template <>
+void PassManager<Module>::StackTraceEntry::print(raw_ostream &OS) const;
+
 extern template class PassManager<Module>;
 
 /// Convenience typedef for a pass manager over modules.
 using ModulePassManager = PassManager<Module>;
 
+template <>
+void PassManager<Function>::StackTraceEntry::print(raw_ostream &OS) const;
+
 extern template class PassManager<Function>;
 
 /// Convenience typedef for a pass manager over functions.
diff --git a/llvm/lib/Analysis/CGSCCPassManager.cpp b/llvm/lib/Analysis/CGSCCPassManager.cpp
index 2ed1d98f80068..c2386e6ab95c4 100644
--- a/llvm/lib/Analysis/CGSCCPassManager.cpp
+++ b/llvm/lib/Analysis/CGSCCPassManager.cpp
@@ -1193,3 +1193,8 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForCGSCCPass(
   return updateCGAndAnalysisManagerForPass(G, InitialC, N, AM, UR, FAM,
                                            /* FunctionPass */ false);
 }
+
+template <>
+void CGSCCPassManager::StackTraceEntry::print(raw_ostream &OS) const {
+  OS << "Running pass \"" << Pass.name() << "\" on cgscc\n";
+}
diff --git a/llvm/lib/CodeGen/MachinePassManager.cpp b/llvm/lib/CodeGen/MachinePassManager.cpp
index 6d540808d4cc9..41eae9491fef9 100644
--- a/llvm/lib/CodeGen/MachinePassManager.cpp
+++ b/llvm/lib/CodeGen/MachinePassManager.cpp
@@ -163,3 +163,10 @@ PreservedAnalyses llvm::getMachineFunctionPassPreservedAnalyses() {
   PA.template preserveSet<AllAnalysesOn<Function>>();
   return PA;
 }
+
+template <>
+void PassManager<MachineFunction>::StackTraceEntry::print(
+    raw_ostream &OS) const {
+  OS << "Running pass \"" << Pass.name() << "\" on machine function \""
+     << IR.getName() << "\"\n";
+}
diff --git a/llvm/lib/IR/PassManager.cpp b/llvm/lib/IR/PassManager.cpp
index cbddf3dfb056c..6f43fba516adf 100644
--- a/llvm/lib/IR/PassManager.cpp
+++ b/llvm/lib/IR/PassManager.cpp
@@ -13,6 +13,7 @@
 using namespace llvm;
 
 namespace llvm {
+
 // Explicit template instantiations and specialization defininitions for core
 // template typedefs.
 template class AllAnalysesOn<Module>;
@@ -144,6 +145,18 @@ PreservedAnalyses ModuleToFunctionPassAdaptor::run(Module &M,
   return PA;
 }
 
+template <>
+void PassManager<Module>::StackTraceEntry::print(raw_ostream &OS) const {
+  OS << "Running pass \"" << Pass.name() << "\" on module \"" << IR.getName()
+     << "\"\n";
+}
+
+template <>
+void PassManager<Function>::StackTraceEntry::print(raw_ostream &OS) const {
+  OS << "Running pass \"" << Pass.name() << "\" on function \"" << IR.getName()
+     << "\"\n";
+}
+
 AnalysisSetKey CFGAnalyses::SetKey;
 
 AnalysisSetKey PreservedAnalyses::AllAnalysesKey;
diff --git a/llvm/unittests/IR/PassManagerTest.cpp b/llvm/unittests/IR/PassManagerTest.cpp
index a6487169224c2..fb1ae17b9ed50 100644
--- a/llvm/unittests/IR/PassManagerTest.cpp
+++ b/llvm/unittests/IR/PassManagerTest.cpp
@@ -1072,3 +1072,9 @@ TEST_F(PassManagerTest, ModulePassMissedModuleAnalysisInvalidation) {
 
 #endif
 }
+
+template <>
+void PassManager<Function, CustomizedAnalysisManager, int,
+                 int &>::StackTraceEntry::print(raw_ostream &OS) const {
+  // Dummy
+}

fhahn · 2024-06-19T14:29:00Z

I originally tried to improve this in 128745c which did things a bit more complicated fashion. That approach turned out to be relatively expensive and was reverted and I never had time to get back to it. Would be good to check the compile-time impact just to be sure

nikic · 2024-06-19T17:31:36Z

I originally tried to improve this in 128745c which did things a bit more complicated fashion. That approach turned out to be relatively expensive and was reverted and I never had time to get back to it. Would be good to check the compile-time impact just to be sure

Looks like there is close to no impact: https://llvm-compile-time-tracker.com/compare.php?from=60984f5be90d3f0378d739792adbd4c5e3dcc0e2&to=3d48d4932284fd29d0ede70f0ea6834a7c204d5e&stat=instructions:u

nikic · 2024-06-20T09:55:18Z

I would have liked to print the pipeline string instead of pass name (so we also get pass options), but the necessary information for that doesn't seem to be available.

I just realized that this is available via PI -> PIC, just currently not exposed in the API. I think I'll change this PR to make use of that.

aeubanks · 2024-06-21T00:56:24Z

I would have liked to print the pipeline string instead of pass name (so we also get pass options), but the necessary information for that doesn't seem to be available.

I just realized that this is available via PI -> PIC, just currently not exposed in the API. I think I'll change this PR to make use of that.

It's only available when shouldPopulateClassToPassNames() is true, unsure of the compile time impact of always populating the pass name map

Initializing this map is somewhat expensive (especially for O0), so we currently only do it if certain flags are used. I would like to make use of it for crash dumps (llvm#96078), where I don't know in advance whether it will be needed or not. This patch changes the initialization to a lazy approach, where a callback is registered that does the actual initialization. The callbacks will be run the first time the pass name is requested.

nikic · 2024-06-21T15:33:19Z

I just realized that this is available via PI -> PIC, just currently not exposed in the API. I think I'll change this PR to make use of that.

It's only available when shouldPopulateClassToPassNames() is true, unsure of the compile time impact of always populating the pass name map

Always initializing the map does have measurable overhead: http://llvm-compile-time-tracker.com/compare.php?from=36c6632eb43bf67e19c8a6a21981cf66e06389b4&to=22e3ed4c5dfd93dcc3684410515ee053ace2edf3&stat=instructions:u

I've put up #96321 to use lazy initialization instead.

Initializing this map is somewhat expensive (especially for O0), so we currently only do it if certain flags are used. I would like to make use of it for crash dumps (#96078), where we don't know in advance whether it will be needed or not. This patch changes the initialization to a lazy approach, where a callback is registered that does the actual initialization. The callbacks will be run the first time the pass name is requested. This way there is no compile-time impact if the mapping is not used.

nikic · 2024-06-24T07:54:11Z

Okay, I think this is ready for review now.

fhahn

LGTM assuming compile-time impact is fine, thanks!

llvm/lib/IR/PassManager.cpp

…lvm#96321) Use [=] instead of [PIC] capture. On Windows, the "this" uses inside decltype result in an error otherwise. We can't explicitly capture "this", because it would result in warnings for the cases where it isn't used. My previous attempt to fix this used [&], which obviously causes lifetime issues instead... ----- Initializing this map is somewhat expensive (especially for O0), so we currently only do it if certain flags are used. I would like to make use of it for crash dumps (llvm#96078), where we don't know in advance whether it will be needed or not. This patch changes the initialization to a lazy approach, where a callback is registered that does the actual initialization. The callbacks will be run the first time the pass name is requested. This way there is no compile-time impact if the mapping is not used.

…96321) (#96462) On MSVC the `this` uses inside `decltype` require a lambda capture. On clang they result in an unused capture warning instead. Add the capture and suppress the warning with `(void)this`. ----- Initializing this map is somewhat expensive (especially for O0), so we currently only do it if certain flags are used. I would like to make use of it for crash dumps (#96078), where we don't know in advance whether it will be needed or not. This patch changes the initialization to a lazy approach, where a callback is registered that does the actual initialization. The callbacks will be run the first time the pass name is requested. This way there is no compile-time impact if the mapping is not used.

In NewPM pass managers, add a "pretty stack frame" that tells you which pass crashed while running which function: ``` Stack dump: 0. Program arguments: build/bin/opt -S -passes=instcombine llvm/test/Transforms/InstCombine/vec_shuffle.ll 1. Running pass "ModuleToFunctionPassAdaptor" on module "llvm/test/Transforms/InstCombine/vec_shuffle.ll" 2. Running pass "InstCombinePass" on function "test16a" ``` While the crashing pass is usually evident from the stack trace, knowing which function caused it is quite handy. Similar functionality existed in the LegacyPM.

aeubanks · 2024-06-24T16:26:12Z

llvm/include/llvm/IR/PassManager.h

@@ -224,11 +224,21 @@ class PassManager : public PassInfoMixin<
  std::vector<std::unique_ptr<PassConceptT>> Passes;
 };

+template <typename IRUnitT>
+void printIRUnitNameForStackTrace(raw_ostream &OS, const IRUnitT &IR);


can we put these in PassManagerImpl.h?

In theory, no, because other places using PassManagerImpl.h also need to specialize these for other types. In practice, yes, we could, because all non-Module/Function users actually specialize the PassManager::run implementation (and as such currently aren't part of the stack trace).

Architecturally, I think putting them in PassManager.h is correct, though possibly this whole implementation is unnecessarily general given that we only end up using it for Module/Function right now.

llvm-ci · 2024-06-27T10:51:13Z

LLVM Buildbot has detected a new failure on builder clang-ppc64le-linux-test-suite running on ppc64le-clang-test-suite while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/530

Here is the relevant piece of the build log for the reference:

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/819/1371' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/unittests/Support/./SupportTests-LLVM-Unit-3438679-819-1371.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=1371 GTEST_SHARD_INDEX=819 /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/unittests/Support/./SupportTests
--

Script:
--
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/build/unittests/Support/./SupportTests --gtest_filter=ProgramEnvTest.TestLockFile
--
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/llvm-project/llvm/unittests/Support/ProgramTest.cpp:558: Failure
Value of: Error.empty()
  Actual: false
Expected: true


/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-test-suite/clang-ppc64le-test-suite/llvm-project/llvm/unittests/Support/ProgramTest.cpp:558
Value of: Error.empty()
  Actual: false
Expected: true



********************

In NewPM pass managers, add a "pretty stack frame" that tells you which pass crashed while running which function. For example `opt -O3 -passes-ep-peephole=trigger-crash-function test.ll` will print something like this: ``` Stack dump: 0. Program arguments: build/bin/opt -S -O3 -passes-ep-peephole=trigger-crash-function test.ll 1. Running pass "function<eager-inv>(mem2reg,instcombine<max-iterations=1;no-use-loop-info;no-verify-fixpoint>,trigger-crash-function,simplifycfg<bonus-inst-threshold=1;no-forward-switch-cond;switch-range-to-icmp;no-switch-to-lookup;keep-loops;no-hoist-common-insts;no-sink-common-insts;speculate-blocks;simplify-cond-branch>)" on module "test.ll" 2. Running pass "trigger-crash-function" on function "fshl_concat_i8_i8" ``` While the crashing pass is usually evident from the stack trace, this also shows which function triggered the crash, as well as the pipeline string for the pass (including options). Similar functionality existed in the LegacyPM.

Initializing this map is somewhat expensive (especially for O0), so we currently only do it if certain flags are used. I would like to make use of it for crash dumps (llvm#96078), where we don't know in advance whether it will be needed or not. This patch changes the initialization to a lazy approach, where a callback is registered that does the actual initialization. The callbacks will be run the first time the pass name is requested. This way there is no compile-time impact if the mapping is not used.

…lvm#96321) (llvm#96462) On MSVC the `this` uses inside `decltype` require a lambda capture. On clang they result in an unused capture warning instead. Add the capture and suppress the warning with `(void)this`. ----- Initializing this map is somewhat expensive (especially for O0), so we currently only do it if certain flags are used. I would like to make use of it for crash dumps (llvm#96078), where we don't know in advance whether it will be needed or not. This patch changes the initialization to a lazy approach, where a callback is registered that does the actual initialization. The callbacks will be run the first time the pass name is requested. This way there is no compile-time impact if the mapping is not used.

In NewPM pass managers, add a "pretty stack frame" that tells you which pass crashed while running which function. For example `opt -O3 -passes-ep-peephole=trigger-crash-function test.ll` will print something like this: ``` Stack dump: 0. Program arguments: build/bin/opt -S -O3 -passes-ep-peephole=trigger-crash-function test.ll 1. Running pass "function<eager-inv>(mem2reg,instcombine<max-iterations=1;no-use-loop-info;no-verify-fixpoint>,trigger-crash-function,simplifycfg<bonus-inst-threshold=1;no-forward-switch-cond;switch-range-to-icmp;no-switch-to-lookup;keep-loops;no-hoist-common-insts;no-sink-common-insts;speculate-blocks;simplify-cond-branch>)" on module "test.ll" 2. Running pass "trigger-crash-function" on function "fshl_concat_i8_i8" ``` While the crashing pass is usually evident from the stack trace, this also shows which function triggered the crash, as well as the pipeline string for the pass (including options). Similar functionality existed in the LegacyPM.

nikic requested a review from aeubanks June 19, 2024 14:17

llvmbot added llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding labels Jun 19, 2024

nikic force-pushed the newpm-pretty-stack branch 2 times, most recently from 8317de2 to 2aaf753 Compare June 21, 2024 08:22

nikic mentioned this pull request Jun 21, 2024

[IR] Lazily initialize the class to pass name mapping (NFC) #96321

Merged

nikic force-pushed the newpm-pretty-stack branch from 2aaf753 to b218927 Compare June 24, 2024 07:49

fhahn approved these changes Jun 24, 2024

View reviewed changes

llvm/lib/IR/PassManager.cpp Outdated Show resolved Hide resolved

nikic force-pushed the newpm-pretty-stack branch from b218927 to 510a920 Compare June 24, 2024 08:06

nikic mentioned this pull request Jun 24, 2024

Reapply [IR] Lazily initialize the class to pass name mapping (NFC) (#96321) #96462

Merged

nikic added 2 commits June 24, 2024 15:01

drop unrelated whitespace change

164f203

nikic force-pushed the newpm-pretty-stack branch from 510a920 to 164f203 Compare June 24, 2024 13:01

aeubanks approved these changes Jun 24, 2024

View reviewed changes

nikic merged commit 253a294 into llvm:main Jun 27, 2024
7 checks passed

nikic deleted the newpm-pretty-stack branch June 27, 2024 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PassManager] Add pretty stack frames #96078

[PassManager] Add pretty stack frames #96078

Uh oh!

nikic commented Jun 19, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jun 19, 2024 •

edited

Loading

Uh oh!

fhahn commented Jun 19, 2024

Uh oh!

nikic commented Jun 19, 2024

Uh oh!

nikic commented Jun 20, 2024

Uh oh!

aeubanks commented Jun 21, 2024

Uh oh!

nikic commented Jun 21, 2024

Uh oh!

nikic commented Jun 24, 2024

Uh oh!

fhahn left a comment

Uh oh!

Uh oh!

aeubanks Jun 24, 2024

Uh oh!

nikic Jun 24, 2024 •

edited

Loading

Uh oh!

Uh oh!

llvm-ci commented Jun 27, 2024

Uh oh!

Uh oh!

[PassManager] Add pretty stack frames #96078

[PassManager] Add pretty stack frames #96078

Uh oh!

Conversation

nikic commented Jun 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fhahn commented Jun 19, 2024

Uh oh!

nikic commented Jun 19, 2024

Uh oh!

nikic commented Jun 20, 2024

Uh oh!

aeubanks commented Jun 21, 2024

Uh oh!

nikic commented Jun 21, 2024

Uh oh!

nikic commented Jun 24, 2024

Uh oh!

fhahn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aeubanks Jun 24, 2024

Choose a reason for hiding this comment

Uh oh!

nikic Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Jun 27, 2024

Uh oh!

Uh oh!

nikic commented Jun 19, 2024 •

edited

Loading

llvmbot commented Jun 19, 2024 •

edited

Loading

nikic Jun 24, 2024 •

edited

Loading