[MSSA] Don't require clone creation to succeed #76819
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sometimes, we create a MemoryAccess for an instruction, which is later simplified (e.g. via devirtualization) such that the new instruction has no memory effects anymore. If we later clone the instruction (e.g. during unswitching), then MSSA will not create a MemoryAccess for the new instruction, triggering an assert. Disable the assertion (by passing CreationMustSucceed=false) and adjust getDefiningAccessForClone() to work correctly in that case.
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesSometimes, we create a MemoryAccess for an instruction, which is later simplified (e.g. via devirtualization) such that the new instruction has no memory effects anymore. If we later clone the instruction (e.g. during unswitching), then MSSA will not create a MemoryAccess for the new instruction, triggering an assert. Disable the assertion (by passing CreationMustSucceed=false) and adjust getDefiningAccessForClone() to work correctly in that case. This PR implements the alternative suggestion by @alinas from #76142. Full diff: https://github.com/llvm/llvm-project/pull/76819.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index 9ad60f774e9f86..e87ae7d71fffe2 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -568,7 +568,6 @@ static MemoryAccess *onlySingleValue(MemoryPhi *MP) {
static MemoryAccess *getNewDefiningAccessForClone(MemoryAccess *MA,
const ValueToValueMapTy &VMap,
PhiToDefMap &MPhiMap,
- bool CloneWasSimplified,
MemorySSA *MSSA) {
MemoryAccess *InsnDefining = MA;
if (MemoryDef *DefMUD = dyn_cast<MemoryDef>(InsnDefining)) {
@@ -578,18 +577,10 @@ static MemoryAccess *getNewDefiningAccessForClone(MemoryAccess *MA,
if (Instruction *NewDefMUDI =
cast_or_null<Instruction>(VMap.lookup(DefMUDI))) {
InsnDefining = MSSA->getMemoryAccess(NewDefMUDI);
- if (!CloneWasSimplified)
- assert(InsnDefining && "Defining instruction cannot be nullptr.");
- else if (!InsnDefining || isa<MemoryUse>(InsnDefining)) {
+ if (!InsnDefining || isa<MemoryUse>(InsnDefining)) {
// The clone was simplified, it's no longer a MemoryDef, look up.
- auto DefIt = DefMUD->getDefsIterator();
- // Since simplified clones only occur in single block cloning, a
- // previous definition must exist, otherwise NewDefMUDI would not
- // have been found in VMap.
- assert(DefIt != MSSA->getBlockDefs(DefMUD->getBlock())->begin() &&
- "Previous def must exist");
InsnDefining = getNewDefiningAccessForClone(
- &*(--DefIt), VMap, MPhiMap, CloneWasSimplified, MSSA);
+ DefMUD->getDefiningAccess(), VMap, MPhiMap, MSSA);
}
}
}
@@ -624,9 +615,9 @@ void MemorySSAUpdater::cloneUsesAndDefs(BasicBlock *BB, BasicBlock *NewBB,
MemoryAccess *NewUseOrDef = MSSA->createDefinedAccess(
NewInsn,
getNewDefiningAccessForClone(MUD->getDefiningAccess(), VMap,
- MPhiMap, CloneWasSimplified, MSSA),
+ MPhiMap, MSSA),
/*Template=*/CloneWasSimplified ? nullptr : MUD,
- /*CreationMustSucceed=*/CloneWasSimplified ? false : true);
+ /*CreationMustSucceed=*/false);
if (NewUseOrDef)
MSSA->insertIntoListsForBlock(NewUseOrDef, NewBB, MemorySSA::End);
}
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll b/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll
new file mode 100644
index 00000000000000..2aaf777683e116
--- /dev/null
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll
@@ -0,0 +1,117 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -S -passes="loop-mssa(loop-instsimplify,simple-loop-unswitch<nontrivial>)" < %s | FileCheck %s
+
+@vtable = constant ptr @foo
+
+declare void @foo() memory(none)
+declare void @bar()
+
+; The call becomes known readnone after simplification, but still have a
+; MemoryAccess. Make sure this does not lead to an assertion failure.
+define void @test(i1 %c) {
+; CHECK-LABEL: define void @test(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: [[C_FR:%.*]] = freeze i1 [[C]]
+; CHECK-NEXT: br i1 [[C_FR]], label [[DOTSPLIT_US:%.*]], label [[DOTSPLIT:%.*]]
+; CHECK: .split.us:
+; CHECK-NEXT: br label [[LOOP_US:%.*]]
+; CHECK: loop.us:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: br label [[EXIT_SPLIT_US:%.*]]
+; CHECK: exit.split.us:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: .split:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: br label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+ br label %loop
+
+loop:
+ %fn = load ptr, ptr @vtable, align 8
+ call void %fn()
+ br i1 %c, label %exit, label %loop
+
+exit:
+ ret void
+}
+
+; Variant with another access after the call.
+define void @test2(i1 %c, ptr %p) {
+; CHECK-LABEL: define void @test2(
+; CHECK-SAME: i1 [[C:%.*]], ptr [[P:%.*]]) {
+; CHECK-NEXT: [[C_FR:%.*]] = freeze i1 [[C]]
+; CHECK-NEXT: br i1 [[C_FR]], label [[DOTSPLIT_US:%.*]], label [[DOTSPLIT:%.*]]
+; CHECK: .split.us:
+; CHECK-NEXT: br label [[LOOP_US:%.*]]
+; CHECK: loop.us:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[EXIT_SPLIT_US:%.*]]
+; CHECK: exit.split.us:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: .split:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+ br label %loop
+
+loop:
+ %fn = load ptr, ptr @vtable, align 8
+ call void %fn()
+ call void @bar()
+ br i1 %c, label %exit, label %loop
+
+exit:
+ ret void
+}
+
+; Variant with another access after the call and no access before the call.
+define void @test3(i1 %c, ptr %p) {
+; CHECK-LABEL: define void @test3(
+; CHECK-SAME: i1 [[C:%.*]], ptr [[P:%.*]]) {
+; CHECK-NEXT: [[C_FR:%.*]] = freeze i1 [[C]]
+; CHECK-NEXT: br i1 [[C_FR]], label [[DOTSPLIT_US:%.*]], label [[DOTSPLIT:%.*]]
+; CHECK: .split.us:
+; CHECK-NEXT: br label [[LOOP_US:%.*]]
+; CHECK: loop.us:
+; CHECK-NEXT: br label [[SPLIT_US:%.*]]
+; CHECK: split.us:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[EXIT_SPLIT_US:%.*]]
+; CHECK: exit.split.us:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: .split:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: br label [[SPLIT:%.*]]
+; CHECK: split:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+ br label %loop
+
+loop:
+ %fn = load ptr, ptr @vtable, align 8
+ br label %split
+
+split:
+ call void %fn()
+ call void @bar()
+ br i1 %c, label %exit, label %loop
+
+exit:
+ ret void
+}
|
|
@llvm/pr-subscribers-llvm-analysis Author: Nikita Popov (nikic) ChangesSometimes, we create a MemoryAccess for an instruction, which is later simplified (e.g. via devirtualization) such that the new instruction has no memory effects anymore. If we later clone the instruction (e.g. during unswitching), then MSSA will not create a MemoryAccess for the new instruction, triggering an assert. Disable the assertion (by passing CreationMustSucceed=false) and adjust getDefiningAccessForClone() to work correctly in that case. This PR implements the alternative suggestion by @alinas from #76142. Full diff: https://github.com/llvm/llvm-project/pull/76819.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp
index 9ad60f774e9f86..e87ae7d71fffe2 100644
--- a/llvm/lib/Analysis/MemorySSAUpdater.cpp
+++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp
@@ -568,7 +568,6 @@ static MemoryAccess *onlySingleValue(MemoryPhi *MP) {
static MemoryAccess *getNewDefiningAccessForClone(MemoryAccess *MA,
const ValueToValueMapTy &VMap,
PhiToDefMap &MPhiMap,
- bool CloneWasSimplified,
MemorySSA *MSSA) {
MemoryAccess *InsnDefining = MA;
if (MemoryDef *DefMUD = dyn_cast<MemoryDef>(InsnDefining)) {
@@ -578,18 +577,10 @@ static MemoryAccess *getNewDefiningAccessForClone(MemoryAccess *MA,
if (Instruction *NewDefMUDI =
cast_or_null<Instruction>(VMap.lookup(DefMUDI))) {
InsnDefining = MSSA->getMemoryAccess(NewDefMUDI);
- if (!CloneWasSimplified)
- assert(InsnDefining && "Defining instruction cannot be nullptr.");
- else if (!InsnDefining || isa<MemoryUse>(InsnDefining)) {
+ if (!InsnDefining || isa<MemoryUse>(InsnDefining)) {
// The clone was simplified, it's no longer a MemoryDef, look up.
- auto DefIt = DefMUD->getDefsIterator();
- // Since simplified clones only occur in single block cloning, a
- // previous definition must exist, otherwise NewDefMUDI would not
- // have been found in VMap.
- assert(DefIt != MSSA->getBlockDefs(DefMUD->getBlock())->begin() &&
- "Previous def must exist");
InsnDefining = getNewDefiningAccessForClone(
- &*(--DefIt), VMap, MPhiMap, CloneWasSimplified, MSSA);
+ DefMUD->getDefiningAccess(), VMap, MPhiMap, MSSA);
}
}
}
@@ -624,9 +615,9 @@ void MemorySSAUpdater::cloneUsesAndDefs(BasicBlock *BB, BasicBlock *NewBB,
MemoryAccess *NewUseOrDef = MSSA->createDefinedAccess(
NewInsn,
getNewDefiningAccessForClone(MUD->getDefiningAccess(), VMap,
- MPhiMap, CloneWasSimplified, MSSA),
+ MPhiMap, MSSA),
/*Template=*/CloneWasSimplified ? nullptr : MUD,
- /*CreationMustSucceed=*/CloneWasSimplified ? false : true);
+ /*CreationMustSucceed=*/false);
if (NewUseOrDef)
MSSA->insertIntoListsForBlock(NewUseOrDef, NewBB, MemorySSA::End);
}
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll b/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll
new file mode 100644
index 00000000000000..2aaf777683e116
--- /dev/null
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll
@@ -0,0 +1,117 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -S -passes="loop-mssa(loop-instsimplify,simple-loop-unswitch<nontrivial>)" < %s | FileCheck %s
+
+@vtable = constant ptr @foo
+
+declare void @foo() memory(none)
+declare void @bar()
+
+; The call becomes known readnone after simplification, but still have a
+; MemoryAccess. Make sure this does not lead to an assertion failure.
+define void @test(i1 %c) {
+; CHECK-LABEL: define void @test(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: [[C_FR:%.*]] = freeze i1 [[C]]
+; CHECK-NEXT: br i1 [[C_FR]], label [[DOTSPLIT_US:%.*]], label [[DOTSPLIT:%.*]]
+; CHECK: .split.us:
+; CHECK-NEXT: br label [[LOOP_US:%.*]]
+; CHECK: loop.us:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: br label [[EXIT_SPLIT_US:%.*]]
+; CHECK: exit.split.us:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: .split:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: br label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+ br label %loop
+
+loop:
+ %fn = load ptr, ptr @vtable, align 8
+ call void %fn()
+ br i1 %c, label %exit, label %loop
+
+exit:
+ ret void
+}
+
+; Variant with another access after the call.
+define void @test2(i1 %c, ptr %p) {
+; CHECK-LABEL: define void @test2(
+; CHECK-SAME: i1 [[C:%.*]], ptr [[P:%.*]]) {
+; CHECK-NEXT: [[C_FR:%.*]] = freeze i1 [[C]]
+; CHECK-NEXT: br i1 [[C_FR]], label [[DOTSPLIT_US:%.*]], label [[DOTSPLIT:%.*]]
+; CHECK: .split.us:
+; CHECK-NEXT: br label [[LOOP_US:%.*]]
+; CHECK: loop.us:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[EXIT_SPLIT_US:%.*]]
+; CHECK: exit.split.us:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: .split:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+ br label %loop
+
+loop:
+ %fn = load ptr, ptr @vtable, align 8
+ call void %fn()
+ call void @bar()
+ br i1 %c, label %exit, label %loop
+
+exit:
+ ret void
+}
+
+; Variant with another access after the call and no access before the call.
+define void @test3(i1 %c, ptr %p) {
+; CHECK-LABEL: define void @test3(
+; CHECK-SAME: i1 [[C:%.*]], ptr [[P:%.*]]) {
+; CHECK-NEXT: [[C_FR:%.*]] = freeze i1 [[C]]
+; CHECK-NEXT: br i1 [[C_FR]], label [[DOTSPLIT_US:%.*]], label [[DOTSPLIT:%.*]]
+; CHECK: .split.us:
+; CHECK-NEXT: br label [[LOOP_US:%.*]]
+; CHECK: loop.us:
+; CHECK-NEXT: br label [[SPLIT_US:%.*]]
+; CHECK: split.us:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[EXIT_SPLIT_US:%.*]]
+; CHECK: exit.split.us:
+; CHECK-NEXT: br label [[EXIT:%.*]]
+; CHECK: .split:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: br label [[SPLIT:%.*]]
+; CHECK: split:
+; CHECK-NEXT: call void @foo()
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: ret void
+;
+ br label %loop
+
+loop:
+ %fn = load ptr, ptr @vtable, align 8
+ br label %split
+
+split:
+ call void %fn()
+ call void @bar()
+ br i1 %c, label %exit, label %loop
+
+exit:
+ ret void
+}
|
alinas
approved these changes
Jan 5, 2024
nikic
added a commit
to nikic/llvm-project
that referenced
this pull request
Jan 9, 2024
Sometimes, we create a MemoryAccess for an instruction, which is later simplified (e.g. via devirtualization) such that the new instruction has no memory effects anymore. If we later clone the instruction (e.g. during unswitching), then MSSA will not create a MemoryAccess for the new instruction, triggering an assert. Disable the assertion (by passing CreationMustSucceed=false) and adjust getDefiningAccessForClone() to work correctly in that case. This PR implements the alternative suggestion by alinas from llvm#76142. (cherry picked from commit d02c793)
nikic
added a commit
to nikic/llvm-project
that referenced
this pull request
Jan 16, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variantion of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case.
nikic
added a commit
that referenced
this pull request
Jan 24, 2024
This is a followup to #76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case.
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jan 26, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
Sometimes, we create a MemoryAccess for an instruction, which is later simplified (e.g. via devirtualization) such that the new instruction has no memory effects anymore. If we later clone the instruction (e.g. during unswitching), then MSSA will not create a MemoryAccess for the new instruction, triggering an assert. Disable the assertion (by passing CreationMustSucceed=false) and adjust getDefiningAccessForClone() to work correctly in that case. This PR implements the alternative suggestion by alinas from llvm#76142.
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jan 28, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
nikic
added a commit
to nikic/llvm-project
that referenced
this pull request
Feb 1, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Feb 5, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
cuviper
pushed a commit
to rust-lang/llvm-project
that referenced
this pull request
Feb 13, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
tstellar
pushed a commit
to tstellar/llvm-project
that referenced
this pull request
Feb 14, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
MingcongBai
pushed a commit
to AOSC-Tracking/llvm-project
that referenced
this pull request
Mar 26, 2024
Sometimes, we create a MemoryAccess for an instruction, which is later simplified (e.g. via devirtualization) such that the new instruction has no memory effects anymore. If we later clone the instruction (e.g. during unswitching), then MSSA will not create a MemoryAccess for the new instruction, triggering an assert. Disable the assertion (by passing CreationMustSucceed=false) and adjust getDefiningAccessForClone() to work correctly in that case. This PR implements the alternative suggestion by alinas from llvm#76142. (cherry picked from commit d02c793)
MingcongBai
pushed a commit
to AOSC-Tracking/llvm-project
that referenced
this pull request
Mar 26, 2024
This is a followup to llvm#76819. After those changes, we can still run into an assertion failure for a slight variation of the test case: When fixing up MemoryPhis, we map the incoming access to the access of the cloned instruction -- which may now no longer exist. Fix this by reusing the getNewDefiningAccessForClone() helper, which will look upwards for a new defining access in that case. (cherry picked from commit a7a1b8b)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometimes, we create a MemoryAccess for an instruction, which is later simplified (e.g. via devirtualization) such that the new instruction has no memory effects anymore.
If we later clone the instruction (e.g. during unswitching), then MSSA will not create a MemoryAccess for the new instruction, triggering an assert.
Disable the assertion (by passing CreationMustSucceed=false) and adjust getDefiningAccessForClone() to work correctly in that case.
This PR implements the alternative suggestion by @alinas from #76142.