[llvm] Fix to generalize the canSkipClobberingStore (PR #174137)

via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 30 03:54:40 PDT 2026


https://github.com/Bhuvan1527 updated https://github.com/llvm/llvm-project/pull/174137

>From 50e9892558023d0413f08d5082edbc608b507db0 Mon Sep 17 00:00:00 2001
From: bhuvan1527 <balabhuvanvarma at gmail.com>
Date: Thu, 1 Jan 2026 15:24:34 +0530
Subject: [PATCH 1/3] Fix to generalize the canSkipClobberingStore

Current code of canSkipClobberingStore is only skipping the store, if its
value operand is an output of a load instruction. That is it is specific.

This pr is an attempt to generalize this condition and include if the operand is
a constant or a function argument.
---
 llvm/lib/Analysis/MemoryDependenceAnalysis.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 1ef762bcf7007..11a67928ff418 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -355,6 +355,11 @@ static bool canSkipClobberingStore(const StoreInst *SI,
       MemLoc.Size.getValue().getKnownMinValue())
     return false;
 
+  auto *StoredVal = SI->getValueOperand();
+  if (isa<Argument>(StoredVal)) {
+    return true;
+  }
+
   auto *LI = dyn_cast<LoadInst>(SI->getValueOperand());
   if (!LI || LI->getParent() != SI->getParent())
     return false;

>From cafb3598d19b37ed949ba4818a5132b8756db97e Mon Sep 17 00:00:00 2001
From: bhuvan1527 <balabhuvanvarma at gmail.com>
Date: Thu, 1 Jan 2026 15:24:34 +0530
Subject: [PATCH 2/3] Fix to generalize the canSkipClobberingStore

Current code of canSkipClobberingStore is only skipping the store, if its
value operand is an output of a load instruction. That is it is specific.

This pr is an attempt to generalize this condition and include if the operand is
a constant or a function argument.
---
 llvm/lib/Analysis/MemoryDependenceAnalysis.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 11a67928ff418..ab1d48c3bcf9a 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -357,6 +357,13 @@ static bool canSkipClobberingStore(const StoreInst *SI,
 
   auto *StoredVal = SI->getValueOperand();
   if (isa<Argument>(StoredVal)) {
+    auto *SIPtr = SI->getPointerOperand();
+    // If this store is the first one to write /read from this pointer
+    // then we can skip this store as well.
+    // i.e. skipping this store won't change the value stored at this
+    // memory location.
+    if (!SIPtr->hasOneUse())
+      return false;
     return true;
   }
 

>From bb12aeb00005e222ac308e01009b0ca23256be12 Mon Sep 17 00:00:00 2001
From: bhuvan1527 <balabhuvanvarma at gmail.com>
Date: Thu, 1 Jan 2026 15:24:34 +0530
Subject: [PATCH 3/3] Fix to generalize the canSkipClobberingStore

Current code of canSkipClobberingStore is only skipping the store, if its
value operand is an output of a load instruction. That is it is specific.

This pr is adding an additional check to canSkipClobberingStore. If the value
stored by the store is same as that of loaded, then we can skip this.

Same-value store bypass
---
 .../lib/Analysis/MemoryDependenceAnalysis.cpp | 76 +++++++++++--------
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index ab1d48c3bcf9a..399db7b069f9e 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -355,30 +355,49 @@ static bool canSkipClobberingStore(const StoreInst *SI,
       MemLoc.Size.getValue().getKnownMinValue())
     return false;
 
-  auto *StoredVal = SI->getValueOperand();
-  if (isa<Argument>(StoredVal)) {
-    auto *SIPtr = SI->getPointerOperand();
-    // If this store is the first one to write /read from this pointer
-    // then we can skip this store as well.
-    // i.e. skipping this store won't change the value stored at this
-    // memory location.
-    if (!SIPtr->hasOneUse())
-      return false;
-    return true;
+
+  if (auto *LI = dyn_cast<LoadInst>(SI->getValueOperand())) {
+    if (LI->getParent() == SI->getParent() &&
+        BatchAA.alias(MemoryLocation::get(LI), MemLoc) ==
+            AliasResult::MustAlias) {
+      unsigned NumVisitedInsts = 0;
+      bool Clean = true;
+      for (const Instruction *I = LI; I != SI; I = I->getNextNode()) {
+        if (++NumVisitedInsts > ScanLimit ||
+            isModSet(BatchAA.getModRefInfo(I, MemLoc))) {
+          Clean = false;
+          break;
+        }
+      }
+      if (Clean)
+        return true;
+    }
   }
 
-  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;
+  // Checking if the store is writing the same value that MemLoc contains now. 
+  // Then we can skip this store as well, because it does not effect the value loaded from MemLoc.
+  auto *StoredVal = SI->getValueOperand();
   unsigned NumVisitedInsts = 0;
-  for (const Instruction *I = LI; I != SI; I = I->getNextNode())
-    if (++NumVisitedInsts > ScanLimit ||
-        isModSet(BatchAA.getModRefInfo(I, MemLoc)))
-      return false;
+  for (auto It = std::next(SI->getReverseIterator()),
+            End = SI->getParent()->rend();
+       It != End; ++It) {
+    if (++NumVisitedInsts > ScanLimit)
+      break;
+    const Instruction *I = &*It;
+    if (!isModSet(BatchAA.getModRefInfo(I, MemLoc)))
+      continue;
+    // I is the most recent modifier of MemLoc before SI.
+    // If it's a must-alias store of the exact same SSA value, SI is a no-op.
+    if (const auto *PrevSI = dyn_cast<StoreInst>(I)) {
+      if (PrevSI->getValueOperand() == StoredVal &&
+          BatchAA.alias(MemoryLocation::get(PrevSI), MemLoc) ==
+              AliasResult::MustAlias)
+        return true;
+    }
+    break;
+  }
 
-  return true;
+  return false;
 }
 
 MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
@@ -386,8 +405,7 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
     BasicBlock *BB, Instruction *QueryInst, unsigned *Limit,
     BatchAAResults &BatchAA) {
   bool isInvariantLoad = false;
-  Align MemLocAlign =
-      MemLoc.Ptr->getPointerAlignment(BB->getDataLayout());
+  Align MemLocAlign = MemLoc.Ptr->getPointerAlignment(BB->getDataLayout());
 
   unsigned DefaultLimit = getDefaultBlockScanLimit();
   if (!Limit)
@@ -435,7 +453,7 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
   // True for volatile instruction.
   // For Load/Store return true if atomic ordering is stronger than AO,
   // for other instruction just true if it can read or write to memory.
-  auto isComplexForReordering = [](Instruction * I, AtomicOrdering AO)->bool {
+  auto isComplexForReordering = [](Instruction *I, AtomicOrdering AO) -> bool {
     if (I->isVolatile())
       return true;
     if (auto *LI = dyn_cast<LoadInst>(I))
@@ -469,7 +487,7 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
       case Intrinsic::masked_load:
       case Intrinsic::masked_store: {
         MemoryLocation Loc;
-        /*ModRefInfo MR =*/ GetLocation(II, Loc, TLI);
+        /*ModRefInfo MR =*/GetLocation(II, Loc, TLI);
         AliasResult R = BatchAA.alias(Loc, MemLoc);
         if (R == AliasResult::NoAlias)
           continue;
@@ -1448,7 +1466,6 @@ bool MemoryDependenceResults::getNonLocalPointerDepFromBB(
 
         I.setResult(MemDepResult::getUnknown());
 
-
         break;
       }
     }
@@ -1769,9 +1786,7 @@ MemoryDependenceWrapperPass::MemoryDependenceWrapperPass() : FunctionPass(ID) {}
 
 MemoryDependenceWrapperPass::~MemoryDependenceWrapperPass() = default;
 
-void MemoryDependenceWrapperPass::releaseMemory() {
-  MemDep.reset();
-}
+void MemoryDependenceWrapperPass::releaseMemory() { MemDep.reset(); }
 
 void MemoryDependenceWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesAll();
@@ -1781,8 +1796,9 @@ void MemoryDependenceWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.addRequiredTransitive<TargetLibraryInfoWrapperPass>();
 }
 
-bool MemoryDependenceResults::invalidate(Function &F, const PreservedAnalyses &PA,
-                               FunctionAnalysisManager::Invalidator &Inv) {
+bool MemoryDependenceResults::invalidate(
+    Function &F, const PreservedAnalyses &PA,
+    FunctionAnalysisManager::Invalidator &Inv) {
   // Check whether our analysis is preserved.
   auto PAC = PA.getChecker<MemoryDependenceAnalysis>();
   if (!PAC.preserved() && !PAC.preservedSet<AllAnalysesOn<Function>>())



More information about the llvm-commits mailing list