Skip to content

Commit 7f3610a

Browse files
committed
Reapply "Revert "[MemCpyOpt] implement multi BB stack-move optimization"
This reverts commit efe8aa2. Differential Revision: https://reviews.llvm.org/D155406
1 parent 2dee316 commit 7f3610a

File tree

8 files changed

+81
-159
lines changed

8 files changed

+81
-159
lines changed

llvm/include/llvm/Transforms/Scalar/MemCpyOptimizer.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class MemMoveInst;
3434
class MemorySSA;
3535
class MemorySSAUpdater;
3636
class MemSetInst;
37+
class PostDominatorTree;
3738
class StoreInst;
3839
class TargetLibraryInfo;
3940
class Value;
@@ -43,6 +44,7 @@ class MemCpyOptPass : public PassInfoMixin<MemCpyOptPass> {
4344
AAResults *AA = nullptr;
4445
AssumptionCache *AC = nullptr;
4546
DominatorTree *DT = nullptr;
47+
PostDominatorTree *PDT = nullptr;
4648
MemorySSA *MSSA = nullptr;
4749
MemorySSAUpdater *MSSAU = nullptr;
4850

@@ -53,7 +55,8 @@ class MemCpyOptPass : public PassInfoMixin<MemCpyOptPass> {
5355

5456
// Glue for the old PM.
5557
bool runImpl(Function &F, TargetLibraryInfo *TLI, AAResults *AA,
56-
AssumptionCache *AC, DominatorTree *DT, MemorySSA *MSSA);
58+
AssumptionCache *AC, DominatorTree *DT, PostDominatorTree *PDT,
59+
MemorySSA *MSSA);
5760

5861
private:
5962
// Helper functions

llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

Lines changed: 57 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
#include "llvm/ADT/iterator_range.h"
2020
#include "llvm/Analysis/AliasAnalysis.h"
2121
#include "llvm/Analysis/AssumptionCache.h"
22+
#include "llvm/Analysis/CFG.h"
2223
#include "llvm/Analysis/CaptureTracking.h"
2324
#include "llvm/Analysis/GlobalsModRef.h"
2425
#include "llvm/Analysis/Loads.h"
2526
#include "llvm/Analysis/MemoryLocation.h"
2627
#include "llvm/Analysis/MemorySSA.h"
2728
#include "llvm/Analysis/MemorySSAUpdater.h"
29+
#include "llvm/Analysis/PostDominators.h"
2830
#include "llvm/Analysis/TargetLibraryInfo.h"
2931
#include "llvm/Analysis/ValueTracking.h"
3032
#include "llvm/IR/BasicBlock.h"
@@ -1440,8 +1442,7 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
14401442
return false;
14411443
}
14421444

1443-
// 1. Check that copy is full. Calculate the static size of the allocas to be
1444-
// merged, bail out if we can't.
1445+
// Check that copy is full with static size.
14451446
const DataLayout &DL = DestAlloca->getModule()->getDataLayout();
14461447
std::optional<TypeSize> SrcSize = SrcAlloca->getAllocationSize(DL);
14471448
if (!SrcSize || SrcSize->isScalable() || Size != SrcSize->getFixedValue()) {
@@ -1455,19 +1456,15 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
14551456
return false;
14561457
}
14571458

1458-
// 2-1. Check that src and dest are static allocas, which are not affected by
1459-
// stacksave/stackrestore.
1460-
if (!SrcAlloca->isStaticAlloca() || !DestAlloca->isStaticAlloca() ||
1461-
SrcAlloca->getParent() != Load->getParent() ||
1462-
SrcAlloca->getParent() != Store->getParent())
1459+
if (!SrcAlloca->isStaticAlloca() || !DestAlloca->isStaticAlloca())
14631460
return false;
14641461

