From b224a734975c4fb2923ebded8e63c21a8a7f2bc1 Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Thu, 8 Oct 2015 00:36:22 +0000 Subject: [PATCH] CodeGen: print and verify after TargetPassConfig::insertPass by default In r224059, we started verifying after addPass, but missed doing so on insertPass. There isn't a good reason for the discrepancy, and skipping the verifier in these cases causes bugs. This also exposes a verifier error that was introduced in r249087, but the verifier doesn't run until after the register coalescer, when the issue happens to have been resolved. I've skipped the verifier after SIFixSGPRLiveRangesID to avoid the failures for now and will follow up with Matt for a proper fix. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@249643 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/Passes.h | 3 +- lib/CodeGen/Passes.cpp | 50 ++++++++++++++--------- lib/Target/AMDGPU/AMDGPUTargetMachine.cpp | 4 +- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/include/llvm/CodeGen/Passes.h b/include/llvm/CodeGen/Passes.h index b06c576cd45..56be662dfc7 100644 --- a/include/llvm/CodeGen/Passes.h +++ b/include/llvm/CodeGen/Passes.h @@ -170,7 +170,8 @@ public: void substitutePass(AnalysisID StandardID, IdentifyingPassPtr TargetID); /// Insert InsertedPassID pass after TargetPassID pass. - void insertPass(AnalysisID TargetPassID, IdentifyingPassPtr InsertedPassID); + void insertPass(AnalysisID TargetPassID, IdentifyingPassPtr InsertedPassID, + bool VerifyAfter = true, bool PrintAfter = true); /// Allow the target to enable a specific standard pass by default. void enablePass(AnalysisID PassID) { substitutePass(PassID, PassID); } diff --git a/lib/CodeGen/Passes.cpp b/lib/CodeGen/Passes.cpp index 114c9d11be8..a7a8a53fa18 100644 --- a/lib/CodeGen/Passes.cpp +++ b/lib/CodeGen/Passes.cpp @@ -189,6 +189,29 @@ char TargetPassConfig::ID = 0; char TargetPassConfig::EarlyTailDuplicateID = 0; char TargetPassConfig::PostRAMachineLICMID = 0; +namespace { +struct InsertedPass { + AnalysisID TargetPassID; + IdentifyingPassPtr InsertedPassID; + bool VerifyAfter; + bool PrintAfter; + + InsertedPass(AnalysisID TargetPassID, IdentifyingPassPtr InsertedPassID, + bool VerifyAfter, bool PrintAfter) + : TargetPassID(TargetPassID), InsertedPassID(InsertedPassID), + VerifyAfter(VerifyAfter), PrintAfter(PrintAfter) {} + + Pass *getInsertedPass() const { + assert(InsertedPassID.isValid() && "Illegal Pass ID!"); + if (InsertedPassID.isInstance()) + return InsertedPassID.getInstance(); + Pass *NP = Pass::createPass(InsertedPassID.getID()); + assert(NP && "Pass ID not registered"); + return NP; + } +}; +} + namespace llvm { class PassConfigImpl { public: @@ -203,7 +226,7 @@ public: /// Store the pairs of of which the second pass /// is inserted after each instance of the first one. - SmallVector, 4> InsertedPasses; + SmallVector InsertedPasses; }; } // namespace llvm @@ -237,14 +260,15 @@ TargetPassConfig::TargetPassConfig(TargetMachine *tm, PassManagerBase &pm) /// Insert InsertedPassID pass after TargetPassID. void TargetPassConfig::insertPass(AnalysisID TargetPassID, - IdentifyingPassPtr InsertedPassID) { + IdentifyingPassPtr InsertedPassID, + bool VerifyAfter, bool PrintAfter) { assert(((!InsertedPassID.isInstance() && TargetPassID != InsertedPassID.getID()) || (InsertedPassID.isInstance() && TargetPassID != InsertedPassID.getInstance()->getPassID())) && "Insert a pass after itself!"); - std::pair P(TargetPassID, InsertedPassID); - Impl->InsertedPasses.push_back(P); + Impl->InsertedPasses.emplace_back(TargetPassID, InsertedPassID, VerifyAfter, + PrintAfter); } /// createPassConfig - Create a pass configuration object to be used by @@ -309,21 +333,9 @@ void TargetPassConfig::addPass(Pass *P, bool verifyAfter, bool printAfter) { } // Add the passes after the pass P if there is any. - for (SmallVectorImpl >::iterator - I = Impl->InsertedPasses.begin(), - E = Impl->InsertedPasses.end(); - I != E; ++I) { - if ((*I).first == PassID) { - assert((*I).second.isValid() && "Illegal Pass ID!"); - Pass *NP; - if ((*I).second.isInstance()) - NP = (*I).second.getInstance(); - else { - NP = Pass::createPass((*I).second.getID()); - assert(NP && "Pass ID not registered"); - } - addPass(NP, false, false); - } + for (auto IP : Impl->InsertedPasses) { + if (IP.TargetPassID == PassID) + addPass(IP.getInsertedPass(), IP.VerifyAfter, IP.PrintAfter); } } else { delete P; diff --git a/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp index 16fff50398e..854b12ab45c 100644 --- a/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp +++ b/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp @@ -309,7 +309,9 @@ void GCNPassConfig::addFastRegAlloc(FunctionPass *RegAllocPass) { void GCNPassConfig::addOptimizedRegAlloc(FunctionPass *RegAllocPass) { // We want to run this after LiveVariables is computed to avoid computing them // twice. - insertPass(&LiveVariablesID, &SIFixSGPRLiveRangesID); + // FIXME: We shouldn't disable the verifier here. r249087 introduced a failure + // that needs to be fixed. + insertPass(&LiveVariablesID, &SIFixSGPRLiveRangesID, /*VerifyAfter=*/false); TargetPassConfig::addOptimizedRegAlloc(RegAllocPass); } -- 2.34.1