[PATCH] D132657: [DSE] Eliminate noop store even through has clobbering between LoadI and StoreI

[luxufan via Phabricator via llvm-commits]

StephenFan created this revision.
StephenFan added reviewers: fhahn, yurai007.
Herald added a subscriber: hiraditya.
Herald added a project: All.
StephenFan requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

For noop store of the form of LoadI and StoreI,
An invariant should be kept is that the memory state of the related
MemoryLoc before LoadI is the same as before StoreI.
For this example:

  define void @pr49927(i32* %q, i32* %p) {
    %v = load i32, i32* %p, align 4
    store i32 %v, i32* %q, align 4
    store i32 %v, i32* %p, align 4
    ret void
  }

Here the definition of the store's destination is different with the
definition of the load's destination, which it seems that the
invariant mentioned above is broken. But the definition of the
store's destination would write a value that is LoadI, actually, the
invariant is still kept. So skip check in this case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132657

Files:
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
  llvm/test/Transforms/DeadStoreElimination/stores-of-existing-values.ll


Index: llvm/test/Transforms/DeadStoreElimination/stores-of-existing-values.ll
===================================================================
--- llvm/test/Transforms/DeadStoreElimination/stores-of-existing-values.ll
+++ llvm/test/Transforms/DeadStoreElimination/stores-of-existing-values.ll
@@ -612,12 +612,10 @@
 ; CHECK-LABEL: @pr49927(
 ; CHECK-NEXT:    [[V:%.*]] = load i32, i32* [[P:%.*]], align 4
 ; CHECK-NEXT:    store i32 [[V]], i32* [[Q:%.*]], align 4
-; CHECK-NEXT:    store i32 [[V]], i32* [[P]], align 4
 ; CHECK-NEXT:    ret void
 ;
   %v = load i32, i32* %p, align 4
   store i32 %v, i32* %q, align 4
-  ; FIXME: this store can be eliminated
   store i32 %v, i32* %p, align 4
   ret void
 }
Index: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -1861,8 +1861,23 @@
           // We are searching for the definition of the store's destination.
           // So, if that is the same definition as the load, then this is a
           // noop. Otherwise, fail.
-          if (LoadAccess != Current)
+          if (LoadAccess != Current) {
+            auto *CurrentStoreI =
+                dyn_cast<StoreInst>(cast<MemoryDef>(Current)->getMemoryInst());
+            if (CurrentStoreI && CurrentStoreI->getOperand(0) == LoadI) {
+              // For noop store of the form of LoadI and StoreI,
+              // An invariant should be kept is that the memory state of the
+              // related MemoryLoc before LoadI is the same as before StoreI.
+              // Here the definition of the store's destination is different
+              // with the definition of the load's destination, which it seems
+              // that the invariant mentioned above is broken. But the
+              // definition of the store's destination would write a value that
+              // is LoadI, actually, the invariant is still kept. So skip in
+              // this case.
+              continue;
+            }
             return false;
+          }
         }
         return true;
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132657.455574.patch
Type: text/x-patch
Size: 2197 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220825/b8acb25b/attachment.bin>