[llvm] d140866 - [MemorySSA] Don't create phi nodes in fixupDefs() (#156021)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 4 01:23:33 PDT 2025
Author: Nikita Popov
Date: 2025-09-04T08:23:29Z
New Revision: d1408667de830da8817c24cb9788da6caae551c7
URL: https://github.com/llvm/llvm-project/commit/d1408667de830da8817c24cb9788da6caae551c7
DIFF: https://github.com/llvm/llvm-project/commit/d1408667de830da8817c24cb9788da6caae551c7.diff
LOG: [MemorySSA] Don't create phi nodes in fixupDefs() (#156021)
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.
Added:
llvm/test/Transforms/LICM/pr117157.ll
Modified:
llvm/lib/Analysis/MemorySSAUpdater.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index ecfecb03c3757..bb3e679219aea 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -411,17 +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.
+ // Update defining access of following defs.
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());
- }
+ fixupDefs(FixupList);
+ assert(NewPhiIndexEnd == InsertedPHIs.size() &&
+ "Should not insert new phis during fixupDefs()");
// Optimize potentially non-minimal phis added in this method.
unsigned NewPhiSize = NewPhiIndexEnd - NewPhiIndex;
@@ -504,11 +498,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
+}
More information about the llvm-commits
mailing list