[IR] Lazily initialize the class to pass name mapping (NFC) #96321
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-backend-aarch64 Author: Nikita Popov (nikic) ChangesInitializing 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. Full diff: https://github.com/llvm/llvm-project/pull/96321.diff 17 Files Affected:
diff --git a/llvm/include/llvm/IR/PassInstrumentation.h b/llvm/include/llvm/IR/PassInstrumentation.h
index ff5e54427814d..f2eb8a95b7881 100644
--- a/llvm/include/llvm/IR/PassInstrumentation.h
+++ b/llvm/include/llvm/IR/PassInstrumentation.h
@@ -149,6 +149,11 @@ class PassInstrumentationCallbacks {
AnalysesClearedCallbacks.emplace_back(std::move(C));
}
+ template <typename CallableT>
+ void registerClassToPassNameCallback(CallableT C) {
+ ClassToPassNameCallbacks.emplace_back(std::move(C));
+ }
+
/// Add a class name to pass name mapping for use by pass instrumentation.
void addClassToPassName(StringRef ClassName, StringRef PassName);
/// Get the pass name for a given pass class name.
@@ -185,6 +190,7 @@ class PassInstrumentationCallbacks {
SmallVector<llvm::unique_function<AnalysesClearedFunc>, 4>
AnalysesClearedCallbacks;
+ SmallVector<llvm::unique_function<void ()>, 4> ClassToPassNameCallbacks;
DenseMap<StringRef, std::string> ClassToPassName;
};
diff --git a/llvm/include/llvm/Passes/TargetPassRegistry.inc b/llvm/include/llvm/Passes/TargetPassRegistry.inc
index b618331c69988..aa5066920cb7c 100644
--- a/llvm/include/llvm/Passes/TargetPassRegistry.inc
+++ b/llvm/include/llvm/Passes/TargetPassRegistry.inc
@@ -21,9 +21,9 @@
#error "must provide <Target>PassRegistry.def"
#endif
-if (PopulateClassToPassNames) {
- auto *PIC = PB.getPassInstrumentationCallbacks();
-
+auto *PIC = PB.getPassInstrumentationCallbacks();
+if (PIC) {
+ PIC->registerClassToPassNameCallback([PIC]() {
#define ADD_CLASS_PASS_TO_PASS_NAME(NAME, CREATE_PASS) \
PIC->addClassToPassName(decltype(CREATE_PASS)::name(), NAME);
#define ADD_CLASS_PASS_TO_PASS_NAME_WITH_PARAMS(NAME, CLASS) \
@@ -69,6 +69,7 @@ if (PopulateClassToPassNames) {
#undef MACHINE_FUNCTION_PASS_WITH_PARAMS
#undef ADD_CLASS_PASS_TO_PASS_NAME
#undef ADD_CLASS_PASS_TO_PASS_NAME_WITH_PARAMS
+ });
}
#define ADD_PASS(NAME, CREATE_PASS) \
diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h
index f09c3b2b347c5..b8e56c755fbda 100644
--- a/llvm/include/llvm/Target/TargetMachine.h
+++ b/llvm/include/llvm/Target/TargetMachine.h
@@ -369,8 +369,7 @@ class TargetMachine {
/// Allow the target to modify the pass pipeline.
// TODO: Populate all pass names by using <Target>PassRegistry.def.
- virtual void registerPassBuilderCallbacks(PassBuilder &,
- bool PopulateClassToPassNames) {}
+ virtual void registerPassBuilderCallbacks(PassBuilder &) {}
/// Allow the target to register alias analyses with the AAManager for use
/// with the new pass manager. Only affects the "default" AAManager.
diff --git a/llvm/lib/IR/PassInstrumentation.cpp b/llvm/lib/IR/PassInstrumentation.cpp
index 86c82a131fd2c..0c4e7698d9fa8 100644
--- a/llvm/lib/IR/PassInstrumentation.cpp
+++ b/llvm/lib/IR/PassInstrumentation.cpp
@@ -24,6 +24,11 @@ void PassInstrumentationCallbacks::addClassToPassName(StringRef ClassName,
StringRef
PassInstrumentationCallbacks::getPassNameForClassName(StringRef ClassName) {
+ if (!ClassToPassNameCallbacks.empty()) {
+ for (auto &Fn : ClassToPassNameCallbacks)
+ Fn();
+ ClassToPassNameCallbacks.clear();
+ }
return ClassToPassName[ClassName];
}
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index af83e2261a333..2c5f5bd82d3c3 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -324,18 +324,6 @@ AnalysisKey NoOpLoopAnalysis::Key;
namespace {
-/// Whether or not we should populate a PassInstrumentationCallbacks's class to
-/// pass name map.
-///
-/// This is for optimization purposes so we don't populate it if we never use
-/// it. This should be updated if new pass instrumentation wants to use the map.
-/// We currently only use this for --print-before/after.
-bool shouldPopulateClassToPassNames() {
- return PrintPipelinePasses || !printBeforePasses().empty() ||
- !printAfterPasses().empty() || !isFilterPassesEmpty() ||
- TargetPassConfig::hasLimitedCodeGenPipeline();
-}
-
// A pass for testing -print-on-crash.
// DO NOT USE THIS EXCEPT FOR TESTING!
class TriggerCrashPass : public PassInfoMixin<TriggerCrashPass> {
@@ -414,10 +402,10 @@ PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
std::optional<PGOOptions> PGOOpt,
PassInstrumentationCallbacks *PIC)
: TM(TM), PTO(PTO), PGOOpt(PGOOpt), PIC(PIC) {
- bool ShouldPopulateClassToPassNames = PIC && shouldPopulateClassToPassNames();
if (TM)
- TM->registerPassBuilderCallbacks(*this, ShouldPopulateClassToPassNames);
- if (ShouldPopulateClassToPassNames) {
+ TM->registerPassBuilderCallbacks(*this);
+ if (PIC) {
+ PIC->registerClassToPassNameCallback([PIC]() {
#define MODULE_PASS(NAME, CREATE_PASS) \
PIC->addClassToPassName(decltype(CREATE_PASS)::name(), NAME);
#define MODULE_PASS_WITH_PARAMS(NAME, CLASS, CREATE_PASS, PARSER, PARAMS) \
@@ -451,6 +439,7 @@ PassBuilder::PassBuilder(TargetMachine *TM, PipelineTuningOptions PTO,
#define MACHINE_FUNCTION_PASS(NAME, CREATE_PASS) \
PIC->addClassToPassName(decltype(CREATE_PASS)::name(), NAME);
#include "llvm/Passes/MachinePassRegistry.def"
+ });
}
}
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index 8c924e7c937cd..37ce07d4a09de 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -554,8 +554,7 @@ class AArch64PassConfig : public TargetPassConfig {
} // end anonymous namespace
-void AArch64TargetMachine::registerPassBuilderCallbacks(
- PassBuilder &PB, bool PopulateClassToPassNames) {
+void AArch64TargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
PB.registerLateLoopOptimizationsEPCallback(
[=](LoopPassManager &LPM, OptimizationLevel Level) {
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.h b/llvm/lib/Target/AArch64/AArch64TargetMachine.h
index e396d9204716a..1a470ca87127c 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.h
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.h
@@ -43,8 +43,7 @@ class AArch64TargetMachine : public LLVMTargetMachine {
// Pass Pipeline Configuration
TargetPassConfig *createPassConfig(PassManagerBase &PM) override;
- void registerPassBuilderCallbacks(PassBuilder &PB,
- bool PopulateClassToPassNames) override;
+ void registerPassBuilderCallbacks(PassBuilder &PB) override;
TargetTransformInfo getTargetTransformInfo(const Function &F) const override;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index 2e7d5cdfc3fe9..8bda424604906 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -658,8 +658,7 @@ Error AMDGPUTargetMachine::buildCodeGenPipeline(
return CGPB.buildPipeline(MPM, Out, DwoOut, FileType);
}
-void AMDGPUTargetMachine::registerPassBuilderCallbacks(
- PassBuilder &PB, bool PopulateClassToPassNames) {
+void AMDGPUTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
#define GET_PASS_REGISTRY "AMDGPUPassRegistry.def"
#include "llvm/Passes/TargetPassRegistry.inc"
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h
index 98b0bc034b5be..0f74fbc22fa84 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.h
@@ -58,8 +58,7 @@ class AMDGPUTargetMachine : public LLVMTargetMachine {
const CGPassBuilderOption &Opts,
PassInstrumentationCallbacks *PIC) override;
- void registerPassBuilderCallbacks(PassBuilder &PB,
- bool PopulateClassToPassNames) override;
+ void registerPassBuilderCallbacks(PassBuilder &PB) override;
void registerDefaultAliasAnalyses(AAManager &) override;
/// Get the integer value of a null pointer in the given address space.
diff --git a/llvm/lib/Target/BPF/BPFTargetMachine.cpp b/llvm/lib/Target/BPF/BPFTargetMachine.cpp
index 7b73c9f4a1e4c..7d91fa8bb824c 100644
--- a/llvm/lib/Target/BPF/BPFTargetMachine.cpp
+++ b/llvm/lib/Target/BPF/BPFTargetMachine.cpp
@@ -113,8 +113,7 @@ static Expected<bool> parseBPFPreserveStaticOffsetOptions(StringRef Params) {
"BPFPreserveStaticOffsetPass");
}
-void BPFTargetMachine::registerPassBuilderCallbacks(
- PassBuilder &PB, bool PopulateClassToPassNames) {
+void BPFTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
#define GET_PASS_REGISTRY "BPFPassRegistry.def"
#include "llvm/Passes/TargetPassRegistry.inc"
diff --git a/llvm/lib/Target/BPF/BPFTargetMachine.h b/llvm/lib/Target/BPF/BPFTargetMachine.h
index 0a28394463b26..4e6adc722e76a 100644
--- a/llvm/lib/Target/BPF/BPFTargetMachine.h
+++ b/llvm/lib/Target/BPF/BPFTargetMachine.h
@@ -42,8 +42,7 @@ class BPFTargetMachine : public LLVMTargetMachine {
return TLOF.get();
}
- void registerPassBuilderCallbacks(PassBuilder &PB,
- bool PopulateClassToPassNames) override;
+ void registerPassBuilderCallbacks(PassBuilder &PB) override;
};
}
diff --git a/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp b/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
index e4886506de19c..b362285d4f16e 100644
--- a/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
@@ -312,8 +312,7 @@ HexagonTargetMachine::getSubtargetImpl(const Function &F) const {
return I.get();
}
-void HexagonTargetMachine::registerPassBuilderCallbacks(
- PassBuilder &PB, bool PopulateClassToPassNames) {
+void HexagonTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
#define GET_PASS_REGISTRY "HexagonPassRegistry.def"
#include "llvm/Passes/TargetPassRegistry.inc"
diff --git a/llvm/lib/Target/Hexagon/HexagonTargetMachine.h b/llvm/lib/Target/Hexagon/HexagonTargetMachine.h
index 34ff45b6acf34..6e9a78b766504 100644
--- a/llvm/lib/Target/Hexagon/HexagonTargetMachine.h
+++ b/llvm/lib/Target/Hexagon/HexagonTargetMachine.h
@@ -35,8 +35,7 @@ class HexagonTargetMachine : public LLVMTargetMachine {
~HexagonTargetMachine() override;
const HexagonSubtarget *getSubtargetImpl(const Function &F) const override;
- void registerPassBuilderCallbacks(PassBuilder &PB,
- bool PopulateClassToPassNames) override;
+ void registerPassBuilderCallbacks(PassBuilder &PB) override;
TargetPassConfig *createPassConfig(PassManagerBase &PM) override;
TargetTransformInfo getTargetTransformInfo(const Function &F) const override;
diff --git a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
index b60a1d747af79..152f200b9d0f3 100644
--- a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.cpp
@@ -224,8 +224,7 @@ void NVPTXTargetMachine::registerDefaultAliasAnalyses(AAManager &AAM) {
AAM.registerFunctionAnalysis<NVPTXAA>();
}
-void NVPTXTargetMachine::registerPassBuilderCallbacks(
- PassBuilder &PB, bool PopulateClassToPassNames) {
+void NVPTXTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
#define GET_PASS_REGISTRY "NVPTXPassRegistry.def"
#include "llvm/Passes/TargetPassRegistry.inc"
diff --git a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h
index 870ea20c26f3f..2b88da67a50f9 100644
--- a/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h
+++ b/llvm/lib/Target/NVPTX/NVPTXTargetMachine.h
@@ -66,8 +66,7 @@ class NVPTXTargetMachine : public LLVMTargetMachine {
void registerDefaultAliasAnalyses(AAManager &AAM) override;
- void registerPassBuilderCallbacks(PassBuilder &PB,
- bool PopulateClassToPassNames) override;
+ void registerPassBuilderCallbacks(PassBuilder &PB) override;
TargetTransformInfo getTargetTransformInfo(const Function &F) const override;
diff --git a/llvm/lib/Target/X86/X86CodeGenPassBuilder.cpp b/llvm/lib/Target/X86/X86CodeGenPassBuilder.cpp
index 9819bfd129855..d979517e12af6 100644
--- a/llvm/lib/Target/X86/X86CodeGenPassBuilder.cpp
+++ b/llvm/lib/Target/X86/X86CodeGenPassBuilder.cpp
@@ -50,8 +50,7 @@ Error X86CodeGenPassBuilder::addInstSelector(AddMachinePass &addPass) const {
} // namespace
-void X86TargetMachine::registerPassBuilderCallbacks(
- PassBuilder &PB, bool PopulateClassToPassNames) {
+void X86TargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
#define GET_PASS_REGISTRY "X86PassRegistry.def"
#include "llvm/Passes/TargetPassRegistry.inc"
}
diff --git a/llvm/lib/Target/X86/X86TargetMachine.h b/llvm/lib/Target/X86/X86TargetMachine.h
index 916445c74bb90..ec4a93e9c9d4b 100644
--- a/llvm/lib/Target/X86/X86TargetMachine.h
+++ b/llvm/lib/Target/X86/X86TargetMachine.h
@@ -66,8 +66,7 @@ class X86TargetMachine final : public LLVMTargetMachine {
SMDiagnostic &Error,
SMRange &SourceRange) const override;
- void registerPassBuilderCallbacks(PassBuilder &PB,
- bool PopulateClassToPassNames) override;
+ void registerPassBuilderCallbacks(PassBuilder &PB) override;
Error buildCodeGenPipeline(ModulePassManager &, raw_pwrite_stream &,
raw_pwrite_stream *, CodeGenFileType,
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/497 Here is the relevant piece of the build log for the reference: |
MaskRay
added a commit
that referenced
this pull request
Jun 24, 2024
nikic
added a commit
that referenced
this pull request
Jun 24, 2024
nikic
added a commit
to nikic/llvm-project
that referenced
this pull request
Jun 24, 2024
…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.
nikic
added a commit
that referenced
this pull request
Jun 24, 2024
…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.
WenleiHe
added a commit
to pytorch/pytorch
that referenced
this pull request
Jun 28, 2024
…)" This reverts commit 20f394f. LLVM upstream changed the pass builder API again, so registerPassBuilderCallbacks no longer takes extra boolean for PopulateClassToPassNames. Update accordingly Relevant LLVM upstream change: llvm/llvm-project#96321 llvm/llvm-project#96462
pytorchmergebot
pushed a commit
to pytorch/pytorch
that referenced
this pull request
Jun 29, 2024
) This reverts commit 20f394f (#117086) LLVM upstream changed the pass builder API again, so registerPassBuilderCallbacks no longer takes extra boolean for PopulateClassToPassNames. Update accordingly. Relevant LLVM upstream change: llvm/llvm-project#96321 llvm/llvm-project#96462 Pull Request resolved: #129797 Approved by: https://github.com/dcci
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
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.
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
…lvm#96321)" My attempt to fix the Windows build made things worse, revert entirely for now. This reverts commit e7137f2. This reverts commit 6eaf204. This reverts commit 957dc43.
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
…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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.