[PATCH] D92045: [DSE] Consider out-of-bound writes in isOverwrite.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: asbirlea, nikic, efriedma, george.burgess.iv.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.
fhahn requested review of this revision.

With cross-bb DSE we need to be more careful when it comes to making
sure the writes are inside the underlying object.

If both writes are in the same basic block, there is no need to check
 if the writes are fully inside the underlying object, because the
out-of-bounds access is guaranteed to execute. Note that the case when
the block is exited early due to unwinding is already handled separately
by DSE, by considering the unwinding calls as clobbers. If the writes
are not in the same block, make sure they are inside the underlying object.

Fixes PR48279.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92045

Files:
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
  llvm/test/Transforms/DeadStoreElimination/MSSA/out-of-bounds-stores.ll


Index: llvm/test/Transforms/DeadStoreElimination/MSSA/out-of-bounds-stores.ll
===================================================================
--- llvm/test/Transforms/DeadStoreElimination/MSSA/out-of-bounds-stores.ll
+++ llvm/test/Transforms/DeadStoreElimination/MSSA/out-of-bounds-stores.ll
@@ -36,6 +36,8 @@
 ; CHECK-NEXT:    [[D:%.*]] = alloca [1 x i32], align 4
 ; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
 ; CHECK:       for.body:
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds [1 x i32], [1 x i32]* [[D]], i64 0, i64 0
+; CHECK-NEXT:    store i32 10, i32* [[ARRAYIDX]], align 4
 ; CHECK-NEXT:    br label [[FOR_INC:%.*]]
 ; CHECK:       for.inc:
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[FOR_BODY_1:%.*]], label [[FOR_END:%.*]]
Index: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -483,12 +483,6 @@
   if (UO1 != UO2)
     return OW_Unknown;
 
-  // If the "Later" store is to a recognizable object, get its size.
-  uint64_t ObjectSize = getPointerSize(UO2, DL, TLI, F);
-  if (ObjectSize != MemoryLocation::UnknownSize)
-    if (ObjectSize == LaterSize && ObjectSize >= EarlierSize)
-      return OW_Complete;
-
   // Okay, we have stores to two completely different pointers.  Try to
   // decompose the pointer into a "base + constant_offset" form.  If the base
   // pointers are equal, then we can reason about the two stores.
@@ -497,6 +491,22 @@
   const Value *BP1 = GetPointerBaseWithConstantOffset(P1, EarlierOff, DL);
   const Value *BP2 = GetPointerBaseWithConstantOffset(P2, LaterOff, DL);
 
+  // If the "Later" store is to a recognizable object, get its size.
+  uint64_t ObjectSize = getPointerSize(UO2, DL, TLI, F);
+  if (ObjectSize != MemoryLocation::UnknownSize)
+    if (ObjectSize == LaterSize && ObjectSize >= EarlierSize)
+      // If both writes are in the same basic block, there is no need to check
+      // if the writes are fully inside the underlying object, because the
+      // out-of-bounds access is guaranteed to execute. Note that the case when
+      // the block is exited early due to unwinding is already handled
+      // separately by DSE, by considering the unwinding calls as clobbers. If
+      // the writes are not in the same block, make sure they are inside the
+      // underlying object.
+      if (EarlierI->getParent() == LaterI->getParent() ||
+          (EarlierOff + EarlierSize <= ObjectSize &&
+           LaterOff + LaterSize <= ObjectSize))
+        return OW_Complete;
+
   // If the base pointers still differ, we have two completely different stores.
   if (BP1 != BP2)
     return OW_Unknown;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92045.307406.patch
Type: text/x-patch
Size: 2774 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201124/d050e127/attachment.bin>