Skip to content

Commit

Permalink
[MemorySSA] Remove -enable-mssa-loop-dependency option
Browse files Browse the repository at this point in the history
This option has been enabled by default for quite a while now.
The practical impact of removing the option is that MSSA use
cannot be disabled in default pipelines (both LPM and NPM) and
in manual LPM invocations. NPM can still choose to enable/disable
MSSA using loop vs loop-mssa.

The next step will be to require MSSA for LICM and drop the
AST-based implementation entirely.

Differential Revision: https://reviews.llvm.org/D108075
  • Loading branch information
nikic committed Aug 16, 2021
1 parent 65532ea commit 735a590
Show file tree
Hide file tree
Showing 21 changed files with 67 additions and 142 deletions.
3 changes: 0 additions & 3 deletions llvm/include/llvm/Analysis/MemorySSA.h
Expand Up @@ -106,9 +106,6 @@

namespace llvm {

/// Enables memory ssa as a dependency for loop passes.
extern cl::opt<bool> EnableMSSALoopDependency;

class AllocaInst;
class Function;
class Instruction;
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/Analysis/MemorySSA.cpp
Expand Up @@ -90,10 +90,6 @@ bool llvm::VerifyMemorySSA = true;
#else
bool llvm::VerifyMemorySSA = false;
#endif
/// Enables memory ssa as a dependency for loop passes in legacy pass manager.
cl::opt<bool> llvm::EnableMSSALoopDependency(
"enable-mssa-loop-dependency", cl::Hidden, cl::init(true),
cl::desc("Enable MemorySSA dependency for loop pass manager"));

static cl::opt<bool, true>
VerifyMemorySSAX("verify-memoryssa", cl::location(VerifyMemorySSA),
Expand Down
16 changes: 8 additions & 8 deletions llvm/lib/Passes/PassBuilder.cpp
Expand Up @@ -617,7 +617,7 @@ PassBuilder::buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
FPM.addPass(
RequireAnalysisPass<OptimizationRemarkEmitterAnalysis, Function>());
FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM1),
EnableMSSALoopDependency,
/*UseMemorySSA=*/true,
/*UseBlockFrequencyInfo=*/true));
FPM.addPass(SimplifyCFGPass());
FPM.addPass(InstCombinePass());
Expand Down Expand Up @@ -791,7 +791,7 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
FPM.addPass(
RequireAnalysisPass<OptimizationRemarkEmitterAnalysis, Function>());
FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM1),
EnableMSSALoopDependency,
/*UseMemorySSA=*/true,
/*UseBlockFrequencyInfo=*/true));
FPM.addPass(SimplifyCFGPass());
FPM.addPass(InstCombinePass());
Expand Down Expand Up @@ -848,7 +848,7 @@ PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
FPM.addPass(DSEPass());
FPM.addPass(createFunctionToLoopPassAdaptor(
LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap),
EnableMSSALoopDependency, /*UseBlockFrequencyInfo=*/true));
/*UseMemorySSA=*/true, /*UseBlockFrequencyInfo=*/true));

FPM.addPass(CoroElidePass());

Expand Down Expand Up @@ -1246,9 +1246,9 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
OptimizationLevel::O3));
FPM.addPass(
RequireAnalysisPass<OptimizationRemarkEmitterAnalysis, Function>());
FPM.addPass(createFunctionToLoopPassAdaptor(
std::move(LPM), EnableMSSALoopDependency,
/*UseBlockFrequencyInfo=*/true));
FPM.addPass(
createFunctionToLoopPassAdaptor(std::move(LPM), /*UseMemorySSA=*/true,
/*UseBlockFrequencyInfo=*/true));
FPM.addPass(SimplifyCFGPass());
FPM.addPass(InstCombinePass());
}
Expand Down Expand Up @@ -1307,7 +1307,7 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
RequireAnalysisPass<OptimizationRemarkEmitterAnalysis, Function>());
FPM.addPass(createFunctionToLoopPassAdaptor(
LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap),
EnableMSSALoopDependency, /*UseBlockFrequencyInfo=*/true));
/*UseMemorySSA=*/true, /*UseBlockFrequencyInfo=*/true));
}

