Fix a bug in LoopStrengthReduce that caused it to emit IR with

use-before-def. The problem comes up in code with multiple PHIs where
one PHI is being rewritten in terms of the other, but the other needs
to be casted first. LLVM rules requre the cast instruction to be
inserted after any PHI instructions, but when instructions were
inserted to replace the second PHI value with a function of the first,
they were ended up going before the cast instruction. Avoid this
problem by remembering the location of the cast instruction, when one
is needed, and inserting the expansion of the new value after it.

This fixes a bug that surfaced in 255.vortex on x86-64 when
instcombine was removed from the middle of the loop optimization
passes.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@51169 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 6d66d14ec9a81ac43fe81610b794948546bd4574..dbd22d90e015d049831beb8589662a046a8deb0e 100644 (file)
--- a/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -543,6 +543,7 @@ namespace {
     // operands of Inst to use the new expression 'NewBase', with 'Imm' added
     // to it.
     void RewriteInstructionToUseNewBase(const SCEVHandle &NewBase,
+                                        Instruction *InsertPt,
                                        SCEVExpander &Rewriter, Loop *L, Pass *P,
                                        SmallPtrSet<Instruction*,16> &DeadInsts);
     
@@ -603,8 +604,12 @@ Value *BasedUser::InsertCodeForBaseAtPosition(const SCEVHandle &NewBase,
 
 // Once we rewrite the code to insert the new IVs we want, update the
 // operands of Inst to use the new expression 'NewBase', with 'Imm' added
-// to it.
+// to it. NewBasePt is the last instruction which contributes to the
+// value of NewBase in the case that it's a diffferent instruction from
+// the PHI that NewBase is computed from, or null otherwise.
+//
 void BasedUser::RewriteInstructionToUseNewBase(const SCEVHandle &NewBase,
+                                               Instruction *NewBasePt,
                                       SCEVExpander &Rewriter, Loop *L, Pass *P,
                                       SmallPtrSet<Instruction*,16> &DeadInsts) {
   if (!isa<PHINode>(Inst)) {
@@ -620,7 +625,10 @@ void BasedUser::RewriteInstructionToUseNewBase(const SCEVHandle &NewBase,
     // value will be pinned to live somewhere after the original computation.
     // In this case, we have to back off.
     if (!isUseOfPostIncrementedValue) {
-      if (Instruction *OpInst = dyn_cast<Instruction>(OperandValToReplace)) { 
+      if (NewBasePt) {
+        InsertPt = NewBasePt;
+        ++InsertPt;
+      } else if (Instruction *OpInst = dyn_cast<Instruction>(OperandValToReplace)) { 
         InsertPt = OpInst;
         while (isa<PHINode>(InsertPt)) ++InsertPt;
       }
@@ -1400,6 +1408,15 @@ void LoopStrengthReduce::StrengthReduceStridedIVUsers(const SCEVHandle &Stride,
 
       SCEVHandle RewriteExpr = SE->getUnknown(RewriteOp);
 
+      // If we had to insert new instrutions for RewriteOp, we have to
+      // consider that they may not have been able to end up immediately
+      // next to RewriteOp, because non-PHI instructions may never precede
+      // PHI instructions in a block. In this case, remember where the last
+      // instruction was inserted so that we can use that point to expand
+      // the final RewriteExpr.
+      Instruction *NewBasePt = dyn_cast<Instruction>(RewriteOp);
+      if (RewriteOp == NewPHI) NewBasePt = 0;
+
       // Clear the SCEVExpander's expression map so that we are guaranteed
       // to have the code emitted where we expect it.
       Rewriter.clear();
@@ -1426,7 +1443,8 @@ void LoopStrengthReduce::StrengthReduceStridedIVUsers(const SCEVHandle &Stride,
         // Add BaseV to the PHI value if needed.
         RewriteExpr = SE->getAddExpr(RewriteExpr, SE->getUnknown(BaseV));
 
-      User.RewriteInstructionToUseNewBase(RewriteExpr, Rewriter, L, this,
+      User.RewriteInstructionToUseNewBase(RewriteExpr, NewBasePt,
+                                          Rewriter, L, this,
                                           DeadInsts);
 
       // Mark old value we replaced as possibly dead, so that it is elminated

diff --git a/test/CodeGen/X86/vortex-bug.ll b/test/CodeGen/X86/vortex-bug.ll
new file mode 100644 (file)
index 0000000..680bfbd
--- /dev/null
+++ b/test/CodeGen/X86/vortex-bug.ll
@@ -0,0 +1,21 @@
+; RUN: llvm-as < %s | llc -march=x86
+
+       %struct.blktkntype = type { i32, i32 }
+       %struct.fieldstruc = type { [128 x i8], %struct.blktkntype*, i32, i32 }
+
+define fastcc i32 @Env_GetFieldStruc(i8* %FieldName, i32* %Status, %struct.fieldstruc* %FieldStruc) nounwind  {
+entry:
+       br label %bb137.i
+
+bb137.i:               ; preds = %bb137.i, %entry
+       %FieldName_addr.0209.rec.i = phi i64 [ %tmp139.rec.i, %bb137.i ], [ 0, %entry ]         ; <i64> [#uses=1]
+       %tmp147213.i = phi i32 [ %tmp147.i, %bb137.i ], [ 1, %entry ]           ; <i32> [#uses=2]
+       %tmp139.rec.i = add i64 %FieldName_addr.0209.rec.i, 1           ; <i64> [#uses=2]
+       %tmp141142.i = sext i32 %tmp147213.i to i64             ; <i64> [#uses=0]
+       %tmp147.i = add i32 %tmp147213.i, 1             ; <i32> [#uses=1]
+       br i1 false, label %bb137.i, label %bb149.i.loopexit
+
+bb149.i.loopexit:              ; preds = %bb137.i
+       %tmp139.i = getelementptr i8* %FieldName, i64 %tmp139.rec.i             ; <i8*> [#uses=0]
+       unreachable
+}