From 3be3b401888ace347018abd077912868bbc416b3 Mon Sep 17 00:00:00 2001 From: Johannes Doerfert Date: Fri, 15 Apr 2022 16:15:07 -0500 Subject: [PATCH] [Attributor][NFCI] Introduce AttributorConfig to bundle all options Instead of lengthy constructors we can now set the members of a read-only struct before the Attributor is created. Should make it clearer what is configurable and also help introducing new options in the future. This actually added IsModulePass and avoids deduction through the Function set size. No functional change was intended. --- llvm/include/llvm/Transforms/IPO/Attributor.h | 159 ++++++++---------- llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp | 7 +- llvm/lib/Transforms/IPO/Attributor.cpp | 57 +++---- llvm/lib/Transforms/IPO/OpenMPOpt.cpp | 35 +++- 4 files changed, 136 insertions(+), 122 deletions(-) diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h index 8222c26100c9..a3109bcb5863 100644 --- a/llvm/include/llvm/Transforms/IPO/Attributor.h +++ b/llvm/include/llvm/Transforms/IPO/Attributor.h @@ -1187,6 +1187,53 @@ private: friend struct Attributor; }; +/// Configuration for the Attributor. +struct AttributorConfig { + + AttributorConfig(CallGraphUpdater &CGUpdater) : CGUpdater(CGUpdater) {} + + /// Is the user of the Attributor a module pass or not. This determines what + /// IR we can look at and modify. If it is a module pass we might deduce facts + /// outside the initial function set and modify functions outside that set, + /// but only as part of the optimization of the functions in the initial + /// function set. For CGSCC passes we can look at the IR of the module slice + /// but never run any deduction, or perform any modification, outside the + /// initial function set (which we assume is the SCC). + bool IsModulePass = true; + + /// Flag to determine if we can delete functions or keep dead ones around. + bool DeleteFns = true; + + /// Flag to determine if we rewrite function signatures. + bool RewriteSignatures = true; + + /// Flag to determine if we want to initialize all default AAs for an internal + /// function marked live. + /// TODO: This should probably be a callback, or maybe + /// identifyDefaultAbstractAttributes should be virtual, something to allow + /// customizable lazy initialization for internal functions. + bool DefaultInitializeLiveInternals = true; + + /// Helper to update an underlying call graph and to delete functions. + CallGraphUpdater &CGUpdater; + + /// If not null, a set limiting the attribute opportunities. + DenseSet *Allowed = nullptr; + + /// Maximum number of iterations to run until fixpoint. + Optional MaxFixpointIterations = None; + + /// A callback function that returns an ORE object from a Function pointer. + ///{ + using OptimizationRemarkGetter = + function_ref; + OptimizationRemarkGetter OREGetter = nullptr; + ///} + + /// The name of the pass running the attributor, used to emit remarks. + const char *PassName = nullptr; +}; + /// The fixpoint analysis framework that orchestrates the attribute deduction. /// /// The Attributor provides a general abstract analysis framework (guided @@ -1216,56 +1263,17 @@ private: /// described in the file comment. struct Attributor { - using OptimizationRemarkGetter = - function_ref; - /// Constructor /// /// \param Functions The set of functions we are deriving attributes for. /// \param InfoCache Cache to hold various information accessible for /// the abstract attributes. - /// \param CGUpdater Helper to update an underlying call graph. - /// \param Allowed If not null, a set limiting the attribute opportunities. - /// \param DeleteFns Whether to delete functions. - /// \param RewriteSignatures Whether to rewrite function signatures. - /// \param DefaultInitializeLiveInternals Whether to initialize default AAs - /// for live internal functions. + /// \param Configuration The Attributor configuration which determines what + /// generic features to use. Attributor(SetVector &Functions, InformationCache &InfoCache, - CallGraphUpdater &CGUpdater, - DenseSet *Allowed = nullptr, bool DeleteFns = true, - bool RewriteSignatures = true, - bool DefaultInitializeLiveInternals = true) + AttributorConfig Configuration) : Allocator(InfoCache.Allocator), Functions(Functions), - InfoCache(InfoCache), CGUpdater(CGUpdater), Allowed(Allowed), - DeleteFns(DeleteFns), RewriteSignatures(RewriteSignatures), - MaxFixpointIterations(None), OREGetter(None), PassName(""), - DefaultInitializeLiveInternals(DefaultInitializeLiveInternals) {} - - /// Constructor - /// - /// \param Functions The set of functions we are deriving attributes for. - /// \param InfoCache Cache to hold various information accessible for - /// the abstract attributes. - /// \param CGUpdater Helper to update an underlying call graph. - /// \param Allowed If not null, a set limiting the attribute opportunities. - /// \param DeleteFns Whether to delete functions - /// \param RewriteSignatures Whether to rewrite function signatures. - /// \param MaxFixpointIterations Maximum number of iterations to run until - /// fixpoint. - /// \param OREGetter A callback function that returns an ORE object from a - /// Function pointer. - /// \param PassName The name of the pass emitting remarks. - Attributor(SetVector &Functions, InformationCache &InfoCache, - CallGraphUpdater &CGUpdater, DenseSet *Allowed, - bool DeleteFns, bool RewriteSignatures, - Optional MaxFixpointIterations, - OptimizationRemarkGetter OREGetter, const char *PassName) - : Allocator(InfoCache.Allocator), Functions(Functions), - InfoCache(InfoCache), CGUpdater(CGUpdater), Allowed(Allowed), - DeleteFns(DeleteFns), RewriteSignatures(RewriteSignatures), - MaxFixpointIterations(MaxFixpointIterations), - OREGetter(Optional(OREGetter)), - PassName(PassName), DefaultInitializeLiveInternals(false) {} + InfoCache(InfoCache), Configuration(Configuration) {} ~Attributor(); @@ -1349,7 +1357,8 @@ struct Attributor { registerAA(AA); // For now we ignore naked and optnone functions. - bool Invalidate = Allowed && !Allowed->count(&AAType::ID); + bool Invalidate = + Configuration.Allowed && !Configuration.Allowed->count(&AAType::ID); const Function *FnScope = IRP.getAnchorScope(); if (FnScope) Invalidate |= FnScope->hasFnAttribute(Attribute::Naked) || @@ -1489,10 +1498,7 @@ struct Attributor { InformationCache &getInfoCache() { return InfoCache; } /// Return true if this is a module pass, false otherwise. - bool isModulePass() const { - return !Functions.empty() && - Functions.size() == Functions.front()->getParent()->size(); - } + bool isModulePass() const { return Configuration.IsModulePass; } /// Return true if we derive attributes for \p Fn bool isRunOn(Function &Fn) const { @@ -1527,7 +1533,7 @@ struct Attributor { assert(F.hasLocalLinkage() && "Only local linkage is assumed dead initially."); - if (DefaultInitializeLiveInternals) + if (Configuration.DefaultInitializeLiveInternals) identifyDefaultAbstractAttributes(const_cast(F)); } @@ -1536,7 +1542,7 @@ struct Attributor { if (!CI) return; - CGUpdater.removeCallSite(*CI); + Configuration.CGUpdater.removeCallSite(*CI); } /// Record that \p U is to be replaces with \p NV after information was @@ -1598,7 +1604,9 @@ struct Attributor { /// Record that \p F is deleted after information was manifested. void deleteAfterManifest(Function &F) { - if (DeleteFns) + errs() << "Delete " << F.getName() << " : " << (Configuration.DeleteFns) + << "\n"; + if (Configuration.DeleteFns) ToBeDeletedFunctions.insert(&F); } @@ -1733,37 +1741,41 @@ public: template void emitRemark(Instruction *I, StringRef RemarkName, RemarkCallBack &&RemarkCB) const { - if (!OREGetter) + if (!Configuration.OREGetter) return; Function *F = I->getFunction(); - auto &ORE = OREGetter.getValue()(F); + auto &ORE = Configuration.OREGetter(F); if (RemarkName.startswith("OMP")) ORE.emit([&]() { - return RemarkCB(RemarkKind(PassName, RemarkName, I)) + return RemarkCB(RemarkKind(Configuration.PassName, RemarkName, I)) << " [" << RemarkName << "]"; }); else - ORE.emit([&]() { return RemarkCB(RemarkKind(PassName, RemarkName, I)); }); + ORE.emit([&]() { + return RemarkCB(RemarkKind(Configuration.PassName, RemarkName, I)); + }); } /// Emit a remark on a function. template void emitRemark(Function *F, StringRef RemarkName, RemarkCallBack &&RemarkCB) const { - if (!OREGetter) + if (!Configuration.OREGetter) return; - auto &ORE = OREGetter.getValue()(F); + auto &ORE = Configuration.OREGetter(F); if (RemarkName.startswith("OMP")) ORE.emit([&]() { - return RemarkCB(RemarkKind(PassName, RemarkName, F)) + return RemarkCB(RemarkKind(Configuration.PassName, RemarkName, F)) << " [" << RemarkName << "]"; }); else - ORE.emit([&]() { return RemarkCB(RemarkKind(PassName, RemarkName, F)); }); + ORE.emit([&]() { + return RemarkCB(RemarkKind(Configuration.PassName, RemarkName, F)); + }); } /// Helper struct used in the communication between an abstract attribute (AA) @@ -2073,9 +2085,6 @@ private: /// The information cache that holds pre-processed (LLVM-IR) information. InformationCache &InfoCache; - /// Helper to update an underlying call graph. - CallGraphUpdater &CGUpdater; - /// Abstract Attribute dependency graph AADepGraph DG; @@ -2101,18 +2110,6 @@ private: using DependenceVector = SmallVector; SmallVector DependenceStack; - /// If not null, a set limiting the attribute opportunities. - const DenseSet *Allowed; - - /// Whether to delete functions. - const bool DeleteFns; - - /// Whether to rewrite signatures. - const bool RewriteSignatures; - - /// Maximum number of fixedpoint iterations. - Optional MaxFixpointIterations; - /// A set to remember the functions we already assume to be live and visited. DenseSet VisitedFunctions; @@ -2151,22 +2148,12 @@ private: SmallSetVector ToBeDeletedInsts; ///} - /// Callback to get an OptimizationRemarkEmitter from a Function *. - Optional OREGetter; - /// Container with all the query AAs that requested an update via /// registerForUpdate. SmallSetVector QueryAAsAwaitingUpdate; - /// The name of the pass to emit remarks for. - const char *PassName = ""; - - /// Flag to determine if we want to initialize all default AAs for an internal - /// function marked live. - /// TODO: This should probably be a callback, or maybe - /// identifyDefaultAbstractAttributes should be virtual, something to allow - /// customizable lazy initialization for internal functions. - const bool DefaultInitializeLiveInternals; + /// User provided configuration for this Attributor instance. + const AttributorConfig Configuration; friend AADepGraph; friend AttributorCallGraph; diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp index 2f1945c8f032..8de0d7e6bff1 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp @@ -745,7 +745,12 @@ public: {&AAAMDAttributes::ID, &AAUniformWorkGroupSize::ID, &AAAMDFlatWorkGroupSize::ID, &AACallEdges::ID, &AAPointerInfo::ID}); - Attributor A(Functions, InfoCache, CGUpdater, &Allowed); + AttributorConfig AC(CGUpdater); + AC.Allowed = &Allowed; + AC.IsModulePass = true; + AC.DefaultInitializeLiveInternals = false; + + Attributor A(Functions, InfoCache, AC); for (Function &F : M) { if (!F.isIntrinsic()) { diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp index 5af5beb6561b..247da10d8769 100644 --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -1633,11 +1633,8 @@ void Attributor::runTillFixpoint() { // the abstract analysis. unsigned IterationCounter = 1; - unsigned MaxFixedPointIterations; - if (MaxFixpointIterations) - MaxFixedPointIterations = MaxFixpointIterations.getValue(); - else - MaxFixedPointIterations = SetFixpointIterations; + unsigned MaxIterations = + Configuration.MaxFixpointIterations.getValueOr(SetFixpointIterations); SmallVector ChangedAAs; SetVector Worklist, InvalidAAs; @@ -1722,21 +1719,20 @@ void Attributor::runTillFixpoint() { QueryAAsAwaitingUpdate.end()); QueryAAsAwaitingUpdate.clear(); - } while (!Worklist.empty() && (IterationCounter++ < MaxFixedPointIterations || - VerifyMaxFixpointIterations)); + } while (!Worklist.empty() && + (IterationCounter++ < MaxIterations || VerifyMaxFixpointIterations)); - if (IterationCounter > MaxFixedPointIterations && !Functions.empty()) { + if (IterationCounter > MaxIterations && !Functions.empty()) { auto Remark = [&](OptimizationRemarkMissed ORM) { return ORM << "Attributor did not reach a fixpoint after " - << ore::NV("Iterations", MaxFixedPointIterations) - << " iterations."; + << ore::NV("Iterations", MaxIterations) << " iterations."; }; Function *F = Functions.front(); emitRemark(F, "FixedPoint", Remark); } LLVM_DEBUG(dbgs() << "\n[Attributor] Fixpoint iteration done after: " - << IterationCounter << "/" << MaxFixpointIterations + << IterationCounter << "/" << MaxIterations << " iterations\n"); // Reset abstract arguments not settled in a sound fixpoint by now. This @@ -1770,11 +1766,9 @@ void Attributor::runTillFixpoint() { << " abstract attributes.\n"; }); - if (VerifyMaxFixpointIterations && - IterationCounter != MaxFixedPointIterations) { + if (VerifyMaxFixpointIterations && IterationCounter != MaxIterations) { errs() << "\n[Attributor] Fixpoint iteration done after: " - << IterationCounter << "/" << MaxFixedPointIterations - << " iterations\n"; + << IterationCounter << "/" << MaxIterations << " iterations\n"; llvm_unreachable("The fixpoint was not reached with exactly the number of " "specified iterations!"); } @@ -1863,7 +1857,7 @@ ChangeStatus Attributor::manifestAttributes() { void Attributor::identifyDeadInternalFunctions() { // Early exit if we don't intend to delete functions. - if (!DeleteFns) + if (!Configuration.DeleteFns) return; // Identify dead internal functions and delete them. This happens outside @@ -2051,7 +2045,7 @@ ChangeStatus Attributor::cleanupIR() { assert(isRunOn(*I->getFunction()) && "Cannot delete an instruction outside the current SCC!"); if (!isa(CB)) - CGUpdater.removeCallSite(*CB); + Configuration.CGUpdater.removeCallSite(*CB); } I->dropDroppableUses(); CGModifiedFunctions.insert(I->getFunction()); @@ -2100,12 +2094,12 @@ ChangeStatus Attributor::cleanupIR() { for (Function *Fn : CGModifiedFunctions) if (!ToBeDeletedFunctions.count(Fn) && Functions.count(Fn)) - CGUpdater.reanalyzeFunction(*Fn); + Configuration.CGUpdater.reanalyzeFunction(*Fn); for (Function *Fn : ToBeDeletedFunctions) { if (!Functions.count(Fn)) continue; - CGUpdater.removeFunction(*Fn); + Configuration.CGUpdater.removeFunction(*Fn); } if (!ToBeChangedUses.empty()) @@ -2344,7 +2338,7 @@ bool Attributor::internalizeFunctions(SmallPtrSetImpl &FnSet, bool Attributor::isValidFunctionSignatureRewrite( Argument &Arg, ArrayRef ReplacementTypes) { - if (!RewriteSignatures) + if (!Configuration.RewriteSignatures) return false; Function *Fn = Arg.getParent(); @@ -2636,13 +2630,13 @@ ChangeStatus Attributor::rewriteFunctionSignatures( assert(OldCB.getType() == NewCB.getType() && "Cannot handle call sites with different types!"); ModifiedFns.insert(OldCB.getFunction()); - CGUpdater.replaceCallSite(OldCB, NewCB); + Configuration.CGUpdater.replaceCallSite(OldCB, NewCB); OldCB.replaceAllUsesWith(&NewCB); OldCB.eraseFromParent(); } // Replace the function in the call graph (if any). - CGUpdater.replaceFunctionWith(*OldFn, *NewFn); + Configuration.CGUpdater.replaceFunctionWith(*OldFn, *NewFn); // If the old function was modified and needed to be reanalyzed, the new one // does now. @@ -3153,7 +3147,7 @@ static bool runAttributorOnFunctions(InformationCache &InfoCache, SetVector &Functions, AnalysisGetter &AG, CallGraphUpdater &CGUpdater, - bool DeleteFns) { + bool DeleteFns, bool IsModulePass) { if (Functions.empty()) return false; @@ -3166,8 +3160,10 @@ static bool runAttributorOnFunctions(InformationCache &InfoCache, // Create an Attributor and initially empty information cache that is filled // while we identify default attribute opportunities. - Attributor A(Functions, InfoCache, CGUpdater, /* Allowed */ nullptr, - DeleteFns); + AttributorConfig AC(CGUpdater); + AC.IsModulePass = IsModulePass; + AC.DeleteFns = DeleteFns; + Attributor A(Functions, InfoCache, AC); // Create shallow wrappers for all functions that are not IPO amendable if (AllowShallowWrappers) @@ -3272,7 +3268,7 @@ PreservedAnalyses AttributorPass::run(Module &M, ModuleAnalysisManager &AM) { BumpPtrAllocator Allocator; InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ nullptr); if (runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater, - /* DeleteFns */ true)) { + /* DeleteFns */ true, /* IsModulePass */ true)) { // FIXME: Think about passes we will preserve and add them here. return PreservedAnalyses::none(); } @@ -3300,7 +3296,8 @@ PreservedAnalyses AttributorCGSCCPass::run(LazyCallGraph::SCC &C, BumpPtrAllocator Allocator; InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ &Functions); if (runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater, - /* DeleteFns */ false)) { + /* DeleteFns */ false, + /* IsModulePass */ false)) { // FIXME: Think about passes we will preserve and add them here. PreservedAnalyses PA; PA.preserve(); @@ -3376,7 +3373,8 @@ struct AttributorLegacyPass : public ModulePass { BumpPtrAllocator Allocator; InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ nullptr); return runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater, - /* DeleteFns*/ true); + /* DeleteFns*/ true, + /* IsModulePass */ true); } void getAnalysisUsage(AnalysisUsage &AU) const override { @@ -3413,7 +3411,8 @@ struct AttributorCGSCCLegacyPass : public CallGraphSCCPass { BumpPtrAllocator Allocator; InformationCache InfoCache(M, AG, Allocator, /* CGSCC */ &Functions); return runAttributorOnFunctions(InfoCache, Functions, AG, CGUpdater, - /* DeleteFns */ false); + /* DeleteFns */ false, + /* IsModulePass */ false); } void getAnalysisUsage(AnalysisUsage &AU) const override { diff --git a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp index c4736521e475..4a911a12b6fa 100644 --- a/llvm/lib/Transforms/IPO/OpenMPOpt.cpp +++ b/llvm/lib/Transforms/IPO/OpenMPOpt.cpp @@ -5001,8 +5001,15 @@ PreservedAnalyses OpenMPOptPass::run(Module &M, ModuleAnalysisManager &AM) { unsigned MaxFixpointIterations = (isOpenMPDevice(M)) ? SetFixpointIterations : 32; - Attributor A(Functions, InfoCache, CGUpdater, nullptr, true, false, - MaxFixpointIterations, OREGetter, DEBUG_TYPE); + + AttributorConfig AC(CGUpdater); + AC.DefaultInitializeLiveInternals = false; + AC.RewriteSignatures = false; + AC.MaxFixpointIterations = MaxFixpointIterations; + AC.OREGetter = OREGetter; + AC.PassName = DEBUG_TYPE; + + Attributor A(Functions, InfoCache, AC); OpenMPOpt OMPOpt(SCC, CGUpdater, OREGetter, InfoCache, A); bool Changed = OMPOpt.run(true); @@ -5068,8 +5075,16 @@ PreservedAnalyses OpenMPOptCGSCCPass::run(LazyCallGraph::SCC &C, unsigned MaxFixpointIterations = (isOpenMPDevice(M)) ? SetFixpointIterations : 32; - Attributor A(Functions, InfoCache, CGUpdater, nullptr, false, true, - MaxFixpointIterations, OREGetter, DEBUG_TYPE); + + AttributorConfig AC(CGUpdater); + AC.DefaultInitializeLiveInternals = false; + AC.IsModulePass = false; + AC.RewriteSignatures = false; + AC.MaxFixpointIterations = MaxFixpointIterations; + AC.OREGetter = OREGetter; + AC.PassName = DEBUG_TYPE; + + Attributor A(Functions, InfoCache, AC); OpenMPOpt OMPOpt(SCC, CGUpdater, OREGetter, InfoCache, A); bool Changed = OMPOpt.run(false); @@ -5139,8 +5154,16 @@ struct OpenMPOptCGSCCLegacyPass : public CallGraphSCCPass { unsigned MaxFixpointIterations = (isOpenMPDevice(M)) ? SetFixpointIterations : 32; - Attributor A(Functions, InfoCache, CGUpdater, nullptr, false, true, - MaxFixpointIterations, OREGetter, DEBUG_TYPE); + + AttributorConfig AC(CGUpdater); + AC.DefaultInitializeLiveInternals = false; + AC.IsModulePass = false; + AC.RewriteSignatures = false; + AC.MaxFixpointIterations = MaxFixpointIterations; + AC.OREGetter = OREGetter; + AC.PassName = DEBUG_TYPE; + + Attributor A(Functions, InfoCache, AC); OpenMPOpt OMPOpt(SCC, CGUpdater, OREGetter, InfoCache, A); bool Result = OMPOpt.run(false);