Skip to content

Commit ccdf744

Browse files
committed
Recommit "[DSE] Track earliest escape, use for loads in isReadClobber."
This reverts the revert commit df56fc6. This version of the patch adjusts the location where the EarliestEscapes cache is cleared when an instruction gets removed. The earliest escaping instruction does not have to be a memory instruction. It could be a ptrtoint instruction like in the added test @earliest_escape_ptrtoint, which subsequently gets removed. We need to invalidate the EarliestEscape entry referring to the ptrtoint when deleting it. This fixes the crash mentioned in https://bugs.chromium.org/p/chromium/issues/detail?id=1252762#c6
1 parent c39c786 commit ccdf744

File tree

4 files changed

+218
-21
lines changed

4 files changed

+218
-21
lines changed

llvm/include/llvm/Analysis/CaptureTracking.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ namespace llvm {
2323
class Instruction;
2424
class DominatorTree;
2525
class LoopInfo;
26+
class Function;
2627

2728
/// getDefaultMaxUsesToExploreForCaptureTracking - Return default value of
2829
/// the maximal number of uses to explore before giving up. It is used by
@@ -63,6 +64,19 @@ namespace llvm {
6364
unsigned MaxUsesToExplore = 0,
6465
const LoopInfo *LI = nullptr);
6566

67+
// Returns the 'earliest' instruction that captures \p V in \F. An instruction
68+
// A is considered earlier than instruction B, if A dominates B. If 2 escapes
69+
// do not dominate each other, the terminator of the common dominator is
70+
// chosen. If not all uses can be analyzed, the earliest escape is set to
71+
// the first instruction in the function entry block. If \p V does not escape,
72+
// nullptr is returned. Note that the caller of the function has to ensure
73+
// that the instruction the result value is compared against is not in a
74+
// cycle.
75+
Instruction *FindEarliestCapture(const Value *V, Function &F,
76+
bool ReturnCaptures, bool StoreCaptures,
77+
const DominatorTree &DT,
78+
unsigned MaxUsesToExplore = 0);
79+
6680
/// This callback is used in conjunction with PointerMayBeCaptured. In
6781
/// addition to the interface here, you'll need to provide your own getters
6882
/// to see whether anything was captured.

llvm/lib/Analysis/CaptureTracking.cpp

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,66 @@ namespace {
143143

144144
const LoopInfo *LI;
145145
};
146+
147+
/// Find the 'earliest' instruction before which the pointer is known not to
148+
/// be captured. Here an instruction A is considered earlier than instruction
149+
/// B, if A dominates B. If 2 escapes do not dominate each other, the
150+
/// terminator of the common dominator is chosen. If not all uses cannot be
151+
/// analyzed, the earliest escape is set to the first instruction in the
152+
/// function entry block.
153+
// NOTE: Users have to make sure instructions compared against the earliest
154+
// escape are not in a cycle.
155+
struct EarliestCaptures : public CaptureTracker {
156+
157+
EarliestCaptures(bool ReturnCaptures, Function &F, const DominatorTree &DT)
158+
: DT(DT), ReturnCaptures(ReturnCaptures), Captured(false), F(F) {}
159+
160+
void tooManyUses() override {
161+
Captured = true;
162+
EarliestCapture = &*F.getEntryBlock().begin();
163+
}
164+
165+
bool captured(const Use *U) override {
166+
Instruction *I = cast<Instruction>(U->getUser());
167+
if (isa<ReturnInst>(I) && !ReturnCaptures)
168+
return false;
169+
170+
if (!EarliestCapture) {
171+
EarliestCapture = I;
172+
} else if (EarliestCapture->getParent() == I->getParent()) {
173+
if (I->comesBefore(EarliestCapture))
174+
EarliestCapture = I;
175+
} else {
176+
BasicBlock *CurrentBB = I->getParent();
177+
BasicBlock *EarliestBB = EarliestCapture->getParent();
178+
if (DT.dominates(EarliestBB, CurrentBB)) {
179+
// EarliestCapture already comes before the current use.
180+
} else if (DT.dominates(CurrentBB, EarliestBB)) {
181+
EarliestCapture = I;
182+
} else {
183+
// Otherwise find the nearest common dominator and use its terminator.
184+
auto *NearestCommonDom =
185+
DT.findNearestCommonDominator(CurrentBB, EarliestBB);
186+
EarliestCapture = NearestCommonDom->getTerminator();
187+
}
188+
}
189+
Captured = true;
190+
191+
// Return false to continue analysis; we need to see all potential
192+
// captures.
193+
return false;
194+
}
195+
196+
Instruction *EarliestCapture = nullptr;
197+
198+
const DominatorTree &DT;
199+
200+
bool ReturnCaptures;
201+
202+
bool Captured;
203+
204+
Function &F;
205+
};
146206
}
147207

148208
/// PointerMayBeCaptured - Return true if this pointer value may be captured
@@ -206,6 +266,22 @@ bool llvm::PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
206266
return CB.Captured;
207267
}
208268

