[llvm] [MemorySSA] Don't create phi nodes in fixupDefs() (PR #156021)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 29 06:06:23 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: Nikita Popov (nikic)
<details>
<summary>Changes</summary>
The general flow when inserting MemoryDefs is:
* Insert the def and set it's defining access (may insert phis)
* Insert IDF phis
* Update defining access for defs after the new one (fixupDefs)
* Rename uses if requested
fixupDefs() uses getPreviousDef() which can create new MemoryPHIs, but for which we're not going to insert IDF phis, so the required dominance property may not hold.
I believe this is a leftover from a time before the "Insert IDF phis" step existed. Now that step should already ensure that all necessary MemoryPhis have been inserted, and we only need to update them.
The fixupDefs() implementation was also returning after updating a single access, which is not right.
Fixes https://github.com/llvm/llvm-project/issues/47875.
Fixes https://github.com/llvm/llvm-project/issues/117157.
Fixes https://github.com/llvm/llvm-project/issues/152998.
Fixes https://github.com/llvm/llvm-project/issues/155161.
Fixes https://github.com/llvm/llvm-project/issues/155184.
---
Full diff: https://github.com/llvm/llvm-project/pull/156021.diff
2 Files Affected:
- (modified) llvm/lib/Analysis/MemorySSAUpdater.cpp (+5-17)
- (added) llvm/test/Transforms/LICM/pr117157.ll (+80)
``````````diff
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index ecfecb03c3757..9f6bd55a87bd5 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -411,20 +411,11 @@ void MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) {
FixupList.push_back(MD);
}
- // Remember the index where we stopped inserting new phis above, since the
- // fixupDefs call in the loop below may insert more, that are already minimal.
- unsigned NewPhiIndexEnd = InsertedPHIs.size();
-
- while (!FixupList.empty()) {
- unsigned StartingPHISize = InsertedPHIs.size();
- fixupDefs(FixupList);
- FixupList.clear();
- // Put any new phis on the fixup list, and process them
- FixupList.append(InsertedPHIs.begin() + StartingPHISize, InsertedPHIs.end());
- }
+ // Update defining access of following defs.
+ fixupDefs(FixupList);
// Optimize potentially non-minimal phis added in this method.
- unsigned NewPhiSize = NewPhiIndexEnd - NewPhiIndex;
+ unsigned NewPhiSize = InsertedPHIs.size() - NewPhiIndex;
if (NewPhiSize)
tryRemoveTrivialPhis(ArrayRef<WeakVH>(&InsertedPHIs[NewPhiIndex], NewPhiSize));
@@ -504,11 +495,8 @@ void MemorySSAUpdater::fixupDefs(const SmallVectorImpl<WeakVH> &Vars) {
assert(MSSA->dominates(NewDef, FirstDef) &&
"Should have dominated the new access");
- // This may insert new phi nodes, because we are not guaranteed the
- // block we are processing has a single pred, and depending where the
- // store was inserted, it may require phi nodes below it.
- cast<MemoryDef>(FirstDef)->setDefiningAccess(getPreviousDef(FirstDef));
- return;
+ cast<MemoryDef>(FirstDef)->setDefiningAccess(NewDef);
+ continue;
}
// We didn't find a def, so we must continue.
for (const auto *S : successors(FixupBlock)) {
diff --git a/llvm/test/Transforms/LICM/pr117157.ll b/llvm/test/Transforms/LICM/pr117157.ll
new file mode 100644
index 0000000000000..3c46e67bb5624
--- /dev/null
+++ b/llvm/test/Transforms/LICM/pr117157.ll
@@ -0,0 +1,80 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=licm -verify-memoryssa < %s | FileCheck %s
+
+define void @test(ptr %p) {
+; CHECK-LABEL: define void @test(
+; CHECK-SAME: ptr [[P:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br label %[[LOOP0:.*]]
+; CHECK: [[LOOP0]]:
+; CHECK-NEXT: br label %[[LOOP1:.*]]
+; CHECK: [[LOOP1]]:
+; CHECK-NEXT: [[DEC10:%.*]] = phi i64 [ 0, %[[LOOP0]] ], [ 1, %[[LOOP1]] ]
+; CHECK-NEXT: br i1 false, label %[[LOOP1_EXIT:.*]], label %[[LOOP1]]
+; CHECK: [[LOOP1_EXIT]]:
+; CHECK-NEXT: [[DEC10_LCSSA:%.*]] = phi i64 [ [[DEC10]], %[[LOOP1]] ]
+; CHECK-NEXT: switch i32 0, label %[[LOOP0_LATCH:.*]] [
+; CHECK-NEXT: i32 0, label %[[LOOP0_LATCH]]
+; CHECK-NEXT: i32 2, label %[[LOOP3_PREHEADER:.*]]
+; CHECK-NEXT: i32 1, label %[[LOOP2:.*]]
+; CHECK-NEXT: ]
+; CHECK: [[LOOP2]]:
+; CHECK-NEXT: br i1 false, label %[[LOOP0_LATCH]], label %[[LOOP3_PREHEADER]]
+; CHECK: [[LOOP3_PREHEADER]]:
+; CHECK-NEXT: br label %[[LOOP3:.*]]
+; CHECK: [[LOOP3]]:
+; CHECK-NEXT: switch i32 0, label %[[EXIT:.*]] [
+; CHECK-NEXT: i32 0, label %[[LOOP3]]
+; CHECK-NEXT: i32 1, label %[[LOOP2_LATCH:.*]]
+; CHECK-NEXT: ]
+; CHECK: [[LOOP2_LATCH]]:
+; CHECK-NEXT: br label %[[LOOP2]]
+; CHECK: [[LOOP0_LATCH]]:
+; CHECK-NEXT: br label %[[LOOP0]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[DEC10_LCSSA_LCSSA:%.*]] = phi i64 [ [[DEC10_LCSSA]], %[[LOOP3]] ]
+; CHECK-NEXT: store i64 [[DEC10_LCSSA_LCSSA]], ptr [[P]], align 4
+; CHECK-NEXT: store i64 1, ptr [[P]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %loop0
+
+loop0: ; preds = %loop0.latch, %entry
+ br label %loop1
+
+loop1: ; preds = %loop1, %loop0
+ %dec10 = phi i64 [ 0, %loop0 ], [ 1, %loop1 ]
+ store i64 %dec10, ptr %p
+ br i1 false, label %loop1.exit, label %loop1
+
+loop1.exit: ; preds = %loop1
+ switch i32 0, label %loop0.latch [
+ i32 0, label %loop0.latch
+ i32 2, label %loop3.preheader
+ i32 1, label %loop2
+ ]
+
+loop2: ; preds = %loop2.latch, %loop1.exit
+ br i1 false, label %loop0.latch, label %loop3.preheader
+
+loop3.preheader: ; preds = %loop1.exit, %loop2
+ br label %loop3
+
+loop3: ; preds = %loop3.preheader, %loop3
+ switch i32 0, label %exit [
+ i32 0, label %loop3
+ i32 1, label %loop2.latch
+ ]
+
+loop2.latch: ; preds = %loop3
+ br label %loop2
+
+loop0.latch: ; preds = %loop2, %loop1.exit, %loop1.exit
+ store i64 0, ptr %p
+ br label %loop0
+
+exit: ; preds = %loop3
+ store i64 1, ptr %p
+ ret void
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/156021
More information about the llvm-commits
mailing list