[MergedLoadStoreMotion] Sink stores to BB with more than 2 predecessors

If we have:

bb5:
  br i1 %arg3, label %bb6, label %bb7

bb6:
  %tmp = getelementptr inbounds i32, i32* %arg1, i64 2
  store i32 3, i32* %tmp, align 4
  br label %bb9

bb7:
  %tmp8 = getelementptr inbounds i32, i32* %arg1, i64 2
  store i32 3, i32* %tmp8, align 4
  br label %bb9

bb9:  ; preds = %bb4, %bb6, %bb7
  ...

We can't sink stores directly into bb9.
This patch creates new BB that is successor of %bb6 and %bb7
and sinks stores into that block.

SplitFooterBB is the parameter to the pass that controls
that behavior.

Change-Id: I7fdf50a772b84633e4b1b860e905bf7e3e29940f
Differential: https://reviews.llvm.org/D66234
llvm-svn: 371089
This commit is contained in:
Denis Bakhvalov 2019-09-05 17:00:32 +00:00
parent 3856512334
commit 58f172f05a
6 changed files with 234 additions and 71 deletions

View File

@ -308,7 +308,7 @@ FunctionPass *createGVNSinkPass();
// MergedLoadStoreMotion - This pass merges loads and stores in diamonds. Loads
// are hoisted into the header, while stores sink into the footer.
//
FunctionPass *createMergedLoadStoreMotionPass();
FunctionPass *createMergedLoadStoreMotionPass(bool SplitFooterBB = false);
//===----------------------------------------------------------------------===//
//

View File

@ -27,12 +27,28 @@
#include "llvm/IR/PassManager.h"
namespace llvm {
class MergedLoadStoreMotionPass
: public PassInfoMixin<MergedLoadStoreMotionPass> {
public:
PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
struct MergedLoadStoreMotionOptions {
bool SplitFooterBB;
MergedLoadStoreMotionOptions(bool SplitFooterBB = false)
: SplitFooterBB(SplitFooterBB) {}
MergedLoadStoreMotionOptions &splitFooterBB(bool SFBB) {
SplitFooterBB = SFBB;
return *this;
}
};
class MergedLoadStoreMotionPass
: public PassInfoMixin<MergedLoadStoreMotionPass> {
MergedLoadStoreMotionOptions Options;
public:
MergedLoadStoreMotionPass()
: MergedLoadStoreMotionPass(MergedLoadStoreMotionOptions()) {}
MergedLoadStoreMotionPass(const MergedLoadStoreMotionOptions &PassOptions)
: Options(PassOptions) {}
PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
};
}
#endif // LLVM_TRANSFORMS_SCALAR_MERGEDLOADSTOREMOTION_H

View File

@ -1581,6 +1581,26 @@ Expected<bool> parseLoopUnswitchOptions(StringRef Params) {
}
return Result;
}
Expected<bool> parseMergedLoadStoreMotionOptions(StringRef Params) {
bool Result = false;
while (!Params.empty()) {
StringRef ParamName;
std::tie(ParamName, Params) = Params.split(';');
bool Enable = !ParamName.consume_front("no-");
if (ParamName == "split-footer-bb") {
Result = Enable;
} else {
return make_error<StringError>(
formatv("invalid MergedLoadStoreMotion pass parameter '{0}' ",
ParamName)
.str(),
inconvertibleErrorCode());
}
}
return Result;
}
} // namespace
/// Tests whether a pass name starts with a valid prefix for a default pipeline

View File

@ -196,7 +196,6 @@ FUNCTION_PASS("lowerinvoke", LowerInvokePass())
FUNCTION_PASS("mem2reg", PromotePass())
FUNCTION_PASS("memcpyopt", MemCpyOptPass())
FUNCTION_PASS("mergeicmps", MergeICmpsPass())
FUNCTION_PASS("mldst-motion", MergedLoadStoreMotionPass())
FUNCTION_PASS("nary-reassociate", NaryReassociatePass())
FUNCTION_PASS("newgvn", NewGVNPass())
FUNCTION_PASS("jump-threading", JumpThreadingPass())
@ -271,6 +270,11 @@ FUNCTION_PASS_WITH_PARAMS("loop-vectorize",
return LoopVectorizePass(Opts);
},
parseLoopVectorizeOptions)
FUNCTION_PASS_WITH_PARAMS("mldst-motion",
[](MergedLoadStoreMotionOptions Opts) {
return MergedLoadStoreMotionPass(Opts);
},
parseMergedLoadStoreMotionOptions)
#undef FUNCTION_PASS_WITH_PARAMS
#ifndef LOOP_ANALYSIS

View File

