[PATCH] Fix Bug in SROA transformation [PR18615]

[Karthik Bhat]

  Hi Rafael,Chandler,
  I was revisiting the patch and looking into SROA in more detail. I think the check was in incorrect position.
  If we have a out-of-bounds access in memtransfer we should never add the instruction into MemTransferSliceMap and also remove any instruction on the other side of the transfer if already added to MemTransferSliceMap.
  The problem in the current code seems to be that in visitMemTransferInst while checking for out of bounds we are just checking the upper bound and not the case were offset is negative.
  Added code to check the same. Please let me know your inputs on the same as i'm a bit new to optimization code.
  Thanks
  Karthik Bhat

Hi chandlerc, rafael,

http://llvm-reviews.chandlerc.com/D2759

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2759?vs=7237&id=7243#toc

Files:
  lib/Transforms/Scalar/SROA.cpp
  test/Transforms/SROA/pr18615.ll

Index: lib/Transforms/Scalar/SROA.cpp
===================================================================
--- lib/Transforms/Scalar/SROA.cpp
+++ lib/Transforms/Scalar/SROA.cpp
@@ -473,12 +473,13 @@
     if (!IsOffsetKnown)
       return PI.setAborted(&II);
 
-    // This side of the transfer is completely out-of-bounds, and so we can
+    // This side of the transfer is completely out-of-bounds
+    // (i.e. Offest < 0 or Offset >= AllocSize), and so we can
     // nuke the entire transfer. However, we also need to nuke the other side
     // if already added to our partitions.
     // FIXME: Yet another place we really should bypass this when
     // instrumenting for ASan.
-    if (!Offset.isNegative() && Offset.uge(AllocSize)) {
+    if (Offset.isNegative() || Offset.uge(AllocSize)) {
       SmallDenseMap<Instruction *, unsigned>::iterator MTPI = MemTransferSliceMap.find(&II);
       if (MTPI != MemTransferSliceMap.end())
         S.Slices[MTPI->second].kill();
Index: test/Transforms/SROA/pr18615.ll
===================================================================
--- test/Transforms/SROA/pr18615.ll
+++ test/Transforms/SROA/pr18615.ll
@@ -0,0 +1,38 @@
+; RUN: opt < %s -early-cse -instcombine -sroa -S
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+
+%struct.S0 = type { i32, i32, i32 }
+
+ at a = common global i32 0, align 4
+
+; Function Attrs: nounwind uwtable
+define void @fn1() #0 {
+entry:
+  %b = alloca i32, align 4
+  %f = alloca [1 x %struct.S0], align 4
+  store i32 -1, i32* %b, align 4
+  %0 = load i32* @a, align 4
+  %tobool = icmp ne i32 %0, 0
+  br i1 %tobool, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  %arrayidx = getelementptr inbounds [1 x %struct.S0]* %f, i32 0, i64 0
+  %1 = load i32* %b, align 4
+  %idxprom = sext i32 %1 to i64
+  %arrayidx1 = getelementptr inbounds [1 x %struct.S0]* %f, i32 0, i64 %idxprom
+  %2 = bitcast %struct.S0* %arrayidx to i8*
+  %3 = bitcast %struct.S0* %arrayidx1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %2, i8* %3, i64 12, i32 4, i1 false)
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  ret void
+}
+
+; Function Attrs: nounwind
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1) #1
+
+attributes #0 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind }
+
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2759.5.patch
Type: text/x-patch
Size: 2649 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140220/6f5a3ee5/attachment.bin>