1465-
// 2-2. Check that src and dest are never captured, unescaped allocas. Also
1466-
// collect lifetime markers first/last users in order to shrink wrap the
1467-
// lifetimes, and instructions with noalias metadata to remove them.
1462+
// Check that src and dest are never captured, unescaped allocas. Also
1463+
// find the nearest common dominator and postdominator for all users in
1464+
// order to shrink wrap the lifetimes, and instructions with noalias metadata
1465+
// to remove them.
14681466

14691467
SmallVector<Instruction *, 4> LifetimeMarkers;
1470-
Instruction *FirstUser = nullptr, *LastUser = nullptr;
14711468
SmallSet<Instruction *, 4> NoAliasInstrs;
14721469

14731470
// Recursively track the user and check whether modified alias exist.
@@ -1505,12 +1502,6 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
15051502
continue;
15061503
case UseCaptureKind::NO_CAPTURE: {
15071504
auto *UI = cast<Instruction>(U.getUser());
1508-
if (DestAlloca->getParent() != UI->getParent())
1509-
return false;
1510-
if (!FirstUser || UI->comesBefore(FirstUser))
1511-
FirstUser = UI;
1512-
if (!LastUser || LastUser->comesBefore(UI))
1513-
LastUser = UI;
15141505
if (UI->isLifetimeStartOrEnd()) {
15151506
// We note the locations of these intrinsic calls so that we can
15161507
// delete them later if the optimization succeeds, this is safe
@@ -1534,37 +1525,64 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
15341525
return true;
15351526
};
15361527

1537-
// 3. Check that dest has no Mod/Ref, except full size lifetime intrinsics,
1538-
// from the alloca to the Store.
1528+
// Check that dest has no Mod/Ref, from the alloca to the Store, except full
1529+
// size lifetime intrinsics. And collect modref inst for the reachability
1530+
// check.
15391531
ModRefInfo DestModRef = ModRefInfo::NoModRef;
15401532
MemoryLocation DestLoc(DestAlloca, LocationSize::precise(Size));
1533+
SmallVector<BasicBlock *, 8> ReachabilityWorklist;
15411534
auto DestModRefCallback = [&](Instruction *UI) -> bool {
15421535
// We don't care about the store itself.
15431536
if (UI == Store)
15441537
return true;
15451538
ModRefInfo Res = BAA.getModRefInfo(UI, DestLoc);
1546-
// FIXME: For multi-BB cases, we need to see reachability from it to
1547-
// store.
1548-
// Bailout if Dest may have any ModRef before Store.
1549-
if (UI->comesBefore(Store) && isModOrRefSet(Res))
1550-
return false;
1551-
DestModRef |= BAA.getModRefInfo(UI, DestLoc);
1539+
DestModRef |= Res;
1540+
if (isModOrRefSet(Res)) {
1541+
// Instructions reachability checks.
1542+
// FIXME: adding the Instruction version isPotentiallyReachableFromMany on
1543+
// lib/Analysis/CFG.cpp (currently only for BasicBlocks) might be helpful.
1544+
if (UI->getParent() == Store->getParent()) {
1545+
// The same block case is special because it's the only time we're
1546+
// looking within a single block to see which instruction comes first.
1547+
// Once we start looking at multiple blocks, the first instruction of
1548+
// the block is reachable, so we only need to determine reachability
1549+
// between whole blocks.
1550+
BasicBlock *BB = UI->getParent();
1551+
1552+
// If A comes before B, then B is definitively reachable from A.
1553+
if (UI->comesBefore(Store))
1554+
return false;
1555+
1556+
// If the user's parent block is entry, no predecessor exists.
1557+
if (BB->isEntryBlock())
1558+
return true;
15521559

1560+
// Otherwise, continue doing the normal per-BB CFG walk.
1561+
ReachabilityWorklist.append(succ_begin(BB), succ_end(BB));
1562+
} else {
1563+
ReachabilityWorklist.push_back(UI->getParent());
1564+
}
1565+
}
15531566
return true;
15541567
};
15551568

15561569
if (!CaptureTrackingWithModRef(DestAlloca, DestModRefCallback))
15571570
return false;
1571+
// Bailout if Dest may have any ModRef before Store.
1572+
if (!ReachabilityWorklist.empty() &&
1573+
isPotentiallyReachableFromMany(ReachabilityWorklist, Store->getParent(),
1574+
nullptr, DT, nullptr))
1575+
return false;
15581576

1559-
// 3. Check that, from after the Load to the end of the BB,
1560-
// 3-1. if the dest has any Mod, src has no Ref, and
1561-
// 3-2. if the dest has any Ref, src has no Mod except full-sized lifetimes.
1577+
// Check that, from after the Load to the end of the BB,
1578+
// - if the dest has any Mod, src has no Ref, and
1579+
// - if the dest has any Ref, src has no Mod except full-sized lifetimes.
15621580
MemoryLocation SrcLoc(SrcAlloca, LocationSize::precise(Size));
15631581

15641582
auto SrcModRefCallback = [&](Instruction *UI) -> bool {
1565-
// Any ModRef before Load doesn't matter, also Load and Store can be
1566-
// ignored.
1567-
if (UI->comesBefore(Load) || UI == Load || UI == Store)
1583+
// Any ModRef post-dominated by Load doesn't matter, also Load and Store
1584+
// themselves can be ignored.
1585+
if (PDT->dominates(Load, UI) || UI == Load || UI == Store)
15681586
return true;
15691587
ModRefInfo Res = BAA.getModRefInfo(UI, SrcLoc);
15701588
if ((isModSet(DestModRef) && isRefSet(Res)) ||
@@ -1588,34 +1606,10 @@ bool MemCpyOptPass::performStackMoveOptzn(Instruction *Load, Instruction *Store,
15881606
// Drop metadata on the source alloca.
15891607
SrcAlloca->dropUnknownNonDebugMetadata();
15901608

1591-
// Do "shrink wrap" the lifetimes, if the original lifetime intrinsics exists.
1609+
// TODO: Reconstruct merged lifetime markers.
1610+
// Remove all other lifetime markers. if the original lifetime intrinsics
1611+
// exists.
15921612
if (!LifetimeMarkers.empty()) {
1593-
LLVMContext &C = SrcAlloca->getContext();
1594-
IRBuilder<> Builder(C);
1595-
1596-
ConstantInt *AllocaSize = ConstantInt::get(Type::getInt64Ty(C), Size);
1597-
// Create a new lifetime start marker before the first user of src or alloca
1598-
// users.
1599-
Builder.SetInsertPoint(FirstUser->getParent(), FirstUser->getIterator());
1600-
auto *Start = Builder.CreateLifetimeStart(SrcAlloca, AllocaSize);
1601-
auto *FirstMA = MSSA->getMemoryAccess(FirstUser);
1602-
auto *StartMA = MSSAU->createMemoryAccessBefore(Start, nullptr, FirstMA);
1603-
MSSAU->insertDef(cast<MemoryDef>(StartMA), /*RenameUses=*/true);
1604-
1605-
// Create a new lifetime end marker after the last user of src or alloca
1606-
// users.
1607-
// FIXME: If the last user is the terminator for the bb, we can insert
1608-
// lifetime.end marker to the immidiate post-dominator, but currently do
1609-
// nothing.
1610-
if (!LastUser->isTerminator()) {
1611-
Builder.SetInsertPoint(LastUser->getParent(), ++LastUser->getIterator());
1612-
auto *End = Builder.CreateLifetimeEnd(SrcAlloca, AllocaSize);
1613-
auto *LastMA = MSSA->getMemoryAccess(LastUser);
1614-
auto *EndMA = MSSAU->createMemoryAccessAfter(End, nullptr, LastMA);
1615-
MSSAU->insertDef(cast<MemoryDef>(EndMA), /*RenameUses=*/true);
1616-
}
1617-
1618-
// Remove all other lifetime markers.
16191613
for (Instruction *I : LifetimeMarkers)
16201614
eraseInstruction(I);
16211615
}
@@ -1999,9 +1993,10 @@ PreservedAnalyses MemCpyOptPass::run(Function &F, FunctionAnalysisManager &AM) {
19991993
auto *AA = &AM.getResult<AAManager>(F);
20001994
auto *AC = &AM.getResult<AssumptionAnalysis>(F);
20011995
auto *DT = &AM.getResult<DominatorTreeAnalysis>(F);
1996+
auto *PDT = &AM.getResult<PostDominatorTreeAnalysis>(F);
20021997
auto *MSSA = &AM.getResult<MemorySSAAnalysis>(F);
20031998

2004-
bool MadeChange = runImpl(F, &TLI, AA, AC, DT, &MSSA->getMSSA());
1999+
bool MadeChange = runImpl(F, &TLI, AA, AC, DT, PDT, &MSSA->getMSSA());
20052000
if (!MadeChange)
20062001
return PreservedAnalyses::all();
20072002

@@ -2013,12 +2008,14 @@ PreservedAnalyses MemCpyOptPass::run(Function &F, FunctionAnalysisManager &AM) {
20132008

20142009
bool MemCpyOptPass::runImpl(Function &F, TargetLibraryInfo *TLI_,
20152010
AliasAnalysis *AA_, AssumptionCache *AC_,
2016-
DominatorTree *DT_, MemorySSA *MSSA_) {
2011+
DominatorTree *DT_, PostDominatorTree *PDT_,
2012+
MemorySSA *MSSA_) {
20172013
bool MadeChange = false;
20182014
TLI = TLI_;
20192015
AA = AA_;
20202016
AC = AC_;
20212017
DT = DT_;
2018+
PDT = PDT_;
20222019
MSSA = MSSA_;
20232020
MemorySSAUpdater MSSAU_(MSSA_);
20242021
MSSAU = &MSSAU_;

llvm/test/Other/new-pm-defaults.ll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@
190190
; CHECK-O23SZ-NEXT: Running pass: GVNPass
191191
; CHECK-O23SZ-NEXT: Running analysis: MemoryDependenceAnalysis
192192
; CHECK-O1-NEXT: Running pass: MemCpyOptPass
193+
; CHECK-O1-NEXT: Running analysis: PostDominatorTreeAnalysis
193194
; CHECK-O-NEXT: Running pass: SCCPPass
194195
; CHECK-O-NEXT: Running pass: BDCEPass
195196
; CHECK-O-NEXT: Running analysis: DemandedBitsAnalysis
@@ -201,7 +202,7 @@
201202
; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
202203
; CHECK-O1-NEXT: Running pass: CoroElidePass
203204
; CHECK-O-NEXT: Running pass: ADCEPass
204-
; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis
205+
; CHECK-O23SZ-NEXT: Running analysis: PostDominatorTreeAnalysis
205206
; CHECK-O23SZ-NEXT: Running pass: MemCpyOptPass
206207
; CHECK-O23SZ-NEXT: Running pass: DSEPass
207208
; CHECK-O23SZ-NEXT: Running pass: MoveAutoInitPass on foo

llvm/test/Other/new-pm-lto-defaults.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@
103103
; CHECK-O23SZ-NEXT: Running pass: GVNPass on foo
104104
; CHECK-O23SZ-NEXT: Running analysis: MemoryDependenceAnalysis on foo
105105
; CHECK-O23SZ-NEXT: Running pass: MemCpyOptPass on foo
106-
; CHECK-O23SZ-NEXT: Running pass: DSEPass on foo
107106
; CHECK-O23SZ-NEXT: Running analysis: PostDominatorTreeAnalysis on foo
107+
; CHECK-O23SZ-NEXT: Running pass: DSEPass on foo
108108
; CHECK-O23SZ-NEXT: Running pass: MoveAutoInitPass on foo
109109
; CHECK-O23SZ-NEXT: Running pass: MergedLoadStoreMotionPass on foo
110110
; CHECK-O23SZ-NEXT: Running pass: LoopSimplifyPass on foo

llvm/test/Other/new-pm-thinlto-postlink-defaults.ll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@
125125
; CHECK-O23SZ-NEXT: Running pass: GVNPass
126126
; CHECK-O23SZ-NEXT: Running analysis: MemoryDependenceAnalysis
127127
; CHECK-O1-NEXT: Running pass: MemCpyOptPass
128+
; CHECK-O1-NEXT: Running analysis: PostDominatorTreeAnalysis
128129
; CHECK-O-NEXT: Running pass: SCCPPass
129130
; CHECK-O-NEXT: Running pass: BDCEPass
130131
; CHECK-O-NEXT: Running analysis: DemandedBitsAnalysis
@@ -135,7 +136,7 @@
135136
; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
136137
; CHECK-O1-NEXT: Running pass: CoroElidePass
137138
; CHECK-O-NEXT: Running pass: ADCEPass
138-
; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis
139+
; CHECK-O23SZ-NEXT: Running analysis: PostDominatorTreeAnalysis
139140
; CHECK-O23SZ-NEXT: Running pass: MemCpyOptPass
140141
; CHECK-O23SZ-NEXT: Running pass: DSEPass
141142
; CHECK-O23SZ-NEXT: Running pass: MoveAutoInitPass on foo

llvm/test/Other/new-pm-thinlto-prelink-defaults.ll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@
157157
; CHECK-O23SZ-NEXT: Running pass: GVNPass
158158
; CHECK-O23SZ-NEXT: Running analysis: MemoryDependenceAnalysis
159159
; CHECK-O1-NEXT: Running pass: MemCpyOptPass
160+
; CHECK-O1-NEXT: Running analysis: PostDominatorTreeAnalysis
160161
; CHECK-O-NEXT: Running pass: SCCPPass
161162
; CHECK-O-NEXT: Running pass: BDCEPass
162163
; CHECK-O-NEXT: Running analysis: DemandedBitsAnalysis
@@ -167,7 +168,7 @@
167168
; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
168169
; CHECK-O1-NEXT: Running pass: CoroElidePass
169170
; CHECK-O-NEXT: Running pass: ADCEPass
170-
; CHECK-O-NEXT: Running analysis: PostDominatorTreeAnalysis
171+
; CHECK-O23SZ-NEXT: Running analysis: PostDominatorTreeAnalysis
171172
; CHECK-O23SZ-NEXT: Running pass: MemCpyOptPass
172173
; CHECK-O23SZ-NEXT: Running pass: DSEPass
173174
; CHECK-O23SZ-NEXT: Running pass: MoveAutoInitPass

llvm/test/Transforms/MemCpyOpt/lifetime-missing.ll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,9 @@ define void @test() {
1515
; CHECK-NEXT: entry:
1616
; CHECK-NEXT: [[AGG_TMP_SROA_14:%.*]] = alloca [20 x i8], align 4
1717
; CHECK-NEXT: [[AGG_TMP_SROA_14_128_SROA_IDX:%.*]] = getelementptr i8, ptr [[AGG_TMP_SROA_14]], i64 4
18-
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 20, ptr [[AGG_TMP_SROA_14]])
1918
; CHECK-NEXT: call void @llvm.memset.p0.i64(ptr [[AGG_TMP_SROA_14_128_SROA_IDX]], i8 0, i64 1, i1 false)
2019
; CHECK-NEXT: [[AGG_TMP3_SROA_35_128_SROA_IDX:%.*]] = getelementptr i8, ptr [[AGG_TMP_SROA_14]], i64 4
2120
; CHECK-NEXT: call void @llvm.memset.p0.i64(ptr inttoptr (i64 4 to ptr), i8 0, i64 1, i1 false)
22-
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 20, ptr [[AGG_TMP_SROA_14]])
2321
; CHECK-NEXT: call void @llvm.memset.p0.i64(ptr null, i8 0, i64 1, i1 false)
2422
; CHECK-NEXT: ret void
2523
;

0 commit comments

Comments
 (0)