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 ↗

(On Diff #345754)

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 ↗

(On Diff #345754)

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 ↗

(On Diff #345754)

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 ↗

(On Diff #346739)

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/lib/Passes/StandardInstrumentations.cpp

 
 #include "llvm/Passes/StandardInstrumentations.h"
 #include "llvm/ADT/Any.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/CallGraphSCCPass.h"
 #include "llvm/Analysis/LazyCallGraph.h"
 #include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/LoopNestAnalysis.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/PrintPasses.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Support/CommandLine.h"
std::string getIRName(Any IR) {
   if (any_isa<const Loop *>(IR)) {
     const Loop *L = any_cast<const Loop *>(IR);
     std::string S;
     raw_string_ostream OS(S);
     L->print(OS, /*Verbose*/ false, /*PrintNested*/ false);
     return OS.str();
   }
 
+  if (any_isa<const LoopNest *>(IR)) {
+    const Loop *L = &any_cast<const LoopNest *>(IR)->getOutermostLoop();
+    std::string S;
+    raw_string_ostream OS(S);
+    assert(L && "Loop should be valid for printing");
+    L->print(OS, /*Verbose*/ false, /*PrintNested*/ false);
+    return OS.str();
+  }
+
   llvm_unreachable("Unknown wrapped IR type");
 }
 
 bool moduleContainsFilterPrintFunc(const Module &M) {
   return any_of(M.functions(),
                 [](const Function &F) {
                   return isFunctionInPrintList(F.getName());
                 }) ||
if (any_isa<const Function *>(IR)) {
     F = any_cast<const Function *>(IR);
   } else if (any_isa<const Loop *>(IR)) {
     F = any_cast<const Loop *>(IR)->getHeader()->getParent();
   }
   bool ShouldRun = !(F && F->hasOptNone());
   if (!ShouldRun && DebugLogging) {
     errs() << "Skipping pass " << PassID << " on " << F->getName()
            << " due to optnone attribute\n";
+  } else if (any_isa<const LoopNest *>(IR)) {
+    const Loop *L = &any_cast<const LoopNest *>(IR)->getOutermostLoop();
+    F = L->getHeader()->getParent();
   }
+
   return ShouldRun;
 }
 
 void OptBisectInstrumentation::registerCallbacks(
     PassInstrumentationCallbacks &PIC) {
   if (!OptBisector->isEnabled())
     return;
   PIC.registerShouldRunOptionalPassCallback([](StringRef PassID, Any IR) {