[DSE] Check for whole object overwrite even if dead store size not known
authorNikita Popov <npopov@redhat.com>
Fri, 24 Dec 2021 11:09:38 +0000 (12:09 +0100)
committerNikita Popov <npopov@redhat.com>
Mon, 3 Jan 2022 08:36:44 +0000 (09:36 +0100)
If the killing store overwrites the whole object, we know that the
preceding store is dead, regardless of the accessed offset or size.
This case was previously only handled if the size of the dead store
was also known.

This allows us to perform conventional DSE for calls that write to
an argument (but without known size).

Differential Revision: https://reviews.llvm.org/D116267

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
llvm/test/Transforms/DeadStoreElimination/trivial-dse-calls.ll

index eadbb42..a8f7c4f 100644 (file)
@@ -835,6 +835,20 @@ struct DSEState {
     if (!isGuaranteedLoopIndependent(DeadI, KillingI, DeadLoc))
       return OW_Unknown;
 
+    const Value *DeadPtr = DeadLoc.Ptr->stripPointerCasts();
+    const Value *KillingPtr = KillingLoc.Ptr->stripPointerCasts();
+    const Value *DeadUndObj = getUnderlyingObject(DeadPtr);
+    const Value *KillingUndObj = getUnderlyingObject(KillingPtr);
+
+    // Check whether the killing store overwrites the whole object, in which
+    // case the size/offset of the dead store does not matter.
+    if (DeadUndObj == KillingUndObj && KillingLoc.Size.isPrecise()) {
+      uint64_t KillingUndObjSize = getPointerSize(KillingUndObj, DL, TLI, &F);
+      if (KillingUndObjSize != MemoryLocation::UnknownSize &&
+          KillingUndObjSize == KillingLoc.Size.getValue())
+        return OW_Complete;
+    }
+
     // FIXME: Vet that this works for size upper-bounds. Seems unlikely that we'll
     // get imprecise values here, though (except for unknown sizes).
     if (!KillingLoc.Size.isPrecise() || !DeadLoc.Size.isPrecise()) {
@@ -875,14 +889,6 @@ struct DSEState {
         return OW_Complete;
     }
 
-    // Check to see if the killing store is to the entire object (either a
-    // global, an alloca, or a byval/inalloca argument).  If so, then it clearly
-    // overwrites any other store to the same object.
-    const Value *DeadPtr = DeadLoc.Ptr->stripPointerCasts();
-    const Value *KillingPtr = KillingLoc.Ptr->stripPointerCasts();
-    const Value *DeadUndObj = getUnderlyingObject(DeadPtr);
-    const Value *KillingUndObj = getUnderlyingObject(KillingPtr);
-
     // If we can't resolve the same pointers to the same object, then we can't
     // analyze them at all.
     if (DeadUndObj != KillingUndObj) {
@@ -896,12 +902,6 @@ struct DSEState {
       return OW_Unknown;
     }
 
-    // If the KillingI store is to a recognizable object, get its size.
-    uint64_t KillingUndObjSize = getPointerSize(KillingUndObj, DL, TLI, &F);
-    if (KillingUndObjSize != MemoryLocation::UnknownSize)
-      if (KillingUndObjSize == KillingSize && KillingUndObjSize >= DeadSize)
-        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.
index 2b3838b..6c4db33 100644 (file)
@@ -252,14 +252,12 @@ define void @test_self_read() {
   ret void
 }
 
-; TODO: We should be able to remove the call because while we don't know
-; the size of the write done by the call, we do know the following store
-; writes to the entire contents of the alloca.
+; We can remove the call because while we don't know the size of the write done
+; by the call, we do know the following store writes to the entire contents of
+; the alloca.
 define i32 @test_dse_overwrite() {
 ; CHECK-LABEL: @test_dse_overwrite(
 ; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
-; CHECK-NEXT:    [[BITCAST:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT:    call void @f(i8* nocapture writeonly [[BITCAST]]) #[[ATTR1]]
 ; CHECK-NEXT:    store i32 0, i32* [[A]], align 4
 ; CHECK-NEXT:    [[V:%.*]] = load i32, i32* [[A]], align 4
 ; CHECK-NEXT:    ret i32 [[V]]