From 83bfb55f3b81271a683ad090724080e4bdbd1858 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Fri, 11 Aug 2017 05:47:13 +0000 Subject: [PATCH] [PM] Switch the CGSCC debug messages to use the standard LLVM debug printing techniques with a DEBUG_TYPE controlling them. It was a mistake to start re-purposing the pass manager `DebugLogging` variable for generic debug printing -- those logs are intended to be very minimal and primarily used for testing. More detailed and comprehensive logging doesn't make sense there (it would only make for brittle tests). Moreover, we kept forgetting to propagate the `DebugLogging` variable to various places making it also ineffective and/or unavailable. Switching to `DEBUG_TYPE` makes this a non-issue. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310695 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/CGSCCPassManager.h | 102 +++++++++----------- lib/Analysis/CGSCCPassManager.cpp | 51 +++++----- lib/Passes/PassBuilder.cpp | 12 +-- test/Other/new-pass-manager.ll | 2 - unittests/Analysis/CGSCCPassManagerTest.cpp | 8 +- 5 files changed, 77 insertions(+), 98 deletions(-) diff --git a/include/llvm/Analysis/CGSCCPassManager.h b/include/llvm/Analysis/CGSCCPassManager.h index 12584b1904c..be87df6f994 100644 --- a/include/llvm/Analysis/CGSCCPassManager.h +++ b/include/llvm/Analysis/CGSCCPassManager.h @@ -98,6 +98,9 @@ namespace llvm { +// Allow debug logging in this inline function. +#define DEBUG_TYPE "cgscc" + struct CGSCCUpdateResult; /// Extern template declaration for the analysis set for this IR unit. @@ -299,20 +302,19 @@ template class ModuleToPostOrderCGSCCPassAdaptor : public PassInfoMixin> { public: - explicit ModuleToPostOrderCGSCCPassAdaptor(CGSCCPassT Pass, bool DebugLogging = false) - : Pass(std::move(Pass)), DebugLogging(DebugLogging) {} + explicit ModuleToPostOrderCGSCCPassAdaptor(CGSCCPassT Pass) + : Pass(std::move(Pass)) {} // We have to explicitly define all the special member functions because MSVC // refuses to generate them. ModuleToPostOrderCGSCCPassAdaptor( const ModuleToPostOrderCGSCCPassAdaptor &Arg) - : Pass(Arg.Pass), DebugLogging(Arg.DebugLogging) {} + : Pass(Arg.Pass) {} ModuleToPostOrderCGSCCPassAdaptor(ModuleToPostOrderCGSCCPassAdaptor &&Arg) - : Pass(std::move(Arg.Pass)), DebugLogging(Arg.DebugLogging) {} + : Pass(std::move(Arg.Pass)) {} friend void swap(ModuleToPostOrderCGSCCPassAdaptor &LHS, ModuleToPostOrderCGSCCPassAdaptor &RHS) { using std::swap; swap(LHS.Pass, RHS.Pass); - swap(LHS.DebugLogging, RHS.DebugLogging); } ModuleToPostOrderCGSCCPassAdaptor & operator=(ModuleToPostOrderCGSCCPassAdaptor RHS) { @@ -369,16 +371,15 @@ public: do { LazyCallGraph::RefSCC *RC = RCWorklist.pop_back_val(); if (InvalidRefSCCSet.count(RC)) { - if (DebugLogging) - dbgs() << "Skipping an invalid RefSCC...\n"; + DEBUG(dbgs() << "Skipping an invalid RefSCC...\n"); continue; } assert(CWorklist.empty() && "Should always start with an empty SCC worklist"); - if (DebugLogging) - dbgs() << "Running an SCC pass across the RefSCC: " << *RC << "\n"; + DEBUG(dbgs() << "Running an SCC pass across the RefSCC: " << *RC + << "\n"); // Push the initial SCCs in reverse post-order as we'll pop off the the // back and so see this in post-order. @@ -392,14 +393,12 @@ public: // other RefSCCs should be queued above, so we just need to skip both // scenarios here. if (InvalidSCCSet.count(C)) { - if (DebugLogging) - dbgs() << "Skipping an invalid SCC...\n"; + DEBUG(dbgs() << "Skipping an invalid SCC...\n"); continue; } if (&C->getOuterRefSCC() != RC) { - if (DebugLogging) - dbgs() << "Skipping an SCC that is now part of some other " - "RefSCC...\n"; + DEBUG(dbgs() << "Skipping an SCC that is now part of some other " + "RefSCC...\n"); continue; } @@ -437,10 +436,10 @@ public: // iterate there too. RC = UR.UpdatedRC ? UR.UpdatedRC : RC; C = UR.UpdatedC ? UR.UpdatedC : C; - if (DebugLogging && UR.UpdatedC) - dbgs() << "Re-running SCC passes after a refinement of the " - "current SCC: " - << *UR.UpdatedC << "\n"; + if (UR.UpdatedC) + DEBUG(dbgs() << "Re-running SCC passes after a refinement of the " + "current SCC: " + << *UR.UpdatedC << "\n"); // Note that both `C` and `RC` may at this point refer to deleted, // invalid SCC and RefSCCs respectively. But we will short circuit @@ -466,15 +465,14 @@ public: private: CGSCCPassT Pass; - bool DebugLogging; }; /// \brief A function to deduce a function pass type and wrap it in the /// templated adaptor. template ModuleToPostOrderCGSCCPassAdaptor -createModuleToPostOrderCGSCCPassAdaptor(CGSCCPassT Pass, bool DebugLogging = false) { - return ModuleToPostOrderCGSCCPassAdaptor(std::move(Pass), DebugLogging); +createModuleToPostOrderCGSCCPassAdaptor(CGSCCPassT Pass) { + return ModuleToPostOrderCGSCCPassAdaptor(std::move(Pass)); } /// A proxy from a \c FunctionAnalysisManager to an \c SCC. @@ -523,7 +521,7 @@ typedef OuterAnalysisManagerProxy /// update result struct for the overall CGSCC walk. LazyCallGraph::SCC &updateCGAndAnalysisManagerForFunctionPass( LazyCallGraph &G, LazyCallGraph::SCC &C, LazyCallGraph::Node &N, - CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR, bool DebugLogging = false); + CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR); /// \brief Adaptor that maps from a SCC to its functions. /// @@ -537,19 +535,18 @@ template class CGSCCToFunctionPassAdaptor : public PassInfoMixin> { public: - explicit CGSCCToFunctionPassAdaptor(FunctionPassT Pass, bool DebugLogging = false) - : Pass(std::move(Pass)), DebugLogging(DebugLogging) {} + explicit CGSCCToFunctionPassAdaptor(FunctionPassT Pass) + : Pass(std::move(Pass)) {} // We have to explicitly define all the special member functions because MSVC // refuses to generate them. CGSCCToFunctionPassAdaptor(const CGSCCToFunctionPassAdaptor &Arg) - : Pass(Arg.Pass), DebugLogging(Arg.DebugLogging) {} + : Pass(Arg.Pass) {} CGSCCToFunctionPassAdaptor(CGSCCToFunctionPassAdaptor &&Arg) - : Pass(std::move(Arg.Pass)), DebugLogging(Arg.DebugLogging) {} + : Pass(std::move(Arg.Pass)) {} friend void swap(CGSCCToFunctionPassAdaptor &LHS, CGSCCToFunctionPassAdaptor &RHS) { using std::swap; swap(LHS.Pass, RHS.Pass); - swap(LHS.DebugLogging, RHS.DebugLogging); } CGSCCToFunctionPassAdaptor &operator=(CGSCCToFunctionPassAdaptor RHS) { swap(*this, RHS); @@ -572,8 +569,7 @@ public: // a pointer we can overwrite. LazyCallGraph::SCC *CurrentC = &C; - if (DebugLogging) - dbgs() << "Running function passes across an SCC: " << C << "\n"; + DEBUG(dbgs() << "Running function passes across an SCC: " << C << "\n"); PreservedAnalyses PA = PreservedAnalyses::all(); for (LazyCallGraph::Node *N : Nodes) { @@ -599,8 +595,8 @@ public: // a smaller, more refined SCC. auto PAC = PA.getChecker(); if (!PAC.preserved() && !PAC.preservedSet>()) { - CurrentC = &updateCGAndAnalysisManagerForFunctionPass( - CG, *CurrentC, *N, AM, UR, DebugLogging); + CurrentC = &updateCGAndAnalysisManagerForFunctionPass(CG, *CurrentC, *N, + AM, UR); assert( CG.lookupSCC(*N) == CurrentC && "Current SCC not updated to the SCC containing the current node!"); @@ -622,16 +618,14 @@ public: private: FunctionPassT Pass; - bool DebugLogging; }; /// \brief A function to deduce a function pass type and wrap it in the /// templated adaptor. template CGSCCToFunctionPassAdaptor -createCGSCCToFunctionPassAdaptor(FunctionPassT Pass, bool DebugLogging = false) { - return CGSCCToFunctionPassAdaptor(std::move(Pass), - DebugLogging); +createCGSCCToFunctionPassAdaptor(FunctionPassT Pass) { + return CGSCCToFunctionPassAdaptor(std::move(Pass)); } /// A helper that repeats an SCC pass each time an indirect call is refined to @@ -652,10 +646,8 @@ template class DevirtSCCRepeatedPass : public PassInfoMixin> { public: - explicit DevirtSCCRepeatedPass(PassT Pass, int MaxIterations, - bool DebugLogging = false) - : Pass(std::move(Pass)), MaxIterations(MaxIterations), - DebugLogging(DebugLogging) {} + explicit DevirtSCCRepeatedPass(PassT Pass, int MaxIterations) + : Pass(std::move(Pass)), MaxIterations(MaxIterations) {} /// Runs the wrapped pass up to \c MaxIterations on the SCC, iterating /// whenever an indirect call is refined. @@ -733,10 +725,9 @@ public: if (!F) return false; - if (DebugLogging) - dbgs() << "Found devirutalized call from " - << CS.getParent()->getParent()->getName() << " to " - << F->getName() << "\n"; + DEBUG(dbgs() << "Found devirutalized call from " + << CS.getParent()->getParent()->getName() << " to " + << F->getName() << "\n"); // We now have a direct call where previously we had an indirect call, // so iterate to process this devirtualization site. @@ -770,17 +761,16 @@ public: // Otherwise, if we've already hit our max, we're done. if (Iteration >= MaxIterations) { - if (DebugLogging) - dbgs() << "Found another devirtualization after hitting the max " - "number of repetitions (" - << MaxIterations << ") on SCC: " << *C << "\n"; + DEBUG(dbgs() << "Found another devirtualization after hitting the max " + "number of repetitions (" + << MaxIterations << ") on SCC: " << *C << "\n"); PA.intersect(std::move(PassPA)); break; } - if (DebugLogging) - dbgs() << "Repeating an SCC pass after finding a devirtualization in: " - << *C << "\n"; + DEBUG(dbgs() + << "Repeating an SCC pass after finding a devirtualization in: " + << *C << "\n"); // Move over the new call counts in preparation for iterating. CallCounts = std::move(NewCallCounts); @@ -800,18 +790,18 @@ public: private: PassT Pass; int MaxIterations; - bool DebugLogging; }; /// \brief A function to deduce a function pass type and wrap it in the /// templated adaptor. template -DevirtSCCRepeatedPass -createDevirtSCCRepeatedPass(PassT Pass, int MaxIterations, - bool DebugLogging = false) { - return DevirtSCCRepeatedPass(std::move(Pass), MaxIterations, - DebugLogging); +DevirtSCCRepeatedPass createDevirtSCCRepeatedPass(PassT Pass, + int MaxIterations) { + return DevirtSCCRepeatedPass(std::move(Pass), MaxIterations); } + +// Clear out the debug logging macro. +#undef DEBUG_TYPE } #endif diff --git a/lib/Analysis/CGSCCPassManager.cpp b/lib/Analysis/CGSCCPassManager.cpp index ce8af755147..46d384922af 100644 --- a/lib/Analysis/CGSCCPassManager.cpp +++ b/lib/Analysis/CGSCCPassManager.cpp @@ -11,6 +11,8 @@ #include "llvm/IR/CallSite.h" #include "llvm/IR/InstIterator.h" +#define DEBUG_TYPE "cgscc" + using namespace llvm; // Explicit template instantiations and specialization defininitions for core @@ -322,8 +324,7 @@ template LazyCallGraph::SCC * incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G, LazyCallGraph::Node &N, LazyCallGraph::SCC *C, - CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR, - bool DebugLogging = false) { + CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR) { typedef LazyCallGraph::SCC SCC; if (NewSCCRange.begin() == NewSCCRange.end()) @@ -331,8 +332,7 @@ incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G, // Add the current SCC to the worklist as its shape has changed. UR.CWorklist.insert(C); - if (DebugLogging) - dbgs() << "Enqueuing the existing SCC in the worklist:" << *C << "\n"; + DEBUG(dbgs() << "Enqueuing the existing SCC in the worklist:" << *C << "\n"); SCC *OldC = C; @@ -368,8 +368,7 @@ incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G, assert(C != &NewC && "No need to re-visit the current SCC!"); assert(OldC != &NewC && "Already handled the original SCC!"); UR.CWorklist.insert(&NewC); - if (DebugLogging) - dbgs() << "Enqueuing a newly formed SCC:" << NewC << "\n"; + DEBUG(dbgs() << "Enqueuing a newly formed SCC:" << NewC << "\n"); // Ensure new SCCs' function analyses are updated. if (NeedFAMProxy) @@ -385,7 +384,7 @@ incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G, LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( LazyCallGraph &G, LazyCallGraph::SCC &InitialC, LazyCallGraph::Node &N, - CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR, bool DebugLogging) { + CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR) { typedef LazyCallGraph::Node Node; typedef LazyCallGraph::Edge Edge; typedef LazyCallGraph::SCC SCC; @@ -475,7 +474,7 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( } else { // Now update the call graph. C = incorporateNewSCCRange(RC->switchInternalEdgeToRef(N, E.getNode()), - G, N, C, AM, UR, DebugLogging); + G, N, C, AM, UR); } } @@ -495,9 +494,8 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( return false; RC->removeOutgoingEdge(N, *TargetN); - if (DebugLogging) - dbgs() << "Deleting outgoing edge from '" << N - << "' to '" << TargetN << "'\n"; + DEBUG(dbgs() << "Deleting outgoing edge from '" << N + << "' to '" << TargetN << "'\n"); return true; }), DeadTargets.end()); @@ -528,9 +526,8 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( assert(NewRC != RC && "Should not encounter the current RefSCC further " "in the postorder list of new RefSCCs."); UR.RCWorklist.insert(NewRC); - if (DebugLogging) - dbgs() << "Enqueuing a new RefSCC in the update worklist: " << *NewRC - << "\n"; + DEBUG(dbgs() << "Enqueuing a new RefSCC in the update worklist: " + << *NewRC << "\n"); } } @@ -547,9 +544,8 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( assert(RC->isAncestorOf(TargetRC) && "Cannot potentially form RefSCC cycles here!"); RC->switchOutgoingEdgeToRef(N, *RefTarget); - if (DebugLogging) - dbgs() << "Switch outgoing call edge to a ref edge from '" << N - << "' to '" << *RefTarget << "'\n"; + DEBUG(dbgs() << "Switch outgoing call edge to a ref edge from '" << N + << "' to '" << *RefTarget << "'\n"); continue; } @@ -563,7 +559,7 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( // Now update the call graph. C = incorporateNewSCCRange(RC->switchInternalEdgeToRef(N, *RefTarget), G, N, - C, AM, UR, DebugLogging); + C, AM, UR); } // Now promote ref edges into call edges. @@ -577,14 +573,12 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( assert(RC->isAncestorOf(TargetRC) && "Cannot potentially form RefSCC cycles here!"); RC->switchOutgoingEdgeToCall(N, *CallTarget); - if (DebugLogging) - dbgs() << "Switch outgoing ref edge to a call edge from '" << N - << "' to '" << *CallTarget << "'\n"; + DEBUG(dbgs() << "Switch outgoing ref edge to a call edge from '" << N + << "' to '" << *CallTarget << "'\n"); continue; } - if (DebugLogging) - dbgs() << "Switch an internal ref edge to a call edge from '" << N - << "' to '" << *CallTarget << "'\n"; + DEBUG(dbgs() << "Switch an internal ref edge to a call edge from '" << N + << "' to '" << *CallTarget << "'\n"); // Otherwise we are switching an internal ref edge to a call edge. This // may merge away some SCCs, and we add those to the UpdateResult. We also @@ -647,15 +641,14 @@ LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass( // post-order sequence, and may end up observing more precise context to // optimize the current SCC. UR.CWorklist.insert(C); - if (DebugLogging) - dbgs() << "Enqueuing the existing SCC in the worklist: " << *C << "\n"; + DEBUG(dbgs() << "Enqueuing the existing SCC in the worklist: " << *C + << "\n"); // Enqueue in reverse order as we pop off the back of the worklist. for (SCC &MovedC : reverse(make_range(RC->begin() + InitialSCCIndex, RC->begin() + NewSCCIndex))) { UR.CWorklist.insert(&MovedC); - if (DebugLogging) - dbgs() << "Enqueuing a newly earlier in post-order SCC: " << MovedC - << "\n"; + DEBUG(dbgs() << "Enqueuing a newly earlier in post-order SCC: " + << MovedC << "\n"); } } } diff --git a/lib/Passes/PassBuilder.cpp b/lib/Passes/PassBuilder.cpp index 84fd98c7c4f..62e00baabaf 100644 --- a/lib/Passes/PassBuilder.cpp +++ b/lib/Passes/PassBuilder.cpp @@ -656,7 +656,7 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, // in postorder (or bottom-up). MPM.addPass( createModuleToPostOrderCGSCCPassAdaptor(createDevirtSCCRepeatedPass( - std::move(MainCGPipeline), MaxDevirtIterations, DebugLogging))); + std::move(MainCGPipeline), MaxDevirtIterations))); return MPM; } @@ -1300,8 +1300,7 @@ bool PassBuilder::parseModulePass(ModulePassManager &MPM, if (!parseCGSCCPassPipeline(CGPM, InnerPipeline, VerifyEachPass, DebugLogging)) return false; - MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM), - DebugLogging)); + MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM))); return true; } if (Name == "function") { @@ -1411,8 +1410,7 @@ bool PassBuilder::parseCGSCCPass(CGSCCPassManager &CGPM, DebugLogging)) return false; // Add the nested pass manager with the appropriate adaptor. - CGPM.addPass( - createCGSCCToFunctionPassAdaptor(std::move(FPM), DebugLogging)); + CGPM.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM))); return true; } if (auto Count = parseRepeatPassName(Name)) { @@ -1428,8 +1426,8 @@ bool PassBuilder::parseCGSCCPass(CGSCCPassManager &CGPM, if (!parseCGSCCPassPipeline(NestedCGPM, InnerPipeline, VerifyEachPass, DebugLogging)) return false; - CGPM.addPass(createDevirtSCCRepeatedPass(std::move(NestedCGPM), - *MaxRepetitions, DebugLogging)); + CGPM.addPass( + createDevirtSCCRepeatedPass(std::move(NestedCGPM), *MaxRepetitions)); return true; } diff --git a/test/Other/new-pass-manager.ll b/test/Other/new-pass-manager.ll index 35f596e7798..0826ecd3152 100644 --- a/test/Other/new-pass-manager.ll +++ b/test/Other/new-pass-manager.ll @@ -24,7 +24,6 @@ ; CHECK-CGSCC-PASS-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*(FunctionAnalysisManager|AnalysisManager<.*Function.*>).*}},{{.*}}Module> ; CHECK-CGSCC-PASS-NEXT: Running analysis: LazyCallGraphAnalysis ; CHECK-CGSCC-PASS-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-CGSCC-PASS-NEXT: Running an SCC pass across the RefSCC: [(foo)] ; CHECK-CGSCC-PASS-NEXT: Starting CGSCC pass manager run ; CHECK-CGSCC-PASS-NEXT: Running pass: NoOpCGSCCPass ; CHECK-CGSCC-PASS-NEXT: Finished CGSCC pass manager run @@ -409,7 +408,6 @@ ; CHECK-REPEAT-CGSCC-PASS-NEXT: Running analysis: InnerAnalysisManagerProxy<{{.*(FunctionAnalysisManager|AnalysisManager<.*Function.*>).*}},{{.*}}Module> ; CHECK-REPEAT-CGSCC-PASS-NEXT: Running analysis: LazyCallGraphAnalysis ; CHECK-REPEAT-CGSCC-PASS-NEXT: Running analysis: TargetLibraryAnalysis -; CHECK-REPEAT-CGSCC-PASS-NEXT: Running an SCC pass across the RefSCC: [(foo)] ; CHECK-REPEAT-CGSCC-PASS-NEXT: Starting CGSCC pass manager run ; CHECK-REPEAT-CGSCC-PASS-NEXT: Running pass: RepeatedPass ; CHECK-REPEAT-CGSCC-PASS-NEXT: Starting CGSCC pass manager run diff --git a/unittests/Analysis/CGSCCPassManagerTest.cpp b/unittests/Analysis/CGSCCPassManagerTest.cpp index e2481826597..aa0be73fdf2 100644 --- a/unittests/Analysis/CGSCCPassManagerTest.cpp +++ b/unittests/Analysis/CGSCCPassManagerTest.cpp @@ -1168,8 +1168,8 @@ TEST_F(CGSCCPassManagerTest, TestAnalysisInvalidationCGSCCUpdate) { "dummy", &*H2F.begin()->begin()); // Now update the call graph. - auto &NewC = updateCGAndAnalysisManagerForFunctionPass( - CG, C, H2N, AM, UR, /*DebugLogging*/ true); + auto &NewC = + updateCGAndAnalysisManagerForFunctionPass(CG, C, H2N, AM, UR); assert(&NewC != &C && "Should get a new SCC due to update!"); (void)&NewC; @@ -1214,8 +1214,8 @@ TEST_F(CGSCCPassManagerTest, TestAnalysisInvalidationCGSCCUpdate) { (void)CallInst::Create(&H3F, {}, "", &*H2F.begin()->begin()); // Now update the call graph. - auto &NewC = updateCGAndAnalysisManagerForFunctionPass( - CG, C, H2N, AM, UR, /*DebugLogging*/ true); + auto &NewC = + updateCGAndAnalysisManagerForFunctionPass(CG, C, H2N, AM, UR); assert(&NewC != &C && "Should get a new SCC due to update!"); (void)&NewC; -- 2.50.1