[PATCH] D102463: Making Instrumentation aware of LoopNest Pass

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:363-365
+  const Loop *L = any_isa<const LoopNest *>(&IR)
+                      ? &any_cast<const LoopNest *>(&IR)->getOutermostLoop()
+                      : any_cast<const Loop *>(&IR);
----------------
raghesh wrote:
> aeubanks wrote:
> > `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&`
> 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;
>  }
> 
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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102463/new/

https://reviews.llvm.org/D102463