[PM] Sink the reference vs. value decision for IR units out of the

templated interface.

So far, every single IR unit I can come up with has address-identity.
That is, when two units of IR are both active in LLVM, their addresses
will be distinct of the IR is distinct. This is clearly true for
Modules, Functions, BasicBlocks, and Instructions. It turns out that the
only practical way to make the CGSCC stuff work the way we want is to
make it true for SCCs as well. I expect this pattern to continue.

When first designing the pass manager code, I kept this dimension of
freedom in the type parameters, essentially allowing for a wrapper-type
whose address did not form identity. But that really no longer makes
sense and is making the code more complex or subtle for no gain. If we
ever have an actual use case for this, we can figure out what makes
sense then and there. It will be better because then we will have the
actual example in hand.

While the simplifications afforded in this patch are fairly small
(mostly sinking the '&' out of many type parameters onto a few
interfaces), it would have become much more pronounced with subsequent
changes. I have a sequence of changes that will completely remove the
code duplication that currently exists between all of the pass managers
and analysis managers. =] Should make things much cleaner and avoid bug
fixing N times for the N pass managers.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@225723 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Analysis/CGSCCPassManager.h

@@ -51,13 +51,13 @@ public:
 
 private:
   // Pull in the concept type and model template specialized for SCCs.
-  typedef detail::PassConcept<LazyCallGraph::SCC &, CGSCCAnalysisManager>
+  typedef detail::PassConcept<LazyCallGraph::SCC, CGSCCAnalysisManager>
   CGSCCPassConcept;
   template <typename PassT>
   struct CGSCCPassModel
