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

Sergey Kachkov via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 10 03:19:20 PDT 2023


https://github.com/skachkov-sc updated https://github.com/llvm/llvm-project/pull/68322

>From 018da8eacdfa8cfa90209bed98bfc67cb4af2468 Mon Sep 17 00:00:00 2001
From: Sergey Kachkov <sergey.kachkov at syntacore.com>
Date: Thu, 5 Oct 2023 15:56:54 +0300
Subject: [PATCH 1/3] [GVN][NFC] Add pre-commit test

---
 .../Transforms/GVN/rle-clobbering-store.ll    | 132 ++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 llvm/test/Transforms/GVN/rle-clobbering-store.ll

diff --git a/llvm/test/Transforms/GVN/rle-clobbering-store.ll b/llvm/test/Transforms/GVN/rle-clobbering-store.ll
new file mode 100644
index 000000000000000..8e00b0c726ad5aa
--- /dev/null
+++ b/llvm/test/Transforms/GVN/rle-clobbering-store.ll
@@ -0,0 +1,132 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt < %s -passes=gvn -S | FileCheck %s
+
+define i1 @test_i1(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define i1 @test_i1(
+; CHECK-SAME: 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]]
+;
+entry:
+  %0 = load i1, ptr %a, align 1
+  store i1 %0, ptr %b, align 1
+  %1 = load i1, ptr %a, align 1
+  ret i1 %1
+}
+
+define i8 @test_i8(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define i8 @test_i8(
+; CHECK-SAME: 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]]
+;
+entry:
+  %0 = load i8, ptr %a, align 1
+  store i8 %0, ptr %b, align 1
+  %1 = load i8, ptr %a, align 1
+  ret i8 %1
+}
+
+define i32 @test_i32(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define i32 @test_i32(
+; CHECK-SAME: 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]]
+;
+entry:
+  %0 = load i32, ptr %a, align 4
+  store i32 %0, ptr %b, align 4
+  %1 = load i32, ptr %a, align 4
+  ret i32 %1
+}
+
+define float @test_float(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define float @test_float(
+; CHECK-SAME: 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]]
+;
+entry:
+  %0 = load float, ptr %a, align 4
+  store float %0, ptr %b, align 4
+  %1 = load float, ptr %a, align 4
+  ret float %1
+}
+
+define i32 @test_unaligned_store(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define i32 @test_unaligned_store(
+; CHECK-SAME: 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 2
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[A]], align 4
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+entry:
+  %0 = load i32, ptr %a, align 4
+  store i32 %0, ptr %b, align 2
+  %1 = load i32, ptr %a, align 4
+  ret i32 %1
+}
+
+define i32 @test_unaligned_loads(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define i32 @test_unaligned_loads(
+; CHECK-SAME: ptr [[A:%.*]], ptr [[B:%.*]], ptr [[C:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr [[A]], align 2
+; CHECK-NEXT:    store i32 [[TMP0]], ptr [[B]], align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[A]], align 2
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+entry:
+  %0 = load i32, ptr %a, align 2
+  store i32 %0, ptr %b, align 4
+  %1 = load i32, ptr %a, align 2
+  ret i32 %1
+}
+
+define i8 @test_modify_between(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define i8 @test_modify_between(
+; CHECK-SAME: ptr [[A:%.*]], ptr [[B:%.*]], ptr [[C:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i8, ptr [[A]], align 1
+; CHECK-NEXT:    store i8 42, ptr [[C]], 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]]
+;
+entry:
+  %0 = load i8, ptr %a, align 1
+  store i8 42, ptr %c, align 1
+  store i8 %0, ptr %b, align 1
+  %1 = load i8, ptr %a, align 1
+  ret i8 %1
+}
+
+define i32 @test_unordered(ptr %a, ptr %b, ptr %c) {
+; CHECK-LABEL: define i32 @test_unordered(
+; CHECK-SAME: 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 atomic i32, ptr [[A]] unordered, align 4
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+entry:
+  %0 = load i32, ptr %a, align 4
+  store i32 %0, ptr %b, align 4
+  %1 = load atomic i32, ptr %a unordered, align 4
+  ret i32 %1
+}

>From 6df3c7bd3c92297757abf257bf48135734d19112 Mon Sep 17 00:00:00 2001
From: Sergey Kachkov <sergey.kachkov at syntacore.com>
Date: Thu, 5 Oct 2023 16:26:28 +0300
Subject: [PATCH 2/3] [GVN] Drop Clobber dependency if store may overwrite only
 the same value

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.
---
 .../lib/Analysis/MemoryDependenceAnalysis.cpp | 46 +++++++++++++++++--
 .../Transforms/GVN/rle-clobbering-store.ll    | 12 ++---
 2 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 071ecdba8a54ace..faee899ec727404 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 auto *I = LI->getNextNode(); I != SI; I = I->getNextNode())
+    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,9 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
         return MemDepResult::getDef(Inst);
       if (isInvariantLoad)
         continue;
+      if (canSkipClobberingStore(SI, MemLoc, MemLocAlign, BatchAA,
+                                 getDefaultBlockScanLimit()))
+        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

>From 40161cd9d845c2d5a741b932684ebb3e54e069c9 Mon Sep 17 00:00:00 2001
From: Sergey Kachkov <sergey.kachkov at syntacore.com>
Date: Tue, 10 Oct 2023 13:16:15 +0300
Subject: [PATCH 3/3] Addressing review comments

---
 llvm/lib/Analysis/MemoryDependenceAnalysis.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index faee899ec727404..49a31a52cf0e850 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -382,7 +382,7 @@ static bool canSkipClobberingStore(const StoreInst *SI,
   if (BatchAA.alias(MemoryLocation::get(LI), MemLoc) != AliasResult::MustAlias)
     return false;
   unsigned NumVisitedInsts = 0;
-  for (const auto *I = LI->getNextNode(); I != SI; I = I->getNextNode())
+  for (const Instruction *I = LI; I != SI; I = I->getNextNonDebugInstruction())
     if (++NumVisitedInsts > ScanLimit ||
         isModSet(BatchAA.getModRefInfo(I, MemLoc)))
       return false;
@@ -610,8 +610,7 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
         return MemDepResult::getDef(Inst);
       if (isInvariantLoad)
         continue;
-      if (canSkipClobberingStore(SI, MemLoc, MemLocAlign, BatchAA,
-                                 getDefaultBlockScanLimit()))
+      if (canSkipClobberingStore(SI, MemLoc, MemLocAlign, BatchAA, *Limit))
         continue;
       return MemDepResult::getClobber(Inst);
     }



More information about the llvm-commits mailing list