[llvm-commits] [llvm] r51169 - in /llvm/trunk: lib/Transforms/Scalar/LoopStrengthReduce.cpp test/CodeGen/X86/vortex-bug.ll

[Dan Gohman]

Author: djg
Date: Thu May 15 18:26:57 2008
New Revision: 51169

URL: http://llvm.org/viewvc/llvm-project?rev=51169&view=rev
Log:
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. 

Added:
    llvm/trunk/test/CodeGen/X86/vortex-bug.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp?rev=51169&r1=51168&r2=51169&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp Thu May 15 18:26:57 2008
@@ -543,6 +543,7 @@
     // 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 @@
 
 // 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 @@
     // 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 @@
 
       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 @@
         // 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

Added: llvm/trunk/test/CodeGen/X86/vortex-bug.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vortex-bug.ll?rev=51169&view=auto

==============================================================================
--- llvm/trunk/test/CodeGen/X86/vortex-bug.ll (added)
+++ llvm/trunk/test/CodeGen/X86/vortex-bug.ll Thu May 15 18:26:57 2008
@@ -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
+}