[llvm] ae64c5a - [DSE][MemLoc] Handle intrinsics more generically

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2021-12-24T09:29:57+01:00
New Revision: ae64c5a0fde55ca53d0c9949562a506e784da66c

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

LOG: [DSE][MemLoc] Handle intrinsics more generically

Remove the special casing for intrinsics in MemoryLocation::getForDest()
and handle them through the general attribute based code. On the DSE
side, this means that isRemovable() now needs to handle more than a
hardcoded list of intrinsics. We consider everything apart from
volatile memory intrinsics and lifetime markers to be removable.

This allows us to perform DSE on intrinsics that DSE has not been
specially taught about, using a matrix store as an example here.

There is an interesting test change for invariant.start, but I
believe that optimization is correct. It only looks a bit odd
because the code is immediate UB anyway.

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

Added: 
    

Modified: 
    llvm/lib/Analysis/MemoryLocation.cpp
    llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
    llvm/test/Transforms/DeadStoreElimination/invariant.start.ll
    llvm/test/Transforms/DeadStoreElimination/simple.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/MemoryLocation.cpp b/llvm/lib/Analysis/MemoryLocation.cpp
index 120f4cda1e351..a877b19df866f 100644
--- a/llvm/lib/Analysis/MemoryLocation.cpp
+++ b/llvm/lib/Analysis/MemoryLocation.cpp
@@ -120,20 +120,6 @@ MemoryLocation MemoryLocation::getForDest(const AnyMemIntrinsic *MI) {
 
 Optional<MemoryLocation>
 MemoryLocation::getForDest(const CallBase *CB, const TargetLibraryInfo &TLI) {
-  if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(CB)) {
-    if (auto *MemInst = dyn_cast<AnyMemIntrinsic>(CB))
-      return getForDest(MemInst);
-
-    switch (II->getIntrinsicID()) {
-    default:
-      return None;
-    case Intrinsic::init_trampoline:
-      return MemoryLocation::getForArgument(CB, 0, TLI);
-    case Intrinsic::masked_store:
-      return MemoryLocation::getForArgument(CB, 1, TLI);
-    }
-  }
-
   if (!CB->onlyAccessesArgMemory())
     return None;
 

diff  --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index 4c17e2c0bdbb2..683ac537bbe42 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -182,33 +182,19 @@ static bool isRemovable(Instruction *I) {
   if (StoreInst *SI = dyn_cast<StoreInst>(I))
     return SI->isUnordered();
 
-  if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
-    switch (II->getIntrinsicID()) {
-    default: llvm_unreachable("Does not have LocForWrite");
-    case Intrinsic::lifetime_end:
-      // Never remove dead lifetime_end's, e.g. because it is followed by a
-      // free.
+  // Note: only get here for calls with analyzable writes.
+  if (auto *CB = dyn_cast<CallBase>(I)) {
+    // Don't remove volatile memory intrinsics.
+    if (auto *MI = dyn_cast<MemIntrinsic>(CB))
+      return !MI->isVolatile();
+
+    // Never remove dead lifetime intrinsics, e.g. because they are followed by
+    // a free.
+    if (CB->isLifetimeStartOrEnd())
       return false;
-    case Intrinsic::init_trampoline:
-      // Always safe to remove init_trampoline.
-      return true;
-    case Intrinsic::memset:
-    case Intrinsic::memmove:
-    case Intrinsic::memcpy:
-    case Intrinsic::memcpy_inline:
-      // Don't remove volatile memory intrinsics.
-      return !cast<MemIntrinsic>(II)->isVolatile();
-    case Intrinsic::memcpy_element_unordered_atomic:
-    case Intrinsic::memmove_element_unordered_atomic:
-    case Intrinsic::memset_element_unordered_atomic:
-    case Intrinsic::masked_store:
-      return true;
-    }
-  }
 
-  // note: only get here for calls with analyzable writes.
-  if (auto *CB = dyn_cast<CallBase>(I))
     return CB->use_empty() && CB->willReturn() && CB->doesNotThrow();
+  }
 
   return false;
 }

diff  --git a/llvm/test/Transforms/DeadStoreElimination/invariant.start.ll b/llvm/test/Transforms/DeadStoreElimination/invariant.start.ll
index f00001f967e6a..45c19c3588b03 100644
--- a/llvm/test/Transforms/DeadStoreElimination/invariant.start.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/invariant.start.ll
@@ -4,13 +4,13 @@
 
 declare {}* @llvm.invariant.start.p0i8(i64, i8* nocapture) nounwind readonly
 
-; We cannot remove the store 1 to %p.
-; FIXME: By the semantics of invariant.start, the store 3 to p is unreachable.
+; We could remove either store here. The first store is dead in the
+; conventional sense, because there is a later killing store. The second store
+; is undefined behavior by the semantics of invariant.start, and as such
+; unreachable.
 define void @test(i8 *%p) {
 ; CHECK-LABEL: @test(
-; CHECK-NEXT:    store i8 1, i8* [[P:%.*]], align 4
-; CHECK-NEXT:    [[I:%.*]] = call {}* @llvm.invariant.start.p0i8(i64 1, i8* [[P]])
-; CHECK-NEXT:    store i8 3, i8* [[P]], align 4
+; CHECK-NEXT:    store i8 3, i8* [[P:%.*]], align 4
 ; CHECK-NEXT:    ret void
 ;
   store i8 1, i8* %p, align 4

diff  --git a/llvm/test/Transforms/DeadStoreElimination/simple.ll b/llvm/test/Transforms/DeadStoreElimination/simple.ll
index eed7d535857d3..cee2b076e8b5a 100644
--- a/llvm/test/Transforms/DeadStoreElimination/simple.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/simple.ll
@@ -194,12 +194,9 @@ define void @test11() {
   ret void
 }
 
-; TODO: Specialized store intrinsics should be removed if dead.
+; Specialized store intrinsics should be removed if dead.
 define void @test_matrix_store(i64 %stride) {
 ; CHECK-LABEL: @test_matrix_store(
-; CHECK-NEXT:    [[A:%.*]] = alloca [6 x float], align 4
-; CHECK-NEXT:    [[CAST:%.*]] = bitcast [6 x float]* [[A]] to float*
-; CHECK-NEXT:    call void @llvm.matrix.column.major.store.v6f32.i64(<6 x float> zeroinitializer, float* [[CAST]], i64 [[STRIDE:%.*]], i1 false, i32 3, i32 2)
 ; CHECK-NEXT:    ret void
 ;
   %a = alloca [6 x float]