Skip to content

Commit

Permalink
Make processing @llvm.assume more efficient by using operand bundles
Browse files Browse the repository at this point in the history
There was an efficiency problem with how we processed @llvm.assume in
ValueTracking (and other places). The AssumptionCache tracked all of the
assumptions in a given function. In order to find assumptions relevant to
computing known bits, etc. we searched every assumption in the function. For
ValueTracking, that means that we did O(#assumes * #values) work in InstCombine
and other passes (with a constant factor that can be quite large because we'd
repeat this search at every level of recursion of the analysis).

Several of us discussed this situation at the last developers' meeting, and
this implements the discussed solution: Make the values that an assume might
affect operands of the assume itself. To avoid exposing this detail to
frontends and passes that need not worry about it, I've used the new
operand-bundle feature to add these extra call "operands" in a way that does
not affect the intrinsic's signature. I think this solution is relatively
clean. InstCombine adds these extra operands based on what ValueTracking, LVI,
etc. will need and then those passes need only search the users of the values
under consideration. This should fix the computational-complexity problem.

At this point, no passes depend on the AssumptionCache, and so I'll remove
that as a follow-up change.

Differential Revision: https://reviews.llvm.org/D27259

llvm-svn: 289755
  • Loading branch information
Hal Finkel committed Dec 15, 2016
1 parent dfe85e2 commit cb9f78e
Show file tree
Hide file tree
Showing 19 changed files with 279 additions and 149 deletions.
9 changes: 9 additions & 0 deletions llvm/docs/LangRef.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1753,6 +1753,15 @@ site, these bundles may contain any values that are needed by the
generated code. For more details, see :ref:`GC Transitions
<gc_transition_args>`.

Affected Operand Bundles
^^^^^^^^^^^^^^^^^^^^^^^^

Affected operand bundles are characterized by the ``"affected"`` operand bundle
tag. These operand bundles indicate that a call, specifically a call to an
intrinsic like ``llvm.assume``, implies some additional knowledge about the
values within the bundle. This enables the optimizer to efficiently find these
relationships. The optimizer will add these automatically.

.. _moduleasm:

Module-Level Inline Assembly
Expand Down
7 changes: 7 additions & 0 deletions llvm/include/llvm/Analysis/ScalarEvolution.h
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,10 @@ class ScalarEvolution {
///
ValueExprMapType ValueExprMap;

/// This is a map of SCEVs to intrinsics (e.g. assumptions) that might affect
/// (i.e. imply something about) them.
DenseMap<const SCEV *, SetVector<Value *>> AffectedMap;

/// Mark predicate values currently being processed by isImpliedCond.
SmallPtrSet<Value *, 6> PendingLoopPredicates;

Expand Down Expand Up @@ -800,6 +804,9 @@ class ScalarEvolution {
ConstantRange getRangeViaFactoring(const SCEV *Start, const SCEV *Stop,
const SCEV *MaxBECount, unsigned BitWidth);

/// Add to the AffectedMap this SCEV if its operands are in the AffectedMap.
void addAffectedFromOperands(const SCEV *S);

/// We know that there is no SCEV for the specified value. Analyze the
/// expression.
const SCEV *createSCEV(Value *V);
Expand Down
36 changes: 12 additions & 24 deletions llvm/lib/Analysis/CodeMetrics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,12 @@ void CodeMetrics::collectEphemeralValues(
SmallPtrSet<const Value *, 32> Visited;
SmallVector<const Value *, 16> Worklist;

for (auto &AssumeVH : AC->assumptions()) {
if (!AssumeVH)
continue;
Instruction *I = cast<Instruction>(AssumeVH);

// Filter out call sites outside of the loop so we don't do a function's
// worth of work for each of its loops (and, in the common case, ephemeral
// values in the loop are likely due to @llvm.assume calls in the loop).
if (!L->contains(I->getParent()))
continue;

if (EphValues.insert(I).second)
appendSpeculatableOperands(I, Visited, Worklist);
}
for (auto &B : L->blocks())
for (auto &I : *B)
if (auto *II = dyn_cast<IntrinsicInst>(&I))
if (II->getIntrinsicID() == Intrinsic::assume &&
EphValues.insert(II).second)
appendSpeculatableOperands(II, Visited, Worklist);

completeEphemeralValues(Visited, Worklist, EphValues);
}
Expand All @@ -100,16 +92,12 @@ void CodeMetrics::collectEphemeralValues(
SmallPtrSet<const Value *, 32> Visited;
SmallVector<const Value *, 16> Worklist;

for (auto &AssumeVH : AC->assumptions()) {
if (!AssumeVH)
continue;
Instruction *I = cast<Instruction>(AssumeVH);
assert(I->getParent()->getParent() == F &&
"Found assumption for the wrong function!");

if (EphValues.insert(I).second)
appendSpeculatableOperands(I, Visited, Worklist);
}
for (auto &B : *F)
for (auto &I : B)
if (auto *II = dyn_cast<IntrinsicInst>(&I))
if (II->getIntrinsicID() == Intrinsic::assume &&
EphValues.insert(II).second)
appendSpeculatableOperands(II, Visited, Worklist);

completeEphemeralValues(Visited, Worklist, EphValues);
}
Expand Down
12 changes: 7 additions & 5 deletions llvm/lib/Analysis/LazyValueInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -924,14 +924,16 @@ void LazyValueInfoImpl::intersectAssumeOrGuardBlockValueConstantRange(
if (!BBI)
return;

for (auto &AssumeVH : AC->assumptions()) {
if (!AssumeVH)
for (auto *U : Val->users()) {
auto *II = dyn_cast<IntrinsicInst>(U);
if (!II)
continue;
auto *I = cast<CallInst>(AssumeVH);
if (!isValidAssumeForContext(I, BBI, DT))
if (II->getIntrinsicID() != Intrinsic::assume)
continue;
if (!isValidAssumeForContext(II, BBI, DT))
continue;

BBLV = intersect(BBLV, getValueFromCondition(Val, I->getArgOperand(0)));
BBLV = intersect(BBLV, getValueFromCondition(Val, II->getArgOperand(0)));
}

// If guards are not used in the module, don't spend time looking for them
Expand Down
92 changes: 72 additions & 20 deletions llvm/lib/Analysis/ScalarEvolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,7 @@ const SCEV *ScalarEvolution::getTruncateExpr(const SCEV *Op,
SCEV *S = new (SCEVAllocator) SCEVTruncateExpr(ID.Intern(SCEVAllocator),
Op, Ty);
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
return S;
}

Expand Down Expand Up @@ -1598,7 +1599,7 @@ const SCEV *ScalarEvolution::getZeroExtendExpr(const SCEV *Op,
// these to prove lack of overflow. Use this fact to avoid
// doing extra work that may not pay off.
if (!isa<SCEVCouldNotCompute>(MaxBECount) || HasGuards ||
!AC.assumptions().empty()) {
!AffectedMap.empty()) {
// If the backedge is guarded by a comparison with the pre-inc
// value the addrec is safe. Also, if the entry is guarded by
// a comparison with the start value and the backedge is
Expand Down Expand Up @@ -1664,6 +1665,7 @@ const SCEV *ScalarEvolution::getZeroExtendExpr(const SCEV *Op,
SCEV *S = new (SCEVAllocator) SCEVZeroExtendExpr(ID.Intern(SCEVAllocator),
Op, Ty);
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
return S;
}

Expand Down Expand Up @@ -1833,7 +1835,7 @@ const SCEV *ScalarEvolution::getSignExtendExpr(const SCEV *Op,
// doing extra work that may not pay off.

if (!isa<SCEVCouldNotCompute>(MaxBECount) || HasGuards ||
!AC.assumptions().empty()) {
!AffectedMap.empty()) {
// If the backedge is guarded by a comparison with the pre-inc
// value the addrec is safe. Also, if the entry is guarded by
// a comparison with the start value and the backedge is
Expand Down Expand Up @@ -1891,6 +1893,7 @@ const SCEV *ScalarEvolution::getSignExtendExpr(const SCEV *Op,
SCEV *S = new (SCEVAllocator) SCEVSignExtendExpr(ID.Intern(SCEVAllocator),
Op, Ty);
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
return S;
}

Expand Down Expand Up @@ -2444,6 +2447,7 @@ const SCEV *ScalarEvolution::getAddExpr(SmallVectorImpl<const SCEV *> &Ops,
S = new (SCEVAllocator) SCEVAddExpr(ID.Intern(SCEVAllocator),
O, Ops.size());
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
}
S->setNoWrapFlags(Flags);
return S;
Expand Down Expand Up @@ -2736,6 +2740,7 @@ const SCEV *ScalarEvolution::getMulExpr(SmallVectorImpl<const SCEV *> &Ops,
S = new (SCEVAllocator) SCEVMulExpr(ID.Intern(SCEVAllocator),
O, Ops.size());
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
}
S->setNoWrapFlags(Flags);
return S;
Expand Down Expand Up @@ -2856,6 +2861,7 @@ const SCEV *ScalarEvolution::getUDivExpr(const SCEV *LHS,
SCEV *S = new (SCEVAllocator) SCEVUDivExpr(ID.Intern(SCEVAllocator),
LHS, RHS);
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
return S;
}

Expand Down Expand Up @@ -3036,6 +3042,7 @@ ScalarEvolution::getAddRecExpr(SmallVectorImpl<const SCEV *> &Operands,
S = new (SCEVAllocator) SCEVAddRecExpr(ID.Intern(SCEVAllocator),
O, Operands.size(), L);
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
}
S->setNoWrapFlags(Flags);
return S;
Expand Down Expand Up @@ -3191,6 +3198,7 @@ ScalarEvolution::getSMaxExpr(SmallVectorImpl<const SCEV *> &Ops) {
SCEV *S = new (SCEVAllocator) SCEVSMaxExpr(ID.Intern(SCEVAllocator),
O, Ops.size());
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
return S;
}

Expand Down Expand Up @@ -3292,6 +3300,7 @@ ScalarEvolution::getUMaxExpr(SmallVectorImpl<const SCEV *> &Ops) {
SCEV *S = new (SCEVAllocator) SCEVUMaxExpr(ID.Intern(SCEVAllocator),
O, Ops.size());
UniqueSCEVs.InsertNode(S, IP);
addAffectedFromOperands(S);
return S;
}

Expand Down Expand Up @@ -3492,9 +3501,38 @@ const SCEV *ScalarEvolution::getSCEV(Value *V) {
ExprValueMap[Stripped].insert({V, Offset});
}
}

// If this value is an instruction or an argument, and might be affected by
// an assumption, and its SCEV to the AffectedMap.
if (isa<Instruction>(V) || isa<Argument>(V)) {
for (auto *U : V->users()) {
auto *II = dyn_cast<IntrinsicInst>(U);
if (!II)
continue;
if (II->getIntrinsicID() != Intrinsic::assume)
continue;

AffectedMap[S].insert(II);
}
}

return S;
}

// If one of this SCEV's operands is in the AffectedMap (meaning that it might
// be affected by an assumption), then this SCEV might be affected by the same
// assumption.
void ScalarEvolution::addAffectedFromOperands(const SCEV *S) {
if (auto *NS = dyn_cast<SCEVNAryExpr>(S))
for (auto *Op : NS->operands()) {
auto AMI = AffectedMap.find(Op);
if (AMI == AffectedMap.end())
continue;

AffectedMap[S].insert(AMI->second.begin(), AMI->second.end());
}
}

const SCEV *ScalarEvolution::getExistingSCEV(Value *V) {
assert(isSCEVable(V->getType()) && "Value is not SCEVable!");

Expand Down Expand Up @@ -7926,16 +7964,23 @@ ScalarEvolution::isLoopBackedgeGuardedByCond(const Loop *L,
}

// Check conditions due to any @llvm.assume intrinsics.
for (auto &AssumeVH : AC.assumptions()) {
if (!AssumeVH)
continue;
auto *CI = cast<CallInst>(AssumeVH);
if (!DT.dominates(CI, Latch->getTerminator()))
continue;
auto CheckAssumptions = [&](const SCEV *S) {
auto AMI = AffectedMap.find(S);
if (AMI != AffectedMap.end())
for (auto *Assume : AMI->second) {
auto *CI = cast<CallInst>(Assume);
if (!DT.dominates(CI, Latch->getTerminator()))
continue;

if (isImpliedCond(Pred, LHS, RHS, CI->getArgOperand(0), false))
return true;
}
if (isImpliedCond(Pred, LHS, RHS, CI->getArgOperand(0), false))
return true;
}

return false;
};

if (CheckAssumptions(LHS) || CheckAssumptions(RHS))
return true;

// If the loop is not reachable from the entry block, we risk running into an
// infinite loop as we walk up into the dom tree. These loops do not matter
Expand Down Expand Up @@ -8020,16 +8065,23 @@ ScalarEvolution::isLoopEntryGuardedByCond(const Loop *L,
}

// Check conditions due to any @llvm.assume intrinsics.
for (auto &AssumeVH : AC.assumptions()) {
if (!AssumeVH)
continue;
auto *CI = cast<CallInst>(AssumeVH);
if (!DT.dominates(CI, L->getHeader()))
continue;
auto CheckAssumptions = [&](const SCEV *S) {
auto AMI = AffectedMap.find(S);
if (AMI != AffectedMap.end())
for (auto *Assume : AMI->second) {
auto *CI = cast<CallInst>(Assume);
if (!DT.dominates(CI, L->getHeader()))
continue;

if (isImpliedCond(Pred, LHS, RHS, CI->getArgOperand(0), false))
return true;
}
if (isImpliedCond(Pred, LHS, RHS, CI->getArgOperand(0), false))
return true;
}

return false;
};

if (CheckAssumptions(LHS) || CheckAssumptions(RHS))
return true;

return false;
}
Expand Down
Loading

0 comments on commit cb9f78e

Please sign in to comment.