From b9389c99a50bebcb68064bd7c1c05c8809eb7f37 Mon Sep 17 00:00:00 2001 From: Dimitry Andric Date: Thu, 26 Jan 2017 20:18:28 +0000 Subject: [PATCH] Revert r312765 for now, since it causes assertions when building lang/spidermonkey24. Reported by: antoine PR: 215649 --- .../include/llvm/Analysis/ScalarEvolution.h | 23 +----- .../llvm/Analysis/ScalarEvolutionExpander.h | 3 +- contrib/llvm/lib/Analysis/ScalarEvolution.cpp | 82 +++++-------------- .../lib/Analysis/ScalarEvolutionExpander.cpp | 31 +++---- 4 files changed, 36 insertions(+), 103 deletions(-) diff --git a/contrib/llvm/include/llvm/Analysis/ScalarEvolution.h b/contrib/llvm/include/llvm/Analysis/ScalarEvolution.h index 023998d41141..535b623d31ac 100644 --- a/contrib/llvm/include/llvm/Analysis/ScalarEvolution.h +++ b/contrib/llvm/include/llvm/Analysis/ScalarEvolution.h @@ -495,29 +495,10 @@ namespace llvm { /// The typedef for ExprValueMap. /// - typedef std::pair ValueOffsetPair; - typedef DenseMap> ExprValueMapType; + typedef DenseMap> 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. @@ -1200,7 +1181,7 @@ namespace llvm { bool containsAddRecurrence(const SCEV *S); /// Return the Value set from which the SCEV expr is generated. - SetVector *getSCEVValues(const SCEV *S); + SetVector *getSCEVValues(const SCEV *S); /// Erase Value from ValueExprMap and ExprValueMap. void eraseValueFromMap(Value *V); diff --git a/contrib/llvm/include/llvm/Analysis/ScalarEvolutionExpander.h b/contrib/llvm/include/llvm/Analysis/ScalarEvolutionExpander.h index 44ce462fe29c..1acf952ab70c 100644 --- a/contrib/llvm/include/llvm/Analysis/ScalarEvolutionExpander.h +++ b/contrib/llvm/include/llvm/Analysis/ScalarEvolutionExpander.h @@ -325,8 +325,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); diff --git a/contrib/llvm/lib/Analysis/ScalarEvolution.cpp b/contrib/llvm/lib/Analysis/ScalarEvolution.cpp index 8fefada6fdbc..e42a4b574d90 100644 --- a/contrib/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/contrib/llvm/lib/Analysis/ScalarEvolution.cpp @@ -3378,28 +3378,8 @@ bool ScalarEvolution::containsAddRecurrence(const SCEV *S) { 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 splitAddExpr(const SCEV *S) { - const auto *Add = dyn_cast(S); - if (!Add) - return {S, nullptr}; - - if (Add->getNumOperands() != 2) - return {S, nullptr}; - - auto *ConstOp = dyn_cast(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::getSCEVValues(const SCEV *S) { +/// Return the Value set from S. +SetVector *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 SCEV *S) { 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 *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 *SV = getSCEVValues(Stripped)) - SV->remove({V, Offset}); - } + SetVector *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(Value *V) { 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 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(Stripped) && - !isa(V)) - ExprValueMap[Stripped].insert({V, Offset}); - } + if (Pair.second) + ExprValueMap[S].insert(V); } return S; } @@ -3478,8 +3436,8 @@ const SCEV *ScalarEvolution::getExistingSCEV(Value *V) { 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(Instruction *PN, const SCEV *SymName) { if (!isa(I) || !isa(Old) || (I != PN && Old == SymName)) { - eraseValueFromMap(It->first); forgetMemoizedResults(Old); + ValueExprMap.erase(It); } } @@ -4097,7 +4055,7 @@ const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) { // 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; @@ -5477,8 +5435,8 @@ ScalarEvolution::getBackedgeTakenInfo(const Loop *L) { // case, createNodeForPHI will perform the necessary updates on its // own when it gets to that point. if (!isa(I) || !isa(Old)) { - eraseValueFromMap(It->first); forgetMemoizedResults(Old); + ValueExprMap.erase(It); } if (PHINode *PN = dyn_cast(I)) ConstantEvolutionLoopExitValue.erase(PN); @@ -5523,8 +5481,8 @@ void ScalarEvolution::forgetLoop(const Loop *L) { ValueExprMapType::iterator It = ValueExprMap.find_as(static_cast(I)); if (It != ValueExprMap.end()) { - eraseValueFromMap(It->first); forgetMemoizedResults(It->second); + ValueExprMap.erase(It); if (PHINode *PN = dyn_cast(I)) ConstantEvolutionLoopExitValue.erase(PN); } @@ -5557,8 +5515,8 @@ void ScalarEvolution::forgetValue(Value *V) { ValueExprMapType::iterator It = ValueExprMap.find_as(static_cast(I)); if (It != ValueExprMap.end()) { - eraseValueFromMap(It->first); forgetMemoizedResults(It->second); + ValueExprMap.erase(It); if (PHINode *PN = dyn_cast(I)) ConstantEvolutionLoopExitValue.erase(PN); } diff --git a/contrib/llvm/lib/Analysis/ScalarEvolutionExpander.cpp b/contrib/llvm/lib/Analysis/ScalarEvolutionExpander.cpp index 4d6cf5a16df9..2e45bb840946 100644 --- a/contrib/llvm/lib/Analysis/ScalarEvolutionExpander.cpp +++ b/contrib/llvm/lib/Analysis/ScalarEvolutionExpander.cpp @@ -1625,10 +1625,9 @@ Value *SCEVExpander::expandCodeFor(const SCEV *SH, Type *Ty) { return V; } -ScalarEvolution::ValueOffsetPair -SCEVExpander::FindValueInExprValueMap(const SCEV *S, - const Instruction *InsertPt) { - SetVector *Set = SE.getSCEVValues(S); +Value *SCEVExpander::FindValueInExprValueMap(const SCEV *S, + const Instruction *InsertPt) { + SetVector *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)) { @@ -1637,21 +1636,21 @@ SCEVExpander::FindValueInExprValueMap(const SCEV *S, // 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(V) && (EntInst = cast(V)) && - S->getType() == V->getType() && + if (Ent && isa(Ent) && + (EntInst = cast(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, @@ -1699,14 +1698,10 @@ Value *SCEVExpander::expand(const SCEV *S) { 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. // @@ -1919,7 +1914,7 @@ Value *SCEVExpander::findExistingExpansion(const SCEV *S, // 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