[PATCH] Fix Bug in SROA transformation [PR18615]

[Karthik Bhat]

  Hi Rafael,
  Thanks for the reduced test case. I have used the same. But i think it is important to run both with "-early-cse -instcombine -sroa" and  "-early-cse -sroa" as the crash logs are different in both cases.
  In the 1st case i.e.  "-early-cse -instcombine -sroa"   the code assets with "Assertion `begin() + idx < end()' failed."
  and in 2nd case i.e. "-early-cse -sroa" the code assets with  "Assertion `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"

  Both the above crashes are due the same reason that in case memtransfer fails we should not insert the instruction into MemTransferSliceMap but return instead.

  Thanks once again for spending your time on this patch. Please let me know if you have any more inputs or if this looks good for commit.

  Regards
  Karthik Bhat

Hi chandlerc, rafael,

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

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

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

Index: test/Transforms/SROA/pr18615.ll
===================================================================
--- test/Transforms/SROA/pr18615.ll
+++ test/Transforms/SROA/pr18615.ll
@@ -0,0 +1,19 @@
+; RUN: opt < %s -early-cse -instcombine -sroa -S
+; RUN: opt < %s -early-cse -sroa -S
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+%struct.S0 = type { i32, i32, i32 }
+
+define void @fn1() {
+entry:
+  %b = alloca i32, align 4
+  %f = alloca [1 x %struct.S0], align 4
+  %arrayidx = getelementptr inbounds [1 x %struct.S0]* %f, i32 0, i64 0
+  %arrayidx1 = getelementptr inbounds [1 x %struct.S0]* %f, i32 0, i64 -1
+  %tmp1 = bitcast %struct.S0* %arrayidx to i8*
+  %tmp2 = bitcast %struct.S0* %arrayidx1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %tmp1, i8* %tmp2, i64 12, i32 4, i1 false)
+  ret void
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)
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();
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2759.6.patch
Type: text/x-patch
Size: 1906 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140223/e2775cf8/attachment.bin>