Skip to content

Commit

Permalink
[SCEV] Move SCEVLostPoisonFlags() check into SCEVExpander
Browse files Browse the repository at this point in the history
Always insert values into ExprValueMap, and instead skip using them
in SCEVExpander if poison-generating flags have been lost. This
ensures that all values that are in ValueExprMap are also in
ExprValueMap, so we can use the latter to invalidate the former.

This change is probably not entirely NFC for the case where
originally the SCEV had no nowrap flags but they were inferred
later, in which case that would now allow reusing the existing
value for expansion.

Differential Revision: https://reviews.llvm.org/D112389
  • Loading branch information
nikic committed Oct 25, 2021
1 parent 4a9db73 commit 3a995c9
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 20 deletions.
20 changes: 1 addition & 19 deletions llvm/lib/Analysis/ScalarEvolution.cpp
Expand Up @@ -4077,24 +4077,6 @@ void ScalarEvolution::eraseValueFromMap(Value *V) {
}
}

/// Check whether value has nuw/nsw/exact set but SCEV does not.
/// TODO: In reality it is better to check the poison recursively
/// but this is better than nothing.
static bool SCEVLostPoisonFlags(const SCEV *S, const Value *V) {
if (auto *I = dyn_cast<Instruction>(V)) {
if (isa<OverflowingBinaryOperator>(I)) {
if (auto *NS = dyn_cast<SCEVNAryExpr>(S)) {
if (I->hasNoSignedWrap() && !NS->hasNoSignedWrap())
return true;
if (I->hasNoUnsignedWrap() && !NS->hasNoUnsignedWrap())
return true;
}
} else if (isa<PossiblyExactOperator>(I) && I->isExact())
return true;
}
return false;
}

/// Return an existing SCEV if it exists, otherwise analyze the expression and
/// create a new one.
const SCEV *ScalarEvolution::getSCEV(Value *V) {
Expand All @@ -4108,7 +4090,7 @@ const SCEV *ScalarEvolution::getSCEV(Value *V) {
// ValueExprMap before insert S->{V, 0} into ExprValueMap.
std::pair<ValueExprMapType::iterator, bool> Pair =
ValueExprMap.insert({SCEVCallbackVH(V, this), S});
if (Pair.second && !SCEVLostPoisonFlags(S, V)) {
if (Pair.second) {
ExprValueMap[S].insert({V, nullptr});

// If S == Stripped + Offset, add Stripped -> {V, Offset} into
Expand Down
19 changes: 18 additions & 1 deletion llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
Expand Up @@ -1835,6 +1835,22 @@ Value *SCEVExpander::expandCodeForImpl(const SCEV *SH, Type *Ty, bool Root) {
return V;
}

/// Check whether value has nuw/nsw/exact set but SCEV does not.
/// TODO: In reality it is better to check the poison recursively
/// but this is better than nothing.
static bool SCEVLostPoisonFlags(const SCEV *S, const Instruction *I) {
if (isa<OverflowingBinaryOperator>(I)) {
if (auto *NS = dyn_cast<SCEVNAryExpr>(S)) {
if (I->hasNoSignedWrap() && !NS->hasNoSignedWrap())
return true;
if (I->hasNoUnsignedWrap() && !NS->hasNoUnsignedWrap())
return true;
}
} else if (isa<PossiblyExactOperator>(I) && I->isExact())
return true;
return false;
}

ScalarEvolution::ValueOffsetPair
SCEVExpander::FindValueInExprValueMap(const SCEV *S,
const Instruction *InsertPt) {
Expand All @@ -1858,7 +1874,8 @@ SCEVExpander::FindValueInExprValueMap(const SCEV *S,
if (S->getType() == V->getType() &&
SE.DT.dominates(EntInst, InsertPt) &&
(SE.LI.getLoopFor(EntInst->getParent()) == nullptr ||
SE.LI.getLoopFor(EntInst->getParent())->contains(InsertPt)))
SE.LI.getLoopFor(EntInst->getParent())->contains(InsertPt)) &&
!SCEVLostPoisonFlags(S, EntInst))
return {V, Offset};
}
}
Expand Down

0 comments on commit 3a995c9

Please sign in to comment.