[llvm-commits] [llvm] r46739 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/2008-02-04-GEPIdxBug.ll

Chris Lattner sabre at nondot.org
Mon Feb 4 20:45:35 PST 2008


Author: lattner
Date: Mon Feb  4 22:45:32 2008
New Revision: 46739

URL: http://llvm.org/viewvc/llvm-project?rev=46739&view=rev
Log:
Fix a bug compiling PR1978 (perhaps not the only one though) which
was incorrectly simplifying "x == (gep x, 1, i)" into false, even 
though i could be negative.  As it turns out, all the code to 
handle this already existed, we just need to disable the incorrect
optimization case and let the general case handle it.


Added:
    llvm/trunk/test/Transforms/InstCombine/2008-02-04-GEPIdxBug.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp

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

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Mon Feb  4 22:45:32 2008
@@ -4629,60 +4629,11 @@
 
   Value *PtrBase = GEPLHS->getOperand(0);
   if (PtrBase == RHS) {
-    // As an optimization, we don't actually have to compute the actual value of
-    // OFFSET if this is a icmp_eq or icmp_ne comparison, just return whether 
-    // each index is zero or not.
-    if (Cond == ICmpInst::ICMP_EQ || Cond == ICmpInst::ICMP_NE) {
-      Instruction *InVal = 0;
-      gep_type_iterator GTI = gep_type_begin(GEPLHS);
-      for (unsigned i = 1, e = GEPLHS->getNumOperands(); i != e; ++i, ++GTI) {
-        bool EmitIt = true;
-        if (Constant *C = dyn_cast<Constant>(GEPLHS->getOperand(i))) {
-          if (isa<UndefValue>(C))  // undef index -> undef.
-            return ReplaceInstUsesWith(I, UndefValue::get(I.getType()));
-          if (C->isNullValue())
-            EmitIt = false;
-          else if (TD->getABITypeSize(GTI.getIndexedType()) == 0) {
-            EmitIt = false;  // This is indexing into a zero sized array?
-          } else if (isa<ConstantInt>(C))
-            return ReplaceInstUsesWith(I, // No comparison is needed here.
-                                 ConstantInt::get(Type::Int1Ty, 
-                                                  Cond == ICmpInst::ICMP_NE));
-        }
-
-        if (EmitIt) {
-          Instruction *Comp =
-            new ICmpInst(Cond, GEPLHS->getOperand(i),
-                    Constant::getNullValue(GEPLHS->getOperand(i)->getType()));
-          if (InVal == 0)
-            InVal = Comp;
-          else {
-            InVal = InsertNewInstBefore(InVal, I);
-            InsertNewInstBefore(Comp, I);
-            if (Cond == ICmpInst::ICMP_NE)   // True if any are unequal
-              InVal = BinaryOperator::createOr(InVal, Comp);
-            else                              // True if all are equal
-              InVal = BinaryOperator::createAnd(InVal, Comp);
-          }
-        }
-      }
-
-      if (InVal)
-        return InVal;
-      else
-        // No comparison is needed here, all indexes = 0
-        ReplaceInstUsesWith(I, ConstantInt::get(Type::Int1Ty, 
-                                                Cond == ICmpInst::ICMP_EQ));
-    }
-
-    // Only lower this if the icmp is the only user of the GEP or if we expect
-    // the result to fold to a constant!
-    if (isa<ConstantExpr>(GEPLHS) || GEPLHS->hasOneUse()) {
-      // ((gep Ptr, OFFSET) cmp Ptr)   ---> (OFFSET cmp 0).
-      Value *Offset = EmitGEPOffset(GEPLHS, I, *this);
-      return new ICmpInst(ICmpInst::getSignedPredicate(Cond), Offset,
-                          Constant::getNullValue(Offset->getType()));
-    }
+    // ((gep Ptr, OFFSET) cmp Ptr)   ---> (OFFSET cmp 0).
+    // This transformation is valid because we know pointers can't overflow.
+    Value *Offset = EmitGEPOffset(GEPLHS, I, *this);
+    return new ICmpInst(ICmpInst::getSignedPredicate(Cond), Offset,
+                        Constant::getNullValue(Offset->getType()));
   } else if (User *GEPRHS = dyn_castGetElementPtr(RHS)) {
     // If the base pointers are different, but the indices are the same, just
     // compare the base pointer.

Added: llvm/trunk/test/Transforms/InstCombine/2008-02-04-GEPIdxBug.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2008-02-04-GEPIdxBug.ll?rev=46739&view=auto

==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/2008-02-04-GEPIdxBug.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/2008-02-04-GEPIdxBug.ll Mon Feb  4 22:45:32 2008
@@ -0,0 +1,33 @@
+; RUN: llvm-as < %s | opt -instcombine | llvm-dis | grep {icmp eq i32 %indvar, 0}
+; PR1978
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
+target triple = "i686-apple-darwin8"
+	%struct.x = type <{ i8 }>
+ at .str = internal constant [6 x i8] c"Main!\00"		; <[6 x i8]*> [#uses=1]
+ at .str1 = internal constant [12 x i8] c"destroy %p\0A\00"		; <[12 x i8]*> [#uses=1]
+
+define i32 @main() nounwind  {
+entry:
+	%orientations = alloca [1 x [1 x %struct.x]]		; <[1 x [1 x %struct.x]]*> [#uses=2]
+	%tmp3 = call i32 @puts( i8* getelementptr ([6 x i8]* @.str, i32 0, i32 0) ) nounwind 		; <i32> [#uses=0]
+	%tmp45 = getelementptr [1 x [1 x %struct.x]]* %orientations, i32 1, i32 0, i32 0		; <%struct.x*> [#uses=1]
+	%orientations62 = getelementptr [1 x [1 x %struct.x]]* %orientations, i32 0, i32 0, i32 0		; <%struct.x*> [#uses=1]
+	br label %bb10
+
+bb10:		; preds = %bb10, %entry
+	%indvar = phi i32 [ 0, %entry ], [ %indvar.next, %bb10 ]		; <i32> [#uses=2]
+	%tmp.0.reg2mem.0.rec = mul i32 %indvar, -1		; <i32> [#uses=1]
+	%tmp12.rec = add i32 %tmp.0.reg2mem.0.rec, -1		; <i32> [#uses=1]
+	%tmp12 = getelementptr %struct.x* %tmp45, i32 %tmp12.rec		; <%struct.x*> [#uses=2]
+	%tmp16 = call i32 (i8*, ...)* @printf( i8* getelementptr ([12 x i8]* @.str1, i32 0, i32 0), %struct.x* %tmp12 ) nounwind 		; <i32> [#uses=0]
+	%tmp84 = icmp eq %struct.x* %tmp12, %orientations62		; <i1> [#uses=1]
+	%indvar.next = add i32 %indvar, 1		; <i32> [#uses=1]
+	br i1 %tmp84, label %bb17, label %bb10
+
+bb17:		; preds = %bb10
+	ret i32 0
+}
+
+declare i32 @puts(i8*)
+
+declare i32 @printf(i8*, ...)





More information about the llvm-commits mailing list