[llvm] 2165360 - [LoopUnrollAndJam] Change LoopUnrollAndJamPass to LoopNest pass

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu May 27 09:20:17 PDT 2021


I know this has been reverted at least once.  It's generally considered 
best practice to explain in the re-commit message what issue caused the 
previous revert.

Philip

On 5/27/21 9:17 AM, via llvm-commits wrote:
> Author: maekawatoshiki
> Date: 2021-05-28T01:17:23+09:00
> New Revision: 21653600034084e8335374ddc1eb8d362158d9a8
>
> URL: https://github.com/llvm/llvm-project/commit/21653600034084e8335374ddc1eb8d362158d9a8
> DIFF: https://github.com/llvm/llvm-project/commit/21653600034084e8335374ddc1eb8d362158d9a8.diff
>
> LOG: [LoopUnrollAndJam] Change LoopUnrollAndJamPass to LoopNest pass
>
> This patch changes LoopUnrollAndJamPass from FunctionPass to LoopNest pass.
> The next patch will utilize LoopNest to effectively handle loop nests.
>
> Reviewed By: Whitney
>
> Differential Revision: https://reviews.llvm.org/D99149
>
> Added:
>      
>
> Modified:
>      llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
>      llvm/include/llvm/Transforms/Scalar/LoopUnrollAndJamPass.h
>      llvm/lib/Passes/PassBuilder.cpp
>      llvm/lib/Passes/PassRegistry.def
>      llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp
>      llvm/test/Transforms/LoopUnrollAndJam/innerloop.ll
>
> Removed:
>      
>
>
> ################################################################################
> diff  --git a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
> index c3833f3635339..480f5e5dfebd8 100644
> --- a/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
> +++ b/llvm/include/llvm/Transforms/Scalar/LoopPassManager.h
> @@ -258,7 +258,7 @@ class LPMUpdater {
>     /// 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()) &&
> +    assert((!LoopNestMode || CurrentL == &L) &&
>              "L should be a top-level loop in loop-nest mode.");
>       LAM.clear(L, Name);
>       assert((&L == CurrentL || CurrentL->contains(&L)) &&
>
> diff  --git a/llvm/include/llvm/Transforms/Scalar/LoopUnrollAndJamPass.h b/llvm/include/llvm/Transforms/Scalar/LoopUnrollAndJamPass.h
> index bd83a6a0cca4d..6125fc7636a06 100644
> --- a/llvm/include/llvm/Transforms/Scalar/LoopUnrollAndJamPass.h
> +++ b/llvm/include/llvm/Transforms/Scalar/LoopUnrollAndJamPass.h
> @@ -10,6 +10,7 @@
>   #define LLVM_TRANSFORMS_SCALAR_LOOPUNROLLANDJAMPASS_H
>   
>   #include "llvm/IR/PassManager.h"
> +#include "llvm/Transforms/Scalar/LoopPassManager.h"
>   
>   namespace llvm {
>   class Function;
> @@ -20,7 +21,8 @@ class LoopUnrollAndJamPass : public PassInfoMixin<LoopUnrollAndJamPass> {
>   
>   public:
>     explicit LoopUnrollAndJamPass(int OptLevel = 2) : OptLevel(OptLevel) {}
> -  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
> +  PreservedAnalyses run(LoopNest &L, LoopAnalysisManager &AM,
> +                        LoopStandardAnalysisResults &AR, LPMUpdater &U);
>   };
>   
>   } // end namespace llvm
>
> diff  --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
> index 3a44dcf82ad63..eb94d284961bc 100644
> --- a/llvm/lib/Passes/PassBuilder.cpp
> +++ b/llvm/lib/Passes/PassBuilder.cpp
> @@ -1207,7 +1207,8 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
>       // across the loop nests.
>       // We do UnrollAndJam in a separate LPM to ensure it happens before unroll
>       if (EnableUnrollAndJam && PTO.LoopUnrolling)
> -      FPM.addPass(LoopUnrollAndJamPass(Level.getSpeedupLevel()));
> +      FPM.addPass(createFunctionToLoopPassAdaptor(
> +          LoopUnrollAndJamPass(Level.getSpeedupLevel())));
>       FPM.addPass(LoopUnrollPass(LoopUnrollOptions(
>           Level.getSpeedupLevel(), /*OnlyWhenForced=*/!PTO.LoopUnrolling,
>           PTO.ForgetAllSCEVInLoopUnroll)));
> @@ -1290,7 +1291,8 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
>       // across the loop nests.
>       // We do UnrollAndJam in a separate LPM to ensure it happens before unroll
>       if (EnableUnrollAndJam && PTO.LoopUnrolling) {
> -      FPM.addPass(LoopUnrollAndJamPass(Level.getSpeedupLevel()));
> +      FPM.addPass(createFunctionToLoopPassAdaptor(
> +          LoopUnrollAndJamPass(Level.getSpeedupLevel())));
>       }
>       FPM.addPass(LoopUnrollPass(LoopUnrollOptions(
>           Level.getSpeedupLevel(), /*OnlyWhenForced=*/!PTO.LoopUnrolling,
>
> diff  --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
> index 99e190b99fc4f..128c9f3bc6357 100644
> --- a/llvm/lib/Passes/PassRegistry.def
> +++ b/llvm/lib/Passes/PassRegistry.def
> @@ -247,7 +247,6 @@ FUNCTION_PASS("guard-widening", GuardWideningPass())
>   FUNCTION_PASS("load-store-vectorizer", LoadStoreVectorizerPass())
>   FUNCTION_PASS("loop-simplify", LoopSimplifyPass())
>   FUNCTION_PASS("loop-sink", LoopSinkPass())
> -FUNCTION_PASS("loop-unroll-and-jam", LoopUnrollAndJamPass())
>   FUNCTION_PASS("loop-flatten", LoopFlattenPass())
>   FUNCTION_PASS("lowerinvoke", LowerInvokePass())
>   FUNCTION_PASS("lowerswitch", LowerSwitchPass())
> @@ -399,6 +398,7 @@ LOOP_PASS("loop-deletion", LoopDeletionPass())
>   LOOP_PASS("loop-simplifycfg", LoopSimplifyCFGPass())
>   LOOP_PASS("loop-reduce", LoopStrengthReducePass())
>   LOOP_PASS("indvars", IndVarSimplifyPass())
> +LOOP_PASS("loop-unroll-and-jam", LoopUnrollAndJamPass())
>   LOOP_PASS("loop-unroll-full", LoopFullUnrollPass())
>   LOOP_PASS("print-access-info", LoopAccessInfoPrinterPass(dbgs()))
>   LOOP_PASS("print<ddg>", DDGAnalysisPrinterPass(dbgs()))
>
> diff  --git a/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp b/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp
> index 495906e1a7630..9b586e83027f8 100644
> --- a/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp
> +++ b/llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp
> @@ -22,6 +22,7 @@
>   #include "llvm/Analysis/DependenceAnalysis.h"
>   #include "llvm/Analysis/LoopAnalysisManager.h"
>   #include "llvm/Analysis/LoopInfo.h"
> +#include "llvm/Analysis/LoopPass.h"
>   #include "llvm/Analysis/OptimizationRemarkEmitter.h"
>   #include "llvm/Analysis/ScalarEvolution.h"
>   #include "llvm/Analysis/TargetTransformInfo.h"
> @@ -424,35 +425,29 @@ tryToUnrollAndJamLoop(Loop *L, DominatorTree &DT, LoopInfo *LI,
>     return UnrollResult;
>   }
>   
> -static bool tryToUnrollAndJamLoop(Function &F, DominatorTree &DT, LoopInfo &LI,
> +static bool tryToUnrollAndJamLoop(LoopNest &LN, DominatorTree &DT, LoopInfo &LI,
>                                     ScalarEvolution &SE,
>                                     const TargetTransformInfo &TTI,
>                                     AssumptionCache &AC, DependenceInfo &DI,
> -                                  OptimizationRemarkEmitter &ORE,
> -                                  int OptLevel) {
> +                                  OptimizationRemarkEmitter &ORE, int OptLevel,
> +                                  LPMUpdater &U) {
>     bool DidSomething = false;
> +  ArrayRef<Loop *> Loops = LN.getLoops();
> +  Loop *OutmostLoop = &LN.getOutermostLoop();
>   
> -  // The loop unroll and jam pass requires loops to be in simplified form, and
> -  // also needs LCSSA. Since simplification may add new inner loops, it has to
> -  // run before the legality and profitability checks. This means running the
> -  // loop unroll and jam pass will simplify all loops, regardless of whether
> -  // anything end up being unroll and jammed.
> -  for (auto &L : LI) {
> -    DidSomething |=
> -        simplifyLoop(L, &DT, &LI, &SE, &AC, nullptr, false /* PreserveLCSSA */);
> -    DidSomething |= formLCSSARecursively(*L, DT, &LI, &SE);
> -  }
> -
> -  // Add the loop nests in the reverse order of LoopInfo. See method
> +  // Add the loop nests in the reverse order of LN. See method
>     // declaration.
>     SmallPriorityWorklist<Loop *, 4> Worklist;
> -  appendLoopsToWorklist(LI, Worklist);
> +  appendLoopsToWorklist(Loops, Worklist);
>     while (!Worklist.empty()) {
>       Loop *L = Worklist.pop_back_val();
> +    std::string LoopName = std::string(L->getName());
>       LoopUnrollResult Result =
>           tryToUnrollAndJamLoop(L, DT, &LI, SE, TTI, AC, DI, ORE, OptLevel);
>       if (Result != LoopUnrollResult::Unmodified)
>         DidSomething = true;
> +    if (L == OutmostLoop && Result == LoopUnrollResult::FullyUnrolled)
> +      U.markLoopAsDeleted(*L, LoopName);
>     }
>   
>     return DidSomething;
> @@ -460,29 +455,35 @@ static bool tryToUnrollAndJamLoop(Function &F, DominatorTree &DT, LoopInfo &LI,
>   
>   namespace {
>   
> -class LoopUnrollAndJam : public FunctionPass {
> +class LoopUnrollAndJam : public LoopPass {
>   public:
>     static char ID; // Pass ID, replacement for typeid
>     unsigned OptLevel;
>   
> -  LoopUnrollAndJam(int OptLevel = 2) : FunctionPass(ID), OptLevel(OptLevel) {
> +  LoopUnrollAndJam(int OptLevel = 2) : LoopPass(ID), OptLevel(OptLevel) {
>       initializeLoopUnrollAndJamPass(*PassRegistry::getPassRegistry());
>     }
>   
> -  bool runOnFunction(Function &F) override {
> -    if (skipFunction(F))
> +  bool runOnLoop(Loop *L, LPPassManager &LPM) override {
> +    if (skipLoop(L))
>         return false;
>   
> -    auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
> -    LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
> -    ScalarEvolution &SE = getAnalysis<ScalarEvolutionWrapperPass>().getSE();
> -    const TargetTransformInfo &TTI =
> -        getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
> -    auto &AC = getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
> +    auto *F = L->getHeader()->getParent();
> +    auto &SE = getAnalysis<ScalarEvolutionWrapperPass>().getSE();
> +    auto *LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
>       auto &DI = getAnalysis<DependenceAnalysisWrapperPass>().getDI();
> +    auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
> +    auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(*F);
>       auto &ORE = getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE();
> +    auto &AC = getAnalysis<AssumptionCacheTracker>().getAssumptionCache(*F);
>   
> -    return tryToUnrollAndJamLoop(F, DT, LI, SE, TTI, AC, DI, ORE, OptLevel);
> +    LoopUnrollResult Result =
> +        tryToUnrollAndJamLoop(L, DT, LI, SE, TTI, AC, DI, ORE, OptLevel);
> +
> +    if (Result == LoopUnrollResult::FullyUnrolled)
> +      LPM.markLoopAsDeleted(*L);
> +
> +    return Result != LoopUnrollResult::Unmodified;
>     }
>   
>     /// This transformation requires natural loop information & requires that
> @@ -505,7 +506,10 @@ char LoopUnrollAndJam::ID = 0;
>   INITIALIZE_PASS_BEGIN(LoopUnrollAndJam, "loop-unroll-and-jam",
>                         "Unroll and Jam loops", false, false)
>   INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
> +INITIALIZE_PASS_DEPENDENCY(LoopPass)
>   INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
> +INITIALIZE_PASS_DEPENDENCY(LoopSimplify)
> +INITIALIZE_PASS_DEPENDENCY(LCSSAWrapperPass)
>   INITIALIZE_PASS_DEPENDENCY(ScalarEvolutionWrapperPass)
>   INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
>   INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
> @@ -518,19 +522,20 @@ Pass *llvm::createLoopUnrollAndJamPass(int OptLevel) {
>     return new LoopUnrollAndJam(OptLevel);
>   }
>   
> -PreservedAnalyses LoopUnrollAndJamPass::run(Function &F,
> -                                            FunctionAnalysisManager &AM) {
> -  ScalarEvolution &SE = AM.getResult<ScalarEvolutionAnalysis>(F);
> -  LoopInfo &LI = AM.getResult<LoopAnalysis>(F);
> -  TargetTransformInfo &TTI = AM.getResult<TargetIRAnalysis>(F);
> -  AssumptionCache &AC = AM.getResult<AssumptionAnalysis>(F);
> -  DominatorTree &DT = AM.getResult<DominatorTreeAnalysis>(F);
> -  DependenceInfo &DI = AM.getResult<DependenceAnalysis>(F);
> -  OptimizationRemarkEmitter &ORE =
> -      AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
> -
> -  if (!tryToUnrollAndJamLoop(F, DT, LI, SE, TTI, AC, DI, ORE, OptLevel))
> +PreservedAnalyses LoopUnrollAndJamPass::run(LoopNest &LN,
> +                                            LoopAnalysisManager &AM,
> +                                            LoopStandardAnalysisResults &AR,
> +                                            LPMUpdater &U) {
> +  Function &F = *LN.getParent();
> +
> +  DependenceInfo DI(&F, &AR.AA, &AR.SE, &AR.LI);
> +  OptimizationRemarkEmitter ORE(&F);
> +
> +  if (!tryToUnrollAndJamLoop(LN, AR.DT, AR.LI, AR.SE, AR.TTI, AR.AC, DI, ORE,
> +                             OptLevel, U))
>       return PreservedAnalyses::all();
>   
> -  return getLoopPassPreservedAnalyses();
> +  auto PA = getLoopPassPreservedAnalyses();
> +  PA.preserve<LoopNestAnalysis>();
> +  return PA;
>   }
>
> diff  --git a/llvm/test/Transforms/LoopUnrollAndJam/innerloop.ll b/llvm/test/Transforms/LoopUnrollAndJam/innerloop.ll
> index 79c32c90174eb..c3a4ebd6dede5 100644
> --- a/llvm/test/Transforms/LoopUnrollAndJam/innerloop.ll
> +++ b/llvm/test/Transforms/LoopUnrollAndJam/innerloop.ll
> @@ -1,5 +1,5 @@
>   ; RUN: opt -loop-unroll-and-jam -allow-unroll-and-jam -verify-loop-info < %s -S | FileCheck %s
> -; RUN: opt -passes='loop-unroll-and-jam,verify<loops>' -allow-unroll-and-jam < %s -S | FileCheck %s
> +; RUN: opt -passes='loop(loop-unroll-and-jam),verify<loops>' -allow-unroll-and-jam < %s -S | FileCheck %s
>   
>   ; Check that the newly created loops to not fail to be added to LI
>   ; This test deliberately disables UnJ on the middle loop, performing it instead on the
>
>
>          
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list