Skip to content

Commit

Permalink
[SimplifyCFG] Remove limitation on sinking of load/store of alloca (l…
Browse files Browse the repository at this point in the history
…lvm#104788)

This is a followup to llvm#104579
to remove the limitation on sinking loads/stores of allocas entirely,
even if this would introduce a phi node.

Nowadays, SROA supports speculating load/store over select/phi.
Additionally, SimplifyCFG with sinking only runs at the end of the
function simplification pipeline, after SROA. I checked that the two
tests modified here still successfully SROA after the SimplifyCFG
transform.

We should, however, keep the limitation on lifetime intrinsics. SROA
does not have speculation support for these, and I've also found that
the way these are handled in the backend is very problematic
(llvm#104776), so I think we
should leave them alone.
  • Loading branch information
nikic authored Aug 26, 2024
1 parent 28fe6dd commit 84497c6
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 30 deletions.
19 changes: 4 additions & 15 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2051,21 +2051,10 @@ static bool canSinkInstructions(
return I->getOperand(OI) == I0->getOperand(OI);
};
if (!all_of(Insts, SameAsI0)) {
// Because SROA historically couldn't handle speculating stores of
// selects, we try not to sink loads, stores or lifetime markers of
// allocas when we'd have to create a PHI for the address operand.
// TODO: SROA supports speculation for loads and stores now -- remove
// this hack?
if (isa<StoreInst>(I0) && OI == 1 &&
any_of(Insts, [](const Instruction *I) {
return isa<AllocaInst>(I->getOperand(1)->stripPointerCasts());
}))
return false;
if (isa<LoadInst>(I0) && OI == 0 &&
any_of(Insts, [](const Instruction *I) {
return isa<AllocaInst>(I->getOperand(0)->stripPointerCasts());
}))
return false;
// SROA can't speculate lifetime markers of selects/phis, and the
// backend may handle such lifetimes incorrectly as well (#104776).
// Don't sink lifetimes if it would introduce a phi on the pointer
// argument.
if (isLifeTimeMarker(I0) && OI == 1 &&
any_of(Insts, [](const Instruction *I) {
return isa<AllocaInst>(I->getOperand(1)->stripPointerCasts());
Expand Down
21 changes: 6 additions & 15 deletions llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
Original file line number Diff line number Diff line change
Expand Up @@ -801,14 +801,8 @@ define i32 @test_pr30188(i1 zeroext %flag, i32 %x) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[Y:%.*]] = alloca i32, align 4
; CHECK-NEXT: [[Z:%.*]] = alloca i32, align 4
; CHECK-NEXT: br i1 [[FLAG:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
; CHECK-NEXT: store i32 [[X:%.*]], ptr [[Y]], align 4
; CHECK-NEXT: br label [[IF_END:%.*]]
; CHECK: if.else:
; CHECK-NEXT: store i32 [[X]], ptr [[Z]], align 4
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: [[Y_Z:%.*]] = select i1 [[FLAG:%.*]], ptr [[Y]], ptr [[Z]]
; CHECK-NEXT: store i32 [[X:%.*]], ptr [[Y_Z]], align 4
; CHECK-NEXT: ret i32 1
;
entry:
Expand All @@ -834,17 +828,14 @@ define i32 @test_pr30188a(i1 zeroext %flag, i32 %x) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[Y:%.*]] = alloca i32, align 4
; CHECK-NEXT: [[Z:%.*]] = alloca i32, align 4
; CHECK-NEXT: br i1 [[FLAG:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK-NEXT: br i1 [[FLAG:%.*]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
; CHECK: if.then:
; CHECK-NEXT: call void @g()
; CHECK-NEXT: [[ONE:%.*]] = load i32, ptr [[Y]], align 4
; CHECK-NEXT: br label [[IF_END:%.*]]
; CHECK: if.else:
; CHECK-NEXT: [[THREE:%.*]] = load i32, ptr [[Z]], align 4
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: [[THREE_SINK:%.*]] = phi i32 [ [[THREE]], [[IF_ELSE]] ], [ [[ONE]], [[IF_THEN]] ]
; CHECK-NEXT: [[FOUR:%.*]] = add i32 [[THREE_SINK]], 2
; CHECK-NEXT: [[Z_SINK:%.*]] = phi ptr [ [[Y]], [[IF_THEN]] ], [ [[Z]], [[ENTRY:%.*]] ]
; CHECK-NEXT: [[THREE:%.*]] = load i32, ptr [[Z_SINK]], align 4
; CHECK-NEXT: [[FOUR:%.*]] = add i32 [[THREE]], 2
; CHECK-NEXT: store i32 [[FOUR]], ptr [[Y]], align 4
; CHECK-NEXT: ret i32 1
;
Expand Down

0 comments on commit 84497c6

Please sign in to comment.