[PATCH] D21859: [DSE] Fix bug in partial overwrite tracking

[Jun Bum Lim via llvm-commits]

junbuml created this revision.
junbuml added reviewers: hfinkel, mcrosier, eeckstein.
junbuml added a subscriber: llvm-commits.
Herald added a subscriber: mcrosier.

Found cases where DSE incorrectly add partially-overwritten intervals.
Please see the test case for details.

http://reviews.llvm.org/D21859

Files:
  lib/Transforms/Scalar/DeadStoreElimination.cpp
  test/Transforms/DeadStoreElimination/combined-partial-overwrites.ll

Index: test/Transforms/DeadStoreElimination/combined-partial-overwrites.ll
===================================================================
--- test/Transforms/DeadStoreElimination/combined-partial-overwrites.ll
+++ test/Transforms/DeadStoreElimination/combined-partial-overwrites.ll
@@ -198,3 +198,42 @@
   ret i8 0
 }
 
+define signext i8 @test6(i32 *%ptr) {
+entry:
+; CHECK-LABEL: @test6
+
+  store i32 0, i32* %ptr
+
+  %bptr = bitcast i32* %ptr to i16*
+  %bptr1 = getelementptr inbounds i16, i16* %bptr, i64 0
+  %bptr2 = getelementptr inbounds i16, i16* %bptr, i64 1
+
+  store i16 1456, i16* %bptr2, align 1
+  store i16 65535, i16* %bptr1, align 1
+
+; CHECK-NOT: store i32 0, i32* %ptr
+
+  ret i8 0
+}
+
+define signext i8 @test7(i64 *%ptr) {
+entry:
+; CHECK-LABEL: @test7
+
+  store i64 0, i64* %ptr
+
+  %bptr = bitcast i64* %ptr to i16*
+  %bptr1 = getelementptr inbounds i16, i16* %bptr, i64 0
+  %bptr2 = getelementptr inbounds i16, i16* %bptr, i64 1
+  %bptr3 = getelementptr inbounds i16, i16* %bptr, i64 2
+  %bptr4 = getelementptr inbounds i16, i16* %bptr, i64 3
+
+  store i16 1346, i16* %bptr1, align 1
+  store i16 1756, i16* %bptr3, align 1
+  store i16 1456, i16* %bptr2, align 1
+  store i16 5656, i16* %bptr4, align 1
+
+; CHECK-NOT: store i64 0, i64* %ptr
+
+  ret i8 0
+}
Index: lib/Transforms/Scalar/DeadStoreElimination.cpp
===================================================================
--- lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -385,18 +385,25 @@
     // Find any intervals ending at, or after, LaterIntStart which start
     // before LaterIntEnd.
     auto ILI = IM.lower_bound(LaterIntStart);
-    if (ILI != IM.end() && ILI->second < LaterIntEnd) {
-      // This existing interval ends in the middle of
-      // [LaterIntStart, LaterIntEnd), erase it adjusting our start.
+    if (ILI != IM.end() && ILI->second <= LaterIntEnd) {
+      // This existing interval is overlapped with the current store somewhere
+      // in [LaterIntStart, LaterIntEnd]. Merge them by erasing the existing
+      // intervals and adjusting our start and end.
       LaterIntStart = std::min(LaterIntStart, ILI->second);
       LaterIntEnd = std::max(LaterIntEnd, ILI->first);
       ILI = IM.erase(ILI);
 
-      while (ILI != IM.end() && ILI->first <= LaterIntEnd)
-        ILI = IM.erase(ILI);
-
-      if (ILI != IM.end() && ILI->second < LaterIntEnd)
+      // Continue erasing and adjusting our end in case other previous
+      // intervals are also overlapped with the current store.
+      //
+      // |--- ealier 1 ---|  |--- ealier 2 ---|
+      //     |------- later---------|
+      //
+      while (ILI != IM.end() && (ILI->second<= LaterIntEnd)) {
+        assert(ILI->second > LaterIntStart && "Unexpected interval");
         LaterIntEnd = std::max(LaterIntEnd, ILI->first);
+        ILI = IM.erase(ILI);
+      }
     }
 
     IM[LaterIntEnd] = LaterIntStart;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D21859.62273.patch
Type: text/x-patch
Size: 2971 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160629/b8480739/attachment.bin>