[llvm] perf/goldsteinn/memdep invalid instead of unknown (PR #125741)

via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 4 10:53:53 PST 2025


https://github.com/goldsteinn created https://github.com/llvm/llvm-project/pull/125741

- **[GVN] Add tests for recovering analysis of a BB after failure; NFC**
- **[MemDep] Mark deps as `Dirty` on translation failure as opposed to `Unknown`**


>From 223989cc09dab8b524649d67f0521659fd15cfec Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Tue, 4 Feb 2025 12:25:35 -0600
Subject: [PATCH 1/2] [GVN] Add tests for recovering analysis of a BB after
 failure; NFC

---
 .../GVN/load-from-unreachable-predecessor.ll  | 63 ++++++++++++++++++-
 1 file changed, 61 insertions(+), 2 deletions(-)

diff --git a/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll b/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll
index 6ad0f59dbe292e..d6af9a9bbcb039 100644
--- a/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll
+++ b/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll
@@ -1,12 +1,23 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt -passes=gvn -S < %s | FileCheck %s
 
 ; Check that an unreachable predecessor to a PHI node doesn't cause a crash.
 ; PR21625.
 
 define i32 @f(ptr %f) {
-; CHECK: bb0:
+; CHECK-LABEL: define i32 @f(
+; CHECK-SAME: ptr [[F:%.*]]) {
+; CHECK-NEXT:  [[BB0:.*]]:
+; CHECK-NEXT:    br label %[[BB2:.*]]
+; CHECK:       [[BB1:.*]]:
+; CHECK-NEXT:    [[ZED:%.*]] = load ptr, ptr [[F]], align 8
+; CHECK-NEXT:    br i1 false, label %[[BB1]], label %[[BB2]]
+; CHECK:       [[BB2]]:
+; CHECK-NEXT:    [[FOO:%.*]] = phi ptr [ null, %[[BB0]] ], [ [[ZED]], %[[BB1]] ]
+; CHECK-NEXT:    [[STOREMERGE:%.*]] = load i32, ptr [[FOO]], align 4
+; CHECK-NEXT:    ret i32 [[STOREMERGE]]
+;
 ; Load should be removed, since it's ignored.
-; CHECK-NEXT: br label
 bb0:
   %bar = load ptr, ptr %f
   br label %bb2
@@ -18,3 +29,51 @@ bb2:
   %storemerge = load i32, ptr %foo
   ret i32 %storemerge
 }
+
+declare void @use.ptr(ptr)
+define i32 @invalidate_and_reuse_dep(ptr %base_pp, i1 %cmp0) {
+; CHECK-LABEL: define i32 @invalidate_and_reuse_dep(
+; CHECK-SAME: ptr [[BASE_PP:%.*]], i1 [[CMP0:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:    [[NODE_P:%.*]] = load ptr, ptr [[BASE_PP]], align 8
+; CHECK-NEXT:    [[VAL:%.*]] = load i32, ptr [[NODE_P]], align 4
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[VAL]], 0
+; CHECK-NEXT:    br i1 [[CMP]], label %[[L0:.*]], label %[[L1:.*]]
+; CHECK:       [[L0]]:
+; CHECK-NEXT:    br label %[[L1]]
+; CHECK:       [[L1]]:
+; CHECK-NEXT:    [[NULL_OR_SELECT_PP:%.*]] = phi ptr [ null, %[[ENTRY]] ], [ [[BASE_PP]], %[[L0]] ]
+; CHECK-NEXT:    br i1 [[CMP0]], label %[[THEN:.*]], label %[[END:.*]]
+; CHECK:       [[THEN]]:
+; CHECK-NEXT:    [[THEN_NODE_P:%.*]] = load ptr, ptr [[BASE_PP]], align 8
+; CHECK-NEXT:    [[THEN_VAL:%.*]] = load i32, ptr [[THEN_NODE_P]], align 4
+; CHECK-NEXT:    ret i32 [[THEN_VAL]]
+; CHECK:       [[END]]:
+; CHECK-NEXT:    [[END_SELECT_P:%.*]] = load ptr, ptr [[NULL_OR_SELECT_PP]], align 8
+; CHECK-NEXT:    call void @use.ptr(ptr [[END_SELECT_P]])
+; CHECK-NEXT:    ret i32 0
+;
+entry:
+  %node_p = load ptr, ptr %base_pp
+  %val = load i32, ptr %node_p
+  %cmp = icmp slt i32 %val, 0
+  br i1 %cmp, label %L0, label %L1
+
+L0:
+  %select_pp = select i1 true, ptr %base_pp, ptr null
+  br label %L1
+
+L1:
+  %null.or.select_pp = phi ptr [ null, %entry ], [ %select_pp, %L0 ]
+  br i1 %cmp0, label %then, label %end
+
+then:
+  %then.node_p = load ptr, ptr %base_pp
+  %then.val = load i32, ptr %then.node_p
+  ret i32 %then.val
+
+end:
+  %end.select_p = load ptr, ptr %null.or.select_pp
+  call void @use.ptr(ptr %end.select_p)
+  ret i32 0
+}

>From 501bcc5b49202d9a7b1c0ada55d13be993b29f38 Mon Sep 17 00:00:00 2001
From: Noah Goldstein <goldstein.w.n at gmail.com>
Date: Tue, 4 Feb 2025 12:25:37 -0600
Subject: [PATCH 2/2] [MemDep] Mark deps as `Dirty` on translation failure as
 opposed to `Unknown`

Setting the `Dep` to `Unknown` blocks future analysis on `BB`/`Inst`,
as future cache hits will return `Unknown` leading to unhelpful
(overly conservative) analysis results.

Set the value to `Dirty` to clear any existing (and potentially
incorrect) results after a translation failure, but leaving room to
later on get results on the `BB`/`Inst`.

Fixes regressions assosiated with #123988
---
 llvm/lib/Analysis/MemoryDependenceAnalysis.cpp                | 4 ++--
 llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll | 4 +---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index dec5ee5361ca33..dec49dcff605c5 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -1391,7 +1391,7 @@ bool MemoryDependenceResults::getNonLocalPointerDepFromBB(
     // results from the set".  Clear out the indicator for this.
     CacheInfo->Pair = BBSkipFirstBlockPair();
 
-    // If *nothing* works, mark the pointer as unknown.
+    // If *nothing* works, mark the pointer as invalid.
     //
     // If this is the magic first block, return this as a clobber of the whole
     // incoming value.  Since we can't phi translate to one of the predecessors,
@@ -1410,7 +1410,7 @@ bool MemoryDependenceResults::getNonLocalPointerDepFromBB(
                 !DT.isReachableFromEntry(BB)) &&
                "Should only be here with transparent block");
 
-        I.setResult(MemDepResult::getUnknown());
+        I.setResult(MemDepResult::getDirty(I.getResult().getInst()));
 
 
         break;
diff --git a/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll b/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll
index d6af9a9bbcb039..726884ab4e3652 100644
--- a/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll
+++ b/llvm/test/Transforms/GVN/load-from-unreachable-predecessor.ll
@@ -45,9 +45,7 @@ define i32 @invalidate_and_reuse_dep(ptr %base_pp, i1 %cmp0) {
 ; CHECK-NEXT:    [[NULL_OR_SELECT_PP:%.*]] = phi ptr [ null, %[[ENTRY]] ], [ [[BASE_PP]], %[[L0]] ]
 ; CHECK-NEXT:    br i1 [[CMP0]], label %[[THEN:.*]], label %[[END:.*]]
 ; CHECK:       [[THEN]]:
-; CHECK-NEXT:    [[THEN_NODE_P:%.*]] = load ptr, ptr [[BASE_PP]], align 8
-; CHECK-NEXT:    [[THEN_VAL:%.*]] = load i32, ptr [[THEN_NODE_P]], align 4
-; CHECK-NEXT:    ret i32 [[THEN_VAL]]
+; CHECK-NEXT:    ret i32 [[VAL]]
 ; CHECK:       [[END]]:
 ; CHECK-NEXT:    [[END_SELECT_P:%.*]] = load ptr, ptr [[NULL_OR_SELECT_PP]], align 8
 ; CHECK-NEXT:    call void @use.ptr(ptr [[END_SELECT_P]])



More information about the llvm-commits mailing list