[LoopNest] Extend `LPMUpdater` and adaptor to handle loop-nest passes

[TaWeiTu]

Summary

This is a follow-up patch of D87045.

The patch implements "loop-nest mode" for LPMUpdater and FunctionToLoopPassAdaptor in which only top-level loops are operated.

createFunctionToLoopPassAdaptor decides whether the returned adaptor is in loop-nest mode or not based on the given pass. If the pass is a loop-nest pass or the pass is a LoopPassManager which contains only loop-nest passes, the loop-nest version of adaptor is returned; otherwise, the normal (loop) version of adaptor is returned.

[Whitney]

LGTM.

[TaWeiTu]

Rebased on top of the current master. Add comments about loop/loop-nest mode.

[asbirlea]

llvm/include/llvm/Transforms/Scalar/LoopPassManager.h

242

s/will no/will not

264

legibility:
(!LoopNestMode || !L.getParentLoop())
or more detailed
(!LoopNestMode || (LoopNestMode && !L.getParentLoop()))
?

[TaWeiTu]

Address comments.

[TaWeiTu]

Rebased.

[asbirlea]

llvm/include/llvm/Transforms/Scalar/LoopPassManager.h

473

Could you clarify this?

[TaWeiTu]

Clarify the specializations of createFunctionToLoopPassAdaptor()

[TaWeiTu]

Rebase

[TaWeiTu]

Since D87045 has been landed, is there any objection against committing this patch as well?
I'll wait for a few days to see whether the reviewers are happy with the patch or not before landing it.
Thanks!

[Whitney]

Since D87045 has been landed, is there any objection against committing this patch as well?

No objection from me.