269+
Instruction *llvm::FindEarliestCapture(const Value *V, Function &F,
270+
bool ReturnCaptures, bool StoreCaptures,
271+
const DominatorTree &DT,
272+
unsigned MaxUsesToExplore) {
273+
assert(!isa<GlobalValue>(V) &&
274+
"It doesn't make sense to ask whether a global is captured.");
275+
276+
EarliestCaptures CB(ReturnCaptures, F, DT);
277+
PointerMayBeCaptured(V, &CB, MaxUsesToExplore);
278+
if (CB.Captured)
279+
++NumCapturedBefore;
280+
else
281+
++NumNotCapturedBefore;
282+
return CB.EarliestCapture;
283+
}
284+
209285
void llvm::PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker,
210286
unsigned MaxUsesToExplore) {
211287
assert(V->getType()->isPointerTy() && "Capture is for pointers only!");

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "llvm/ADT/Statistic.h"
3939
#include "llvm/ADT/StringRef.h"
4040
#include "llvm/Analysis/AliasAnalysis.h"
41+
#include "llvm/Analysis/CFG.h"
4142
#include "llvm/Analysis/CaptureTracking.h"
4243
#include "llvm/Analysis/GlobalsModRef.h"
4344
#include "llvm/Analysis/LoopInfo.h"
@@ -892,6 +893,9 @@ struct DSEState {
892893
/// basic block.
893894
DenseMap<BasicBlock *, InstOverlapIntervalsTy> IOLs;
894895

896+
DenseMap<const Value *, Instruction *> EarliestEscapes;
897+
DenseMap<Instruction *, TinyPtrVector<const Value *>> Inst2Obj;
898+
895899
DSEState(Function &F, AliasAnalysis &AA, MemorySSA &MSSA, DominatorTree &DT,
896900
PostDominatorTree &PDT, const TargetLibraryInfo &TLI,
897901
const LoopInfo &LI)
@@ -1253,6 +1257,30 @@ struct DSEState {
12531257
InstWriteOffset) == OW_Complete;
12541258
}
12551259

1260+
/// Returns true if \p Object is not captured before or by \p I.
1261+
bool notCapturedBeforeOrAt(const Value *Object, Instruction *I) {
1262+
if (!isIdentifiedFunctionLocal(Object))
1263+
return false;
1264+
1265+
auto Iter = EarliestEscapes.insert({Object, nullptr});
1266+
if (Iter.second) {
1267+
Instruction *EarliestCapture = FindEarliestCapture(
1268+
Object, F, /*ReturnCaptures=*/false, /*StoreCaptures=*/true, DT);
1269+
if (EarliestCapture) {
1270+
auto Ins = Inst2Obj.insert({EarliestCapture, {}});
1271+
Ins.first->second.push_back(Object);
1272+
}
1273+
Iter.first->second = EarliestCapture;
1274+
}
1275+
1276+
// No capturing instruction.
1277+
if (!Iter.first->second)
1278+
return true;
1279+
1280+
return I != Iter.first->second &&
1281+
!isPotentiallyReachable(Iter.first->second, I, nullptr, &DT, &LI);
1282+
}
1283+
12561284
// Returns true if \p Use may read from \p DefLoc.
12571285
bool isReadClobber(const MemoryLocation &DefLoc, Instruction *UseInst) {
12581286
if (isNoopIntrinsic(UseInst))
@@ -1270,6 +1298,25 @@ struct DSEState {
12701298
if (CB->onlyAccessesInaccessibleMemory())
12711299
return false;
12721300

1301+
// BasicAA does not spend linear time to check whether local objects escape
1302+
// before potentially aliasing accesses. To improve DSE results, compute and
1303+
// cache escape info for local objects in certain circumstances.
1304+
if (auto *LI = dyn_cast<LoadInst>(UseInst)) {
1305+
// If the loads reads from a loaded underlying object accesses the load
1306+
// cannot alias DefLoc, if DefUO is a local object that has not escaped
1307+
// before the load.
1308+
auto *ReadUO = getUnderlyingObject(LI->getPointerOperand());
1309+
auto *DefUO = getUnderlyingObject(DefLoc.Ptr);
1310+
if (DefUO && ReadUO && isa<LoadInst>(ReadUO) &&
1311+
notCapturedBeforeOrAt(DefUO, UseInst)) {
1312+
assert(
1313+
!PointerMayBeCapturedBefore(DefLoc.Ptr, false, true, UseInst, &DT,
1314+
false, 0, &this->LI) &&
1315+
"cached analysis disagrees with fresh PointerMayBeCapturedBefore");
1316+
return false;
1317+
}
1318+
}
1319+
12731320
// NOTE: For calls, the number of stores removed could be slightly improved
12741321
// by using AA.callCapturesBefore(UseInst, DefLoc, &DT), but that showed to
12751322
// be expensive compared to the benefits in practice. For now, avoid more
@@ -1701,6 +1748,7 @@ struct DSEState {
17011748
if (MemoryDef *MD = dyn_cast<MemoryDef>(MA)) {
17021749
SkipStores.insert(MD);
17031750
}
1751+
17041752
Updater.removeMemoryAccess(MA);
17051753
}
17061754

@@ -1715,6 +1763,14 @@ struct DSEState {
17151763
NowDeadInsts.push_back(OpI);
17161764
}
17171765

1766+
// Clear any cached escape info for objects associated with the
1767+
// removed instructions.
1768+
auto Iter = Inst2Obj.find(DeadInst);
1769+
if (Iter != Inst2Obj.end()) {
1770+
for (const Value *Obj : Iter->second)
1771+
EarliestEscapes.erase(Obj);
1772+
Inst2Obj.erase(DeadInst);
1773+
}
17181774
DeadInst->eraseFromParent();
17191775
}
17201776
}

0 commit comments

Comments
 (0)