[llvm] r268446 - PM: Port LoopSimplifyCFG to the new pass manager

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 17 11:23:03 PDT 2016


On Fri, Jun 17, 2016 at 11:01 AM, Justin Bogner <mail at justinbogner.com> wrote:
> Davide Italiano <davide at freebsd.org> writes:
>> On Tue, May 3, 2016 at 2:47 PM, Justin Bogner via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> Author: bogner
>>> Date: Tue May  3 16:47:32 2016
>>> New Revision: 268446
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=268446&view=rev
>>> Log:
>>> PM: Port LoopSimplifyCFG to the new pass manager
>>>
>>> Added:
>>>     llvm/trunk/include/llvm/Transforms/Scalar/LoopSimplifyCFG.h
>>> Modified:
>>>     llvm/trunk/include/llvm/InitializePasses.h
>>>     llvm/trunk/lib/Passes/PassBuilder.cpp
>>>     llvm/trunk/lib/Passes/PassRegistry.def
>>>     llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
>>>     llvm/trunk/lib/Transforms/Scalar/Scalar.cpp
>>>     llvm/trunk/test/Transforms/LoopSimplifyCFG/merge-header.ll
>>>
>>> Modified: llvm/trunk/include/llvm/InitializePasses.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/InitializePasses.h?rev=268446&r1=268445&r2=268446&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/InitializePasses.h (original)
>>> +++ llvm/trunk/include/llvm/InitializePasses.h Tue May  3 16:47:32 2016
>>> @@ -180,7 +180,7 @@ void initializeLoopInterchangePass(PassR
>>>  void initializeLoopInstSimplifyPass(PassRegistry&);
>>>  void initializeLoopRotatePass(PassRegistry&);
>>>  void initializeLoopSimplifyPass(PassRegistry&);
>>> -void initializeLoopSimplifyCFGPass(PassRegistry&);
>>> +void initializeLoopSimplifyCFGLegacyPassPass(PassRegistry&);
>>>  void initializeLoopStrengthReducePass(PassRegistry&);
>>>  void initializeGlobalMergePass(PassRegistry&);
>>>  void initializeLoopRerollPass(PassRegistry&);
>>>
>>> Added: llvm/trunk/include/llvm/Transforms/Scalar/LoopSimplifyCFG.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scalar/LoopSimplifyCFG.h?rev=268446&view=auto
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Transforms/Scalar/LoopSimplifyCFG.h (added)
>>> +++ llvm/trunk/include/llvm/Transforms/Scalar/LoopSimplifyCFG.h Tue May  3 16:47:32 2016
>>> @@ -0,0 +1,32 @@
>>> +//===- LoopSimplifyCFG.cpp - Loop CFG Simplification Pass -------*- C++ -*-===//
>>> +//
>>> +//                     The LLVM Compiler Infrastructure
>>> +//
>>> +// This file is distributed under the University of Illinois Open Source
>>> +// License. See LICENSE.TXT for details.
>>> +//
>>> +//===----------------------------------------------------------------------===//
>>> +//
>>> +// This file implements the Loop SimplifyCFG Pass. This pass is responsible for
>>> +// basic loop CFG cleanup, primarily to assist other loop passes. If you
>>> +// encounter a noncanonical CFG construct that causes another loop pass to
>>> +// perform suboptimally, this is the place to fix it up.
>>> +//
>>> +//===----------------------------------------------------------------------===//
>>> +
>>> +#ifndef LLVM_TRANSFORMS_SCALAR_LOOPSIMPLIFYCFG_H
>>> +#define LLVM_TRANSFORMS_SCALAR_LOOPSIMPLIFYCFG_H
>>> +
>>> +#include "llvm/Analysis/LoopInfo.h"
>>> +#include "llvm/IR/PassManager.h"
>>> +
>>> +namespace llvm {
>>> +
>>> +/// Performs basic CFG simplifications to assist other loop passes.
>>> +class LoopSimplifyCFGPass : public PassInfoMixin<LoopSimplifyCFGPass> {
>>> +public:
>>> +  PreservedAnalyses run(Loop &L, AnalysisManager<Loop> &AM);
>>> +};
>>> +} // end namespace llvm
>>> +
>>> +#endif // LLVM_TRANSFORMS_SCALAR_LOOPSIMPLIFYCFG_H
>>>
>>> Modified: llvm/trunk/lib/Passes/PassBuilder.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Passes/PassBuilder.cpp?rev=268446&r1=268445&r2=268446&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Passes/PassBuilder.cpp (original)
>>> +++ llvm/trunk/lib/Passes/PassBuilder.cpp Tue May  3 16:47:32 2016
>>> @@ -57,6 +57,7 @@
>>>  #include "llvm/Transforms/Scalar/ADCE.h"
>>>  #include "llvm/Transforms/Scalar/DCE.h"
>>>  #include "llvm/Transforms/Scalar/EarlyCSE.h"
>>> +#include "llvm/Transforms/Scalar/LoopSimplifyCFG.h"
>>>  #include "llvm/Transforms/Scalar/LowerExpectIntrinsic.h"
>>>  #include "llvm/Transforms/Scalar/GVN.h"
>>>  #include "llvm/Transforms/Scalar/Reassociate.h"
>>>
>>> Modified: llvm/trunk/lib/Passes/PassRegistry.def
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Passes/PassRegistry.def?rev=268446&r1=268445&r2=268446&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Passes/PassRegistry.def (original)
>>> +++ llvm/trunk/lib/Passes/PassRegistry.def Tue May  3 16:47:32 2016
>>> @@ -136,4 +136,5 @@ LOOP_ANALYSIS("no-op-loop", NoOpLoopAnal
>>>  LOOP_PASS("invalidate<all>", InvalidateAllAnalysesPass())
>>>  LOOP_PASS("no-op-loop", NoOpLoopPass())
>>>  LOOP_PASS("print", PrintLoopPass(dbgs()))
>>> +LOOP_PASS("simplify-cfg", LoopSimplifyCFGPass())
>>>  #undef LOOP_PASS
>>>
>>> Modified: llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp?rev=268446&r1=268445&r2=268446&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp (original)
>>> +++ llvm/trunk/lib/Transforms/Scalar/LoopSimplifyCFG.cpp Tue May  3 16:47:32 2016
>>> @@ -14,7 +14,7 @@
>>>  //
>>>  //===----------------------------------------------------------------------===//
>>>
>>> -#include "llvm/Transforms/Scalar.h"
>>> +#include "llvm/Transforms/Scalar/LoopSimplifyCFG.h"
>>>  #include "llvm/ADT/SmallVector.h"
>>>  #include "llvm/ADT/Statistic.h"
>>>  #include "llvm/Analysis/AliasAnalysis.h"
>>> @@ -24,47 +24,23 @@
>>>  #include "llvm/Analysis/GlobalsModRef.h"
>>>  #include "llvm/Analysis/LoopInfo.h"
>>>  #include "llvm/Analysis/LoopPass.h"
>>> +#include "llvm/Analysis/LoopPassManager.h"
>>>  #include "llvm/Analysis/ScalarEvolution.h"
>>>  #include "llvm/Analysis/ScalarEvolutionAliasAnalysis.h"
>>>  #include "llvm/Analysis/TargetTransformInfo.h"
>>>  #include "llvm/IR/Dominators.h"
>>> +#include "llvm/Transforms/Scalar.h"
>>>  #include "llvm/Transforms/Utils/Local.h"
>>>  #include "llvm/Transforms/Utils/LoopUtils.h"
>>>  using namespace llvm;
>>>
>>>  #define DEBUG_TYPE "loop-simplifycfg"
>>>
>>> -namespace {
>>> -class LoopSimplifyCFG : public LoopPass {
>>> -public:
>>> -  static char ID; // Pass ID, replacement for typeid
>>> -  LoopSimplifyCFG() : LoopPass(ID) {
>>> -    initializeLoopSimplifyCFGPass(*PassRegistry::getPassRegistry());
>>> -  }
>>> -
>>> -  bool runOnLoop(Loop *L, LPPassManager &) override;
>>> -
>>> -  void getAnalysisUsage(AnalysisUsage &AU) const override {
>>> -    AU.addPreserved<DependenceAnalysis>();
>>> -    getLoopAnalysisUsage(AU);
>>> -  }
>>> -};
>>> -}
>>> -
>>> -char LoopSimplifyCFG::ID = 0;
>>> -INITIALIZE_PASS_BEGIN(LoopSimplifyCFG, "loop-simplifycfg", "Simplify loop CFG",
>>> -                      false, false)
>>> -INITIALIZE_PASS_DEPENDENCY(LoopPass)
>>> -INITIALIZE_PASS_END(LoopSimplifyCFG, "loop-simplifycfg", "Simplify loop CFG",
>>> -                    false, false)
>>> -
>>> -Pass *llvm::createLoopSimplifyCFGPass() { return new LoopSimplifyCFG(); }
>>> -
>>> -static bool simplifyLoopCFG(Loop *L, DominatorTree *DT, LoopInfo *LI) {
>>> +static bool simplifyLoopCFG(Loop &L, DominatorTree &DT, LoopInfo &LI) {
>>>    bool Changed = false;
>>>    // Copy blocks into a temporary array to avoid iterator invalidation issues
>>>    // as we remove them.
>>> -  SmallVector<WeakVH, 16> Blocks(L->blocks());
>>> +  SmallVector<WeakVH, 16> Blocks(L.blocks());
>>>
>>>    for (auto &Block : Blocks) {
>>>      // Attempt to merge blocks in the trivial case. Don't modify blocks which
>>> @@ -74,27 +50,64 @@ static bool simplifyLoopCFG(Loop *L, Dom
>>>        continue;
>>>
>>>      BasicBlock *Pred = Succ->getSinglePredecessor();
>>> -    if (!Pred || !Pred->getSingleSuccessor() || LI->getLoopFor(Pred) != L)
>>> +    if (!Pred || !Pred->getSingleSuccessor() || LI.getLoopFor(Pred) != &L)
>>>        continue;
>>>
>>>      // Pred is going to disappear, so we need to update the loop info.
>>> -    if (L->getHeader() == Pred)
>>> -      L->moveToHeader(Succ);
>>> -    LI->removeBlock(Pred);
>>> -    MergeBasicBlockIntoOnlyPred(Succ, DT);
>>> +    if (L.getHeader() == Pred)
>>> +      L.moveToHeader(Succ);
>>> +    LI.removeBlock(Pred);
>>> +    MergeBasicBlockIntoOnlyPred(Succ, &DT);
>>>      Changed = true;
>>>    }
>>>
>>>    return Changed;
>>>  }
>>>
>>> -/// runOnLoop - Perform basic CFG simplifications to assist other loop passes.
>>> -/// For now, this only attempts to merge blocks in the trivial case.
>>> -bool LoopSimplifyCFG::runOnLoop(Loop *L, LPPassManager &) {
>>> -  if (skipLoop(L))
>>> -    return false;
>>> -
>>> -  DominatorTree *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
>>> -  LoopInfo *LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
>>> -  return simplifyLoopCFG(L, DT, LI);
>>> +PreservedAnalyses LoopSimplifyCFGPass::run(Loop &L, AnalysisManager<Loop> &AM) {
>>> +  auto &FAM = AM.getResult<FunctionAnalysisManagerLoopProxy>(L).getManager();
>>> +  Function *F = L.getHeader()->getParent();
>>> +
>>> +  auto *LI = FAM.getCachedResult<LoopAnalysis>(*F);
>>> +  auto *DT = FAM.getCachedResult<DominatorTreeAnalysis>(*F);
>>> +  assert((LI && DT) && "Analyses for LoopSimplifyCFG not available");
>>> +
>>
>> Is there a particular reason why this uses getCachedResult() +
>> assert() instead of getResult() ?
>
> Simply put, getResult() isn't allowed here. Try to change it and check
> the compile errors ;)
>
> This is one of those cases where writing "auto &" hurt clarity.
> getManager returns const&, and getResult isn't const so it can't be
> called. We've discussed that it might be better to add more API for
> these "required to have already been calculated" results, since this
> kind of assert is a bit fragile.

Thanks for the clarification, Justin. I definitely agree we need
another API (as Sean pointed out when we were discussing this in
person this might trigger UB because users make a mistake on the
cmdline, which, IMHO, is far from ideal).

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare


More information about the llvm-commits mailing list