[MSSAUpdater] Handle simplified accesses when updating phis

[nikic]

This is a followup to #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.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

This is a followup to #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.

---

Full diff: https://github.com/llvm/llvm-project/pull/78272.diff

2 Files Affected:

• (modified) llvm/lib/Analysis/MemorySSAUpdater.cpp (+3-19)
• (modified) llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll (+104)

diff --git a/llvm/lib/Analysis/MemorySSAUpdater.cpp b/llvm/lib/Analysis/MemorySSAUpdater.cpp index e87ae7d71fffe2..aa550f0b6a7bfd 100644 --- a/llvm/lib/Analysis/MemorySSAUpdater.cpp +++ b/llvm/lib/Analysis/MemorySSAUpdater.cpp @@ -692,25 +692,9 @@ void MemorySSAUpdater::updateForClonedLoop(const LoopBlocksRPO &LoopBlocks, continue; // Determine incoming value and add it as incoming from IncBB. - if (MemoryUseOrDef *IncMUD = dyn_cast<MemoryUseOrDef>(IncomingAccess)) { - if (!MSSA->isLiveOnEntryDef(IncMUD)) { - Instruction *IncI = IncMUD->getMemoryInst(); - assert(IncI && "Found MemoryUseOrDef with no Instruction."); - if (Instruction *NewIncI = - cast_or_null<Instruction>(VMap.lookup(IncI))) { - IncMUD = MSSA->getMemoryAccess(NewIncI); - assert(IncMUD && - "MemoryUseOrDef cannot be null, all preds processed."); - } - } - NewPhi->addIncoming(IncMUD, IncBB); - } else { - MemoryPhi *IncPhi = cast<MemoryPhi>(IncomingAccess); - if (MemoryAccess *NewDefPhi = MPhiMap.lookup(IncPhi)) - NewPhi->addIncoming(NewDefPhi, IncBB); - else - NewPhi->addIncoming(IncPhi, IncBB); - } + NewPhi->addIncoming( + getNewDefiningAccessForClone(IncomingAccess, VMap, MPhiMap, MSSA), + IncBB); } if (auto *SingleAccess = onlySingleValue(NewPhi)) { MPhiMap[Phi] = SingleAccess; diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll b/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll index 2aaf777683e116..c6e6608d4be383 100644 --- a/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll +++ b/llvm/test/Transforms/SimpleLoopUnswitch/memssa-readnone-access.ll @@ -115,3 +115,107 @@ split: exit: ret void } + +; Variants of the above test with swapped branch destinations. + +define void @test1_swapped(i1 %c) { +; CHECK-LABEL: define void @test1_swapped( +; CHECK-SAME: i1 [[C:%.*]]) { +; CHECK-NEXT: start: +; CHECK-NEXT: [[C_FR:%.*]] = freeze i1 [[C]] +; CHECK-NEXT: br i1 [[C_FR]], label [[START_SPLIT_US:%.*]], label [[START_SPLIT:%.*]] +; CHECK: start.split.us: +; CHECK-NEXT: br label [[LOOP_US:%.*]] +; CHECK: loop.us: +; CHECK-NEXT: call void @foo() +; CHECK-NEXT: br label [[LOOP_US]] +; CHECK: start.split: +; CHECK-NEXT: br label [[LOOP:%.*]] +; CHECK: loop: +; CHECK-NEXT: call void @foo() +; CHECK-NEXT: br label [[EXIT:%.*]] +; CHECK: exit: +; CHECK-NEXT: ret void +; +start: + br label %loop + +loop: + %fn = load ptr, ptr @vtable, align 8 + call void %fn() + br i1 %c, label %loop, label %exit + +exit: + ret void +} + +define void @test2_swapped(i1 %c, ptr %p) { +; CHECK-LABEL: define void @test2_swapped( +; 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 [[LOOP_US]] +; CHECK: .split: +; CHECK-NEXT: br label [[LOOP:%.*]] +; CHECK: loop: +; CHECK-NEXT: call void @foo() +; CHECK-NEXT: call void @bar() +; CHECK-NEXT: br label [[EXIT:%.*]] +; 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 %loop, label %exit + +exit: + ret void +} + +define void @test3_swapped(i1 %c, ptr %p) { +; CHECK-LABEL: define void @test3_swapped( +; 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 [[LOOP_US]] +; 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 [[EXIT:%.*]] +; 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 %loop, label %exit + +exit: + ret void +} 

[nikic]

ping

[alinas]

LGTM.