[llvm] r247749 - [IndVars] Fix PR24783.

[Sanjoy Das via llvm-commits]

Author: sanjoy
Date: Tue Sep 15 18:45:39 2015
New Revision: 247749

URL: http://llvm.org/viewvc/llvm-project?rev=247749&view=rev
Log:
[IndVars] Fix PR24783.

In `IndVarSimplify::ExpandSCEVIfNeeded`,
`SCEVExpander::findExistingExpansion` may return an `llvm::Value` that
differs in type from the SCEV it was asked to find an expansion for (but
computes the same value).  In such cases, we fall back on
`expandCodeFor`; and rely on LLVM to CSE the two equivalent
expressions (different only by a no-op cast) into a single computation.

I tried a few other approaches to fixing PR24783, all of which turned
out to be more complex than this current version:

 1. Move the `ExpandSCEVIfNeeded` logic into `expandCodeFor`.  This got
    problematic because currently we do not pass in the `Loop *` into
    `expandCodeFor`.  Changing the interface to do this is a more
    invasive change, and really does not make much semantic sense unless
    the SCEV being passed in is an add recurrence.

    There is also the problem of `expandCodeFor` being used in places
    other than `indvars` -- there may be performance / correctness
    issues elsewhere if `expandCodeFor` is moved from always generating
    IR from scratch to cache-like model.

 2. Have `findExistingExpansion` only return expression with the correct
    type.  This would make `isHighCostExpansionHelper` and thus
    `isHighCostExpansion` more conservative than necessary.

 3. Insert casts on the value returned by `findExistingExpansion` if
    needed using `InsertNoopCastOfTo`.  This is complicated because
    `InsertNoopCastOfTo` depends on internal state of its
    `SCEVExpander` (specifically `Builder.GetInserPoint()`), and this
    may not be set up when `ExpandSCEVIfNeeded` is called.

 4. Manually insert casts on the value returned by
    `findExistingExpansion` if needed using `InsertNoopCastOfTo` via
    `CastInst::Create`.  This is probably workable, but figuring out the
    location where the cast instruction needs to be inserted has enough
    edge cases (arguments, constants, invokes, LCSSA must be preserved)
    makes me feel what I have right now is simplest solution.

Added:
    llvm/trunk/test/Transforms/IndVarSimplify/pr24783.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=247749&r1=247748&r2=247749&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Tue Sep 15 18:45:39 2015
@@ -509,7 +509,8 @@ Value *IndVarSimplify::ExpandSCEVIfNeede
   // Before expanding S into an expensive LLVM expression, see if we can use an
   // already existing value as the expansion for S.
   if (Value *ExistingValue = Rewriter.findExistingExpansion(S, InsertPt, L))
-    return ExistingValue;
+    if (ExistingValue->getType() == ResultTy)
+      return ExistingValue;
 
   // We didn't find anything, fall back to using SCEVExpander.
   return Rewriter.expandCodeFor(S, ResultTy, InsertPt);

Added: llvm/trunk/test/Transforms/IndVarSimplify/pr24783.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/pr24783.ll?rev=247749&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/IndVarSimplify/pr24783.ll (added)
+++ llvm/trunk/test/Transforms/IndVarSimplify/pr24783.ll Tue Sep 15 18:45:39 2015
@@ -0,0 +1,30 @@
+; RUN: opt -S -indvars < %s | FileCheck %s
+
+target datalayout = "E-m:e-i64:64-n32:64"
+target triple = "powerpc64-unknown-linux-gnu"
+
+define void @f(i32* %end.s, i8** %loc, i32 %p) {
+; CHECK-LABEL: @f(
+entry:
+; CHECK:  [[P_SEXT:%[0-9a-z]+]] = sext i32 %p to i64
+; CHECK:  [[END:%[0-9a-z]+]] = getelementptr i32, i32* %end.s, i64 [[P_SEXT]]
+
+  %end = getelementptr inbounds i32, i32* %end.s, i32 %p
+  %init = bitcast i32* %end.s to i8*
+  br label %while.body.i
+
+while.body.i:
+  %ptr = phi i8* [ %ptr.inc, %while.body.i ], [ %init, %entry ]
+  %ptr.inc = getelementptr inbounds i8, i8* %ptr, i8 1
+  %ptr.inc.cast = bitcast i8* %ptr.inc to i32*
+  %cmp.i = icmp eq i32* %ptr.inc.cast, %end
+  br i1 %cmp.i, label %loop.exit, label %while.body.i
+
+loop.exit:
+; CHECK: loop.exit:
+; CHECK: [[END_BCASTED:%[a-z0-9]+]] = bitcast i32* %scevgep to i8*
+; CHECK: store i8* [[END_BCASTED]], i8** %loc
+  %ptr.inc.lcssa = phi i8* [ %ptr.inc, %while.body.i ]
+  store i8* %ptr.inc.lcssa, i8** %loc
+  ret void
+}