// Now that we've vectorized and unrolled loops, we may have more refined
Expand Down Expand Up @@ -1828,7 +1828,7 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
FunctionPassManager MainFPM;
MainFPM.addPass(createFunctionToLoopPassAdaptor(
LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap),
EnableMSSALoopDependency, /*UseBlockFrequencyInfo=*/true));
/*USeMemorySSA=*/true, /*UseBlockFrequencyInfo=*/true));

if (RunNewGVN)
MainFPM.addPass(NewGVNPass());
Expand Down
10 changes: 3 additions & 7 deletions llvm/lib/Transforms/Scalar/LICM.cpp
Expand Up @@ -229,9 +229,7 @@ struct LegacyLICMPass : public LoopPass {
<< L->getHeader()->getNameOrAsOperand() << "\n");

auto *SE = getAnalysisIfAvailable<ScalarEvolutionWrapperPass>();
MemorySSA *MSSA = EnableMSSALoopDependency
? (&getAnalysis<MemorySSAWrapperPass>().getMSSA())
: nullptr;
MemorySSA *MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
bool hasProfileData = L->getHeader()->getParent()->hasProfileData();
BlockFrequencyInfo *BFI =
hasProfileData ? &getAnalysis<LazyBlockFrequencyInfoPass>().getBFI()
Expand All @@ -258,10 +256,8 @@ struct LegacyLICMPass : public LoopPass {
AU.addPreserved<DominatorTreeWrapperPass>();
AU.addPreserved<LoopInfoWrapperPass>();
AU.addRequired<TargetLibraryInfoWrapperPass>();
if (EnableMSSALoopDependency) {
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
}
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
AU.addRequired<TargetTransformInfoWrapperPass>();
getLoopAnalysisUsage(AU);
LazyBlockFrequencyInfoPass::getLazyBFIAnalysisUsage(AU);
Expand Down
17 changes: 5 additions & 12 deletions llvm/lib/Transforms/Scalar/LoopInstSimplify.cpp
Expand Up @@ -195,26 +195,19 @@ class LoopInstSimplifyLegacyPass : public LoopPass {
const TargetLibraryInfo &TLI =
getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(
*L->getHeader()->getParent());
MemorySSA *MSSA = nullptr;
Optional<MemorySSAUpdater> MSSAU;
if (EnableMSSALoopDependency) {
MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MSSAU = MemorySSAUpdater(MSSA);
}
MemorySSA *MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MemorySSAUpdater MSSAU(MSSA);

return simplifyLoopInst(*L, DT, LI, AC, TLI,
MSSAU.hasValue() ? MSSAU.getPointer() : nullptr);
return simplifyLoopInst(*L, DT, LI, AC, TLI, &MSSAU);
}

void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addRequired<AssumptionCacheTracker>();
AU.addRequired<DominatorTreeWrapperPass>();
AU.addRequired<TargetLibraryInfoWrapperPass>();
AU.setPreservesCFG();
if (EnableMSSALoopDependency) {
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
}
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
getLoopAnalysisUsage(AU);
}
};
Expand Down
15 changes: 6 additions & 9 deletions llvm/lib/Transforms/Scalar/LoopRotation.cpp
Expand Up @@ -99,8 +99,7 @@ class LoopRotateLegacyPass : public LoopPass {
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addRequired<AssumptionCacheTracker>();
AU.addRequired<TargetTransformInfoWrapperPass>();
if (EnableMSSALoopDependency)
AU.addPreserved<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
getLoopAnalysisUsage(AU);

// Lazy BFI and BPI are marked as preserved here so LoopRotate
Expand All @@ -121,13 +120,11 @@ class LoopRotateLegacyPass : public LoopPass {
auto &SE = getAnalysis<ScalarEvolutionWrapperPass>().getSE();
const SimplifyQuery SQ = getBestSimplifyQuery(*this, F);
Optional<MemorySSAUpdater> MSSAU;
if (EnableMSSALoopDependency) {
// Not requiring MemorySSA and getting it only if available will split
// the loop pass pipeline when LoopRotate is being run first.
auto *MSSAA = getAnalysisIfAvailable<MemorySSAWrapperPass>();
if (MSSAA)
MSSAU = MemorySSAUpdater(&MSSAA->getMSSA());
}
// Not requiring MemorySSA and getting it only if available will split
// the loop pass pipeline when LoopRotate is being run first.
auto *MSSAA = getAnalysisIfAvailable<MemorySSAWrapperPass>();
if (MSSAA)
MSSAU = MemorySSAUpdater(&MSSAA->getMSSA());
// Vectorization requires loop-rotation. Use default threshold for loops the
// user explicitly marked for vectorization, even when header duplication is
// disabled.
Expand Down
21 changes: 7 additions & 14 deletions llvm/lib/Transforms/Scalar/LoopSimplifyCFG.cpp
Expand Up @@ -733,27 +733,20 @@ class LoopSimplifyCFGLegacyPass : public LoopPass {
DominatorTree &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
LoopInfo &LI = getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
ScalarEvolution &SE = getAnalysis<ScalarEvolutionWrapperPass>().getSE();
Optional<MemorySSAUpdater> MSSAU;
if (EnableMSSALoopDependency) {
MemorySSA *MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MSSAU = MemorySSAUpdater(MSSA);
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();
}
MemorySSA *MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MemorySSAUpdater MSSAU(MSSA);
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();
bool DeleteCurrentLoop = false;
bool Changed = simplifyLoopCFG(
*L, DT, LI, SE, MSSAU.hasValue() ? MSSAU.getPointer() : nullptr,
DeleteCurrentLoop);
bool Changed = simplifyLoopCFG(*L, DT, LI, SE, &MSSAU, DeleteCurrentLoop);
if (DeleteCurrentLoop)
LPM.markLoopAsDeleted(*L);
return Changed;
}

void getAnalysisUsage(AnalysisUsage &AU) const override {
if (EnableMSSALoopDependency) {
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
}
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
AU.addPreserved<DependenceAnalysisWrapperPass>();
getLoopAnalysisUsage(AU);
}
Expand Down
19 changes: 7 additions & 12 deletions llvm/lib/Transforms/Scalar/LoopUnswitch.cpp
Expand Up @@ -232,10 +232,8 @@ namespace {
AU.addPreserved<LazyBranchProbabilityInfoPass>();
AU.addRequired<AssumptionCacheTracker>();
AU.addRequired<TargetTransformInfoWrapperPass>();
if (EnableMSSALoopDependency) {
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
}
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
if (HasBranchDivergence)
AU.addRequired<LegacyDivergenceAnalysis>();
getLoopAnalysisUsage(AU);
Expand Down Expand Up @@ -539,31 +537,28 @@ bool LoopUnswitch::runOnLoop(Loop *L, LPPassManager &LPMRef) {
LPM = &LPMRef;
DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
if (EnableMSSALoopDependency) {
MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MSSAU = std::make_unique<MemorySSAUpdater>(MSSA);
assert(DT && "Cannot update MemorySSA without a valid DomTree.");
}
MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MSSAU = std::make_unique<MemorySSAUpdater>(MSSA);
CurrentLoop = L;
Function *F = CurrentLoop->getHeader()->getParent();

SanitizeMemory = F->hasFnAttribute(Attribute::SanitizeMemory);
if (SanitizeMemory)
SafetyInfo.computeLoopSafetyInfo(L);

if (MSSA && VerifyMemorySSA)
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();

bool Changed = false;
do {
assert(CurrentLoop->isLCSSAForm(*DT));
if (MSSA && VerifyMemorySSA)
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();
RedoLoop = false;
Changed |= processCurrentLoop();
} while (RedoLoop);

if (MSSA && VerifyMemorySSA)
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();

return Changed;
Expand Down
23 changes: 8 additions & 15 deletions llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
Expand Up @@ -3126,10 +3126,8 @@ class SimpleLoopUnswitchLegacyPass : public LoopPass {
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addRequired<AssumptionCacheTracker>();
AU.addRequired<TargetTransformInfoWrapperPass>();
if (EnableMSSALoopDependency) {
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
}
AU.addRequired<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
getLoopAnalysisUsage(AU);
}
};
Expand All @@ -3150,12 +3148,8 @@ bool SimpleLoopUnswitchLegacyPass::runOnLoop(Loop *L, LPPassManager &LPM) {
auto &AC = getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
auto &AA = getAnalysis<AAResultsWrapperPass>().getAAResults();
auto &TTI = getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
MemorySSA *MSSA = nullptr;
Optional<MemorySSAUpdater> MSSAU;
if (EnableMSSALoopDependency) {
MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MSSAU = MemorySSAUpdater(MSSA);
}
MemorySSA *MSSA = &getAnalysis<MemorySSAWrapperPass>().getMSSA();
MemorySSAUpdater MSSAU(MSSA);

auto *SEWP = getAnalysisIfAvailable<ScalarEvolutionWrapperPass>();
auto *SE = SEWP ? &SEWP->getSE() : nullptr;
Expand All @@ -3179,14 +3173,13 @@ bool SimpleLoopUnswitchLegacyPass::runOnLoop(Loop *L, LPPassManager &LPM) {
LPM.markLoopAsDeleted(*L);
};

if (MSSA && VerifyMemorySSA)
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();

bool Changed =
unswitchLoop(*L, DT, LI, AC, AA, TTI, true, NonTrivial, UnswitchCB, SE,
MSSAU.hasValue() ? MSSAU.getPointer() : nullptr);
bool Changed = unswitchLoop(*L, DT, LI, AC, AA, TTI, true, NonTrivial,
UnswitchCB, SE, &MSSAU);

if (MSSA && VerifyMemorySSA)
if (VerifyMemorySSA)
MSSA->verifyMemorySSA();

// Historically this pass has had issues with the dominator tree so verify it
Expand Down
13 changes: 5 additions & 8 deletions llvm/lib/Transforms/Utils/LoopSimplify.cpp
Expand Up @@ -779,8 +779,7 @@ namespace {
AU.addPreserved<DependenceAnalysisWrapperPass>();
AU.addPreservedID(BreakCriticalEdgesID); // No critical edges added.
AU.addPreserved<BranchProbabilityInfoWrapperPass>();
if (EnableMSSALoopDependency)
AU.addPreserved<MemorySSAWrapperPass>();
AU.addPreserved<MemorySSAWrapperPass>();
}

/// verifyAnalysis() - Verify LoopSimplifyForm's guarantees.
Expand Down Expand Up @@ -814,12 +813,10 @@ bool LoopSimplify::runOnFunction(Function &F) {
&getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
MemorySSA *MSSA = nullptr;
std::unique_ptr<MemorySSAUpdater> MSSAU;
if (EnableMSSALoopDependency) {
auto *MSSAAnalysis = getAnalysisIfAvailable<MemorySSAWrapperPass>();
if (MSSAAnalysis) {
MSSA = &MSSAAnalysis->getMSSA();
MSSAU = std::make_unique<MemorySSAUpdater>(MSSA);
}
auto *MSSAAnalysis = getAnalysisIfAvailable<MemorySSAWrapperPass>();
if (MSSAAnalysis) {
MSSA = &MSSAAnalysis->getMSSA();
MSSAU = std::make_unique<MemorySSAUpdater>(MSSA);
}

bool PreserveLCSSA = mustPreserveAnalysisID(LCSSAID);
Expand Down
1 change: 0 additions & 1 deletion llvm/test/Analysis/BasicAA/store-promote.ll
Expand Up @@ -2,7 +2,6 @@
; disambiguating some obvious cases. If LICM is able to disambiguate the
; two pointers, then the load should be hoisted, and the store sunk.

; RUN: opt < %s -basic-aa -licm -enable-mssa-loop-dependency=false -enable-new-pm=0 -S | FileCheck %s -check-prefixes=CHECK,AST
; RUN: opt < %s -basic-aa -licm -enable-new-pm=0 -S | FileCheck %s -check-prefixes=CHECK,MSSA
; RUN: opt < %s -aa-pipeline=basic-aa -passes='loop(licm)' -S | FileCheck %s -check-prefixes=CHECK,AST
; RUN: opt < %s -aa-pipeline=basic-aa -passes='loop-mssa(licm)' -S | FileCheck %s -check-prefixes=CHECK,MSSA
Expand Down
2 changes: 0 additions & 2 deletions llvm/test/Analysis/MemorySSA/pr42294.ll
@@ -1,8 +1,6 @@
; REQUIRES: asserts
; RUN: opt -loop-rotate -licm %s -disable-output -debug-only=licm 2>&1 | FileCheck %s -check-prefix=LICM
; RUN: opt -loop-rotate -licm %s -disable-output -enable-mssa-loop-dependency=false -debug-only=licm 2>&1 | FileCheck %s -check-prefix=LICM
; RUN: opt -loop-rotate -licm %s -S | FileCheck %s
; RUN: opt -loop-rotate -licm %s -S -enable-mssa-loop-dependency=false | FileCheck %s

; LICM: Using
; LICM-NOT: LICM sinking instruction: %.pre = load i8, i8* %arrayidx.phi.trans.insert
Expand Down
45 changes: 13 additions & 32 deletions llvm/test/CodeGen/PowerPC/pr35688.ll
@@ -1,45 +1,26 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -enable-mssa-loop-dependency=false -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown < %s | \
; RUN: FileCheck %s
; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown < %s | \
; RUN: FileCheck %s --check-prefix=MSSA
; Function Attrs: nounwind
; RUN: FileCheck %s

; With MemorySSA, everything is taken out of the loop by licm.
; Loads and stores to undef are treated as non-aliasing.
define void @ec_GFp_nistp256_points_mul() {
; CHECK-LABEL: ec_GFp_nistp256_points_mul:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: ld 5, 0(3)
; CHECK-NEXT: li 3, 127
; CHECK-NEXT: ld 3, 0(3)
; CHECK-NEXT: li 4, 0
; CHECK-NEXT: .p2align 5
; CHECK-NEXT: subfic 5, 3, 0
; CHECK-NEXT: subfze 5, 4
; CHECK-NEXT: sradi 5, 5, 63
; CHECK-NEXT: subc 3, 5, 3
; CHECK-NEXT: subfe 3, 4, 5
; CHECK-NEXT: sradi 3, 3, 63
; CHECK-NEXT: std 3, 0(3)
; CHECK-NEXT: .p2align 4
; CHECK-NEXT: .LBB0_1: # %fe_cmovznz.exit.i534.i.15
; CHECK-NEXT: #
; CHECK-NEXT: subfic 6, 5, 0
; CHECK-NEXT: subfze 6, 4
; CHECK-NEXT: sradi 7, 6, 63
; CHECK-NEXT: srad 6, 6, 3
; CHECK-NEXT: subc 5, 7, 5
; CHECK-NEXT: subfe 5, 4, 6
; CHECK-NEXT: sradi 5, 5, 63
; CHECK-NEXT: b .LBB0_1
;
; MSSA-LABEL: ec_GFp_nistp256_points_mul:
; MSSA: # %bb.0: # %entry
; MSSA-NEXT: ld 3, 0(3)
; MSSA-NEXT: li 4, 0
; MSSA-NEXT: subfic 5, 3, 0
; MSSA-NEXT: subfze 5, 4
; MSSA-NEXT: sradi 5, 5, 63
; MSSA-NEXT: subc 3, 5, 3
; MSSA-NEXT: subfe 3, 4, 5
; MSSA-NEXT: sradi 3, 3, 63
; MSSA-NEXT: std 3, 0(3)
; MSSA-NEXT: .p2align 4
; MSSA-NEXT: .LBB0_1: # %fe_cmovznz.exit.i534.i.15
; MSSA-NEXT: #
; MSSA-NEXT: b .LBB0_1

; With MemorySSA, everything is taken out of the loop by licm.
; Loads and stores to undef are treated as non-aliasing.
entry:
br label %fe_cmovznz.exit.i534.i.15

Expand Down

0 comments on commit 735a590

Please sign in to comment.