Skip to content

Commit

Permalink
[SimplifyCFG] performBranchToCommonDestFolding(): require block-close…
Browse files Browse the repository at this point in the history
…d SSA form for bonus instructions (PR51125)

I can't seem to wrap my head around the proper fix here,
we should be fine without this requirement, iff we can form this form,
but the naive attempt (https://reviews.llvm.org/D106317) has failed.
So just to unblock the release, put up a restriction.

Fixes https://bugs.llvm.org/show_bug.cgi?id=51125

(cherry picked from commit 909cba9)
  • Loading branch information
LebedevRI authored and tstellar committed Sep 10, 2021
1 parent bd8cc85 commit 84a3be8
Show file tree
Hide file tree
Showing 7 changed files with 240 additions and 212 deletions.
36 changes: 27 additions & 9 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1094,17 +1094,24 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(

// Update (liveout) uses of bonus instructions,
// now that the bonus instruction has been cloned into predecessor.
SSAUpdater SSAUpdate;
SSAUpdate.Initialize(BonusInst.getType(),
(NewBonusInst->getName() + ".merge").str());
SSAUpdate.AddAvailableValue(BB, &BonusInst);
SSAUpdate.AddAvailableValue(PredBlock, NewBonusInst);
// Note that we expect to be in a block-closed SSA form for this to work!
for (Use &U : make_early_inc_range(BonusInst.uses())) {
auto *UI = cast<Instruction>(U.getUser());
if (UI->getParent() != PredBlock)
SSAUpdate.RewriteUseAfterInsertions(U);
else // Use is in the same block as, and comes before, NewBonusInst.
SSAUpdate.RewriteUse(U);
auto *PN = dyn_cast<PHINode>(UI);
if (!PN) {
assert(UI->getParent() == BB && BonusInst.comesBefore(UI) &&
"If the user is not a PHI node, then it should be in the same "
"block as, and come after, the original bonus instruction.");
continue; // Keep using the original bonus instruction.
}
// Is this the block-closed SSA form PHI node?
if (PN->getIncomingBlock(U) == BB)
continue; // Great, keep using the original bonus instruction.
// The only other alternative is an "use" when coming from
// the predecessor block - here we should refer to the cloned bonus instr.
assert(PN->getIncomingBlock(U) == PredBlock &&
"Not in block-closed SSA form?");
U.set(NewBonusInst);
}
}
}
Expand Down Expand Up @@ -3207,6 +3214,17 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
// Early exits once we reach the limit.
if (NumBonusInsts > BonusInstThreshold)
return false;

auto IsBCSSAUse = [BB, &I](Use &U) {
auto *UI = cast<Instruction>(U.getUser());
if (auto *PN = dyn_cast<PHINode>(UI))
return PN->getIncomingBlock(U) == BB;
return UI->getParent() == BB && I.comesBefore(UI);
};

// Does this instruction require rewriting of uses?
if (!all_of(I.uses(), IsBCSSAUse))
return false;
}

// Ok, we have the budget. Perform the transformation.
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/Thumb2/mve-float16regloops.ll
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ define void @fir(%struct.arm_fir_instance_f32* nocapture readonly %S, half* noca
; CHECK-NEXT: cmp r3, #8
; CHECK-NEXT: str r1, [sp, #20] @ 4-byte Spill
; CHECK-NEXT: blo.w .LBB16_12
; CHECK-NEXT: @ %bb.1: @ %entry
; CHECK-NEXT: @ %bb.1: @ %if.then
; CHECK-NEXT: lsrs.w r12, r3, #2
; CHECK-NEXT: beq.w .LBB16_12
; CHECK-NEXT: @ %bb.2: @ %while.body.lr.ph
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/Thumb2/mve-float32regloops.ll
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ define void @fir(%struct.arm_fir_instance_f32* nocapture readonly %S, float* noc
; CHECK-NEXT: sub sp, #32
; CHECK-NEXT: cmp r3, #8
; CHECK-NEXT: blo.w .LBB16_12
; CHECK-NEXT: @ %bb.1: @ %entry
; CHECK-NEXT: @ %bb.1: @ %if.then
; CHECK-NEXT: lsrs.w r12, r3, #2
; CHECK-NEXT: beq.w .LBB16_12
; CHECK-NEXT: @ %bb.2: @ %while.body.lr.ph
Expand Down
Loading

0 comments on commit 84a3be8

Please sign in to comment.