[llvm] 9f12f65 - [GVN] Drop Clobber dependency if store may overwrite only the same value (#68322)

[via llvm-commits]

Author: Sergey Kachkov
Date: 2023-10-10T19:22:00+03:00
New Revision: 9f12f655c44faf47e5c42c016b543cc431b66af8

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

LOG: [GVN] Drop Clobber dependency if store may overwrite only the same value (#68322)

In some cases clobbering store can be safely skipped if it can only must
or no alias with memory location and it writes the same value. This
patch supports simple case when the value from memory location was
loaded in the same basic block before the store and there are no
modifications between them.

Added: 
    

Modified: 
    llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
    llvm/test/Transforms/GVN/rle-clobbering-store.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 071ecdba8a54ace..49a31a52cf0e850 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -360,11 +360,43 @@ MemoryDependenceResults::getInvariantGroupPointerDependency(LoadInst *LI,
   return MemDepResult::getNonLocal();
 }
 
+// Check if SI that may alias with MemLoc can be safely skipped. This is
+// possible in case if SI can only must alias or no alias with MemLoc (no
+// partial overlapping possible) and it writes the same value that MemLoc
+// contains now (it was loaded before this store and was not modified in
+// between).
+static bool canSkipClobberingStore(const StoreInst *SI,
+                                   const MemoryLocation &MemLoc,
+                                   Align MemLocAlign, BatchAAResults &BatchAA,
+                                   unsigned ScanLimit) {
+  if (!MemLoc.Size.hasValue())
+    return false;
+  if (MemoryLocation::get(SI).Size != MemLoc.Size)
+    return false;
+  if (std::min(MemLocAlign, SI->getAlign()).value() < MemLoc.Size.getValue())
+    return false;
+
+  auto *LI = dyn_cast<LoadInst>(SI->getValueOperand());
+  if (!LI || LI->getParent() != SI->getParent())
+    return false;
+  if (BatchAA.alias(MemoryLocation::get(LI), MemLoc) != AliasResult::MustAlias)
+    return false;
+  unsigned NumVisitedInsts = 0;
+  for (const Instruction *I = LI; I != SI; I = I->getNextNonDebugInstruction())
+    if (++NumVisitedInsts > ScanLimit ||
+        isModSet(BatchAA.getModRefInfo(I, MemLoc)))
+      return false;
+
+  return true;
+}
+
 MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
     const MemoryLocation &MemLoc, bool isLoad, BasicBlock::iterator ScanIt,
     BasicBlock *BB, Instruction *QueryInst, unsigned *Limit,
     BatchAAResults &BatchAA) {
   bool isInvariantLoad = false;
+  Align MemLocAlign =
+      MemLoc.Ptr->getPointerAlignment(BB->getModule()->getDataLayout());
 
   unsigned DefaultLimit = getDefaultBlockScanLimit();
   if (!Limit)
@@ -402,11 +434,12 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
   // do want to respect mustalias results since defs are useful for value
   // forwarding, but any mayalias write can be assumed to be noalias.
   // Arguably, this logic should be pushed inside AliasAnalysis itself.
-  if (isLoad && QueryInst) {
-    LoadInst *LI = dyn_cast<LoadInst>(QueryInst);
-    if (LI && LI->hasMetadata(LLVMContext::MD_invariant_load))
-      isInvariantLoad = true;
-  }
+  if (isLoad && QueryInst)
+    if (LoadInst *LI = dyn_cast<LoadInst>(QueryInst)) {
+      if (LI->hasMetadata(LLVMContext::MD_invariant_load))
+        isInvariantLoad = true;
+      MemLocAlign = LI->getAlign();
+    }
 
   // True for volatile instruction.
   // For Load/Store return true if atomic ordering is stronger than AO,
@@ -577,6 +610,8 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
         return MemDepResult::getDef(Inst);
       if (isInvariantLoad)
         continue;
+      if (canSkipClobberingStore(SI, MemLoc, MemLocAlign, BatchAA, *Limit))
+        continue;
       return MemDepResult::getClobber(Inst);
     }
 

diff  --git a/llvm/test/Transforms/GVN/rle-clobbering-store.ll b/llvm/test/Transforms/GVN/rle-clobbering-store.ll
index 8e00b0c726ad5aa..b60f6c323d42d56 100644
--- a/llvm/test/Transforms/GVN/rle-clobbering-store.ll
+++ b/llvm/test/Transforms/GVN/rle-clobbering-store.ll
@@ -7,8 +7,7 @@ define i1 @test_i1(ptr %a, ptr %b, ptr %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i1, ptr [[A]], align 1
 ; CHECK-NEXT:    store i1 [[TMP0]], ptr [[B]], align 1
-; CHECK-NEXT:    [[TMP1:%.*]] = load i1, ptr [[A]], align 1
-; CHECK-NEXT:    ret i1 [[TMP1]]
+; CHECK-NEXT:    ret i1 [[TMP0]]
 ;
 entry:
   %0 = load i1, ptr %a, align 1
@@ -23,8 +22,7 @@ define i8 @test_i8(ptr %a, ptr %b, ptr %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i8, ptr [[A]], align 1
 ; CHECK-NEXT:    store i8 [[TMP0]], ptr [[B]], align 1
-; CHECK-NEXT:    [[TMP1:%.*]] = load i8, ptr [[A]], align 1
-; CHECK-NEXT:    ret i8 [[TMP1]]
+; CHECK-NEXT:    ret i8 [[TMP0]]
 ;
 entry:
   %0 = load i8, ptr %a, align 1
@@ -39,8 +37,7 @@ define i32 @test_i32(ptr %a, ptr %b, ptr %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 4
 ; CHECK-NEXT:    store i32 [[TMP0]], ptr [[B]], align 4
-; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[A]], align 4
-; CHECK-NEXT:    ret i32 [[TMP1]]
+; CHECK-NEXT:    ret i32 [[TMP0]]
 ;
 entry:
   %0 = load i32, ptr %a, align 4
@@ -55,8 +52,7 @@ define float @test_float(ptr %a, ptr %b, ptr %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load float, ptr [[A]], align 4
 ; CHECK-NEXT:    store float [[TMP0]], ptr [[B]], align 4
-; CHECK-NEXT:    [[TMP1:%.*]] = load float, ptr [[A]], align 4
-; CHECK-NEXT:    ret float [[TMP1]]
+; CHECK-NEXT:    ret float [[TMP0]]
 ;
 entry:
   %0 = load float, ptr %a, align 4