[llvm] r276822 - Revert r276136 "Use ValueOffsetPair to enhance value reuse during SCEV expansion."
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 26 16:25:13 PDT 2016
Author: hans
Date: Tue Jul 26 18:25:13 2016
New Revision: 276822
URL: http://llvm.org/viewvc/llvm-project?rev=276822&view=rev
Log:
Revert r276136 "Use ValueOffsetPair to enhance value reuse during SCEV expansion."
It causes Clang tests to fail after Windows self-host (PR28705).
(Also reverts follow-up r276139.)
Removed:
llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-existing-value-offset.ll
Modified:
llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h
llvm/trunk/lib/Analysis/ScalarEvolution.cpp
llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=276822&r1=276821&r2=276822&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
+++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Tue Jul 26 18:25:13 2016
@@ -495,29 +495,10 @@ namespace llvm {
/// The typedef for ExprValueMap.
///
- typedef std::pair<Value *, ConstantInt *> ValueOffsetPair;
- typedef DenseMap<const SCEV *, SetVector<ValueOffsetPair>> ExprValueMapType;
+ typedef DenseMap<const SCEV *, SetVector<Value *>> ExprValueMapType;
/// ExprValueMap -- This map records the original values from which
/// the SCEV expr is generated from.
- ///
- /// We want to represent the mapping as SCEV -> ValueOffsetPair instead
- /// of SCEV -> Value:
- /// Suppose we know S1 expands to V1, and
- /// S1 = S2 + C_a
- /// S3 = S2 + C_b
- /// where C_a and C_b are different SCEVConstants. Then we'd like to
- /// expand S3 as V1 - C_a + C_b instead of expanding S2 literally.
- /// It is helpful when S2 is a complex SCEV expr.
- ///
- /// In order to do that, we represent ExprValueMap as a mapping from
- /// SCEV to ValueOffsetPair. We will save both S1->{V1, 0} and
- /// S2->{V1, C_a} into the map when we create SCEV for V1. When S3
- /// is expanded, it will first expand S2 to V1 - C_a because of
- /// S2->{V1, C_a} in the map, then expand S3 to V1 - C_a + C_b.
- ///
- /// Note: S->{V, Offset} in the ExprValueMap means S can be expanded
- /// to V - Offset.
ExprValueMapType ExprValueMap;
/// The typedef for ValueExprMap.
@@ -1204,7 +1185,7 @@ namespace llvm {
bool containsAddRecurrence(const SCEV *S);
/// Return the Value set from which the SCEV expr is generated.
- SetVector<ValueOffsetPair> *getSCEVValues(const SCEV *S);
+ SetVector<Value *> *getSCEVValues(const SCEV *S);
/// Erase Value from ValueExprMap and ExprValueMap.
void eraseValueFromMap(Value *V);
Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h?rev=276822&r1=276821&r2=276822&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h (original)
+++ llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpander.h Tue Jul 26 18:25:13 2016
@@ -309,8 +309,7 @@ namespace llvm {
PointerType *PTy, Type *Ty, Value *V);
/// \brief Find a previous Value in ExprValueMap for expand.
- ScalarEvolution::ValueOffsetPair
- FindValueInExprValueMap(const SCEV *S, const Instruction *InsertPt);
+ Value *FindValueInExprValueMap(const SCEV *S, const Instruction *InsertPt);
Value *expand(const SCEV *S);
Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=276822&r1=276821&r2=276822&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Tue Jul 26 18:25:13 2016
@@ -3378,28 +3378,8 @@ bool ScalarEvolution::containsAddRecurre
return F.FoundOne;
}
-/// Try to split a SCEVAddExpr into a pair of {SCEV, ConstantInt}.
-/// If \p S is a SCEVAddExpr and is composed of a sub SCEV S' and an
-/// offset I, then return {S', I}, else return {\p S, nullptr}.
-static std::pair<const SCEV *, ConstantInt *> splitAddExpr(const SCEV *S) {
- const auto *Add = dyn_cast<SCEVAddExpr>(S);
- if (!Add)
- return {S, nullptr};
-
- if (Add->getNumOperands() != 2)
- return {S, nullptr};
-
- auto *ConstOp = dyn_cast<SCEVConstant>(Add->getOperand(0));
- if (!ConstOp)
- return {S, nullptr};
-
- return {Add->getOperand(1), ConstOp->getValue()};
-}
-
-/// Return the ValueOffsetPair set for \p S. \p S can be represented
-/// by the value and offset from any ValueOffsetPair in the set.
-SetVector<ScalarEvolution::ValueOffsetPair> *
-ScalarEvolution::getSCEVValues(const SCEV *S) {
+/// Return the Value set from S.
+SetVector<Value *> *ScalarEvolution::getSCEVValues(const SCEV *S) {
ExprValueMapType::iterator SI = ExprValueMap.find_as(S);
if (SI == ExprValueMap.end())
return nullptr;
@@ -3407,31 +3387,24 @@ ScalarEvolution::getSCEVValues(const SCE
if (VerifySCEVMap) {
// Check there is no dangling Value in the set returned.
for (const auto &VE : SI->second)
- assert(ValueExprMap.count(VE.first));
+ assert(ValueExprMap.count(VE));
}
#endif
return &SI->second;
}
-/// Erase Value from ValueExprMap and ExprValueMap. ValueExprMap.erase(V)
-/// cannot be used separately. eraseValueFromMap should be used to remove
-/// V from ValueExprMap and ExprValueMap at the same time.
+/// Erase Value from ValueExprMap and ExprValueMap. If ValueExprMap.erase(V) is
+/// not used together with forgetMemoizedResults(S), eraseValueFromMap should be
+/// used instead to ensure whenever V->S is removed from ValueExprMap, V is also
+/// removed from the set of ExprValueMap[S].
void ScalarEvolution::eraseValueFromMap(Value *V) {
ValueExprMapType::iterator I = ValueExprMap.find_as(V);
if (I != ValueExprMap.end()) {
const SCEV *S = I->second;
- // Remove {V, 0} from the set of ExprValueMap[S]
- if (SetVector<ValueOffsetPair> *SV = getSCEVValues(S))
- SV->remove({V, nullptr});
-
- // Remove {V, Offset} from the set of ExprValueMap[Stripped]
- const SCEV *Stripped;
- ConstantInt *Offset;
- std::tie(Stripped, Offset) = splitAddExpr(S);
- if (Offset != nullptr) {
- if (SetVector<ValueOffsetPair> *SV = getSCEVValues(Stripped))
- SV->remove({V, Offset});
- }
+ SetVector<Value *> *SV = getSCEVValues(S);
+ // Remove V from the set of ExprValueMap[S]
+ if (SV)
+ SV->remove(V);
ValueExprMap.erase(V);
}
}
@@ -3446,26 +3419,11 @@ const SCEV *ScalarEvolution::getSCEV(Val
S = createSCEV(V);
// During PHI resolution, it is possible to create two SCEVs for the same
// V, so it is needed to double check whether V->S is inserted into
- // ValueExprMap before insert S->{V, 0} into ExprValueMap.
+ // ValueExprMap before insert S->V into ExprValueMap.
std::pair<ValueExprMapType::iterator, bool> Pair =
ValueExprMap.insert({SCEVCallbackVH(V, this), S});
- if (Pair.second) {
- ExprValueMap[S].insert({V, nullptr});
-
- // If S == Stripped + Offset, add Stripped -> {V, Offset} into
- // ExprValueMap.
- const SCEV *Stripped = S;
- ConstantInt *Offset = nullptr;
- std::tie(Stripped, Offset) = splitAddExpr(S);
- // If stripped is SCEVUnknown, don't bother to save
- // Stripped -> {V, offset}. It doesn't simplify and sometimes even
- // increase the complexity of the expansion code.
- // If V is GetElementPtrInst, don't save Stripped -> {V, offset}
- // because it may generate add/sub instead of GEP in SCEV expansion.
- if (Offset != nullptr && !isa<SCEVUnknown>(Stripped) &&
- !isa<GetElementPtrInst>(V))
- ExprValueMap[Stripped].insert({V, Offset});
- }
+ if (Pair.second)
+ ExprValueMap[S].insert(V);
}
return S;
}
@@ -3478,8 +3436,8 @@ const SCEV *ScalarEvolution::getExisting
const SCEV *S = I->second;
if (checkValidity(S))
return S;
- eraseValueFromMap(V);
forgetMemoizedResults(S);
+ ValueExprMap.erase(I);
}
return nullptr;
}
@@ -3717,8 +3675,8 @@ void ScalarEvolution::forgetSymbolicName
if (!isa<PHINode>(I) ||
!isa<SCEVUnknown>(Old) ||
(I != PN && Old == SymName)) {
- eraseValueFromMap(It->first);
forgetMemoizedResults(Old);
+ ValueExprMap.erase(It);
}
}
@@ -4097,7 +4055,7 @@ const SCEV *ScalarEvolution::createAddRe
// to create an AddRecExpr for this PHI node. We can not keep this temporary
// as it will prevent later (possibly simpler) SCEV expressions to be added
// to the ValueExprMap.
- eraseValueFromMap(PN);
+ ValueExprMap.erase(PN);
}
return nullptr;
@@ -5473,8 +5431,8 @@ ScalarEvolution::getBackedgeTakenInfo(co
// case, createNodeForPHI will perform the necessary updates on its
// own when it gets to that point.
if (!isa<PHINode>(I) || !isa<SCEVUnknown>(Old)) {
- eraseValueFromMap(It->first);
forgetMemoizedResults(Old);
+ ValueExprMap.erase(It);
}
if (PHINode *PN = dyn_cast<PHINode>(I))
ConstantEvolutionLoopExitValue.erase(PN);
@@ -5519,8 +5477,8 @@ void ScalarEvolution::forgetLoop(const L
ValueExprMapType::iterator It =
ValueExprMap.find_as(static_cast<Value *>(I));
if (It != ValueExprMap.end()) {
- eraseValueFromMap(It->first);
forgetMemoizedResults(It->second);
+ ValueExprMap.erase(It);
if (PHINode *PN = dyn_cast<PHINode>(I))
ConstantEvolutionLoopExitValue.erase(PN);
}
@@ -5553,8 +5511,8 @@ void ScalarEvolution::forgetValue(Value
ValueExprMapType::iterator It =
ValueExprMap.find_as(static_cast<Value *>(I));
if (It != ValueExprMap.end()) {
- eraseValueFromMap(It->first);
forgetMemoizedResults(It->second);
+ ValueExprMap.erase(It);
if (PHINode *PN = dyn_cast<PHINode>(I))
ConstantEvolutionLoopExitValue.erase(PN);
}
Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=276822&r1=276821&r2=276822&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Tue Jul 26 18:25:13 2016
@@ -1626,10 +1626,9 @@ Value *SCEVExpander::expandCodeFor(const
return V;
}
-ScalarEvolution::ValueOffsetPair
-SCEVExpander::FindValueInExprValueMap(const SCEV *S,
- const Instruction *InsertPt) {
- SetVector<ScalarEvolution::ValueOffsetPair> *Set = SE.getSCEVValues(S);
+Value *SCEVExpander::FindValueInExprValueMap(const SCEV *S,
+ const Instruction *InsertPt) {
+ SetVector<Value *> *Set = SE.getSCEVValues(S);
// If the expansion is not in CanonicalMode, and the SCEV contains any
// sub scAddRecExpr type SCEV, it is required to expand the SCEV literally.
if (CanonicalMode || !SE.containsAddRecurrence(S)) {
@@ -1638,21 +1637,21 @@ SCEVExpander::FindValueInExprValueMap(co
// Choose a Value from the set which dominates the insertPt.
// insertPt should be inside the Value's parent loop so as not to break
// the LCSSA form.
- for (auto const &VOPair : *Set) {
- Value *V = VOPair.first;
- ConstantInt *Offset = VOPair.second;
+ for (auto const &Ent : *Set) {
Instruction *EntInst = nullptr;
- if (V && isa<Instruction>(V) && (EntInst = cast<Instruction>(V)) &&
- S->getType() == V->getType() &&
+ if (Ent && isa<Instruction>(Ent) &&
+ (EntInst = cast<Instruction>(Ent)) &&
+ S->getType() == Ent->getType() &&
EntInst->getFunction() == InsertPt->getFunction() &&
SE.DT.dominates(EntInst, InsertPt) &&
(SE.LI.getLoopFor(EntInst->getParent()) == nullptr ||
- SE.LI.getLoopFor(EntInst->getParent())->contains(InsertPt)))
- return {V, Offset};
+ SE.LI.getLoopFor(EntInst->getParent())->contains(InsertPt))) {
+ return Ent;
+ }
}
}
}
- return {nullptr, nullptr};
+ return nullptr;
}
// The expansion of SCEV will either reuse a previous Value in ExprValueMap,
@@ -1700,14 +1699,10 @@ Value *SCEVExpander::expand(const SCEV *
Builder.SetInsertPoint(InsertPt);
// Expand the expression into instructions.
- ScalarEvolution::ValueOffsetPair VO = FindValueInExprValueMap(S, InsertPt);
- Value *V = VO.first;
+ Value *V = FindValueInExprValueMap(S, InsertPt);
if (!V)
V = visit(S);
- else if (VO.second) {
- V = Builder.CreateSub(V, VO.second);
- }
// Remember the expanded value for this SCEV at this location.
//
@@ -1920,7 +1915,7 @@ Value *SCEVExpander::findExistingExpansi
// Use expand's logic which is used for reusing a previous Value in
// ExprValueMap.
- if (Value *Val = FindValueInExprValueMap(S, At).first)
+ if (Value *Val = FindValueInExprValueMap(S, At))
return Val;
// There is potential to make this significantly smarter, but this simple
Removed: llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-existing-value-offset.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-existing-value-offset.ll?rev=276821&view=auto
==============================================================================
--- llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-existing-value-offset.ll (original)
+++ llvm/trunk/test/Analysis/ScalarEvolution/scev-expander-existing-value-offset.ll (removed)
@@ -1,44 +0,0 @@
-; RUN: opt < %s -loop-vectorize -force-vector-interleave=1 -force-vector-width=4 -S |FileCheck %s
-; SCEV expansion uses existing value or value + offset to reduce duplicate code expansion so foo should only generate one select inst after loop vectorization.
-; CHECK-LABEL: @foo(
-; CHECK: select
-; CHECK-NOT: select
-
- at ySrcL = common global i8* null, align 8
- at smL = common global i32 0, align 4
-
-define void @foo(i32 %rwL, i32 %kL, i32 %xfL) {
-entry:
- %sub = add nsw i32 %rwL, -1
- %shr = ashr i32 %xfL, 6
- %cmp.i = icmp slt i32 %sub, %shr
- %cond.i = select i1 %cmp.i, i32 %sub, i32 %shr
- %cmp6 = icmp sgt i32 %cond.i, %kL
- br i1 %cmp6, label %for.body.lr.ph, label %for.end
-
-for.body.lr.ph: ; preds = %entry
- %tmp = load i8*, i8** @ySrcL, align 8
- %tmp1 = sext i32 %kL to i64
- %tmp2 = sext i32 %cond.i to i64
- br label %for.body
-
-for.body: ; preds = %for.body, %for.body.lr.ph
- %indvars.iv = phi i64 [ %tmp1, %for.body.lr.ph ], [ %indvars.iv.next, %for.body ]
- %reduct.07 = phi i32 [ 0, %for.body.lr.ph ], [ %add, %for.body ]
- %arrayidx = getelementptr inbounds i8, i8* %tmp, i64 %indvars.iv
- %tmp3 = load i8, i8* %arrayidx, align 1
- %conv = zext i8 %tmp3 to i32
- %add = add nsw i32 %conv, %reduct.07
- %indvars.iv.next = add nsw i64 %indvars.iv, 1
- %cmp = icmp slt i64 %indvars.iv.next, %tmp2
- br i1 %cmp, label %for.body, label %for.end.loopexit
-
-for.end.loopexit: ; preds = %for.body
- %add.lcssa = phi i32 [ %add, %for.body ]
- br label %for.end
-
-for.end: ; preds = %for.end.loopexit, %entry
- %reduct.0.lcssa = phi i32 [ 0, %entry ], [ %add.lcssa, %for.end.loopexit ]
- store i32 %reduct.0.lcssa, i32* @smL, align 4
- ret void
-}
More information about the llvm-commits
mailing list