[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

@@ -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<const char *> *Allowed = nullptr;
+
+  /// Maximum number of iterations to run until fixpoint.
+  Optional<unsigned> MaxFixpointIterations = None;
+
+  /// A callback function that returns an ORE object from a Function pointer.
+  ///{
+  using OptimizationRemarkGetter =
+      function_ref<OptimizationRemarkEmitter &(Function *)>;
+  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<OptimizationRemarkEmitter &(Function *)>;
-
   /// 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 Configuration The Attributor configuration which determines what
-  /// \param Allowed If not null, a set limiting the attribute opportunities.
+  ///                      generic features to use.
-  /// \param DeleteFns Whether to delete functions.
-  /// \param RewriteSignatures Whether to rewrite function signatures.
-  /// \param DefaultInitializeLiveInternals Whether to initialize default AAs
-  ///                                       for live internal functions.
   Attributor(SetVector<Function *> &Functions, InformationCache &InfoCache,
-             CallGraphUpdater &CGUpdater,
+             AttributorConfig Configuration)
-             DenseSet<const char *> *Allowed = nullptr, bool DeleteFns = true,
-             bool RewriteSignatures = true,
-             bool DefaultInitializeLiveInternals = true)
       : Allocator(InfoCache.Allocator), Functions(Functions),
-        InfoCache(InfoCache), CGUpdater(CGUpdater), Allowed(Allowed),
+        InfoCache(InfoCache), Configuration(Configuration) {}
-        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<Function *> &Functions, InformationCache &InfoCache,
-             CallGraphUpdater &CGUpdater, DenseSet<const char *> *Allowed,
-             bool DeleteFns, bool RewriteSignatures,
-             Optional<unsigned> 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<OptimizationRemarkGetter>(OREGetter)),
-        PassName(PassName), DefaultInitializeLiveInternals(false) {}
 
   ~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 {
+  bool isModulePass() const { return Configuration.IsModulePass; }
-    return !Functions.empty() &&
-           Functions.size() == Functions.front()->getParent()->size();
-  }
 
   /// 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<Function &>(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 <typename RemarkKind, typename RemarkCallBack>
   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 <typename RemarkKind, typename RemarkCallBack>
   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<DepInfo, 8>;
   SmallVector<DependenceVector *, 16> DependenceStack;
 
-  /// If not null, a set limiting the attribute opportunities.
-  const DenseSet<const char *> *Allowed;
-
-  /// Whether to delete functions.
-  const bool DeleteFns;
-
-  /// Whether to rewrite signatures.
-  const bool RewriteSignatures;
-
-  /// Maximum number of fixedpoint iterations.
-  Optional<unsigned> MaxFixpointIterations;
-
   /// A set to remember the functions we already assume to be live and visited.
   DenseSet<const Function *> VisitedFunctions;
 
@@ -2151,22 +2148,12 @@ private:
   SmallSetVector<WeakVH, 8> ToBeDeletedInsts;
   ///}
 
-  /// Callback to get an OptimizationRemarkEmitter from a Function *.
-  Optional<OptimizationRemarkGetter> OREGetter;
-
   /// Container with all the query AAs that requested an update via
   /// registerForUpdate.
   SmallSetVector<AbstractAttribute *, 16> QueryAAsAwaitingUpdate;
 
-  /// The name of the pass to emit remarks for.
+  /// User provided configuration for this Attributor instance.
-  const char *PassName = "";
+  const AttributorConfig Configuration;
-
-  /// 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;
 
   friend AADepGraph;
   friend AttributorCallGraph;

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()) {

llvm/lib/Transforms/IPO/Attributor.cpp

@@ -1633,11 +1633,8 @@ void Attributor::runTillFixpoint() {
   // the abstract analysis.
 
   unsigned IterationCounter = 1;
-  unsigned MaxFixedPointIterations;
+  unsigned MaxIterations =
-  if (MaxFixpointIterations)
+      Configuration.MaxFixpointIterations.getValueOr(SetFixpointIterations);
-    MaxFixedPointIterations = MaxFixpointIterations.getValue();
-  else
-    MaxFixedPointIterations = SetFixpointIterations;
 
   SmallVector<AbstractAttribute *, 32> ChangedAAs;
   SetVector<AbstractAttribute *> Worklist, InvalidAAs;
@@ -1722,21 +1719,20 @@ void Attributor::runTillFixpoint() {
                     QueryAAsAwaitingUpdate.end());
     QueryAAsAwaitingUpdate.clear();
 
-  } while (!Worklist.empty() && (IterationCounter++ < MaxFixedPointIterations ||
+  } while (!Worklist.empty() &&
-                                 VerifyMaxFixpointIterations));
+           (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)
+                 << ore::NV("Iterations", MaxIterations) << " iterations.";
-                 << " iterations.";
     };
     Function *F = Functions.front();
     emitRemark<OptimizationRemarkMissed>(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 &&
+  if (VerifyMaxFixpointIterations && IterationCounter != MaxIterations) {
-      IterationCounter != MaxFixedPointIterations) {
     errs() << "\n[Attributor] Fixpoint iteration done after: "
-           << IterationCounter << "/" << MaxFixedPointIterations
+           << IterationCounter << "/" << MaxIterations << " iterations\n";
-           << " 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<IntrinsicInst>(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<Function *> &FnSet,
 bool Attributor::isValidFunctionSignatureRewrite(
     Argument &Arg, ArrayRef<Type *> 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<Function *> &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,
+  AttributorConfig AC(CGUpdater);
-               DeleteFns);
+  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<FunctionAnalysisManagerCGSCCProxy>();
@@ -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 {

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);