Skip to content

Commit f9599bb

Browse files
caojoshuacaojoshua
committed
[AssumptionCache] caches @llvm.experimental.guard's
As discussed in #59901 This change is not NFC. There is one SCEV and EarlyCSE test that have an improved analysis/optimization case. Rest of the tests are not failing. I've mostly only added cleanup to SCEV since that is where this issue started. As a follow up, I believe there is more cleanup opportunity in SCEV and other affected passes. There could be cases where there are missed registerAssumption of guards, but this case is not so bad because there will be no miscompilation. AssumptionCacheTracker should take care of deleted guards. Differential Revision: https://reviews.llvm.org/D142330
1 parent 5ba8ecb commit f9599bb

File tree

11 files changed

+57
-85
lines changed

11 files changed

+57
-85
lines changed

‎llvm/include/llvm/Analysis/AssumptionCache.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
namespace llvm {
2828

29-
class AssumeInst;
29+
class CondGuardInst;
3030
class Function;
3131
class raw_ostream;
3232
class TargetTransformInfo;
@@ -120,15 +120,15 @@ class AssumptionCache {
120120
///
121121
/// The call passed in must be an instruction within this function and must
122122
/// not already be in the cache.
123-
void registerAssumption(AssumeInst *CI);
123+
void registerAssumption(CondGuardInst *CI);
124124

125125
/// Remove an \@llvm.assume intrinsic from this function's cache if it has
126126
/// been added to the cache earlier.
127-
void unregisterAssumption(AssumeInst *CI);
127+
void unregisterAssumption(CondGuardInst *CI);
128128

129129
/// Update the cache of values being affected by this assumption (i.e.
130130
/// the values about which this assumption provides information).
131-
void updateAffectedValues(AssumeInst *CI);
131+
void updateAffectedValues(CondGuardInst *CI);
132132

133133
/// Clear the cache of \@llvm.assume intrinsics for a function.
134134
///

‎llvm/include/llvm/IR/IntrinsicInst.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1513,9 +1513,20 @@ class GCResultInst : public GCProjectionInst {
15131513
}
15141514
};
15151515

1516+
/// This represents intrinsics that guard a condition
1517+
class CondGuardInst : public IntrinsicInst {
1518+
public:
1519+
static bool classof(const IntrinsicInst *I) {
1520+
return I->getIntrinsicID() == Intrinsic::assume ||
1521+
I->getIntrinsicID() == Intrinsic::experimental_guard;
1522+
}
1523+
static bool classof(const Value *V) {
1524+
return isa<IntrinsicInst>(V) && classof(cast<IntrinsicInst>(V));
1525+
}
1526+
};
15161527

15171528
/// This represents the llvm.assume intrinsic.
1518-
class AssumeInst : public IntrinsicInst {
1529+
class AssumeInst : public CondGuardInst {
15191530
public:
15201531
static bool classof(const IntrinsicInst *I) {
15211532
return I->getIntrinsicID() == Intrinsic::assume;

‎llvm/lib/Analysis/AssumeBundleQueries.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ llvm::getKnowledgeForValue(const Value *V,
162162
return RetainedKnowledge::none();
163163
if (AC) {
164164
for (AssumptionCache::ResultElem &Elem : AC->assumptionsFor(V)) {
165-
auto *II = cast_or_null<AssumeInst>(Elem.Assume);
165+
auto *II = dyn_cast_or_null<AssumeInst>(Elem.Assume);
166166
if (!II || Elem.Index == AssumptionCache::ExprResultIdx)
167167
continue;
168168
if (RetainedKnowledge RK = getKnowledgeFromBundle(

‎llvm/lib/Analysis/AssumptionCache.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
//
77
//===----------------------------------------------------------------------===//
88
//
9-
// This file contains a pass that keeps track of @llvm.assume intrinsics in
10-
// the functions of a module.
9+
// This file contains a pass that keeps track of @llvm.assume and
10+
// @llvm.experimental.guard intrinsics in the functions of a module.
1111
//
1212
//===----------------------------------------------------------------------===//
1313

@@ -140,7 +140,7 @@ findAffectedValues(CallBase *CI, TargetTransformInfo *TTI,
140140
}
141141
}
142142

143-
void AssumptionCache::updateAffectedValues(AssumeInst *CI) {
143+
void AssumptionCache::updateAffectedValues(CondGuardInst *CI) {
144144
SmallVector<AssumptionCache::ResultElem, 16> Affected;
145145
findAffectedValues(CI, TTI, Affected);
146146

@@ -153,7 +153,7 @@ void AssumptionCache::updateAffectedValues(AssumeInst *CI) {
153153
}
154154
}
155155

156-
void AssumptionCache::unregisterAssumption(AssumeInst *CI) {
156+
void AssumptionCache::unregisterAssumption(CondGuardInst *CI) {
157157
SmallVector<AssumptionCache::ResultElem, 16> Affected;
158158
findAffectedValues(CI, TTI, Affected);
159159

@@ -217,18 +217,18 @@ void AssumptionCache::scanFunction() {
217217
// to this cache.
218218
for (BasicBlock &B : F)
219219
for (Instruction &I : B)
220-
if (isa<AssumeInst>(&I))
220+
if (isa<CondGuardInst>(&I))
221221
AssumeHandles.push_back({&I, ExprResultIdx});
222222

223223
// Mark the scan as complete.
224224
Scanned = true;
225225

226226
// Update affected values.
227227
for (auto &A : AssumeHandles)
228-
updateAffectedValues(cast<AssumeInst>(A));
228+
updateAffectedValues(cast<CondGuardInst>(A));
229229
}
230230

231-
void AssumptionCache::registerAssumption(AssumeInst *CI) {
231+
void AssumptionCache::registerAssumption(CondGuardInst *CI) {
232232
// If we haven't scanned the function yet, just drop this assumption. It will
233233
// be found when we scan later.
234234
if (!Scanned)
@@ -238,9 +238,9 @@ void AssumptionCache::registerAssumption(AssumeInst *CI) {
238238

239239
#ifndef NDEBUG
240240
assert(CI->getParent() &&
241-
"Cannot register @llvm.assume call not in a basic block");
241+
"Cannot a register CondGuardInst not in a basic block");
242242
assert(&F == CI->getParent()->getParent() &&
243-
"Cannot register @llvm.assume call not in this function");
243+
"Cannot a register CondGuardInst not in this function");
244244

245245
// We expect the number of assumptions to be small, so in an asserts build
246246
// check that we don't accumulate duplicates and that all assumptions point
@@ -252,8 +252,8 @@ void AssumptionCache::registerAssumption(AssumeInst *CI) {
252252

253253
assert(&F == cast<Instruction>(VH)->getParent()->getParent() &&
254254
"Cached assumption not inside this function!");
255-
assert(match(cast<CallInst>(VH), m_Intrinsic<Intrinsic::assume>()) &&
256-
"Cached something other than a call to @llvm.assume!");
255+
assert(isa<CondGuardInst>(VH) &&
256+
"Cached something other than CondGuardInst!");
257257
assert(AssumptionSet.insert(VH).second &&
258258
"Cache contains multiple copies of a call!");
259259
}

‎llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1771,8 +1771,7 @@ const SCEV *ScalarEvolution::getZeroExtendExprImpl(const SCEV *Op, Type *Ty,
17711771
// these to compute max backedge taken counts, but can still use
17721772
// these to prove lack of overflow. Use this fact to avoid
17731773
// doing extra work that may not pay off.
1774-
if (!isa<SCEVCouldNotCompute>(MaxBECount) || HasGuards ||
1775-
!AC.assumptions().empty()) {
1774+
if (!isa<SCEVCouldNotCompute>(MaxBECount) || !AC.assumptions().empty()) {
17761775

17771776
auto NewFlags = proveNoUnsignedWrapViaInduction(AR);
17781777
setNoWrapFlags(const_cast<SCEVAddRecExpr *>(AR), NewFlags);
@@ -5113,8 +5112,7 @@ ScalarEvolution::proveNoSignedWrapViaInduction(const SCEVAddRecExpr *AR) {
51135112
// these to prove lack of overflow. Use this fact to avoid
51145113
// doing extra work that may not pay off.
51155114

5116-
if (isa<SCEVCouldNotCompute>(MaxBECount) && !HasGuards &&
5117-
AC.assumptions().empty())
5115+
if (isa<SCEVCouldNotCompute>(MaxBECount) && AC.assumptions().empty())
51185116
return Result;
51195117

51205118
// If the backedge is guarded by a comparison with the pre-inc value the
@@ -5167,8 +5165,7 @@ ScalarEvolution::proveNoUnsignedWrapViaInduction(const SCEVAddRecExpr *AR) {
51675165
// these to prove lack of overflow. Use this fact to avoid
51685166
// doing extra work that may not pay off.
51695167

5170-
if (isa<SCEVCouldNotCompute>(MaxBECount) && !HasGuards &&
5171-
AC.assumptions().empty())
5168+
if (isa<SCEVCouldNotCompute>(MaxBECount) && AC.assumptions().empty())
51725169
return Result;
51735170

51745171
// If the backedge is guarded by a comparison with the pre-inc value the
@@ -11356,7 +11353,7 @@ bool ScalarEvolution::isImpliedViaGuard(const BasicBlock *BB,
1135611353
ICmpInst::Predicate Pred,
1135711354
const SCEV *LHS, const SCEV *RHS) {
1135811355
// No need to even try if we know the module has no guards.
11359-
if (!HasGuards)
11356+
if (AC.assumptions().empty())
1136011357
return false;
1136111358

1136211359
return any_of(*BB, [&](const Instruction &I) {
@@ -11566,15 +11563,6 @@ bool ScalarEvolution::isBasicBlockEntryGuardedByCond(const BasicBlock *BB,
1156611563
return true;
1156711564
}
1156811565

11569-
// Check conditions due to any @llvm.experimental.guard intrinsics.
11570-
auto *GuardDecl = F.getParent()->getFunction(
11571-
Intrinsic::getName(Intrinsic::experimental_guard));
11572-
if (GuardDecl)
11573-
for (const auto *GU : GuardDecl->users())
11574-
if (const auto *Guard = dyn_cast<IntrinsicInst>(GU))
11575-
if (Guard->getFunction() == BB->getParent() && DT.dominates(Guard, BB))
11576-
if (ProveViaCond(Guard->getArgOperand(0), false))
11577-
return true;
1157811566
return false;
1157911567
}
1158011568

@@ -13447,25 +13435,11 @@ ScalarEvolution::ScalarEvolution(Function &F, TargetLibraryInfo &TLI,
1344713435
LoopInfo &LI)
1344813436
: F(F), TLI(TLI), AC(AC), DT(DT), LI(LI),
1344913437
CouldNotCompute(new SCEVCouldNotCompute()), ValuesAtScopes(64),
13450-
LoopDispositions(64), BlockDispositions(64) {
13451-
// To use guards for proving predicates, we need to scan every instruction in
13452-
// relevant basic blocks, and not just terminators. Doing this is a waste of
13453-
// time if the IR does not actually contain any calls to
13454-
// @llvm.experimental.guard, so do a quick check and remember this beforehand.
13455-
//
13456-
// This pessimizes the case where a pass that preserves ScalarEvolution wants
13457-
// to _add_ guards to the module when there weren't any before, and wants
13458-
// ScalarEvolution to optimize based on those guards. For now we prefer to be
13459-
// efficient in lieu of being smart in that rather obscure case.
13460-
13461-
auto *GuardDecl = F.getParent()->getFunction(
13462-
Intrinsic::getName(Intrinsic::experimental_guard));
13463-
HasGuards = GuardDecl && !GuardDecl->use_empty();
13464-
}
13438+
LoopDispositions(64), BlockDispositions(64) {}
1346513439

1346613440
ScalarEvolution::ScalarEvolution(ScalarEvolution &&Arg)
13467-
: F(Arg.F), HasGuards(Arg.HasGuards), TLI(Arg.TLI), AC(Arg.AC), DT(Arg.DT),
13468-
LI(Arg.LI), CouldNotCompute(std::move(Arg.CouldNotCompute)),
13441+
: F(Arg.F), TLI(Arg.TLI), AC(Arg.AC), DT(Arg.DT), LI(Arg.LI),
13442+
CouldNotCompute(std::move(Arg.CouldNotCompute)),
1346913443
ValueExprMap(std::move(Arg.ValueExprMap)),
1347013444
PendingLoopPredicates(std::move(Arg.PendingLoopPredicates)),
1347113445
PendingPhiRanges(std::move(Arg.PendingPhiRanges)),
@@ -15138,16 +15112,7 @@ const SCEV *ScalarEvolution::applyLoopGuards(const SCEV *Expr, const Loop *L) {
1513815112
Terms.emplace_back(AssumeI->getOperand(0), true);
1513915113
}
1514015114

15141-
// Second, collect information from llvm.experimental.guards dominating the loop.
15142-
auto *GuardDecl = F.getParent()->getFunction(
15143-
Intrinsic::getName(Intrinsic::experimental_guard));
15144-
if (GuardDecl)
15145-
for (const auto *GU : GuardDecl->users())
15146-
if (const auto *Guard = dyn_cast<IntrinsicInst>(GU))
15147-
if (Guard->getFunction() == Header->getParent() && DT.dominates(Guard, Header))
15148-
Terms.emplace_back(Guard->getArgOperand(0), true);
15149-
15150-
// Third, collect conditions from dominating branches. Starting at the loop
15115+
// Second, collect conditions from dominating branches. Starting at the loop
1515115116
// predecessor, climb up the predecessor chain, as long as there are
1515215117
// predecessors that can be found that have unique successors leading to the
1515315118
// original header.

‎llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -616,17 +616,14 @@ static bool isKnownNonZeroFromAssume(const Value *V, const Query &Q) {
616616
for (auto &AssumeVH : Q.AC->assumptionsFor(V)) {
617617
if (!AssumeVH)
618618
continue;
619-
CallInst *I = cast<CallInst>(AssumeVH);
619+
CondGuardInst *I = cast<CondGuardInst>(AssumeVH);
620620
assert(I->getFunction() == Q.CxtI->getFunction() &&
621621
"Got assumption for the wrong function!");
622622

623623
// Warning: This loop can end up being somewhat performance sensitive.
624624
// We're running this loop for once for each value queried resulting in a
625625
// runtime of ~O(#assumes * #values).
626626

627-
assert(I->getCalledFunction()->getIntrinsicID() == Intrinsic::assume &&
628-
"must be an assume intrinsic");
629-
630627
Value *RHS;
631628
CmpInst::Predicate Pred;
632629
auto m_V = m_CombineOr(m_Specific(V), m_PtrToInt(m_Specific(V)));
@@ -664,17 +661,14 @@ static void computeKnownBitsFromAssume(const Value *V, KnownBits &Known,
664661
for (auto &AssumeVH : Q.AC->assumptionsFor(V)) {
665662
if (!AssumeVH)
666663
continue;
667-
CallInst *I = cast<CallInst>(AssumeVH);
664+
CondGuardInst *I = cast<CondGuardInst>(AssumeVH);
668665
assert(I->getParent()->getParent() == Q.CxtI->getParent()->getParent() &&
669666
"Got assumption for the wrong function!");
670667

671668
// Warning: This loop can end up being somewhat performance sensitive.
672669
// We're running this loop for once for each value queried resulting in a
673670
// runtime of ~O(#assumes * #values).
674671

675-
assert(I->getCalledFunction()->getIntrinsicID() == Intrinsic::assume &&
676-
"must be an assume intrinsic");
677-
678672
Value *Arg = I->getArgOperand(0);
679673

680674
if (Arg == V && isValidAssumeForContext(I, Q.CxtI, Q.DT)) {
@@ -7446,11 +7440,9 @@ ConstantRange llvm::computeConstantRange(const Value *V, bool ForSigned,
74467440
for (auto &AssumeVH : AC->assumptionsFor(V)) {
74477441
if (!AssumeVH)
74487442
continue;
7449-
CallInst *I = cast<CallInst>(AssumeVH);
7443+
IntrinsicInst *I = cast<IntrinsicInst>(AssumeVH);
74507444
assert(I->getParent()->getParent() == CtxI->getParent()->getParent() &&
74517445
"Got assumption for the wrong function!");
7452-
assert(I->getCalledFunction()->getIntrinsicID() == Intrinsic::assume &&
7453-
"must be an assume intrinsic");
74547446

74557447
if (!isValidAssumeForContext(I, CtxI, DT))
74567448
continue;

‎llvm/lib/Transforms/Utils/CodeExtractor.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1663,14 +1663,14 @@ CodeExtractor::extractCodeRegion(const CodeExtractorAnalysisCache &CEAC,
16631663
}
16641664
}
16651665

1666-
// Remove @llvm.assume calls that will be moved to the new function from the
1667-
// old function's assumption cache.
1666+
// Remove CondGuardInsts that will be moved to the new function from the old
1667+
// function's assumption cache.
16681668
for (BasicBlock *Block : Blocks) {
16691669
for (Instruction &I : llvm::make_early_inc_range(*Block)) {
1670-
if (auto *AI = dyn_cast<AssumeInst>(&I)) {
1670+
if (auto *CI = dyn_cast<CondGuardInst>(&I)) {
16711671
if (AC)
1672-
AC->unregisterAssumption(AI);
1673-
AI->eraseFromParent();
1672+
AC->unregisterAssumption(CI);
1673+
CI->eraseFromParent();
16741674
}
16751675
}
16761676
}
@@ -1864,7 +1864,7 @@ bool CodeExtractor::verifyAssumptionCache(const Function &OldFunc,
18641864
const Function &NewFunc,
18651865
AssumptionCache *AC) {
18661866
for (auto AssumeVH : AC->assumptions()) {
1867-
auto *I = dyn_cast_or_null<CallInst>(AssumeVH);
1867+
auto *I = dyn_cast_or_null<CondGuardInst>(AssumeVH);
18681868
if (!I)
18691869
continue;
18701870

@@ -1876,7 +1876,7 @@ bool CodeExtractor::verifyAssumptionCache(const Function &OldFunc,
18761876
// that were previously in the old function, but that have now been moved
18771877
// to the new function.
18781878
for (auto AffectedValVH : AC->assumptionsFor(I->getOperand(0))) {
1879-
auto *AffectedCI = dyn_cast_or_null<CallInst>(AffectedValVH);
1879+
auto *AffectedCI = dyn_cast_or_null<CondGuardInst>(AffectedValVH);
18801880
if (!AffectedCI)
18811881
continue;
18821882
if (AffectedCI->getFunction() != &OldFunc)

‎llvm/lib/Transforms/Utils/InlineFunction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2333,7 +2333,7 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
23332333
for (BasicBlock &NewBlock :
23342334
make_range(FirstNewBlock->getIterator(), Caller->end()))
23352335
for (Instruction &I : NewBlock)
2336-
if (auto *II = dyn_cast<AssumeInst>(&I))
2336+
if (auto *II = dyn_cast<CondGuardInst>(&I))
23372337
IFI.GetAssumptionCache(*Caller).registerAssumption(II);
23382338
}
23392339

‎llvm/test/Analysis/AssumptionCache/basic.ll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@
33
target datalayout = "e-i64:64-f80:128-n8:16:32:64-S128"
44

55
declare void @llvm.assume(i1)
6+
declare void @llvm.experimental.guard(i1, ...)
67

78
define void @test1(i32 %a) {
89
; CHECK-LABEL: Cached assumptions for function: test1
910
; CHECK-NEXT: icmp ne i32 %{{.*}}, 0
1011
; CHECK-NEXT: icmp slt i32 %{{.*}}, 0
1112
; CHECK-NEXT: icmp sgt i32 %{{.*}}, 0
13+
; CHECK-NEXT: icmp ult i32 %{{.*}}, 0
14+
; CHECK-NEXT: icmp ugt i32 %{{.*}}, 0
1215

1316
entry:
1417
%cond1 = icmp ne i32 %a, 0
@@ -17,6 +20,10 @@ entry:
1720
call void @llvm.assume(i1 %cond2)
1821
%cond3 = icmp sgt i32 %a, 0
1922
call void @llvm.assume(i1 %cond3)
23+
%cond4 = icmp ult i32 %a, 0
24+
call void (i1, ...) @llvm.experimental.guard(i1 %cond4) [ "deopt"() ]
25+
%cond5 = icmp ugt i32 %a, 0
26+
call void (i1, ...) @llvm.experimental.guard(i1 %cond5) [ "deopt"() ]
2027

2128
ret void
2229
}

‎llvm/test/Analysis/ScalarEvolution/guards.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ entry:
8686
loop:
8787
; CHECK: loop:
8888
; CHECK: call void (i1, ...) @llvm.experimental.guard(i1 true) [ "deopt"() ]
89-
; CHECK: %iv.inc.cmp = icmp slt i32 %iv.inc, %len
89+
; CHECK: %iv.inc.cmp = icmp ult i32 %iv.inc, %len
9090
; CHECK: call void (i1, ...) @llvm.experimental.guard(i1 %iv.inc.cmp) [ "deopt"() ]
9191
; CHECK: leave:
9292
%iv = phi i32 [ 0, %entry ], [ %iv.inc, %loop ]
@@ -129,7 +129,7 @@ left:
129129

130130
be:
131131
; CHECK: be:
132-
; CHECK-NEXT: %iv.cmp = icmp slt i32 %iv, %len
132+
; CHECK-NEXT: %iv.cmp = icmp ult i32 %iv, %len
133133
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 %iv.cmp) [ "deopt"() ]
134134
; CHECK: leave:
135135

0 commit comments

Comments
 (0)