From 4a4f767235dac50965fc266fb822d9a2e37d5212 Mon Sep 17 00:00:00 2001
From: Dan Gohman <gohman@apple.com>
Date: Wed, 27 May 2009 02:00:53 +0000
Subject: [PATCH] Teach SCEVExpander to avoid creating over-indexed GEP indices
 when possible. For example, it now emits

  %p.2.ip.1 = getelementptr [3 x [3 x double]]* %p, i64 2, i64 %tmp, i64 1

instead of the equivalent but less obvious

  %p.2.ip.1 = getelementptr [3 x [3 x double]]* %p, i64 0, i64 %tmp, i64 19


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@72452 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Analysis/ScalarEvolutionExpander.cpp      | 35 +++++++++++++------
 .../IndVarSimplify/preserve-gep-remainder.ll  | 19 ++++++++++
 2 files changed, 44 insertions(+), 10 deletions(-)
 create mode 100644 test/Transforms/IndVarSimplify/preserve-gep-remainder.ll

diff --git a/lib/Analysis/ScalarEvolutionExpander.cpp b/lib/Analysis/ScalarEvolutionExpander.cpp
index 6992b998802..4e2600986b1 100644
--- a/lib/Analysis/ScalarEvolutionExpander.cpp
+++ b/lib/Analysis/ScalarEvolutionExpander.cpp
@@ -144,12 +144,15 @@ Value *SCEVExpander::InsertBinop(Instruction::BinaryOps Opcode, Value *LHS,
   return BO;
 }
 
-/// FactorOutConstant - Test if S is evenly divisible by Factor, using signed
+/// FactorOutConstant - Test if S is divisible by Factor, using signed
 /// division. If so, update S with Factor divided out and return true.
+/// S need not be evenly divisble 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(SCEVHandle &S,
+                              SCEVHandle &Remainder,
                               const APInt &Factor,
                               ScalarEvolution &SE) {
   // Everything is divisible by one.
@@ -157,14 +160,21 @@ static bool FactorOutConstant(SCEVHandle &S,
     return true;
 
   // For a Constant, check for a multiple of the given factor.
-  if (const SCEVConstant *C = dyn_cast<SCEVConstant>(S))
-    if (!C->getValue()->getValue().srem(Factor)) {
-      ConstantInt *CI =
-        ConstantInt::get(C->getValue()->getValue().sdiv(Factor));
+  if (const SCEVConstant *C = dyn_cast<SCEVConstant>(S)) {
+    ConstantInt *CI =
+      ConstantInt::get(C->getValue()->getValue().sdiv(Factor));
+    // If the quotient is zero and the remainder is non-zero, reject
+    // the value at this scale. It will be considered for subsequent
+    // smaller scales.
+    if (C->isZero() || !CI->isZero()) {
       SCEVHandle Div = SE.getConstant(CI);
       S = Div;
+      Remainder =
+        SE.getAddExpr(Remainder,
+                      SE.getConstant(C->getValue()->getValue().srem(Factor)));
       return true;
     }
+  }
 
   // In a Mul, check if there is a constant operand which is a multiple
   // of the given factor.
@@ -180,11 +190,14 @@ static bool FactorOutConstant(SCEVHandle &S,
 
   // In an AddRec, check if both start and step are divisible.
   if (const SCEVAddRecExpr *A = dyn_cast<SCEVAddRecExpr>(S)) {
-    SCEVHandle Start = A->getStart();
-    if (!FactorOutConstant(Start, Factor, SE))
-      return false;
     SCEVHandle Step = A->getStepRecurrence(SE);
-    if (!FactorOutConstant(Step, Factor, SE))
+    SCEVHandle StepRem = SE.getIntegerSCEV(0, Step->getType());
+    if (!FactorOutConstant(Step, StepRem, Factor, SE))
+      return false;
+    if (!StepRem->isZero())
+      return false;
+    SCEVHandle Start = A->getStart();
+    if (!FactorOutConstant(Start, Remainder, Factor, SE))
       return false;
     S = SE.getAddRecExpr(Start, Step, A->getLoop());
     return true;
@@ -253,8 +266,10 @@ Value *SCEVExpander::expandAddToGEP(const SCEVHandle *op_begin,
       // If the scale size is not 0, attempt to factor out a scale.
       if (ElSize != 0) {
         SCEVHandle Op = Ops[i];
-        if (FactorOutConstant(Op, ElSize, SE)) {
+        SCEVHandle Remainder = SE.getIntegerSCEV(0, Op->getType());
+        if (FactorOutConstant(Op, Remainder, ElSize, SE)) {
           ScaledOps.push_back(Op); // Op now has ElSize factored out.
+          NewOps.push_back(Remainder);
           continue;
         }
       }
diff --git a/test/Transforms/IndVarSimplify/preserve-gep-remainder.ll b/test/Transforms/IndVarSimplify/preserve-gep-remainder.ll
new file mode 100644
index 00000000000..95726ea0818
--- /dev/null
+++ b/test/Transforms/IndVarSimplify/preserve-gep-remainder.ll
@@ -0,0 +1,19 @@
+; RUN: llvm-as < %s | opt -indvars | llvm-dis \
+; RUN:   | grep {\[%\]p.2.ip.1 = getelementptr \\\[3 x \\\[3 x double\\\]\\\]\\* \[%\]p, i64 2, i64 \[%\]tmp, i64 1}
+
+; Indvars shouldn't expand this to
+;   %p.2.ip.1 = getelementptr [3 x [3 x double]]* %p, i64 0, i64 %tmp, i64 19
+; or something. That's valid, but more obscure.
+
+define void @foo([3 x [3 x double]]* noalias %p) nounwind {
+entry:
+  br label %loop
+
+loop:
+  %i = phi i64 [ 0, %entry ], [ %i.next, %loop ]
+  %ip = add i64 %i, 1
+  %p.2.ip.1 = getelementptr [3 x [3 x double]]* %p, i64 2, i64 %ip, i64 1
+  volatile store double 0.0, double* %p.2.ip.1
+  %i.next = add i64 %i, 1
+  br label %loop
+}