[llvm] Revert [LICM] Remove unnecessary check during store hoisting (PR #195606)
via llvm-commits
llvm-commits at lists.llvm.org
Mon May 4 01:05:49 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Nikita Popov (nikic)
<details>
<summary>Changes</summary>
This check is needed after all, to handle the case where the load aliases only on the first iteration. Even with correct cross-iteration handling in MSSA, it's legal to return an out of loop clobbering memory accesses in this case.
Reverts https://github.com/llvm/llvm-project/pull/187529.
Fixes https://github.com/llvm/llvm-project/issues/195513.
---
Full diff: https://github.com/llvm/llvm-project/pull/195606.diff
5 Files Affected:
- (modified) llvm/lib/Transforms/Scalar/LICM.cpp (+6)
- (modified) llvm/test/Transforms/LICM/call-hoisting.ll (+2-1)
- (modified) llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll (+5-3)
- (modified) llvm/test/Transforms/LICM/pr54495.ll (+1-1)
- (modified) llvm/test/Transforms/LICM/store-hoisting.ll (+35)
``````````diff
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 330464fdb64ff..9f3535a506b7e 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -2336,6 +2336,12 @@ static bool noConflictingReadWrites(Instruction *I, MemorySSA *MSSA,
const_cast<MemoryUse *>(MU));
if (!MSSA->isLiveOnEntryDef(MD) && CurLoop->contains(MD->getBlock()))
return false;
+ // Disable hoisting past potentially interfering loads. Optimized
+ // Uses may point to an access outside the loop, as getClobbering
+ // checks the previous iteration when walking the backedge.
+ // FIXME: More precise: no Uses that alias I.
+ if (!Flags.getIsSink() && !MSSA->dominates(IMD, MU))
+ return false;
} else if (const auto *MD = dyn_cast<MemoryDef>(&MA)) {
if (auto *LI = dyn_cast<LoadInst>(MD->getMemoryInst())) {
(void)LI; // Silence warning.
diff --git a/llvm/test/Transforms/LICM/call-hoisting.ll b/llvm/test/Transforms/LICM/call-hoisting.ll
index 2520e2277cf56..748810be3a709 100644
--- a/llvm/test/Transforms/LICM/call-hoisting.ll
+++ b/llvm/test/Transforms/LICM/call-hoisting.ll
@@ -277,16 +277,17 @@ exit:
ret i32 %val
}
+; FIXME: It's safe to hoist @store(), because @load() does not alias.
define i32 @unrelated_read(ptr noalias %loc, ptr noalias %otherloc) {
; CHECK-LABEL: define i32 @unrelated_read(
; CHECK-SAME: ptr noalias [[LOC:%.*]], ptr noalias [[OTHERLOC:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*]]:
-; CHECK-NEXT: call void @store(i32 0, ptr [[LOC]])
; CHECK-NEXT: br label %[[LOOP:.*]]
; CHECK: [[LOOP]]:
; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, %[[ENTRY]] ], [ [[IV_NEXT:%.*]], %[[LOOP]] ]
; CHECK-NEXT: [[OTHERLOC_GEP:%.*]] = getelementptr i32, ptr [[OTHERLOC]], i32 [[IV]]
; CHECK-NEXT: [[VAL:%.*]] = call i32 @load(ptr [[OTHERLOC_GEP]])
+; CHECK-NEXT: call void @store(i32 0, ptr [[LOC]])
; CHECK-NEXT: [[IV_NEXT]] = add i32 [[IV]], 1
; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[IV]], 200
; CHECK-NEXT: br i1 [[CMP]], label %[[LOOP]], label %[[EXIT:.*]]
diff --git a/llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll b/llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll
index 03ab270de12e0..fd73080365993 100644
--- a/llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll
+++ b/llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll
@@ -4,16 +4,18 @@
;; It should hoist fn_write_inaccessible_mem
;; because there is no conflict between inaccessible memory
;; fn_read_inaccessible_mem is a nice side effect
+; FIXME: fn_write_inaccessible_mem is currently not hoisted due to the preceding
+; load, even though it does not alias.
define i32 @loop_alias(i64 %x, ptr %start) {
; CHECK-LABEL: define i32 @loop_alias(
; CHECK-SAME: i64 [[X:%.*]], ptr [[START:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*]]:
-; CHECK-NEXT: call void @fn_write_inaccessible_mem()
; CHECK-NEXT: br label %[[LOOP:.*]]
; CHECK: [[LOOP]]:
; CHECK-NEXT: [[PHI:%.*]] = phi ptr [ [[START]], %[[ENTRY]] ], [ [[GEP:%.*]], %[[LOOP]] ]
-; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr [[PHI]]
+; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr [[PHI]], align 4
; CHECK-NEXT: [[VAL:%.*]] = call i32 @fn_args(i32 [[LOAD]])
+; CHECK-NEXT: call void @fn_write_inaccessible_mem()
; CHECK-NEXT: call void @fn_read_inaccessible_mem(i32 [[LOAD]])
; CHECK-NEXT: [[GEP]] = getelementptr inbounds nuw i32, ptr [[PHI]], i64 [[X]]
; CHECK-NEXT: [[ACC:%.*]] = add nuw nsw i32 [[VAL]], 1
@@ -48,7 +50,7 @@ define i32 @ne_loop_alias(i64 %x, ptr %start) {
; CHECK-NEXT: br label %[[LOOP:.*]]
; CHECK: [[LOOP]]:
; CHECK-NEXT: [[PHI:%.*]] = phi ptr [ [[START]], %[[ENTRY]] ], [ [[GEP:%.*]], %[[LOOP]] ]
-; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr [[PHI]]
+; CHECK-NEXT: [[LOAD:%.*]] = load i32, ptr [[PHI]], align 4
; CHECK-NEXT: [[VAL:%.*]] = call i32 @fn_read_inaccessible_mem_2(i32 [[LOAD]])
; CHECK-NEXT: call void @fn_write_inaccessible_mem()
; CHECK-NEXT: call void @fn_read_inaccessible_mem(i32 [[VAL]])
diff --git a/llvm/test/Transforms/LICM/pr54495.ll b/llvm/test/Transforms/LICM/pr54495.ll
index 5e66758257ef0..d01ca69d55242 100644
--- a/llvm/test/Transforms/LICM/pr54495.ll
+++ b/llvm/test/Transforms/LICM/pr54495.ll
@@ -6,7 +6,6 @@
define void @test(ptr %p1, ptr %p2, ptr noalias %p3) {
; CHECK-LABEL: @test(
; CHECK-NEXT: entry:
-; CHECK-NEXT: store ptr [[P3:%.*]], ptr [[P3]], align 8
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[P1:%.*]], [[ENTRY:%.*]] ], [ [[P2:%.*]], [[LOOP]] ]
@@ -14,6 +13,7 @@ define void @test(ptr %p1, ptr %p2, ptr noalias %p3) {
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i64 [[V]], 0
; CHECK-NEXT: br i1 [[CMP]], label [[LOOP]], label [[LOOP_EXIT:%.*]]
; CHECK: loop.exit:
+; CHECK-NEXT: store ptr [[P3:%.*]], ptr [[P3]], align 8
; CHECK-NEXT: ret void
;
entry:
diff --git a/llvm/test/Transforms/LICM/store-hoisting.ll b/llvm/test/Transforms/LICM/store-hoisting.ll
index a746caec18e54..207a1831bb6e8 100644
--- a/llvm/test/Transforms/LICM/store-hoisting.ll
+++ b/llvm/test/Transforms/LICM/store-hoisting.ll
@@ -689,3 +689,38 @@ merge:
exit:
ret void
}
+
+ at g1 = external global i8
+ at g2 = external global i8
+
+; The store should not be hoisted here, because the location is accessed by the
+; preceding load. However, the clobbering memory access for that load is
+; liveOnEntry, because the pointer on the backedge does not alias. As such,
+; only checking whether the clobbering access is outside the loop is
+; insufficient.
+define i8 @load_before_store_with_out_of_loop_def(i1 %c) {
+; CHECK-LABEL: define i8 @load_before_store_with_out_of_loop_def(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[LOOP:.*]]
+; CHECK: [[LOOP]]:
+; CHECK-NEXT: [[PHI:%.*]] = phi ptr [ @g1, %[[ENTRY]] ], [ @g2, %[[LOOP]] ]
+; CHECK-NEXT: [[V:%.*]] = load i8, ptr [[PHI]], align 1
+; CHECK-NEXT: store i8 1, ptr @g1, align 1
+; CHECK-NEXT: br i1 [[C]], label %[[LOOP]], label %[[EXIT:.*]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[V_LCSSA:%.*]] = phi i8 [ [[V]], %[[LOOP]] ]
+; CHECK-NEXT: ret i8 [[V_LCSSA]]
+;
+entry:
+ br label %loop
+
+loop:
+ %phi = phi ptr [ @g1, %entry ], [ @g2, %loop ]
+ %v = load i8, ptr %phi
+ store i8 1, ptr @g1
+ br i1 %c, label %loop, label %exit
+
+exit:
+ ret i8 %v
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/195606
More information about the llvm-commits
mailing list