Making Instrumentation aware of LoopNest Pass

[raghesh]

Summary

Intrumentation callbacks are not made aware of LoopNest passes. From the loop pass manager, we can pass the outermost loop of the LoopNest to instrumentation in case of LoopNest passes.

The current patch made the change in two places in StandardInstrumentation.cpp. I will submit a proper patch where the OuterMostLoop is passed from the LoopPassManager to the call backs. That way we will avoid making changes at multiple places in StandardInstrumentation.cpp.

A testcase also will be submitted.

[aeubanks]

I thought we were going to make changes to always pass the Loop (as mentioned in the description), and not do this patch

[raghesh]

In D102463#2760201, @aeubanks wrote:

I thought we were going to make changes to always pass the Loop (as mentioned in the description), and not do this patch

Yes. I would like to amend this patch in such a way finally as mentioned in the description. Created this so that I don't miss to follow it up.

[raghesh]

Passing the outermost loop from LoopPassManager to the call backs in case of LoopNest pass.

[aeubanks]

do you have a test case? probably a super simple one that crashes without this patch is good enough
it probably goes somewhere in llvm/test/Other/

[aeubanks]

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

363–365

IR is either a Loop& or a LoopNest& right? not an Any
so we need to extract a Loop out of either Loop or LoopNest. with templates, it should be as simple as creating two functions with the same name that both return a Loop&, where one takes Loop& and the other takes a LoopNest&

[raghesh]

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

363–365

Sorry!! I dont really get you. Are you suggesting something similar to the following

@@ -183,6 +183,9 @@ protected:

PreservedAnalyses runWithoutLoopNestPasses(Loop &L, LoopAnalysisManager &AM,
                                           LoopStandardAnalysisResults &AR,
                                           LPMUpdater &U);

+private:
+ const Loop& getLoopFromIR(Loop &L) { return L; }
+ const Loop& getLoopFromIR(LoopNest &LN) { return LN.getOutermostLoop(); }
};

/// The Loop pass manager.
@@ -360,12 +363,10 @@ Optional<PreservedAnalyses> LoopPassManager::runSinglePass(

  LoopStandardAnalysisResults &AR, LPMUpdater &U, PassInstrumentation &PI) {
// Pass the outermost loop to BeforePass and AfterPass callbacks in case of
// LoopNest Pass.

+ const Loop &L = getLoopFromIR(IR);

// Check the PassInstrumentation's BeforePass callbacks before running the
// pass, skip its execution completely if asked to (callback returns false).

• if (!PI.runBeforePass<Loop>(*Pass, *L))

+ if (!PI.runBeforePass<Loop>(*Pass, L))

  return None;

PreservedAnalyses PA;

@@ -378,7 +379,7 @@ Optional<PreservedAnalyses> LoopPassManager::runSinglePass(

if (U.skipCurrentLoop())
  PI.runAfterPassInvalidated<IRUnitT>(*Pass, PA);
else

• PI.runAfterPass<Loop>(*Pass, *L, PA);

+ PI.runAfterPass<Loop>(*Pass, L, PA);

return PA;

}

[aeubanks]

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

363–365

yup that's right

I don't think the current patch does anything because there's no Any in this code (a test case would be nice)

[raghesh]

Made the suggested change. Added a simple testcase which uses -print-after for LoopInterchange pass (a LoopNest pass). This would crash without this patch.

[aeubanks]

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

188

these probably make more sense as static functions, but not a huge deal

llvm/test/Other/loopnest-callback.ll

1 ↗

(On Diff #346739)

-passes=loop-interchange

2 ↗

(On Diff #346739)

--print-after-all, --print-after isn't super well supported in the new PM due to pass naming being weird

3 ↗

(On Diff #346739)

this is already the default, just FileCheck %s works

[raghesh]

Addressed the comments. Changed functions to static and made testcase changes

[aeubanks]

looks good, thanks!
can you land this yourself or do you need help doing that?

[raghesh]

In D102463#2777867, @aeubanks wrote:

looks good, thanks!
can you land this yourself or do you need help doing that?

I do not have commit access. Please push it for me. Thanks a lot for your help and reviews with this.

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

protected:
   PreservedAnalyses runWithLoopNestPasses(Loop &L, LoopAnalysisManager &AM,
                                           LoopStandardAnalysisResults &AR,
                                           LPMUpdater &U);
   PreservedAnalyses runWithoutLoopNestPasses(Loop &L, LoopAnalysisManager &AM,
                                              LoopStandardAnalysisResults &AR,
                                              LPMUpdater &U);
 };
 
 /// The Loop pass manager.
 ///
 /// See the documentation for the PassManager template for details. It runs
 /// a sequence of Loop passes over each Loop that the manager is run over. This
 /// typedef serves as a convenient way to refer to this construct.
 typedef PassManager<Loop, LoopAnalysisManager, LoopStandardAnalysisResults &,
                     LPMUpdater &>
     LoopPassManager;
 
LPMUpdater(SmallPriorityWorklist<Loop *, 4> &Worklist,
              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) {
+  // Pass the outermost loop to BeforePass and AfterPass callbacks in case of
+  // LoopNest Pass.
+  const Loop *L = any_isa<const LoopNest *>(&IR)
+                      ? &any_cast<const LoopNest *>(&IR)->getOutermostLoop()
+                      : any_cast<const Loop *>(&IR);
   // Check the PassInstrumentation's BeforePass callbacks before running the
   // pass, skip its execution completely if asked to (callback returns false).
-  if (!PI.runBeforePass<IRUnitT>(*Pass, IR))
+  if (!PI.runBeforePass<Loop>(*Pass, *L))
     return None;
 
   PreservedAnalyses PA;
   {
     TimeTraceScope TimeScope(Pass->name(), IR.getName());
     PA = Pass->run(IR, AM, AR, U);
   }
 
   // do not pass deleted Loop into the instrumentation
   if (U.skipCurrentLoop())
     PI.runAfterPassInvalidated<IRUnitT>(*Pass, PA);
   else
-    PI.runAfterPass<IRUnitT>(*Pass, IR, PA);
+    PI.runAfterPass<Loop>(*Pass, *L, PA);
   return PA;
 }
 
 /// 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