[llvm] 8f0466e - [DSE] Unify & fix mem terminator location checks.

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2020-09-26T13:47:50+01:00
New Revision: 8f0466edc0cb3782af4108bfe8840e2ad3de9b49

URL: https://github.com/llvm/llvm-project/commit/8f0466edc0cb3782af4108bfe8840e2ad3de9b49
DIFF: https://github.com/llvm/llvm-project/commit/8f0466edc0cb3782af4108bfe8840e2ad3de9b49.diff

LOG: [DSE] Unify & fix mem terminator location checks.

When looking for memory defs killed by memory terminators the code
currently incorrectly ignores the size argument of llvm.lifetime.end.

This patch updates the code to use isMemTerminator and updates
isMemTerminator to use isOverwrite() to make sure locations that are
outside the range marked as dead by llvm.lifetime.end are not
considered. Note that isOverwrite is only used for llvm.lifetime.end,
because free-like functions make the whole underlying object dead.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
    llvm/test/Transforms/DeadStoreElimination/MSSA/lifetime.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index 6615f6b1c32e..42fa2d72560f 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -1824,9 +1824,10 @@ struct DSEState {
            isFreeCall(I, &TLI);
   }
 
-  /// Returns true if \p MaybeTerm is a memory terminator for the same
-  /// underlying object as \p DefLoc.
-  bool isMemTerminator(MemoryLocation DefLoc, Instruction *MaybeTerm) {
+  /// Returns true if \p MaybeTerm is a memory terminator for \p Loc from
+  /// instruction \p AccessI.
+  bool isMemTerminator(MemoryLocation Loc, Instruction *AccessI,
+                       Instruction *MaybeTerm) {
     Optional<std::pair<MemoryLocation, bool>> MaybeTermLoc =
         getLocForTerminator(MaybeTerm);
 
@@ -1835,9 +1836,15 @@ struct DSEState {
 
     // If the terminator is a free-like call, all accesses to the underlying
     // object can be considered terminated.
-    if (MaybeTermLoc->second)
-      DefLoc = MemoryLocation(getUnderlyingObject(DefLoc.Ptr));
-    return BatchAA.isMustAlias(MaybeTermLoc->first, DefLoc);
+    if (getUnderlyingObject(Loc.Ptr) !=
+        getUnderlyingObject(MaybeTermLoc->first.Ptr))
+      return false;
+
+    int64_t InstWriteOffset, DepWriteOffset;
+    return MaybeTermLoc->second ||
+           isOverwrite(MaybeTerm, AccessI, MaybeTermLoc->first, Loc, DL, TLI,
+                       DepWriteOffset, InstWriteOffset, BatchAA,
+                       &F) == OW_Complete;
   }
 
   // Returns true if \p Use may read from \p DefLoc.
@@ -2011,8 +2018,7 @@ struct DSEState {
         // If the killing def is a memory terminator (e.g. lifetime.end), check
         // the next candidate if the current Current does not write the same
         // underlying object as the terminator.
-        const Value *NIUnd = getUnderlyingObject(CurrentLoc->Ptr);
-        if (DefUO != NIUnd) {
+        if (!isMemTerminator(*CurrentLoc, CurrentI, KillingI)) {
           StepAgain = true;
           Current = CurrentDef->getDefiningAccess();
         }
@@ -2134,7 +2140,7 @@ struct DSEState {
 
       // A memory terminator kills all preceeding MemoryDefs and all succeeding
       // MemoryAccesses. We do not have to check it's users.
-      if (isMemTerminator(DefLoc, UseInst))
+      if (isMemTerminator(DefLoc, KillingI, UseInst))
         continue;
 
       if (UseInst->mayThrow() && !isInvisibleToCallerBeforeRet(DefUO)) {

diff  --git a/llvm/test/Transforms/DeadStoreElimination/MSSA/lifetime.ll b/llvm/test/Transforms/DeadStoreElimination/MSSA/lifetime.ll
index 3ed68cc7227a..6184c89be5c4 100644
--- a/llvm/test/Transforms/DeadStoreElimination/MSSA/lifetime.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/MSSA/lifetime.ll
@@ -47,6 +47,8 @@ define void @test3_lifetime_end_partial() {
 ; CHECK-NEXT:    [[A_0:%.*]] = bitcast i32* [[A]] to i8*
 ; CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 2, i8* [[A_0]])
 ; CHECK-NEXT:    [[A_1:%.*]] = getelementptr i8, i8* [[A_0]], i64 1
+; CHECK-NEXT:    [[A_2:%.*]] = getelementptr i8, i8* [[A_0]], i64 2
+; CHECK-NEXT:    store i8 20, i8* [[A_2]], align 1
 ; CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 2, i8* [[A_0]])
 ; CHECK-NEXT:    call void @use(i8* [[A_1]])
 ; CHECK-NEXT:    ret void
@@ -126,6 +128,7 @@ define void @test5_lifetime_end_partial(i32* %A) {
 ; CHECK-NEXT:    call void @llvm.lifetime.start.p0i8(i64 2, i8* [[A_0]])
 ; CHECK-NEXT:    [[A_1:%.*]] = getelementptr i8, i8* [[A_0]], i64 1
 ; CHECK-NEXT:    [[A_2:%.*]] = getelementptr i8, i8* [[A_0]], i64 2
+; CHECK-NEXT:    store i8 20, i8* [[A_2]], align 1
 ; CHECK-NEXT:    call void @llvm.lifetime.end.p0i8(i64 2, i8* [[A_0]])
 ; CHECK-NEXT:    call void @use(i8* [[A_1]])
 ; CHECK-NEXT:    store i8 30, i8* [[A_1]], align 1