[PATCH] D26217: Consider all SCEVMulExpr operands for factoring in FactorOutConstant

[Victor Campos via llvm-commits]

vhscampos created this revision.
vhscampos added reviewers: sanjoy, sunfish.
vhscampos added a subscriber: llvm-commits.
Herald added a subscriber: mzolotukhin.

In FactorOutConstant function, when the expression is a SCEVMulExpr, the code was considering only the first operand for factoring. As the comment in line 258 states, all operands should be tested until one suitable is found.

I also removed the TODO about SCEVSDivExpr since this class is not present anymore in LLVM.


https://reviews.llvm.org/D26217

Files:
  lib/Analysis/ScalarEvolutionExpander.cpp


Index: lib/Analysis/ScalarEvolutionExpander.cpp
===================================================================
--- lib/Analysis/ScalarEvolutionExpander.cpp
+++ lib/Analysis/ScalarEvolutionExpander.cpp
@@ -220,9 +220,6 @@
 /// division. If so, update S with Factor divided out and return true.
 /// S need not be evenly divisible if a reasonable remainder can be
 /// computed.
-/// TODO: When ScalarEvolution gets a SCEVSDivExpr, this can be made
-/// unnecessary; in its place, just signed-divide Ops[i] by the scale and
-/// check to see if the divide was folded.
 static bool FactorOutConstant(const SCEV *&S, const SCEV *&Remainder,
                               const SCEV *Factor, ScalarEvolution &SE,
                               const DataLayout &DL) {
@@ -264,13 +261,15 @@
     // Size is known, check if there is a constant operand which is a multiple
     // of the given factor. If so, we can factor it.
     const SCEVConstant *FC = cast<SCEVConstant>(Factor);
-    if (const SCEVConstant *C = dyn_cast<SCEVConstant>(M->getOperand(0)))
-      if (!C->getAPInt().srem(FC->getAPInt())) {
-        SmallVector<const SCEV *, 4> NewMulOps(M->op_begin(), M->op_end());
-        NewMulOps[0] = SE.getConstant(C->getAPInt().sdiv(FC->getAPInt()));
-        S = SE.getMulExpr(NewMulOps);
-        return true;
-      }
+    for (unsigned i = 0, e = M->getNumOperands(); i < e; ++i) {
+      if (const SCEVConstant *C = dyn_cast<SCEVConstant>(M->getOperand(i)))
+        if (!C->getAPInt().srem(FC->getAPInt())) {
+          SmallVector<const SCEV *, 4> NewMulOps(M->op_begin(), M->op_end());
+          NewMulOps[i] = SE.getConstant(C->getAPInt().sdiv(FC->getAPInt()));
+          S = SE.getMulExpr(NewMulOps);
+          return true;
+        }
+    }
   }
 
   // In an AddRec, check if both start and step are divisible.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26217.76638.patch
Type: text/x-patch
Size: 1836 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161101/6b7f6493/attachment.bin>