llvm/include/llvm/Transforms/Scalar/LoopPassManager.h

 #include "llvm/Transforms/Utils/LoopUtils.h"
 #include <memory>
 
 namespace llvm {
 
 // Forward declarations of an update tracking API used in the pass manager.
 class LPMUpdater;
 
+namespace {
+
+template <typename PassT>
+using HasRunOnLoopT = decltype(std::declval<PassT>().run(
+    std::declval<Loop &>(), std::declval<LoopAnalysisManager &>(),
+    std::declval<LoopStandardAnalysisResults &>(),
+    std::declval<LPMUpdater &>()));
+
+} // namespace
+
 // Explicit specialization and instantiation declarations for the pass manager.
 // See the comments on the definition of the specialization for details on how
 // it differs from the primary template.
 template <>
 class PassManager<Loop, LoopAnalysisManager, LoopStandardAnalysisResults &,
                   LPMUpdater &>
     : public PassInfoMixin<
           PassManager<Loop, LoopAnalysisManager, LoopStandardAnalysisResults &,
                       LPMUpdater &>> {
-private:
-  template <typename PassT>
-  using HasRunOnLoopT = decltype(std::declval<PassT>().run(
-      std::declval<Loop &>(), std::declval<LoopAnalysisManager &>(),
-      std::declval<LoopStandardAnalysisResults &>(),
-      std::declval<LPMUpdater &>()));
-
 public:
   /// Construct a pass manager.
   ///
   /// If \p DebugLogging is true, we'll log our progress to llvm::dbgs().
   explicit PassManager(bool DebugLogging = false)
       : DebugLogging(DebugLogging) {}
 
   // FIXME: These are equivalent to the default move constructor/move
addPass(RepeatedPass<PassT> Pass) {
     LoopNestPasses.emplace_back(
         new RepeatedLoopNestPassModelT(std::move(Pass)));
   }
 
   bool isEmpty() const { return LoopPasses.empty() && LoopNestPasses.empty(); }
 
   static bool isRequired() { return true; }
 
+  size_t getNumLoopPasses() const { return LoopPasses.size(); }
+  size_t getNumLoopNestPasses() const { return LoopNestPasses.size(); }
+
 protected:
   using LoopPassConceptT =
       detail::PassConcept<Loop, LoopAnalysisManager,
                           LoopStandardAnalysisResults &, LPMUpdater &>;
   using LoopNestPassConceptT =
       detail::PassConcept<LoopNest, LoopAnalysisManager,
                           LoopStandardAnalysisResults &, LPMUpdater &>;
 

 class FunctionToLoopPassAdaptor;
 
 /// This class provides an interface for updating the loop pass manager based
 /// on mutations to the loop nest.
 ///
 /// A reference to an instance of this class is passed as an argument to each
 /// Loop pass, and Loop passes should use it to update LPM infrastructure if
 /// they modify the loop nest structure.
+///
+/// \c LPMUpdater comes with two modes: the loop mode and the loop-nest mode. In
+/// loop mode, all the loops in the function will be pushed into the worklist
+/// and when new loops are added to the pipeline, their subloops are also
+/// inserted recursively. On the other hand, in loop-nest mode, only top-level
+/// loops are contained in the worklist and the addition of new (top-level)
+/// loops will not trigger the addition of their subloops.
 class LPMUpdater {
 public:
   /// This can be queried by loop passes which run other loop passes (like pass
   /// managers) to know whether the loop needs to be skipped due to updates to
   /// the loop nest.
   ///
   /// If this returns true, the loop object may have been deleted, so passes
   /// should take care not to touch the object.
   bool skipCurrentLoop() const { return SkipCurrentLoop; }
 
   /// Loop passes should use this method to indicate they have deleted a loop
   /// from the nest.
   ///
   /// Note that this loop must either be the current loop or a subloop of the
   /// current loop. This routine must be called prior to removing the loop from
   /// the loop nest.
   ///
   /// If this is called for the current loop, in addition to clearing any
   /// state, this routine will mark that the current loop should be skipped by
   /// the rest of the pass management infrastructure.
   void markLoopAsDeleted(Loop &L, llvm::StringRef Name) {
+    assert((!LoopNestMode || L.isOutermost()) &&
+           "L should be a top-level loop in loop-nest mode.");
     LAM.clear(L, Name);
     assert((&L == CurrentL || CurrentL->contains(&L)) &&
            "Cannot delete a loop outside of the "
            "subloop tree currently being processed.");
     if (&L == CurrentL)
       SkipCurrentLoop = true;
   }
 
   /// Loop passes should use this method to indicate they have added new child
   /// loops of the current loop.
   ///
   /// \p NewChildLoops must contain only the immediate children. Any nested
   /// loops within them will be visited in postorder as usual for the loop pass
   /// manager.
   void addChildLoops(ArrayRef<Loop *> NewChildLoops) {
+    assert(!LoopNestMode &&
+           "Child loops should not be pushed in loop-nest mode.");
     // Insert ourselves back into the worklist first, as this loop should be
     // revisited after all the children have been processed.
     Worklist.insert(CurrentL);
 
 #ifndef NDEBUG
     for (Loop *NewL : NewChildLoops)
       assert(NewL->getParentLoop() == CurrentL && "All of the new loops must "
                                                   "be immediate children of "
#endif
   /// manager.
   void addSiblingLoops(ArrayRef<Loop *> NewSibLoops) {
 #ifndef NDEBUG
     for (Loop *NewL : NewSibLoops)
       assert(NewL->getParentLoop() == ParentL &&
              "All of the new loops must be siblings of the current loop!");
 #endif
 
+    if (LoopNestMode)
+      Worklist.insert(NewSibLoops);
+    else
       appendLoopsToWorklist(NewSibLoops, Worklist);
 
     // No need to skip the current loop or revisit it, as sibling loops
     // shouldn't impact anything.
   }
 
   /// Restart the current loop.
   ///
   /// Loop passes should call this method to indicate the current loop has been
private:
   /// The \c FunctionToLoopPassAdaptor's worklist of loops to process.
   SmallPriorityWorklist<Loop *, 4> &Worklist;
 
   /// The analysis manager for use in the current loop nest.
   LoopAnalysisManager &LAM;
 
   Loop *CurrentL;
   bool SkipCurrentLoop;
+  const bool LoopNestMode;
 
 #ifndef NDEBUG
   // In debug builds we also track the parent loop to implement asserts even in
   // the face of loop deletion.
   Loop *ParentL;
 #endif
 
   LPMUpdater(SmallPriorityWorklist<Loop *, 4> &Worklist,
-             LoopAnalysisManager &LAM)
-      : Worklist(Worklist), LAM(LAM) {}
+             LoopAnalysisManager &LAM, bool LoopNestMode = false)
+      : Worklist(Worklist), LAM(LAM), LoopNestMode(LoopNestMode) {}
 };
 
 template <typename IRUnitT, typename PassT>
 Optional<PreservedAnalyses> LoopPassManager::runSinglePass(
     IRUnitT &IR, PassT &Pass, LoopAnalysisManager &AM,
     LoopStandardAnalysisResults &AR, LPMUpdater &U, PassInstrumentation &PI) {
   // Check the PassInstrumentation's BeforePass callbacks before running the
   // pass, skip its execution completely if asked to (callback returns false).

 
 /// Adaptor that maps from a function to its loops.
 ///
 /// Designed to allow composition of a LoopPass(Manager) and a
 /// FunctionPassManager. Note that if this pass is constructed with a \c
 /// FunctionAnalysisManager it will run the \c LoopAnalysisManagerFunctionProxy
 /// analysis prior to running the loop passes over the function to enable a \c
 /// LoopAnalysisManager to be used within this run safely.
+///
+/// The adaptor comes with two modes: the loop mode and the loop-nest mode, and
+/// the worklist updater lived inside will be in the same mode as the adaptor
+/// (refer to the documentation of \c LPMUpdater for more detailed explanation).
+/// Specifically, in loop mode, all loops in the funciton will be pushed into
+/// the worklist and processed by \p Pass, while only top-level loops are
+/// processed in loop-nest mode. Please refer to the various specializations of
+/// \fn createLoopFunctionToLoopPassAdaptor to see when loop mode and loop-nest
+/// mode are used.
 class FunctionToLoopPassAdaptor
     : public PassInfoMixin<FunctionToLoopPassAdaptor> {
 public:
   using PassConceptT =
       detail::PassConcept<Loop, LoopAnalysisManager,
                           LoopStandardAnalysisResults &, LPMUpdater &>;
 
   explicit FunctionToLoopPassAdaptor(std::unique_ptr<PassConceptT> Pass,
                                      bool UseMemorySSA = false,
                                      bool UseBlockFrequencyInfo = false,
-                                     bool DebugLogging = false)
+                                     bool DebugLogging = false,
+                                     bool LoopNestMode = false)
       : Pass(std::move(Pass)), LoopCanonicalizationFPM(DebugLogging),
         UseMemorySSA(UseMemorySSA),
-        UseBlockFrequencyInfo(UseBlockFrequencyInfo) {
+        UseBlockFrequencyInfo(UseBlockFrequencyInfo),
+        LoopNestMode(LoopNestMode) {
     LoopCanonicalizationFPM.addPass(LoopSimplifyPass());
     LoopCanonicalizationFPM.addPass(LCSSAPass());
   }
 
   /// Runs the loop passes across every loop in the function.
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 
   static bool isRequired() { return true; }
 
+  bool isLoopNestMode() const { return LoopNestMode; }
+
 private:
   std::unique_ptr<PassConceptT> Pass;
 
   FunctionPassManager LoopCanonicalizationFPM;
 
   bool UseMemorySSA = false;
   bool UseBlockFrequencyInfo = false;
+  const bool LoopNestMode;
 };
 
 /// A function to deduce a loop pass type and wrap it in the templated
 /// adaptor.
+///
+/// If \p Pass is a loop pass, the returned adaptor will be in loop mode.
 template <typename LoopPassT>
-FunctionToLoopPassAdaptor
+inline std::enable_if_t<is_detected<HasRunOnLoopT, LoopPassT>::value,
+                        FunctionToLoopPassAdaptor>
 createFunctionToLoopPassAdaptor(LoopPassT Pass, bool UseMemorySSA = false,
                                 bool UseBlockFrequencyInfo = false,
                                 bool DebugLogging = false) {
   using PassModelT =
       detail::PassModel<Loop, LoopPassT, PreservedAnalyses, LoopAnalysisManager,
                         LoopStandardAnalysisResults &, LPMUpdater &>;
   return FunctionToLoopPassAdaptor(
       std::make_unique<PassModelT>(std::move(Pass)), UseMemorySSA,
-      UseBlockFrequencyInfo, DebugLogging);
+      UseBlockFrequencyInfo, DebugLogging, false);
+}
+
+/// If \p Pass is a loop-nest pass, \p Pass will first be wrapped into a
+/// \c LoopPassManager and the returned adaptor will be in loop-nest mode.
+template <typename LoopNestPassT>
+inline std::enable_if_t<!is_detected<HasRunOnLoopT, LoopNestPassT>::value,
+                        FunctionToLoopPassAdaptor>
+createFunctionToLoopPassAdaptor(LoopNestPassT Pass, bool UseMemorySSA = false,
+                                bool UseBlockFrequencyInfo = false,
+                                bool DebugLogging = false) {
+  LoopPassManager LPM(DebugLogging);
+  LPM.addPass(std::move(Pass));
+  using PassModelT =
+      detail::PassModel<Loop, LoopPassManager, PreservedAnalyses,
+                        LoopAnalysisManager, LoopStandardAnalysisResults &,
+                        LPMUpdater &>;
+  return FunctionToLoopPassAdaptor(std::make_unique<PassModelT>(std::move(LPM)),
+                                   UseMemorySSA, UseBlockFrequencyInfo,
+                                   DebugLogging, true);
+}
+
+/// If \p Pass is an instance of \c LoopPassManager, the returned adaptor will
+/// be in loop-nest mode if the pass manager contains only loop-nest passes.
+template <>
+inline FunctionToLoopPassAdaptor
+createFunctionToLoopPassAdaptor<LoopPassManager>(LoopPassManager LPM,
+                                                 bool UseMemorySSA,
+                                                 bool UseBlockFrequencyInfo,
+                                                 bool DebugLogging) {
+  // Check if LPM contains any loop pass and if it does not, returns an adaptor
+  // in loop-nest mode.
+  using PassModelT =
+      detail::PassModel<Loop, LoopPassManager, PreservedAnalyses,
+                        LoopAnalysisManager, LoopStandardAnalysisResults &,
+                        LPMUpdater &>;
+  bool LoopNestMode = (LPM.getNumLoopPasses() == 0);
+  return FunctionToLoopPassAdaptor(std::make_unique<PassModelT>(std::move(LPM)),
+                                   UseMemorySSA, UseBlockFrequencyInfo,
+                                   DebugLogging, LoopNestMode);
 }
 
 /// Pass for printing a loop's contents as textual IR.
 class PrintLoopPass : public PassInfoMixin<PrintLoopPass> {
   raw_ostream &OS;
   std::string Banner;
 
 public:

llvm/lib/Transforms/Scalar/LoopPassManager.cpp

if (UseMemorySSA)
     LAMFP.markMSSAUsed();
   LoopAnalysisManager &LAM = LAMFP.getManager();
 
   // A postorder worklist of loops to process.
   SmallPriorityWorklist<Loop *, 4> Worklist;
 
   // Register the worklist and loop analysis manager so that loop passes can
   // update them when they mutate the loop nest structure.
-  LPMUpdater Updater(Worklist, LAM);
+  LPMUpdater Updater(Worklist, LAM, LoopNestMode);
 
   // Add the loop nests in the reverse order of LoopInfo. See method
   // declaration.
+  if (!LoopNestMode) {
     appendLoopsToWorklist(LI, Worklist);
+  } else {
+    for (Loop *L : LI)
+      Worklist.insert(L);
+  }
 
 #ifndef NDEBUG
   PI.pushBeforeNonSkippedPassCallback([&LAR, &LI](StringRef PassID, Any IR) {
     if (isSpecialPass(PassID, {"PassManager"}))
       return;
     assert(any_isa<const Loop *>(IR) || any_isa<const LoopNest *>(IR));
     const Loop *L = any_isa<const Loop *>(IR)
                         ? any_cast<const Loop *>(IR)
                         : &any_cast<const LoopNest *>(IR)->getOutermostLoop();
     assert(L && "Loop should be valid for printing");
 
     // Verify the loop structure and LCSSA form before visiting the loop.
     L->verifyLoop();
     assert(L->isRecursivelyLCSSAForm(LAR.DT, LI) &&
            "Loops must remain in LCSSA form!");
   });
 #endif
 
   do {
     Loop *L = Worklist.pop_back_val();
+    assert(!(LoopNestMode && L->getParentLoop()) &&
+           "L should be a top-level loop in loop-nest mode.");
 
     // Reset the update structure for this loop.
     Updater.CurrentL = L;
     Updater.SkipCurrentLoop = false;
 
 #ifndef NDEBUG
     // Save a parent loop pointer for asserts.
     Updater.ParentL = L->getParentLoop();

llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp

TEST_F(LoopPassManagerTest, LoopDeletion) {
 
   // Add the function pass pipeline now that it is fully built up and run it
   // over the module's one function.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   MPM.run(*M, MAM);
 }
 
 TEST_F(LoopPassManagerTest, HandleLoopNestPass) {
-  ::testing::InSequence MakeExpectationsSequenced;
+  ::testing::Sequence FSequence, GSequence;
+
+  EXPECT_CALL(MLPHandle, run(HasName("loop.0.0"), _, _, _))
+      .Times(2)
+      .InSequence(FSequence);
+  EXPECT_CALL(MLPHandle, run(HasName("loop.0.1"), _, _, _))
+      .Times(2)
+      .InSequence(FSequence);
+  EXPECT_CALL(MLPHandle, run(HasName("loop.0"), _, _, _)).InSequence(FSequence);
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.0"), _, _, _))
+      .InSequence(FSequence);
+  EXPECT_CALL(MLPHandle, run(HasName("loop.0"), _, _, _)).InSequence(FSequence);
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.0"), _, _, _))
+      .InSequence(FSequence);
+  EXPECT_CALL(MLPHandle, run(HasName("loop.g.0"), _, _, _))
+      .InSequence(GSequence);
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.g.0"), _, _, _))
+      .InSequence(GSequence);
+  EXPECT_CALL(MLPHandle, run(HasName("loop.g.0"), _, _, _))
+      .InSequence(GSequence);
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.g.0"), _, _, _))
+      .InSequence(GSequence);
+
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.0"), _, _, _))
+      .InSequence(FSequence);
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.g.0"), _, _, _))
+      .InSequence(GSequence);
+
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.0"), _, _, _))
+      .InSequence(FSequence);
+  EXPECT_CALL(MLNPHandle, run(HasName("loop.g.0"), _, _, _))
+      .InSequence(GSequence);
 
-  EXPECT_CALL(MLPHandle, run(HasName("loop.0.0"), _, _, _)).Times(2);
-  EXPECT_CALL(MLPHandle, run(HasName("loop.0.1"), _, _, _)).Times(2);
-  EXPECT_CALL(MLPHandle, run(HasName("loop.0"), _, _, _));
-  EXPECT_CALL(MLNPHandle, run(HasName("loop.0"), _, _, _));
-  EXPECT_CALL(MLPHandle, run(HasName("loop.0"), _, _, _));
-  EXPECT_CALL(MLNPHandle, run(HasName("loop.0"), _, _, _));
-  EXPECT_CALL(MLPHandle, run(HasName("loop.g.0"), _, _, _));
-  EXPECT_CALL(MLNPHandle, run(HasName("loop.g.0"), _, _, _));
-  EXPECT_CALL(MLPHandle, run(HasName("loop.g.0"), _, _, _));
-  EXPECT_CALL(MLNPHandle, run(HasName("loop.g.0"), _, _, _));
+  ModulePassManager MPM(true);
+  FunctionPassManager FPM(true);
 
+  {
     LoopPassManager LPM(true);
     LPM.addPass(MLPHandle.getPass());
     LPM.addPass(MLNPHandle.getPass());
     LPM.addPass(MLPHandle.getPass());
     LPM.addPass(MLNPHandle.getPass());
 
-  ModulePassManager MPM(true);
-  MPM.addPass(createModuleToFunctionPassAdaptor(
-      createFunctionToLoopPassAdaptor(std::move(LPM))));
+    auto Adaptor = createFunctionToLoopPassAdaptor(std::move(LPM));
+    ASSERT_FALSE(Adaptor.isLoopNestMode());
+    FPM.addPass(std::move(Adaptor));
+  }
+
+  {
+    auto Adaptor = createFunctionToLoopPassAdaptor(MLNPHandle.getPass());
+    ASSERT_TRUE(Adaptor.isLoopNestMode());
+    FPM.addPass(std::move(Adaptor));
+  }
+
+  {
+    LoopPassManager LPM(true);
+    LPM.addPass(MLNPHandle.getPass());
+    auto Adaptor = createFunctionToLoopPassAdaptor(MLNPHandle.getPass());
+    ASSERT_TRUE(Adaptor.isLoopNestMode());
+    FPM.addPass(std::move(Adaptor));
+  }
+
+  MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
   MPM.run(*M, MAM);
 }
 
 } // namespace