-      : detail::PassModel<LazyCallGraph::SCC &, CGSCCAnalysisManager, PassT> {
+      : detail::PassModel<LazyCallGraph::SCC, CGSCCAnalysisManager, PassT> {
     CGSCCPassModel(PassT Pass)
-        : detail::PassModel<LazyCallGraph::SCC &, CGSCCAnalysisManager, PassT>(
+        : detail::PassModel<LazyCallGraph::SCC, CGSCCAnalysisManager, PassT>(
               std::move(Pass)) {}
   };
 
@@ -70,11 +70,11 @@ private:
 /// \brief A function analysis manager to coordinate and cache analyses run over
 /// a module.
 class CGSCCAnalysisManager : public detail::AnalysisManagerBase<
-                                 CGSCCAnalysisManager, LazyCallGraph::SCC &> {
+                                 CGSCCAnalysisManager, LazyCallGraph::SCC> {
   friend class detail::AnalysisManagerBase<CGSCCAnalysisManager,
-                                           LazyCallGraph::SCC &>;
+                                           LazyCallGraph::SCC>;
   typedef detail::AnalysisManagerBase<CGSCCAnalysisManager,
-                                      LazyCallGraph::SCC &> BaseT;
+                                      LazyCallGraph::SCC> BaseT;
   typedef BaseT::ResultConceptT ResultConceptT;
   typedef BaseT::PassConceptT PassConceptT;
 
@@ -129,7 +129,7 @@ private:
   /// half of a bijection and provides storage for the actual result concept.
   typedef std::list<
       std::pair<void *, std::unique_ptr<detail::AnalysisResultConcept<
-                            LazyCallGraph::SCC &>>>> CGSCCAnalysisResultListT;
+                            LazyCallGraph::SCC>>>> CGSCCAnalysisResultListT;
 
   /// \brief Map type from function pointer to our custom list type.
   typedef DenseMap<LazyCallGraph::SCC *, CGSCCAnalysisResultListT>

include/llvm/IR/PassManager.h

@@ -203,13 +203,13 @@ public:
 
 private:
   // Pull in the concept type and model template specialized for modules.
-  typedef detail::PassConcept<Module &, ModuleAnalysisManager>
+  typedef detail::PassConcept<Module, ModuleAnalysisManager>
       ModulePassConcept;
   template <typename PassT>
   struct ModulePassModel
-      : detail::PassModel<Module &, ModuleAnalysisManager, PassT> {
+      : detail::PassModel<Module, ModuleAnalysisManager, PassT> {
     ModulePassModel(PassT Pass)
-        : detail::PassModel<Module &, ModuleAnalysisManager, PassT>(
+        : detail::PassModel<Module, ModuleAnalysisManager, PassT>(
               std::move(Pass)) {}
   };
 
@@ -268,13 +268,13 @@ public:
 
 private:
   // Pull in the concept type and model template specialized for functions.
-  typedef detail::PassConcept<Function &, FunctionAnalysisManager>
+  typedef detail::PassConcept<Function, FunctionAnalysisManager>
       FunctionPassConcept;
   template <typename PassT>
   struct FunctionPassModel
-      : detail::PassModel<Function &, FunctionAnalysisManager, PassT> {
+      : detail::PassModel<Function, FunctionAnalysisManager, PassT> {
     FunctionPassModel(PassT Pass)
-        : detail::PassModel<Function &, FunctionAnalysisManager, PassT>(
+        : detail::PassModel<Function, FunctionAnalysisManager, PassT>(
               std::move(Pass)) {}
   };
 
@@ -330,7 +330,7 @@ public:
   ///
   /// If there is not a valid cached result in the manager already, this will
   /// re-run the analysis to produce a valid result.
-  template <typename PassT> typename PassT::Result &getResult(IRUnitT IR) {
+  template <typename PassT> typename PassT::Result &getResult(IRUnitT &IR) {
     assert(AnalysisPasses.count(PassT::ID()) &&
            "This analysis pass was not registered prior to being queried");
 
@@ -347,7 +347,7 @@ public:
   ///
   /// \returns null if there is no cached result.
   template <typename PassT>
-  typename PassT::Result *getCachedResult(IRUnitT IR) const {
+  typename PassT::Result *getCachedResult(IRUnitT &IR) const {
     assert(AnalysisPasses.count(PassT::ID()) &&
            "This analysis pass was not registered prior to being queried");
 
@@ -376,7 +376,7 @@ public:
   /// \brief Invalidate a specific analysis pass for an IR module.
   ///
   /// Note that the analysis result can disregard invalidation.
-  template <typename PassT> void invalidate(IRUnitT IR) {
+  template <typename PassT> void invalidate(IRUnitT &IR) {
     assert(AnalysisPasses.count(PassT::ID()) &&
            "This analysis pass was not registered prior to being invalidated");
     derived_this()->invalidateImpl(PassT::ID(), IR);
@@ -389,7 +389,7 @@ public:
   /// We accept the PreservedAnalyses set by value and update it with each
   /// analyis pass which has been successfully invalidated and thus can be
   /// preserved going forward. The updated set is returned.
-  PreservedAnalyses invalidate(IRUnitT IR, PreservedAnalyses PA) {
+  PreservedAnalyses invalidate(IRUnitT &IR, PreservedAnalyses PA) {
     return derived_this()->invalidateImpl(IR, std::move(PA));
   }
 
@@ -423,9 +423,9 @@ private:
 /// \brief A module analysis pass manager with lazy running and caching of
 /// results.
 class ModuleAnalysisManager
-    : public detail::AnalysisManagerBase<ModuleAnalysisManager, Module &> {
-  friend class detail::AnalysisManagerBase<ModuleAnalysisManager, Module &>;
-  typedef detail::AnalysisManagerBase<ModuleAnalysisManager, Module &> BaseT;
+    : public detail::AnalysisManagerBase<ModuleAnalysisManager, Module> {
+  friend class detail::AnalysisManagerBase<ModuleAnalysisManager, Module>;
+  typedef detail::AnalysisManagerBase<ModuleAnalysisManager, Module> BaseT;
   typedef BaseT::ResultConceptT ResultConceptT;
   typedef BaseT::PassConceptT PassConceptT;
 
@@ -462,7 +462,7 @@ private:
   /// \brief Map type from module analysis pass ID to pass result concept
   /// pointer.
   typedef DenseMap<void *,
-                   std::unique_ptr<detail::AnalysisResultConcept<Module &>>>
+                   std::unique_ptr<detail::AnalysisResultConcept<Module>>>
       ModuleAnalysisResultMapT;
 
   /// \brief Cache of computed module analysis results for this module.
@@ -472,9 +472,9 @@ private:
 /// \brief A function analysis manager to coordinate and cache analyses run over
 /// a module.
 class FunctionAnalysisManager
-    : public detail::AnalysisManagerBase<FunctionAnalysisManager, Function &> {
-  friend class detail::AnalysisManagerBase<FunctionAnalysisManager, Function &>;
-  typedef detail::AnalysisManagerBase<FunctionAnalysisManager, Function &>
+    : public detail::AnalysisManagerBase<FunctionAnalysisManager, Function> {
+  friend class detail::AnalysisManagerBase<FunctionAnalysisManager, Function>;
+  typedef detail::AnalysisManagerBase<FunctionAnalysisManager, Function>
       BaseT;
   typedef BaseT::ResultConceptT ResultConceptT;
   typedef BaseT::PassConceptT PassConceptT;
@@ -529,7 +529,7 @@ private:
   /// erases. Provides both the pass ID and concept pointer such that it is
   /// half of a bijection and provides storage for the actual result concept.
   typedef std::list<std::pair<
-      void *, std::unique_ptr<detail::AnalysisResultConcept<Function &>>>>
+      void *, std::unique_ptr<detail::AnalysisResultConcept<Function>>>>
       FunctionAnalysisResultListT;
 
   /// \brief Map type from function pointer to our custom list type.
@@ -788,10 +788,10 @@ template <typename AnalysisT> struct RequireAnalysisPass {
   /// provided they satisfy the basic API requirements. When this pass is
   /// created, these methods can be instantiated to satisfy whatever the
   /// context requires.
-  template <typename T, typename AnalysisManagerT>
-  PreservedAnalyses run(T &&Arg, AnalysisManagerT *AM) {
+  template <typename IRUnitT, typename AnalysisManagerT>
+  PreservedAnalyses run(IRUnitT &Arg, AnalysisManagerT *AM) {
     if (AM)
-      (void)AM->template getResult<AnalysisT>(std::forward<T>(Arg));
+      (void)AM->template getResult<AnalysisT>(Arg);
 
     return PreservedAnalyses::all();
   }
@@ -811,12 +811,12 @@ template <typename AnalysisT> struct InvalidateAnalysisPass {
   /// provided they satisfy the basic API requirements. When this pass is
   /// created, these methods can be instantiated to satisfy whatever the
   /// context requires.
-  template <typename T, typename AnalysisManagerT>
-  PreservedAnalyses run(T &&Arg, AnalysisManagerT *AM) {
+  template <typename IRUnitT, typename AnalysisManagerT>
+  PreservedAnalyses run(IRUnitT &Arg, AnalysisManagerT *AM) {
     if (AM)
       // We have to directly invalidate the analysis result as we can't
       // enumerate all other analyses and use the preserved set to control it.
-      (void)AM->template invalidate<AnalysisT>(std::forward<T>(Arg));
+      (void)AM->template invalidate<AnalysisT>(Arg);
 
     return PreservedAnalyses::all();
   }

include/llvm/IR/PassManagerInternal.h

@@ -40,7 +40,7 @@ template <typename IRUnitT, typename AnalysisManagerT> struct PassConcept {
   /// Note that actual pass object can omit the analysis manager argument if
   /// desired. Also that the analysis manager may be null if there is no
   /// analysis manager in the pass pipeline.
-  virtual PreservedAnalyses run(IRUnitT IR, AnalysisManagerT *AM) = 0;
+  virtual PreservedAnalyses run(IRUnitT &IR, AnalysisManagerT *AM) = 0;
 
   /// \brief Polymorphic method to access the name of a pass.
   virtual StringRef name() = 0;
@@ -56,7 +56,7 @@ class PassRunAcceptsAnalysisManager {
     char a, b;
   };
 
-  template <typename T, ResultT (T::*)(IRUnitT, AnalysisManagerT *)>
+  template <typename T, ResultT (T::*)(IRUnitT &, AnalysisManagerT *)>
   struct Checker;
 
   template <typename T> static SmallType f(Checker<T, &T::run> *);
@@ -97,7 +97,7 @@ struct PassModel<IRUnitT, AnalysisManagerT, PassT, PreservedAnalysesT, true>
     return *this;
   }
 
-  PreservedAnalysesT run(IRUnitT IR, AnalysisManagerT *AM) override {
+  PreservedAnalysesT run(IRUnitT &IR, AnalysisManagerT *AM) override {
     return Pass.run(IR, AM);
   }
   StringRef name() override { return PassT::name(); }
@@ -124,7 +124,7 @@ struct PassModel<IRUnitT, AnalysisManagerT, PassT, PreservedAnalysesT, false>
     return *this;
   }
 
-  PreservedAnalysesT run(IRUnitT IR, AnalysisManagerT *AM) override {
+  PreservedAnalysesT run(IRUnitT &IR, AnalysisManagerT *AM) override {
     return Pass.run(IR);
   }
   StringRef name() override { return PassT::name(); }
@@ -148,7 +148,7 @@ template <typename IRUnitT> struct AnalysisResultConcept {
   /// took care to update or preserve the analysis result in some way.
   ///
   /// \returns true if the result is indeed invalid (the default).
-  virtual bool invalidate(IRUnitT IR, const PreservedAnalyses &PA) = 0;
+  virtual bool invalidate(IRUnitT &IR, const PreservedAnalyses &PA) = 0;
 };
 
 /// \brief SFINAE metafunction for computing whether \c ResultT provides an
@@ -159,7 +159,7 @@ template <typename IRUnitT, typename ResultT> class ResultHasInvalidateMethod {
     char a, b;
   };
 
-  template <typename T, bool (T::*)(IRUnitT, const PreservedAnalyses &)>
+  template <typename T, bool (T::*)(IRUnitT &, const PreservedAnalyses &)>
   struct Checker;
 
   template <typename T> static SmallType f(Checker<T, &T::invalidate> *);
@@ -207,7 +207,7 @@ struct AnalysisResultModel<IRUnitT, PassT, ResultT, PreservedAnalysesT, false>
   // FIXME: We should actually use two different concepts for analysis results
   // rather than two different models, and avoid the indirect function call for
   // ones that use the trivial behavior.
-  bool invalidate(IRUnitT, const PreservedAnalysesT &PA) override {
+  bool invalidate(IRUnitT &, const PreservedAnalysesT &PA) override {
     return !PA.preserved(PassT::ID());
   }
 
@@ -236,7 +236,7 @@ struct AnalysisResultModel<IRUnitT, PassT, ResultT, PreservedAnalysesT, true>
   }
 
   /// \brief The model delegates to the \c ResultT method.
-  bool invalidate(IRUnitT IR, const PreservedAnalysesT &PA) override {
+  bool invalidate(IRUnitT &IR, const PreservedAnalysesT &PA) override {
     return Result.invalidate(IR, PA);
   }
 
@@ -255,7 +255,7 @@ struct AnalysisPassConcept {
   /// \returns A unique_ptr to the analysis result object to be queried by
   /// users.
   virtual std::unique_ptr<AnalysisResultConcept<IRUnitT>>
-  run(IRUnitT IR, AnalysisManagerT *AM) = 0;
+  run(IRUnitT &IR, AnalysisManagerT *AM) = 0;
 
   /// \brief Polymorphic method to access the name of a pass.
   virtual StringRef name() = 0;
@@ -298,7 +298,7 @@ struct AnalysisPassModel<IRUnitT, AnalysisManagerT, PassT, true>
   ///
   /// The return is wrapped in an \c AnalysisResultModel.
   std::unique_ptr<AnalysisResultConcept<IRUnitT>>
-  run(IRUnitT IR, AnalysisManagerT *AM) override {
+  run(IRUnitT &IR, AnalysisManagerT *AM) override {
     return make_unique<ResultModelT>(Pass.run(IR, AM));
   }
 
@@ -337,7 +337,7 @@ struct AnalysisPassModel<IRUnitT, AnalysisManagerT, PassT, false>
   ///
   /// The return is wrapped in an \c AnalysisResultModel.
   std::unique_ptr<AnalysisResultConcept<IRUnitT>>
-  run(IRUnitT IR, AnalysisManagerT *) override {
+  run(IRUnitT &IR, AnalysisManagerT *) override {
     return make_unique<ResultModelT>(Pass.run(IR));
   }
 

lib/IR/PassManager.cpp

@@ -57,7 +57,7 @@ ModuleAnalysisManager::getResultImpl(void *PassID, Module &M) {
   ModuleAnalysisResultMapT::iterator RI;
   bool Inserted;
   std::tie(RI, Inserted) = ModuleAnalysisResults.insert(std::make_pair(
-      PassID, std::unique_ptr<detail::AnalysisResultConcept<Module &>>()));
+      PassID, std::unique_ptr<detail::AnalysisResultConcept<Module>>()));
 
   // If we don't have a cached result for this module, look up the pass and run
   // it to produce a result, which we then add to the cache.