@ -14,9 +14,11 @@
// diamond (hammock) and merges them into a single load in the header. Similar
// it sinks and merges two stores to the tail block (footer). The algorithm
// iterates over the instructions of one side of the diamond and attempts to
// find a matching load/store on the other side. It hoists / sinks when it
// thinks it safe to do so. This optimization helps with eg. hiding load
// latencies, triggering if-conversion, and reducing static code size.
// find a matching load/store on the other side. New tail/footer block may be
// insterted if the tail/footer block has more predecessors (not only the two
// predecessors that are forming the diamond). It hoists / sinks when it thinks
// it safe to do so. This optimization helps with eg. hiding load latencies,
// triggering if-conversion, and reducing static code size.
//
// NOTE: This code no longer performs load hoisting, it is subsumed by GVNHoist.
//
@ -103,7 +105,9 @@ class MergedLoadStoreMotion {
// Control is enforced by the check Size0 * Size1 < MagicCompileTimeControl.
const int MagicCompileTimeControl = 250;
const bool SplitFooterBB;
public:
MergedLoadStoreMotion(bool SplitFooterBB) : SplitFooterBB(SplitFooterBB) {}
bool run(Function &F, AliasAnalysis &AA);
private:
@ -114,7 +118,9 @@ private:
PHINode *getPHIOperand(BasicBlock *BB, StoreInst *S0, StoreInst *S1);
bool isStoreSinkBarrierInRange(const Instruction &Start,
const Instruction &End, MemoryLocation Loc);
bool sinkStore(BasicBlock *BB, StoreInst *SinkCand, StoreInst *ElseInst);
bool canSinkStoresAndGEPs(StoreInst *S0, StoreInst *S1) const;
void sinkStoresAndGEPs(BasicBlock *BB, StoreInst *SinkCand,
StoreInst *ElseInst);
bool mergeStores(BasicBlock *BB);
};
} // end anonymous namespace
@ -216,75 +222,83 @@ PHINode *MergedLoadStoreMotion::getPHIOperand(BasicBlock *BB, StoreInst *S0,
return NewPN;
}
///
/// Check if 2 stores can be sunk together with corresponding GEPs
///
bool MergedLoadStoreMotion::canSinkStoresAndGEPs(StoreInst *S0,
StoreInst *S1) const {
auto *A0 = dyn_cast<Instruction>(S0->getPointerOperand());
auto *A1 = dyn_cast<Instruction>(S1->getPointerOperand());
return A0 && A1 && A0->isIdenticalTo(A1) && A0->hasOneUse() &&
(A0->getParent() == S0->getParent()) && A1->hasOneUse() &&
(A1->getParent() == S1->getParent()) && isa<GetElementPtrInst>(A0);
}
///
/// Merge two stores to same address and sink into \p BB
///
/// Also sinks GEP instruction computing the store address
///
bool MergedLoadStoreMotion::sinkStore(BasicBlock *BB, StoreInst *S0,
StoreInst *S1) {
void MergedLoadStoreMotion::sinkStoresAndGEPs(BasicBlock *BB, StoreInst *S0,
StoreInst *S1) {
// Only one definition?
auto *A0 = dyn_cast<Instruction>(S0->getPointerOperand());
auto *A1 = dyn_cast<Instruction>(S1->getPointerOperand());
if (A0 && A1 && A0->isIdenticalTo(A1) && A0->hasOneUse() &&
(A0->getParent() == S0->getParent()) && A1->hasOneUse() &&
(A1->getParent() == S1->getParent()) && isa<GetElementPtrInst>(A0)) {
LLVM_DEBUG(dbgs() << "Sink Instruction into BB \n"; BB->dump();
dbgs() << "Instruction Left\n"; S0->dump(); dbgs() << "\n";
dbgs() << "Instruction Right\n"; S1->dump(); dbgs() << "\n");
// Hoist the instruction.
BasicBlock::iterator InsertPt = BB->getFirstInsertionPt();
// Intersect optional metadata.
S0->andIRFlags(S1);
S0->dropUnknownNonDebugMetadata();
LLVM_DEBUG(dbgs() << "Sink Instruction into BB \n"; BB->dump();
dbgs() << "Instruction Left\n"; S0->dump(); dbgs() << "\n";
dbgs() << "Instruction Right\n"; S1->dump(); dbgs() << "\n");
// Hoist the instruction.
BasicBlock::iterator InsertPt = BB->getFirstInsertionPt();
// Intersect optional metadata.
S0->andIRFlags(S1);
S0->dropUnknownNonDebugMetadata();
// Create the new store to be inserted at the join point.
StoreInst *SNew = cast<StoreInst>(S0->clone());
Instruction *ANew = A0->clone();
SNew->insertBefore(&*InsertPt);
ANew->insertBefore(SNew);
// Create the new store to be inserted at the join point.
StoreInst *SNew = cast<StoreInst>(S0->clone());
Instruction *ANew = A0->clone();
SNew->insertBefore(&*InsertPt);
ANew->insertBefore(SNew);
assert(S0->getParent() == A0->getParent());
assert(S1->getParent() == A1->getParent());
assert(S0->getParent() == A0->getParent());
assert(S1->getParent() == A1->getParent());
// New PHI operand? Use it.
if (PHINode *NewPN = getPHIOperand(BB, S0, S1))
SNew->setOperand(0, NewPN);
S0->eraseFromParent();
S1->eraseFromParent();
A0->replaceAllUsesWith(ANew);
A0->eraseFromParent();
A1->replaceAllUsesWith(ANew);
A1->eraseFromParent();
return true;
}
return false;
// New PHI operand? Use it.
if (PHINode *NewPN = getPHIOperand(BB, S0, S1))
SNew->setOperand(0, NewPN);
S0->eraseFromParent();
S1->eraseFromParent();
A0->replaceAllUsesWith(ANew);
A0->eraseFromParent();
A1->replaceAllUsesWith(ANew);
A1->eraseFromParent();
}
///
/// True when two stores are equivalent and can sink into the footer
///
/// Starting from a diamond tail block, iterate over the instructions in one
/// predecessor block and try to match a store in the second predecessor.
/// Starting from a diamond head block, iterate over the instructions in one
/// successor block and try to match a store in the second successor.
///
bool MergedLoadStoreMotion::mergeStores(BasicBlock *T) {
bool MergedLoadStoreMotion::mergeStores(BasicBlock *HeadBB) {
bool MergedStores = false;
assert(T && "Footer of a diamond cannot be empty");
BasicBlock *TailBB = getDiamondTail(HeadBB);
BasicBlock *SinkBB = TailBB;
assert(SinkBB && "Footer of a diamond cannot be empty");
pred_iterator PI = pred_begin(T), E = pred_end(T);
assert(PI != E);
BasicBlock *Pred0 = *PI;
++PI;
BasicBlock *Pred1 = *PI;
++PI;
succ_iterator SI = succ_begin(HeadBB);
assert(SI != succ_end(HeadBB) && "Diamond head cannot have zero successors");
BasicBlock *Pred0 = *SI;
++SI;
assert(SI != succ_end(HeadBB) && "Diamond head cannot have single successor");
BasicBlock *Pred1 = *SI;
// tail block of a diamond/hammock?
if (Pred0 == Pred1)
return false; // No.
if (PI != E)
return false; // No. More than 2 predecessors.
// #Instructions in Succ1 for Compile Time Control
// bail out early if we can not merge into the footer BB
if (!SplitFooterBB && TailBB->hasNPredecessorsOrMore(3))
return false;
// #Instructions in Pred1 for Compile Time Control
auto InstsNoDbg = Pred1->instructionsWithoutDebug();
int Size1 = std::distance(InstsNoDbg.begin(), InstsNoDbg.end());
int NStores = 0;
@ -304,14 +318,23 @@ bool MergedLoadStoreMotion::mergeStores(BasicBlock *T) {
if (NStores * Size1 >= MagicCompileTimeControl)
break;
if (StoreInst *S1 = canSinkFromBlock(Pred1, S0)) {
bool Res = sinkStore(T, S0, S1);
MergedStores |= Res;
// Don't attempt to sink below stores that had to stick around
// But after removal of a store and some of its feeding
// instruction search again from the beginning since the iterator
// is likely stale at this point.
if (!Res)
if (!canSinkStoresAndGEPs(S0, S1))
// Don't attempt to sink below stores that had to stick around
// But after removal of a store and some of its feeding
// instruction search again from the beginning since the iterator
// is likely stale at this point.
break;
if (SinkBB == TailBB && TailBB->hasNPredecessorsOrMore(3)) {
// We have more than 2 predecessors. Insert a new block
// postdominating 2 predecessors we're going to sink from.
SinkBB = SplitBlockPredecessors(TailBB, {Pred0, Pred1}, ".sink.split");
if (!SinkBB)
break;
}
MergedStores = true;
sinkStoresAndGEPs(SinkBB, S0, S1);
RBI = Pred0->rbegin();
RBE = Pred0->rend();
LLVM_DEBUG(dbgs() << "Search again\n"; Instruction *I = &*RBI; I->dump());
@ -328,13 +351,15 @@ bool MergedLoadStoreMotion::run(Function &F, AliasAnalysis &AA) {
// Merge unconditional branches, allowing PRE to catch more
// optimization opportunities.
// This loop doesn't care about newly inserted/split blocks
// since they never will be diamond heads.
for (Function::iterator FI = F.begin(), FE = F.end(); FI != FE;) {
BasicBlock *BB = &*FI++;
// Hoist equivalent loads and sink stores
// outside diamonds when possible
if (isDiamondHead(BB)) {
Changed |= mergeStores(getDiamondTail(BB));
Changed |= mergeStores(BB);
}
}
return Changed;
@ -342,9 +367,11 @@ bool MergedLoadStoreMotion::run(Function &F, AliasAnalysis &AA) {
namespace {
class MergedLoadStoreMotionLegacyPass : public FunctionPass {
const bool SplitFooterBB;
public:
static char ID; // Pass identification, replacement for typeid
MergedLoadStoreMotionLegacyPass() : FunctionPass(ID) {
MergedLoadStoreMotionLegacyPass(bool SplitFooterBB = false)
: FunctionPass(ID), SplitFooterBB(SplitFooterBB) {
initializeMergedLoadStoreMotionLegacyPassPass(
*PassRegistry::getPassRegistry());
}
@ -355,13 +382,14 @@ public:
bool runOnFunction(Function &F) override {
if (skipFunction(F))
return false;
MergedLoadStoreMotion Impl;
MergedLoadStoreMotion Impl(SplitFooterBB);
return Impl.run(F, getAnalysis<AAResultsWrapperPass>().getAAResults());
}
private:
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.setPreservesCFG();
if (!SplitFooterBB)
AU.setPreservesCFG();
AU.addRequired<AAResultsWrapperPass>();
AU.addPreserved<GlobalsAAWrapperPass>();
}
@ -373,8 +401,8 @@ char MergedLoadStoreMotionLegacyPass::ID = 0;
///
/// createMergedLoadStoreMotionPass - The public interface to this file.
///
FunctionPass *llvm::createMergedLoadStoreMotionPass() {
return new MergedLoadStoreMotionLegacyPass();
FunctionPass *llvm::createMergedLoadStoreMotionPass(bool SplitFooterBB) {
return new MergedLoadStoreMotionLegacyPass(SplitFooterBB);
}
INITIALIZE_PASS_BEGIN(MergedLoadStoreMotionLegacyPass, "mldst-motion",
@ -385,13 +413,14 @@ INITIALIZE_PASS_END(MergedLoadStoreMotionLegacyPass, "mldst-motion",
PreservedAnalyses
MergedLoadStoreMotionPass::run(Function &F, FunctionAnalysisManager &AM) {
MergedLoadStoreMotion Impl;
MergedLoadStoreMotion Impl(Options.SplitFooterBB);
auto &AA = AM.getResult<AAManager>(F);
if (!Impl.run(F, AA))
return PreservedAnalyses::all();
PreservedAnalyses PA;
PA.preserveSet<CFGAnalyses>();
if (!Options.SplitFooterBB)
PA.preserveSet<CFGAnalyses>();
PA.preserve<GlobalsAA>();
return PA;
}

View File

@ -0,0 +1,94 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; Test to make sure that a new block is inserted if we
; have more than 2 predecessors for the block we're going to sink to.
; RUN: opt -basicaa -memdep -mldst-motion -S < %s | FileCheck %s --check-prefix=CHECK-NO
; RUN: opt -debug-pass-manager -aa-pipeline=basic-aa -passes='require<memdep>,mldst-motion' -S < %s 2>&1 | FileCheck %s --check-prefixes=CHECK-NO,CHECK-INV-NO
; RUN: opt -debug-pass-manager -aa-pipeline=basic-aa -passes='require<memdep>,mldst-motion<split-footer-bb>' -S < %s 2>&1 | FileCheck %s --check-prefixes=CHECK-YES,CHECK-INV-YES
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
; When passing split-footer-bb to MLSM, it invalidates CFG analyses
; CHECK-INV-NO: Running pass: MergedLoadStoreMotionPass
; CHECK-INV-NO-NOT: Invalidating analysis: DominatorTreeAnalysis
; CHECK-INV-YES: Running pass: MergedLoadStoreMotionPass
; CHECK-INV-YES: Invalidating analysis: DominatorTreeAnalysis
; Function Attrs: nounwind uwtable
define dso_local void @st_sink_split_bb(i32* nocapture %arg, i32* nocapture %arg1, i1 zeroext %arg2, i1 zeroext %arg3) local_unnamed_addr {
; CHECK-NO-LABEL: @st_sink_split_bb(
; CHECK-NO-NEXT: bb:
; CHECK-NO-NEXT: br i1 [[ARG2:%.*]], label [[BB4:%.*]], label [[BB5:%.*]]
; CHECK-NO: bb4:
; CHECK-NO-NEXT: store i32 1, i32* [[ARG:%.*]], align 4
; CHECK-NO-NEXT: br label [[BB9:%.*]]
; CHECK-NO: bb5:
; CHECK-NO-NEXT: br i1 [[ARG3:%.*]], label [[BB6:%.*]], label [[BB7:%.*]]
; CHECK-NO: bb6:
; CHECK-NO-NEXT: store i32 2, i32* [[ARG]], align 4
; CHECK-NO-NEXT: [[TMP1:%.*]] = getelementptr inbounds i32, i32* [[ARG1:%.*]], i64 1
; CHECK-NO-NEXT: store i32 3, i32* [[TMP1]], align 4
; CHECK-NO-NEXT: [[TMP:%.*]] = getelementptr inbounds i32, i32* [[ARG1]], i64 2
; CHECK-NO-NEXT: store i32 3, i32* [[TMP]], align 4
; CHECK-NO-NEXT: br label [[BB9]]
; CHECK-NO: bb7:
; CHECK-NO-NEXT: store i32 3, i32* [[ARG]], align 4
; CHECK-NO-NEXT: [[TMP2:%.*]] = getelementptr inbounds i32, i32* [[ARG1]], i64 1
; CHECK-NO-NEXT: store i32 3, i32* [[TMP2]], align 4
; CHECK-NO-NEXT: [[TMP8:%.*]] = getelementptr inbounds i32, i32* [[ARG1]], i64 2
; CHECK-NO-NEXT: store i32 3, i32* [[TMP8]], align 4
; CHECK-NO-NEXT: br label [[BB9]]
; CHECK-NO: bb9:
; CHECK-NO-NEXT: ret void
;
; CHECK-YES-LABEL: @st_sink_split_bb(
; CHECK-YES-NEXT: bb:
; CHECK-YES-NEXT: br i1 [[ARG2:%.*]], label [[BB4:%.*]], label [[BB5:%.*]]
; CHECK-YES: bb4:
; CHECK-YES-NEXT: store i32 1, i32* [[ARG:%.*]], align 4
; CHECK-YES-NEXT: br label [[BB9:%.*]]
; CHECK-YES: bb5:
; CHECK-YES-NEXT: br i1 [[ARG3:%.*]], label [[BB6:%.*]], label [[BB7:%.*]]
; CHECK-YES: bb6:
; CHECK-YES-NEXT: store i32 2, i32* [[ARG]], align 4
; CHECK-YES-NEXT: br label [[BB9_SINK_SPLIT:%.*]]
; CHECK-YES: bb7:
; CHECK-YES-NEXT: store i32 3, i32* [[ARG]], align 4
; CHECK-YES-NEXT: br label [[BB9_SINK_SPLIT]]
; CHECK-YES: bb9.sink.split:
; CHECK-YES-NEXT: [[TMP0:%.*]] = getelementptr inbounds i32, i32* [[ARG1:%.*]], i64 1
; CHECK-YES-NEXT: store i32 3, i32* [[TMP0]], align 4
; CHECK-YES-NEXT: [[TMP1:%.*]] = getelementptr inbounds i32, i32* [[ARG1]], i64 2
; CHECK-YES-NEXT: store i32 3, i32* [[TMP1]], align 4
; CHECK-YES-NEXT: br label [[BB9]]
; CHECK-YES: bb9:
; CHECK-YES-NEXT: ret void
;
bb:
br i1 %arg2, label %bb4, label %bb5
bb4: ; preds = %bb
store i32 1, i32* %arg, align 4
br label %bb9
bb5: ; preds = %bb
br i1 %arg3, label %bb6, label %bb7
bb6: ; preds = %bb5
store i32 2, i32* %arg, align 4
%tmp1 = getelementptr inbounds i32, i32* %arg1, i64 1
store i32 3, i32* %tmp1, align 4
%tmp = getelementptr inbounds i32, i32* %arg1, i64 2
store i32 3, i32* %tmp, align 4
br label %bb9
bb7: ; preds = %bb5
store i32 3, i32* %arg, align 4
%tmp2 = getelementptr inbounds i32, i32* %arg1, i64 1
store i32 3, i32* %tmp2, align 4
%tmp8 = getelementptr inbounds i32, i32* %arg1, i64 2
store i32 3, i32* %tmp8, align 4
br label %bb9
bb9: ; preds = %bb7, %bb6, %bb4
ret void
}