[llvm] [SimpleLoopUnswitch] Keep one PHI when removing a predecessor of a BB (PR #116813)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 19 14:39:31 PST 2024
https://github.com/DianQK updated https://github.com/llvm/llvm-project/pull/116813
>From b2eb1f4d3f536fe82812943ded40f8e94cb0be40 Mon Sep 17 00:00:00 2001
From: DianQK <dianqk at dianqk.net>
Date: Tue, 19 Nov 2024 22:01:12 +0800
Subject: [PATCH 1/6] Pre-commit test cases
---
.../nontrivial-unswitch-pred-removed.ll | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
create mode 100644 llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
new file mode 100644
index 00000000000000..a5fa6c8f2acdf2
--- /dev/null
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
@@ -0,0 +1,20 @@
+; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),print<memoryssa>' -verify-memoryssa -disable-output -S < %s 2>&1 | FileCheck %s
+
+; CHECK: preds = %bb2{{$}}
+; CHECK-NEXT: MemoryDef
+; CHECK-NEXT: call i32 @bar()
+
+define i32 @foo(i1 %arg, ptr %arg1) {
+bb:
+ br label %bb2
+
+bb2: ; preds = %bb2, %bb
+ %i = select i1 %arg, ptr %arg1, ptr @bar
+ %i3 = call i32 %i()
+ br i1 %arg, label %bb2, label %bb4
+
+bb4: ; preds = %bb2
+ ret i32 %i3
+}
+
+declare i32 @bar() nounwind willreturn memory(none)
>From b164143bd0cf537cd17958628cda31acdeb86a8e Mon Sep 17 00:00:00 2001
From: DianQK <dianqk at dianqk.net>
Date: Tue, 19 Nov 2024 22:11:46 +0800
Subject: [PATCH 2/6] [SimpleLoopUnswitch] Preserve one PHI when removing a
predecessor of a BB
---
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp | 2 +-
.../nontrivial-unswitch-pred-removed.ll | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index aa3cbc5e4bddc2..d6fcf57f2b0647 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -1716,7 +1716,7 @@ static void deleteDeadBlocksFromLoop(Loop &L,
auto *BB = DeathCandidates.pop_back_val();
if (!DeadBlockSet.count(BB) && !DT.isReachableFromEntry(BB)) {
for (BasicBlock *SuccBB : successors(BB)) {
- SuccBB->removePredecessor(BB);
+ SuccBB->removePredecessor(BB, /*KeepOneInputPHIs*/ true);
DeathCandidates.push_back(SuccBB);
}
DeadBlockSet.insert(BB);
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
index a5fa6c8f2acdf2..3545492f1e5e23 100644
--- a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
@@ -1,8 +1,12 @@
; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),print<memoryssa>' -verify-memoryssa -disable-output -S < %s 2>&1 | FileCheck %s
+; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>,licm)' -verify-memoryssa -disable-output -S
-; CHECK: preds = %bb2{{$}}
+; Check that SimpleLoopUnswitch preserves the MemoryDef of `call i32 @bar()` by preserving the PHI node.
+; Also, check that executing LICM after SimpleLoopUnswitch does not result in a crash.
+
+; CHECK: %unswitched.select = phi ptr [ @bar, %bb2 ]
; CHECK-NEXT: MemoryDef
-; CHECK-NEXT: call i32 @bar()
+; CHECK-NEXT: call i32 %unswitched.select()
define i32 @foo(i1 %arg, ptr %arg1) {
bb:
>From 6b80bdb95627e4047c98e28991c503e3e90fddfa Mon Sep 17 00:00:00 2001
From: DianQK <dianqk at dianqk.net>
Date: Tue, 19 Nov 2024 22:21:25 +0800
Subject: [PATCH 3/6] [SimpleLoopUnswitch] Preserve one PHI when removing a
predecessor of a cloned BB
---
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index d6fcf57f2b0647..d807c66bd96195 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -1678,7 +1678,7 @@ deleteDeadClonedBlocks(Loop &L, ArrayRef<BasicBlock *> ExitBlocks,
if (BasicBlock *ClonedBB = cast_or_null<BasicBlock>(VMap->lookup(BB)))
if (!DT.isReachableFromEntry(ClonedBB)) {
for (BasicBlock *SuccBB : successors(ClonedBB))
- SuccBB->removePredecessor(ClonedBB);
+ SuccBB->removePredecessor(ClonedBB, /*KeepOneInputPHIs*/ true);
DeadBlocks.push_back(ClonedBB);
}
>From 5d2867be862430d5327571f1baeb01ba62fd6be8 Mon Sep 17 00:00:00 2001
From: DianQK <dianqk at dianqk.net>
Date: Wed, 20 Nov 2024 05:43:15 +0800
Subject: [PATCH 4/6] Revert "[SimpleLoopUnswitch] Preserve one PHI when
removing a predecessor of a"
This reverts commit 6b80bdb95627e4047c98e28991c503e3e90fddfa.
---
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index d807c66bd96195..d6fcf57f2b0647 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -1678,7 +1678,7 @@ deleteDeadClonedBlocks(Loop &L, ArrayRef<BasicBlock *> ExitBlocks,
if (BasicBlock *ClonedBB = cast_or_null<BasicBlock>(VMap->lookup(BB)))
if (!DT.isReachableFromEntry(ClonedBB)) {
for (BasicBlock *SuccBB : successors(ClonedBB))
- SuccBB->removePredecessor(ClonedBB, /*KeepOneInputPHIs*/ true);
+ SuccBB->removePredecessor(ClonedBB);
DeadBlocks.push_back(ClonedBB);
}
>From e6b291a0a1d11c3937a3a99e827a9818848d051b Mon Sep 17 00:00:00 2001
From: DianQK <dianqk at dianqk.net>
Date: Wed, 20 Nov 2024 05:43:36 +0800
Subject: [PATCH 5/6] Revert "[SimpleLoopUnswitch] Preserve one PHI when
removing a predecessor of a BB"
This reverts commit b164143bd0cf537cd17958628cda31acdeb86a8e.
---
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp | 2 +-
.../nontrivial-unswitch-pred-removed.ll | 8 ++------
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
index d6fcf57f2b0647..aa3cbc5e4bddc2 100644
--- a/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
+++ b/llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
@@ -1716,7 +1716,7 @@ static void deleteDeadBlocksFromLoop(Loop &L,
auto *BB = DeathCandidates.pop_back_val();
if (!DeadBlockSet.count(BB) && !DT.isReachableFromEntry(BB)) {
for (BasicBlock *SuccBB : successors(BB)) {
- SuccBB->removePredecessor(BB, /*KeepOneInputPHIs*/ true);
+ SuccBB->removePredecessor(BB);
DeathCandidates.push_back(SuccBB);
}
DeadBlockSet.insert(BB);
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
index 3545492f1e5e23..a5fa6c8f2acdf2 100644
--- a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
@@ -1,12 +1,8 @@
; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),print<memoryssa>' -verify-memoryssa -disable-output -S < %s 2>&1 | FileCheck %s
-; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>,licm)' -verify-memoryssa -disable-output -S
-; Check that SimpleLoopUnswitch preserves the MemoryDef of `call i32 @bar()` by preserving the PHI node.
-; Also, check that executing LICM after SimpleLoopUnswitch does not result in a crash.
-
-; CHECK: %unswitched.select = phi ptr [ @bar, %bb2 ]
+; CHECK: preds = %bb2{{$}}
; CHECK-NEXT: MemoryDef
-; CHECK-NEXT: call i32 %unswitched.select()
+; CHECK-NEXT: call i32 @bar()
define i32 @foo(i1 %arg, ptr %arg1) {
bb:
>From e553a412bf217490fbeb13e93218b8d79aa81190 Mon Sep 17 00:00:00 2001
From: DianQK <dianqk at dianqk.net>
Date: Wed, 20 Nov 2024 05:57:15 +0800
Subject: [PATCH 6/6] [LICM] allow MemoryAccess creation failure
---
llvm/include/llvm/Analysis/MemorySSAUpdater.h | 3 ++-
llvm/lib/Analysis/MemorySSAUpdater.cpp | 8 +++++---
llvm/lib/Transforms/Scalar/LICM.cpp | 5 ++++-
.../PR116813-memoryssa-outdated.ll} | 4 ++++
4 files changed, 15 insertions(+), 5 deletions(-)
rename llvm/test/Transforms/{SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll => LICM/PR116813-memoryssa-outdated.ll} (68%)
diff --git a/llvm/include/llvm/Analysis/MemorySSAUpdater.h b/llvm/include/llvm/Analysis/MemorySSAUpdater.h
index d4da3ef1146db7..055feceefb0546 100644
--- a/llvm/include/llvm/Analysis/MemorySSAUpdater.h
+++ b/llvm/include/llvm/Analysis/MemorySSAUpdater.h
@@ -190,7 +190,8 @@ class MemorySSAUpdater {
/// inaccessible and it *must* have removeMemoryAccess called on it.
MemoryAccess *createMemoryAccessInBB(Instruction *I, MemoryAccess *Definition,
const BasicBlock *BB,
- MemorySSA::InsertionPlace Point);
+ MemorySSA::InsertionPlace Point,
+ bool CreationMustSucceed = true);
/// Create a MemoryAccess in MemorySSA before an existing MemoryAccess.
///
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index aa550f0b6a7bfd..f672bd0e1e133e 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -1403,9 +1403,11 @@ void MemorySSAUpdater::changeToUnreachable(const Instruction *I) {
MemoryAccess *MemorySSAUpdater::createMemoryAccessInBB(
Instruction *I, MemoryAccess *Definition, const BasicBlock *BB,
- MemorySSA::InsertionPlace Point) {
- MemoryUseOrDef *NewAccess = MSSA->createDefinedAccess(I, Definition);
- MSSA->insertIntoListsForBlock(NewAccess, BB, Point);
+ MemorySSA::InsertionPlace Point, bool CreationMustSucceed) {
+ MemoryUseOrDef *NewAccess = MSSA->createDefinedAccess(
+ I, Definition, /*Template=*/nullptr, CreationMustSucceed);
+ if (NewAccess)
+ MSSA->insertIntoListsForBlock(NewAccess, BB, Point);
return NewAccess;
}
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index fa04ced7182dc7..94bfe44a847a37 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -1465,8 +1465,11 @@ static Instruction *cloneInstructionInExitBlock(
if (MSSAU.getMemorySSA()->getMemoryAccess(&I)) {
// Create a new MemoryAccess and let MemorySSA set its defining access.
+ // After running some passes, MemorySSA might be outdated, and the
+ // instruction `I` may have become a non-memory touching instruction.
MemoryAccess *NewMemAcc = MSSAU.createMemoryAccessInBB(
- New, nullptr, New->getParent(), MemorySSA::Beginning);
+ New, nullptr, New->getParent(), MemorySSA::Beginning,
+ /*CreationMustSucceed=*/false);
if (NewMemAcc) {
if (auto *MemDef = dyn_cast<MemoryDef>(NewMemAcc))
MSSAU.insertDef(MemDef, /*RenameUses=*/true);
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll b/llvm/test/Transforms/LICM/PR116813-memoryssa-outdated.ll
similarity index 68%
rename from llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
rename to llvm/test/Transforms/LICM/PR116813-memoryssa-outdated.ll
index a5fa6c8f2acdf2..50f2940a61dff1 100644
--- a/llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
+++ b/llvm/test/Transforms/LICM/PR116813-memoryssa-outdated.ll
@@ -1,4 +1,8 @@
; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),print<memoryssa>' -verify-memoryssa -disable-output -S < %s 2>&1 | FileCheck %s
+; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>,licm)' -verify-memoryssa -disable-output -S
+
+; Check that SimpleLoopUnswitch preserves the MemoryDef of `call i32 @bar()`.
+; Also, check that running LICM after SimpleLoopUnswitch does not result in a crash.
; CHECK: preds = %bb2{{$}}
; CHECK-NEXT: MemoryDef
More information about the llvm-commits
mailing list