[llvm-commits] [llvm] r90007 - in /llvm/trunk: lib/Transforms/Scalar/ScalarReplAggregates.cpp test/Transforms/ScalarRepl/badarray.ll

[Chris Lattner]

Author: lattner
Date: Fri Nov 27 10:37:41 2009
New Revision: 90007

URL: http://llvm.org/viewvc/llvm-project?rev=90007&view=rev
Log:
fix PR5436 by making the 'simple' case of SRoA not promote out of range 
array indexes.  The "complex" case of SRoA still handles them, and correctly.

This fixes a weirdness where we'd correctly avoid transforming A[0][42] if
the 42 was too large, but we'd only do it if it was one gep, not two separate
ones.

Modified:
    llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp
    llvm/trunk/test/Transforms/ScalarRepl/badarray.ll

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

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp Fri Nov 27 10:37:41 2009
@@ -469,15 +469,41 @@
     case Instruction::GetElementPtr: {
       GetElementPtrInst *GEP = cast<GetElementPtrInst>(User);
       bool AreAllZeroIndices = isFirstElt;
-      if (GEP->getNumOperands() > 1) {
-        if (!isa<ConstantInt>(GEP->getOperand(1)) ||
-            !cast<ConstantInt>(GEP->getOperand(1))->isZero())
-          // Using pointer arithmetic to navigate the array.
-          return MarkUnsafe(Info);
-       
-        if (AreAllZeroIndices)
-          AreAllZeroIndices = GEP->hasAllZeroIndices();
+      if (GEP->getNumOperands() > 1 &&
+          (!isa<ConstantInt>(GEP->getOperand(1)) ||
+           !cast<ConstantInt>(GEP->getOperand(1))->isZero()))
+        // Using pointer arithmetic to navigate the array.
+        return MarkUnsafe(Info);
+      
+      // Verify that any array subscripts are in range.
+      for (gep_type_iterator GEPIt = gep_type_begin(GEP),
+           E = gep_type_end(GEP); GEPIt != E; ++GEPIt) {
+        // Ignore struct elements, no extra checking needed for these.
+        if (isa<StructType>(*GEPIt))
+          continue;
+
+        // This GEP indexes an array.  Verify that this is an in-range
+        // constant integer. Specifically, consider A[0][i]. We cannot know that
+        // the user isn't doing invalid things like allowing i to index an
+        // out-of-range subscript that accesses A[1].  Because of this, we have
+        // to reject SROA of any accesses into structs where any of the
+        // components are variables. 
+        ConstantInt *IdxVal = dyn_cast<ConstantInt>(GEPIt.getOperand());
+        if (!IdxVal) return MarkUnsafe(Info);
+        
+        // Are all indices still zero?
+        AreAllZeroIndices &= IdxVal->isZero();
+        
+        if (const ArrayType *AT = dyn_cast<ArrayType>(*GEPIt)) {
+          if (IdxVal->getZExtValue() >= AT->getNumElements())
+            return MarkUnsafe(Info);
+        } else if (const VectorType *VT = dyn_cast<VectorType>(*GEPIt)) {
+          if (IdxVal->getZExtValue() >= VT->getNumElements())
+            return MarkUnsafe(Info);
+        }
       }
+      
+      
       isSafeElementUse(GEP, AreAllZeroIndices, AI, Info);
       if (Info.isUnsafe) return;
       break;

Modified: llvm/trunk/test/Transforms/ScalarRepl/badarray.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ScalarRepl/badarray.ll?rev=90007&r1=90006&r2=90007&view=diff

==============================================================================
--- llvm/trunk/test/Transforms/ScalarRepl/badarray.ll (original)
+++ llvm/trunk/test/Transforms/ScalarRepl/badarray.ll Fri Nov 27 10:37:41 2009
@@ -1,10 +1,14 @@
 ; RUN: opt < %s -scalarrepl -S | FileCheck %s
 
+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:32:32-n8:16:32"
+target triple = "i386-pc-linux-gnu"
+
+
 ; PR3466
 ; Off end of array, don't transform.
 define i32 @test1() {
 ; CHECK: @test1
-; CHECK: %X = alloca
+; CHECK-NOT: = alloca
 	%X = alloca [4 x i32]
 	%Y = getelementptr [4 x i32]* %X, i64 0, i64 6		; <i32*> [#uses=2]
 	store i32 0, i32* %Y
@@ -17,9 +21,37 @@
 define i32 @test2() nounwind {
 entry:
 ; CHECK: @test2
-; CHECK: %yx2.i = alloca
+; CHECK-NOT: = alloca
         %yx2.i = alloca float, align 4          ; <float*> [#uses=1]            
         %yx26.i = bitcast float* %yx2.i to i64*         ; <i64*> [#uses=1]      
         %0 = load i64* %yx26.i, align 8         ; <i64> [#uses=0]               
         unreachable
 }
+
+%base = type { i32, [0 x i8] }
+%padded = type { %base, [1 x i32] }
+
+; PR5436
+define void @test3() {
+entry:
+; CHECK: @test3
+; CHECK-NOT: = alloca
+; CHECK: store i64
+  %var_1 = alloca %padded, align 8                ; <%padded*> [#uses=3]
+  %0 = getelementptr inbounds %padded* %var_1, i32 0, i32 0 ; <%base*> [#uses=2]
+  
+  %p2 = getelementptr inbounds %base* %0, i32 0, i32 1, i32 0 ; <i8*> [#uses=1]
+  store i8 72, i8* %p2, align 1
+  
+  ; 72 -> a[0].
+
+  %callret = call %padded *@test3f() ; <i32> [#uses=2]
+  %callretcast = bitcast %padded* %callret to i8*                     ; <i8*> [#uses=1]
+  %var_11 = bitcast %padded* %var_1 to i8*        ; <i8*> [#uses=1]
+  call void @llvm.memcpy.i32(i8* %callretcast, i8* %var_11, i32 8, i32 4)
+  ret void
+}
+
+declare void @llvm.memcpy.i32(i8* nocapture, i8* nocapture, i32, i32) nounwind
+
+declare %padded* @test3f()