Remove NormalizeAutodetect; NFC
authorSanjoy Das <sanjoy@playingwithpointers.com>
Fri, 14 Apr 2017 15:49:53 +0000 (15:49 +0000)
committerSanjoy Das <sanjoy@playingwithpointers.com>
Fri, 14 Apr 2017 15:49:53 +0000 (15:49 +0000)
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.

llvm-svn: 300330

llvm/include/llvm/Analysis/ScalarEvolutionNormalization.h
llvm/lib/Analysis/IVUsers.cpp
llvm/lib/Analysis/ScalarEvolutionExpander.cpp
llvm/lib/Analysis/ScalarEvolutionNormalization.cpp
llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp

index 7c6423a..cc79aa9 100644 (file)
 
 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
index 4b7d15b..de38cf8 100644 (file)
@@ -117,6 +117,50 @@ static bool isSimplifiedLoopNest(BasicBlock *BB, const DominatorTree *DT,
   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 *I,
       // 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(const IVStrideUse &IU) const {
 
 /// 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) {
index d15a7db..66df34d 100644 (file)
@@ -1268,8 +1268,8 @@ Value *SCEVExpander::expandAddRecExprLiterally(const SCEVAddRecExpr *S) {
   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.
index 17d4c01..becc4bb 100644 (file)
 //
 //===----------------------------------------------------------------------===//
 
-#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 *User, Value *OperandValToReplace) {
     // 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 *User, Value *OperandValToReplace) {
       //     (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 *User, Value *OperandValToReplace) {
       // 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 *User, Value *OperandValToReplace) {
     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 *User, Value *OperandValToReplace) {
   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 *User, Value *OperandValToReplace) {
 
 /// 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, Instruction *User, Value *OperandValToReplace) {
   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);
 }
index 1dad080..b584e00 100644 (file)
@@ -3160,8 +3160,7 @@ void LSRInstance::CollectFixupsAndInitialFormulae() {
         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 &LU,
 
     // 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 &LU,
 
     // 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.