Skip to content

[LICM] allow MemoryAccess creation failure #116813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 20, 2024
Merged

[LICM] allow MemoryAccess creation failure #116813

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b2eb1f4
Pre-commit test cases
dianqk Nov 19, 2024
b164143
[SimpleLoopUnswitch] Preserve one PHI when removing a predecessor of …
dianqk Nov 19, 2024
6b80bdb
[SimpleLoopUnswitch] Preserve one PHI when removing a predecessor of a
dianqk Nov 19, 2024
5d2867b
Revert "[SimpleLoopUnswitch] Preserve one PHI when removing a predece…
dianqk Nov 19, 2024
e6b291a
Revert "[SimpleLoopUnswitch] Preserve one PHI when removing a predece…
dianqk Nov 19, 2024
e553a41
[LICM] allow MemoryAccess creation failure
dianqk Nov 19, 2024
de04782
Update test
dianqk Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member Author

@dianqk dianqk Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a test case for now yet, but I think we also need to keep one phi here.

SuccBB->removePredecessor(ClonedBB, /*KeepOneInputPHIs*/ true);
DeadBlocks.push_back(ClonedBB);
}

Expand Down Expand Up @@ -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);
Copy link
Member Author

@dianqk dianqk Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: why can't MemorySSA be updated here? Is it because MemorySSAUpdater does not yet support this?

}
DeadBlockSet.insert(BB);
Expand Down
24 changes: 24 additions & 0 deletions llvm/test/Transforms/SimpleLoopUnswitch/nontrivial-unswitch-pred-removed.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
; 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
Copy link
Member Author

@dianqk dianqk Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, I like /llvm/utils/update_test_checks.py, but verifying that it does not crash is sufficient.


; 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
Copy link
Member Author

@dianqk dianqk Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot use /llvm/utils/update_test_checks.py because I want to check MemoryDef here.

; CHECK-NEXT: call i32 %unswitched.select()

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)
Loading