Skip to content

Commit

Permalink
Recommit "[DSE] Track earliest escape, use for loads in isReadClobber."
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fhahn committed Sep 24, 2021
1 parent e254652 commit 6f28fb7
Show file tree
Hide file tree
Showing 4 changed files with 218 additions and 21 deletions.
14 changes: 14 additions & 0 deletions llvm/include/llvm/Analysis/CaptureTracking.h
Expand Up @@ -23,6 +23,7 @@ namespace llvm {
class Instruction;
class DominatorTree;
class LoopInfo;
class Function;

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

// Returns the 'earliest' instruction that captures \p V in \F. An instruction
// A is considered earlier than instruction B, if A dominates B. If 2 escapes
// do not dominate each other, the terminator of the common dominator is
// chosen. If not all uses can be analyzed, the earliest escape is set to
// the first instruction in the function entry block. If \p V does not escape,
// nullptr is returned. Note that the caller of the function has to ensure
// that the instruction the result value is compared against is not in a
// cycle.
Instruction *FindEarliestCapture(const Value *V, Function &F,
bool ReturnCaptures, bool StoreCaptures,
const DominatorTree &DT,
unsigned MaxUsesToExplore = 0);

/// This callback is used in conjunction with PointerMayBeCaptured. In
/// addition to the interface here, you'll need to provide your own getters
/// to see whether anything was captured.
Expand Down
76 changes: 76 additions & 0 deletions llvm/lib/Analysis/CaptureTracking.cpp
Expand Up @@ -143,6 +143,66 @@ namespace {

const LoopInfo *LI;
};

/// Find the 'earliest' instruction before which the pointer is known not to
/// be captured. Here an instruction A is considered earlier than instruction
/// B, if A dominates B. If 2 escapes do not dominate each other, the
/// terminator of the common dominator is chosen. If not all uses cannot be
/// analyzed, the earliest escape is set to the first instruction in the
/// function entry block.
// NOTE: Users have to make sure instructions compared against the earliest
// escape are not in a cycle.
struct EarliestCaptures : public CaptureTracker {

EarliestCaptures(bool ReturnCaptures, Function &F, const DominatorTree &DT)
: DT(DT), ReturnCaptures(ReturnCaptures), Captured(false), F(F) {}

void tooManyUses() override {
Captured = true;
EarliestCapture = &*F.getEntryBlock().begin();
}

bool captured(const Use *U) override {
Instruction *I = cast<Instruction>(U->getUser());
if (isa<ReturnInst>(I) && !ReturnCaptures)
return false;

if (!EarliestCapture) {
EarliestCapture = I;
} else if (EarliestCapture->getParent() == I->getParent()) {
if (I->comesBefore(EarliestCapture))
EarliestCapture = I;
} else {
BasicBlock *CurrentBB = I->getParent();
BasicBlock *EarliestBB = EarliestCapture->getParent();
if (DT.dominates(EarliestBB, CurrentBB)) {
// EarliestCapture already comes before the current use.
} else if (DT.dominates(CurrentBB, EarliestBB)) {
EarliestCapture = I;
} else {
// Otherwise find the nearest common dominator and use its terminator.
auto *NearestCommonDom =
DT.findNearestCommonDominator(CurrentBB, EarliestBB);
EarliestCapture = NearestCommonDom->getTerminator();
}
}
Captured = true;

// Return false to continue analysis; we need to see all potential
// captures.
return false;
}

Instruction *EarliestCapture = nullptr;

const DominatorTree &DT;

bool ReturnCaptures;

bool Captured;

Function &F;
};
}

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

Instruction *llvm::FindEarliestCapture(const Value *V, Function &F,
bool ReturnCaptures, bool StoreCaptures,
const DominatorTree &DT,
unsigned MaxUsesToExplore) {
assert(!isa<GlobalValue>(V) &&
"It doesn't make sense to ask whether a global is captured.");

EarliestCaptures CB(ReturnCaptures, F, DT);
PointerMayBeCaptured(V, &CB, MaxUsesToExplore);
if (CB.Captured)
++NumCapturedBefore;
else
++NumNotCapturedBefore;
return CB.EarliestCapture;
}

