[llvm] r300330 - Remove NormalizeAutodetect; NFC
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 14 08:49:53 PDT 2017
Author: sanjoy
Date: Fri Apr 14 10:49:53 2017
New Revision: 300330
URL: http://llvm.org/viewvc/llvm-project?rev=300330&view=rev
Log:
Remove NormalizeAutodetect; NFC
It is cleaner to have a callback based system where the logic of
whether an add recurrence is normalized or not lives on IVUsers.
This is one step in a multi-step cleanup.
Modified:
llvm/trunk/include/llvm/Analysis/ScalarEvolutionNormalization.h
llvm/trunk/lib/Analysis/IVUsers.cpp
llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
llvm/trunk/lib/Analysis/ScalarEvolutionNormalization.cpp
llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp
Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolutionNormalization.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolutionNormalization.h?rev=300330&r1=300329&r2=300330&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/ScalarEvolutionNormalization.h (original)
+++ llvm/trunk/include/llvm/Analysis/ScalarEvolutionNormalization.h Fri Apr 14 10:49:53 2017
@@ -40,21 +40,15 @@
namespace llvm {
-class Instruction;
-class DominatorTree;
class Loop;
class ScalarEvolution;
class SCEV;
-class Value;
/// TransformKind - Different types of transformations that
/// TransformForPostIncUse can do.
enum TransformKind {
/// Normalize - Normalize according to the given loops.
Normalize,
- /// NormalizeAutodetect - Detect post-inc opportunities on new expressions,
- /// update the given loop set, and normalize.
- NormalizeAutodetect,
/// Denormalize - Perform the inverse transform on the expression with the
/// given loop set.
Denormalize
@@ -63,16 +57,13 @@ enum TransformKind {
/// PostIncLoopSet - A set of loops.
typedef SmallPtrSet<const Loop *, 2> PostIncLoopSet;
+typedef function_ref<bool(const SCEVAddRecExpr *)> NormalizePredTy;
+
/// TransformForPostIncUse - Transform the given expression according to the
/// given transformation kind.
-const SCEV *TransformForPostIncUse(TransformKind Kind,
- const SCEV *S,
- Instruction *User,
- Value *OperandValToReplace,
- PostIncLoopSet &Loops,
- ScalarEvolution &SE,
- DominatorTree &DT);
-
+const SCEV *TransformForPostIncUse(TransformKind Kind, const SCEV *S,
+ Optional<NormalizePredTy> Pred,
+ PostIncLoopSet &Loops, ScalarEvolution &SE);
}
#endif
Modified: llvm/trunk/lib/Analysis/IVUsers.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IVUsers.cpp?rev=300330&r1=300329&r2=300330&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/IVUsers.cpp (original)
+++ llvm/trunk/lib/Analysis/IVUsers.cpp Fri Apr 14 10:49:53 2017
@@ -117,6 +117,50 @@ static bool isSimplifiedLoopNest(BasicBl
return true;
}
+/// IVUseShouldUsePostIncValue - We have discovered a "User" of an IV expression
+/// and now we need to decide whether the user should use the preinc or post-inc
+/// value. If this user should use the post-inc version of the IV, return true.
+///
+/// Choosing wrong here can break dominance properties (if we choose to use the
+/// post-inc value when we cannot) or it can end up adding extra live-ranges to
+/// the loop, resulting in reg-reg copies (if we use the pre-inc value when we
+/// should use the post-inc value).
+static bool IVUseShouldUsePostIncValue(Instruction *User, Value *Operand,
+ const Loop *L, DominatorTree *DT) {
+ // If the user is in the loop, use the preinc value.
+ if (L->contains(User))
+ return false;
+
+ BasicBlock *LatchBlock = L->getLoopLatch();
+ if (!LatchBlock)
+ return false;
+
+ // Ok, the user is outside of the loop. If it is dominated by the latch
+ // block, use the post-inc value.
+ if (DT->dominates(LatchBlock, User->getParent()))
+ return true;
+
+ // There is one case we have to be careful of: PHI nodes. These little guys
+ // can live in blocks that are not dominated by the latch block, but (since
+ // their uses occur in the predecessor block, not the block the PHI lives in)
+ // should still use the post-inc value. Check for this case now.
+ PHINode *PN = dyn_cast<PHINode>(User);
+ if (!PN || !Operand)
+ return false; // not a phi, not dominated by latch block.
+
+ // Look at all of the uses of Operand by the PHI node. If any use corresponds
+ // to a block that is not dominated by the latch block, give up and use the
+ // preincremented value.
+ for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
+ if (PN->getIncomingValue(i) == Operand &&
+ !DT->dominates(LatchBlock, PN->getIncomingBlock(i)))
+ return false;
+
+ // Okay, all uses of Operand by PN are in predecessor blocks that really are
+ // dominated by the latch block. Use the post-incremented value.
+ return true;
+}
+
/// AddUsersImpl - Inspect the specified instruction. If it is a
/// reducible SCEV, recursively add its users to the IVUsesByStride set and
/// return true. Otherwise, return false.
@@ -207,19 +251,36 @@ bool IVUsers::AddUsersImpl(Instruction *
// The regular return value here is discarded; instead of recording
// it, we just recompute it when we need it.
const SCEV *OriginalISE = ISE;
- ISE = TransformForPostIncUse(NormalizeAutodetect,
- ISE, User, I,
- NewUse.PostIncLoops,
- *SE, *DT);
+
+ auto NormalizePred = [&](const SCEVAddRecExpr *AR) {
+ // We only allow affine AddRecs to be normalized, otherwise we would not
+ // be able to correctly denormalize.
+ // e.g. {1,+,3,+,2} == {-2,+,1,+,2} + {3,+,2}
+ // Normalized form: {-2,+,1,+,2}
+ // Denormalized form: {1,+,3,+,2}
+ //
+ // However, denormalization would use a different step expression than
+ // normalization (see getPostIncExpr), generating the wrong final
+ // expression: {-2,+,1,+,2} + {1,+,2} => {-1,+,3,+,2}
+ auto *L = AR->getLoop();
+ bool Result =
+ AR->isAffine() && IVUseShouldUsePostIncValue(User, I, L, DT);
+ if (Result)
+ NewUse.PostIncLoops.insert(L);
+ return Result;
+ };
+
+ ISE = TransformForPostIncUse(Normalize, ISE,
+ Optional<NormalizePredTy>(NormalizePred),
+ NewUse.PostIncLoops, *SE);
// PostIncNormalization effectively simplifies the expression under
// pre-increment assumptions. Those assumptions (no wrapping) might not
// hold for the post-inc value. Catch such cases by making sure the
// transformation is invertible.
if (OriginalISE != ISE) {
- const SCEV *DenormalizedISE =
- TransformForPostIncUse(Denormalize, ISE, User, I,
- NewUse.PostIncLoops, *SE, *DT);
+ const SCEV *DenormalizedISE = TransformForPostIncUse(
+ Denormalize, ISE, None, NewUse.PostIncLoops, *SE);
// If we normalized the expression, but denormalization doesn't give the
// original one, discard this user.
@@ -337,11 +398,9 @@ const SCEV *IVUsers::getReplacementExpr(
/// getExpr - Return the expression for the use.
const SCEV *IVUsers::getExpr(const IVStrideUse &IU) const {
- return
- TransformForPostIncUse(Normalize, getReplacementExpr(IU),
- IU.getUser(), IU.getOperandValToReplace(),
- const_cast<PostIncLoopSet &>(IU.getPostIncLoops()),
- *SE, *DT);
+ return TransformForPostIncUse(
+ Normalize, getReplacementExpr(IU), None,
+ const_cast<PostIncLoopSet &>(IU.getPostIncLoops()), *SE);
}
static const SCEVAddRecExpr *findAddRecForLoop(const SCEV *S, const Loop *L) {
Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=300330&r1=300329&r2=300330&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Fri Apr 14 10:49:53 2017
@@ -1268,8 +1268,8 @@ Value *SCEVExpander::expandAddRecExprLit
if (PostIncLoops.count(L)) {
PostIncLoopSet Loops;
Loops.insert(L);
- Normalized = cast<SCEVAddRecExpr>(TransformForPostIncUse(
- Normalize, S, nullptr, nullptr, Loops, SE, SE.DT));
+ Normalized = cast<SCEVAddRecExpr>(
+ TransformForPostIncUse(Normalize, S, None, Loops, SE));
}
// Strip off any non-loop-dominating component from the addrec start.
Modified: llvm/trunk/lib/Analysis/ScalarEvolutionNormalization.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionNormalization.cpp?rev=300330&r1=300329&r2=300330&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolutionNormalization.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolutionNormalization.cpp Fri Apr 14 10:49:53 2017
@@ -12,88 +12,41 @@
//
//===----------------------------------------------------------------------===//
-#include "llvm/IR/Dominators.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/ScalarEvolutionExpressions.h"
#include "llvm/Analysis/ScalarEvolutionNormalization.h"
using namespace llvm;
-/// IVUseShouldUsePostIncValue - We have discovered a "User" of an IV expression
-/// and now we need to decide whether the user should use the preinc or post-inc
-/// value. If this user should use the post-inc version of the IV, return true.
-///
-/// Choosing wrong here can break dominance properties (if we choose to use the
-/// post-inc value when we cannot) or it can end up adding extra live-ranges to
-/// the loop, resulting in reg-reg copies (if we use the pre-inc value when we
-/// should use the post-inc value).
-static bool IVUseShouldUsePostIncValue(Instruction *User, Value *Operand,
- const Loop *L, DominatorTree *DT) {
- // If the user is in the loop, use the preinc value.
- if (L->contains(User)) return false;
-
- BasicBlock *LatchBlock = L->getLoopLatch();
- if (!LatchBlock)
- return false;
-
- // Ok, the user is outside of the loop. If it is dominated by the latch
- // block, use the post-inc value.
- if (DT->dominates(LatchBlock, User->getParent()))
- return true;
-
- // There is one case we have to be careful of: PHI nodes. These little guys
- // can live in blocks that are not dominated by the latch block, but (since
- // their uses occur in the predecessor block, not the block the PHI lives in)
- // should still use the post-inc value. Check for this case now.
- PHINode *PN = dyn_cast<PHINode>(User);
- if (!PN || !Operand) return false; // not a phi, not dominated by latch block.
-
- // Look at all of the uses of Operand by the PHI node. If any use corresponds
- // to a block that is not dominated by the latch block, give up and use the
- // preincremented value.
- for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
- if (PN->getIncomingValue(i) == Operand &&
- !DT->dominates(LatchBlock, PN->getIncomingBlock(i)))
- return false;
-
- // Okay, all uses of Operand by PN are in predecessor blocks that really are
- // dominated by the latch block. Use the post-incremented value.
- return true;
-}
-
namespace {
/// Hold the state used during post-inc expression transformation, including a
/// map of transformed expressions.
class PostIncTransform {
TransformKind Kind;
+ Optional<NormalizePredTy> Pred;
PostIncLoopSet &Loops;
ScalarEvolution &SE;
- DominatorTree &DT;
DenseMap<const SCEV*, const SCEV*> Transformed;
public:
- PostIncTransform(TransformKind kind, PostIncLoopSet &loops,
- ScalarEvolution &se, DominatorTree &dt):
- Kind(kind), Loops(loops), SE(se), DT(dt) {}
+ PostIncTransform(TransformKind kind, Optional<NormalizePredTy> Pred,
+ PostIncLoopSet &loops, ScalarEvolution &se)
+ : Kind(kind), Pred(Pred), Loops(loops), SE(se) {}
- const SCEV *TransformSubExpr(const SCEV *S, Instruction *User,
- Value *OperandValToReplace);
+ const SCEV *TransformSubExpr(const SCEV *S);
protected:
- const SCEV *TransformImpl(const SCEV *S, Instruction *User,
- Value *OperandValToReplace);
+ const SCEV *TransformImpl(const SCEV *S);
};
} // namespace
/// Implement post-inc transformation for all valid expression types.
-const SCEV *PostIncTransform::
-TransformImpl(const SCEV *S, Instruction *User, Value *OperandValToReplace) {
-
+const SCEV *PostIncTransform::TransformImpl(const SCEV *S) {
if (const SCEVCastExpr *X = dyn_cast<SCEVCastExpr>(S)) {
const SCEV *O = X->getOperand();
- const SCEV *N = TransformSubExpr(O, User, OperandValToReplace);
+ const SCEV *N = TransformSubExpr(O);
if (O != N)
switch (S->getSCEVType()) {
case scZeroExtend: return SE.getZeroExtendExpr(N, S->getType());
@@ -108,44 +61,13 @@ TransformImpl(const SCEV *S, Instruction
// An addrec. This is the interesting part.
SmallVector<const SCEV *, 8> Operands;
const Loop *L = AR->getLoop();
- // The addrec conceptually uses its operands at loop entry.
- Instruction *LUser = &L->getHeader()->front();
- transform(
- AR->operands(), std::back_inserter(Operands),
- [&](const SCEV *Op) { return TransformSubExpr(Op, LUser, nullptr); });
+ transform(AR->operands(), std::back_inserter(Operands),
+ [&](const SCEV *Op) { return TransformSubExpr(Op); });
// Conservatively use AnyWrap until/unless we need FlagNW.
const SCEV *Result = SE.getAddRecExpr(Operands, L, SCEV::FlagAnyWrap);
switch (Kind) {
- case NormalizeAutodetect:
- // Normalize this SCEV by subtracting the expression for the final step.
- // We only allow affine AddRecs to be normalized, otherwise we would not
- // be able to correctly denormalize.
- // e.g. {1,+,3,+,2} == {-2,+,1,+,2} + {3,+,2}
- // Normalized form: {-2,+,1,+,2}
- // Denormalized form: {1,+,3,+,2}
- //
- // However, denormalization would use a different step expression than
- // normalization (see getPostIncExpr), generating the wrong final
- // expression: {-2,+,1,+,2} + {1,+,2} => {-1,+,3,+,2}
- if (AR->isAffine() &&
- IVUseShouldUsePostIncValue(User, OperandValToReplace, L, &DT)) {
- const SCEV *TransformedStep =
- TransformSubExpr(AR->getStepRecurrence(SE),
- User, OperandValToReplace);
- Result = SE.getMinusSCEV(Result, TransformedStep);
- Loops.insert(L);
- }
-#if 0
- // This assert is conceptually correct, but ScalarEvolution currently
- // sometimes fails to canonicalize two equal SCEVs to exactly the same
- // form. It's possibly a pessimization when this happens, but it isn't a
- // correctness problem, so disable this assert for now.
- assert(S == TransformSubExpr(Result, User, OperandValToReplace) &&
- "SCEV normalization is not invertible!");
-#endif
- break;
case Normalize:
// We want to normalize step expression, because otherwise we might not be
// able to denormalize to the original expression.
@@ -161,10 +83,9 @@ TransformImpl(const SCEV *S, Instruction
// (100 /u {1,+,1}<%bb16>)}<%bb25>
// Note that the initial value changes after normalization +
// denormalization, which isn't correct.
- if (Loops.count(L)) {
+ if ((Pred && (*Pred)(AR)) || (!Pred && Loops.count(L))) {
const SCEV *TransformedStep =
- TransformSubExpr(AR->getStepRecurrence(SE),
- User, OperandValToReplace);
+ TransformSubExpr(AR->getStepRecurrence(SE));
Result = SE.getMinusSCEV(Result, TransformedStep);
}
#if 0
@@ -178,8 +99,7 @@ TransformImpl(const SCEV *S, Instruction
// stated above.
if (Loops.count(L)) {
const SCEV *TransformedStep =
- TransformSubExpr(AR->getStepRecurrence(SE),
- User, OperandValToReplace);
+ TransformSubExpr(AR->getStepRecurrence(SE));
Result = SE.getAddExpr(Result, TransformedStep);
}
break;
@@ -194,7 +114,7 @@ TransformImpl(const SCEV *S, Instruction
for (SCEVNAryExpr::op_iterator I = X->op_begin(), E = X->op_end();
I != E; ++I) {
const SCEV *O = *I;
- const SCEV *N = TransformSubExpr(O, User, OperandValToReplace);
+ const SCEV *N = TransformSubExpr(O);
Changed |= N != O;
Operands.push_back(N);
}
@@ -213,8 +133,8 @@ TransformImpl(const SCEV *S, Instruction
if (const SCEVUDivExpr *X = dyn_cast<SCEVUDivExpr>(S)) {
const SCEV *LO = X->getLHS();
const SCEV *RO = X->getRHS();
- const SCEV *LN = TransformSubExpr(LO, User, OperandValToReplace);
- const SCEV *RN = TransformSubExpr(RO, User, OperandValToReplace);
+ const SCEV *LN = TransformSubExpr(LO);
+ const SCEV *RN = TransformSubExpr(RO);
if (LO != LN || RO != RN)
return SE.getUDivExpr(LN, RN);
return S;
@@ -225,9 +145,7 @@ TransformImpl(const SCEV *S, Instruction
/// Manage recursive transformation across an expression DAG. Revisiting
/// expressions would lead to exponential recursion.
-const SCEV *PostIncTransform::
-TransformSubExpr(const SCEV *S, Instruction *User, Value *OperandValToReplace) {
-
+const SCEV *PostIncTransform::TransformSubExpr(const SCEV *S) {
if (isa<SCEVConstant>(S) || isa<SCEVUnknown>(S))
return S;
@@ -235,20 +153,17 @@ TransformSubExpr(const SCEV *S, Instruct
if (Result)
return Result;
- Result = TransformImpl(S, User, OperandValToReplace);
+ Result = TransformImpl(S);
Transformed[S] = Result;
return Result;
}
/// Top level driver for transforming an expression DAG into its requested
/// post-inc form (either "Normalized" or "Denormalized").
-const SCEV *llvm::TransformForPostIncUse(TransformKind Kind,
- const SCEV *S,
- Instruction *User,
- Value *OperandValToReplace,
+const SCEV *llvm::TransformForPostIncUse(TransformKind Kind, const SCEV *S,
+ Optional<NormalizePredTy> Pred,
PostIncLoopSet &Loops,
- ScalarEvolution &SE,
- DominatorTree &DT) {
- PostIncTransform Transform(Kind, Loops, SE, DT);
- return Transform.TransformSubExpr(S, User, OperandValToReplace);
+ ScalarEvolution &SE) {
+ PostIncTransform Transform(Kind, Pred, Loops, SE);
+ return Transform.TransformSubExpr(S);
}
Modified: llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp?rev=300330&r1=300329&r2=300330&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp Fri Apr 14 10:49:53 2017
@@ -3160,8 +3160,7 @@ void LSRInstance::CollectFixupsAndInitia
if (SE.isLoopInvariant(N, L) && isSafeToExpand(N, SE)) {
// S is normalized, so normalize N before folding it into S
// to keep the result normalized.
- N = TransformForPostIncUse(Normalize, N, CI, nullptr,
- TmpPostIncLoops, SE, DT);
+ N = TransformForPostIncUse(Normalize, N, None, TmpPostIncLoops, SE);
Kind = LSRUse::ICmpZero;
S = SE.getMinusSCEV(N, S);
}
@@ -4800,10 +4799,7 @@ Value *LSRInstance::Expand(const LSRUse
// If we're expanding for a post-inc user, make the post-inc adjustment.
PostIncLoopSet &Loops = const_cast<PostIncLoopSet &>(LF.PostIncLoops);
- Reg = TransformForPostIncUse(Denormalize, Reg,
- LF.UserInst, LF.OperandValToReplace,
- Loops, SE, DT);
-
+ Reg = TransformForPostIncUse(Denormalize, Reg, None, Loops, SE);
Ops.push_back(SE.getUnknown(Rewriter.expandCodeFor(Reg, nullptr)));
}
@@ -4814,9 +4810,7 @@ Value *LSRInstance::Expand(const LSRUse
// If we're expanding for a post-inc user, make the post-inc adjustment.
PostIncLoopSet &Loops = const_cast<PostIncLoopSet &>(LF.PostIncLoops);
- ScaledS = TransformForPostIncUse(Denormalize, ScaledS,
- LF.UserInst, LF.OperandValToReplace,
- Loops, SE, DT);
+ ScaledS = TransformForPostIncUse(Denormalize, ScaledS, None, Loops, SE);
if (LU.Kind == LSRUse::ICmpZero) {
// Expand ScaleReg as if it was part of the base regs.
More information about the llvm-commits
mailing list