Skip to content

Commit edaf656

Browse files
authored
[CodeGen][NPM] Differentiate pipeline-required and opt-required passes (#135752)
"Required" passes relate to actually running the pass on the IR, regardless of whether they are in the pipeline. CGPassBuilder was mistakenly still adding them to the pipeline. The test `llc -stop-after=greedy -enable-new-pm` would still add `greedy` to the pipeline otherwise.
1 parent 7ea3714 commit edaf656

File tree

4 files changed

+17
-16
lines changed

4 files changed

+17
-16
lines changed

llvm/include/llvm/Passes/CodeGenPassBuilder.h

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,6 @@ template <typename DerivedT, typename TargetMachineT> class CodeGenPassBuilder {
188188
}
189189

190190
protected:
191-
template <typename PassT>
192-
using has_required_t = decltype(std::declval<PassT &>().isRequired());
193-
194191
template <typename PassT>
195192
using is_module_pass_t = decltype(std::declval<PassT &>().run(
196193
std::declval<Module &>(), std::declval<ModuleAnalysisManager &>()));
@@ -214,14 +211,12 @@ template <typename DerivedT, typename TargetMachineT> class CodeGenPassBuilder {
214211
~AddIRPass() { flushFPMToMPM(); }
215212

216213
template <typename PassT>
217-
void operator()(PassT &&Pass, StringRef Name = PassT::name()) {
214+
void operator()(PassT &&Pass, bool Force = false,
215+
StringRef Name = PassT::name()) {
218216
static_assert((is_detected<is_function_pass_t, PassT>::value ||
219217
is_detected<is_module_pass_t, PassT>::value) &&
220218
"Only module pass and function pass are supported.");
221-
bool Required = false;
222-
if constexpr (is_detected<has_required_t, PassT>::value)
223-
Required = PassT::isRequired();
224-
if (!PB.runBeforeAdding(Name) && !Required)
219+
if (!Force && !PB.runBeforeAdding(Name))
225220
return;
226221

227222
// Add Function Pass
@@ -625,9 +620,12 @@ Error CodeGenPassBuilder<Derived, TargetMachineT>::buildPipeline(
625620

626621
{
627622
AddIRPass addIRPass(MPM, derived());
628-
addIRPass(RequireAnalysisPass<MachineModuleAnalysis, Module>());
629-
addIRPass(RequireAnalysisPass<ProfileSummaryAnalysis, Module>());
630-
addIRPass(RequireAnalysisPass<CollectorMetadataAnalysis, Module>());
623+
addIRPass(RequireAnalysisPass<MachineModuleAnalysis, Module>(),
624+
/*Force=*/true);
625+
addIRPass(RequireAnalysisPass<ProfileSummaryAnalysis, Module>(),
626+
/*Force=*/true);
627+
addIRPass(RequireAnalysisPass<CollectorMetadataAnalysis, Module>(),
628+
/*Force=*/true);
631629
addISelPasses(addIRPass);
632630
}
633631

@@ -743,7 +741,7 @@ void CodeGenPassBuilder<Derived, TargetMachineT>::addIRPasses(
743741
// Before running any passes, run the verifier to determine if the input
744742
// coming from the front-end and/or optimizer is valid.
745743
if (!Opt.DisableVerify)
746-
addPass(VerifierPass());
744+
addPass(VerifierPass(), /*Force=*/true);
747745

748746
// Run loop strength reduction before anything else.
749747
if (getOptLevel() != CodeGenOptLevel::None && !Opt.DisableLSR) {
@@ -883,7 +881,7 @@ void CodeGenPassBuilder<Derived, TargetMachineT>::addISelPrepare(
883881
// All passes which modify the LLVM IR are now complete; run the verifier
884882
// to ensure that the IR is valid.
885883
if (!Opt.DisableVerify)
886-
addPass(VerifierPass());
884+
addPass(VerifierPass(), /*Force=*/true);
887885
}
888886

889887
template <typename Derived, typename TargetMachineT>

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2147,7 +2147,8 @@ void AMDGPUCodeGenPassBuilder::addPreISel(AddIRPass &addPass) const {
21472147

21482148
// FIXME: Why isn't this queried as required from AMDGPUISelDAGToDAG, and why
21492149
// isn't this in addInstSelector?
2150-
addPass(RequireAnalysisPass<UniformityInfoAnalysis, Function>());
2150+
addPass(RequireAnalysisPass<UniformityInfoAnalysis, Function>(),
2151+
/*Force=*/true);
21512152
}
21522153

21532154
void AMDGPUCodeGenPassBuilder::addILPOpts(AddMachinePass &addPass) const {

llvm/test/tools/llc/new-pm/pipeline.mir

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# RUN: llc -mtriple=x86_64-pc-linux-gnu -x mir -passes=no-op-machine-function --print-pipeline-passes -filetype=null < %s | FileCheck %s --match-full-lines
22
# RUN: llc -mtriple=x86_64-pc-linux-gnu -x mir -passes='require<machine-dom-tree>,print<machine-dom-tree>' -print-pipeline-passes < %s | FileCheck --check-prefix=ANALYSIS %s
3+
# RUN: llc -mtriple=x86_64-pc-linux-gnu -x mir -enable-new-pm -stop-before=greedy -O3 -filetype=null --print-pipeline-passes < %s | FileCheck %s --check-prefix=CHECK-REQ
34

45
# Check same nested pass managers
56
# RUN: llc -mtriple=x86_64-pc-linux-gnu -x mir \
@@ -10,6 +11,7 @@
1011

1112
# CHECK: function(machine-function(no-op-machine-function)),PrintMIRPreparePass,function(machine-function(verify,print))
1213

14+
# CHECK-REQ-NOT: greedy
1315
# ANALYSIS: require<machine-dom-tree>,print<machine-dom-tree>
1416

1517
# NESTED: function(machine-function(machine-cp))
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
; RUN: llc -mtriple=x86_64-pc-linux-gnu -enable-new-pm -print-pipeline-passes -start-before=mergeicmps -stop-after=gc-lowering -filetype=null %s | FileCheck --match-full-lines %s --check-prefix=NULL
22
; RUN: llc -mtriple=x86_64-pc-linux-gnu -enable-new-pm -print-pipeline-passes -start-before=mergeicmps -stop-after=gc-lowering -o /dev/null %s | FileCheck --match-full-lines %s --check-prefix=OBJ
33

4-
; NULL: require<MachineModuleAnalysis>,require<profile-summary>,require<collector-metadata>,function(verify,loop-mssa(loop-reduce),mergeicmps,expand-memcmp,gc-lowering,ee-instrument<post-inline>,verify)
5-
; OBJ: require<MachineModuleAnalysis>,require<profile-summary>,require<collector-metadata>,function(verify,loop-mssa(loop-reduce),mergeicmps,expand-memcmp,gc-lowering,ee-instrument<post-inline>,verify),PrintMIRPreparePass,function(machine-function(print),invalidate<machine-function-info>)
4+
; NULL: require<MachineModuleAnalysis>,require<profile-summary>,require<collector-metadata>,function(verify,mergeicmps,expand-memcmp,gc-lowering,verify)
5+
; OBJ: require<MachineModuleAnalysis>,require<profile-summary>,require<collector-metadata>,function(verify,mergeicmps,expand-memcmp,gc-lowering,verify),PrintMIRPreparePass,function(machine-function(print),invalidate<machine-function-info>)

0 commit comments

Comments
 (0)