void llvm::PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker,
unsigned MaxUsesToExplore) {
assert(V->getType()->isPointerTy() && "Capture is for pointers only!");
Expand Down
56 changes: 56 additions & 0 deletions llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
Expand Up @@ -38,6 +38,7 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/CFG.h"
#include "llvm/Analysis/CaptureTracking.h"
#include "llvm/Analysis/GlobalsModRef.h"
#include "llvm/Analysis/LoopInfo.h"
Expand Down Expand Up @@ -897,6 +898,9 @@ struct DSEState {
/// basic block.
DenseMap<BasicBlock *, InstOverlapIntervalsTy> IOLs;

DenseMap<const Value *, Instruction *> EarliestEscapes;
DenseMap<Instruction *, TinyPtrVector<const Value *>> Inst2Obj;

DSEState(Function &F, AliasAnalysis &AA, MemorySSA &MSSA, DominatorTree &DT,
PostDominatorTree &PDT, const TargetLibraryInfo &TLI,
const LoopInfo &LI)
Expand Down Expand Up @@ -1264,6 +1268,30 @@ struct DSEState {
DepWriteOffset) == OW_Complete;
}

/// Returns true if \p Object is not captured before or by \p I.
bool notCapturedBeforeOrAt(const Value *Object, Instruction *I) {
if (!isIdentifiedFunctionLocal(Object))
return false;

auto Iter = EarliestEscapes.insert({Object, nullptr});
if (Iter.second) {
Instruction *EarliestCapture = FindEarliestCapture(
Object, F, /*ReturnCaptures=*/false, /*StoreCaptures=*/true, DT);
if (EarliestCapture) {
auto Ins = Inst2Obj.insert({EarliestCapture, {}});
Ins.first->second.push_back(Object);
}
Iter.first->second = EarliestCapture;
}

// No capturing instruction.
if (!Iter.first->second)
return true;

return I != Iter.first->second &&
!isPotentiallyReachable(Iter.first->second, I, nullptr, &DT, &LI);
}

// Returns true if \p Use may read from \p DefLoc.
bool isReadClobber(const MemoryLocation &DefLoc, Instruction *UseInst) {
if (isNoopIntrinsic(UseInst))
Expand All @@ -1281,6 +1309,25 @@ struct DSEState {
if (CB->onlyAccessesInaccessibleMemory())
return false;

// BasicAA does not spend linear time to check whether local objects escape
// before potentially aliasing accesses. To improve DSE results, compute and
// cache escape info for local objects in certain circumstances.
if (auto *LI = dyn_cast<LoadInst>(UseInst)) {
// If the loads reads from a loaded underlying object accesses the load
// cannot alias DefLoc, if DefUO is a local object that has not escaped
// before the load.
auto *ReadUO = getUnderlyingObject(LI->getPointerOperand());
auto *DefUO = getUnderlyingObject(DefLoc.Ptr);
if (DefUO && ReadUO && isa<LoadInst>(ReadUO) &&
notCapturedBeforeOrAt(DefUO, UseInst)) {
assert(
!PointerMayBeCapturedBefore(DefLoc.Ptr, false, true, UseInst, &DT,
false, 0, &this->LI) &&
"cached analysis disagrees with fresh PointerMayBeCapturedBefore");
return false;
}
}

// NOTE: For calls, the number of stores removed could be slightly improved
// by using AA.callCapturesBefore(UseInst, DefLoc, &DT), but that showed to
// be expensive compared to the benefits in practice. For now, avoid more
Expand Down Expand Up @@ -1707,6 +1754,7 @@ struct DSEState {
if (MemoryDef *MD = dyn_cast<MemoryDef>(MA)) {
SkipStores.insert(MD);
}

Updater.removeMemoryAccess(MA);
}

Expand All @@ -1721,6 +1769,14 @@ struct DSEState {
NowDeadInsts.push_back(OpI);
}

// Clear any cached escape info for objects associated with the
// removed instructions.
auto Iter = Inst2Obj.find(DeadInst);
if (Iter != Inst2Obj.end()) {
for (const Value *Obj : Iter->second)
EarliestEscapes.erase(Obj);
Inst2Obj.erase(DeadInst);
}
DeadInst->eraseFromParent();
}
}
Expand Down

0 comments on commit 6f28fb7

Please sign in to comment.