[llvm] [LICM] Relax Hoistable Branch if all of the incoming node are hoistable (PR #225599)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 23 00:00:38 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: aokblast
<details>
<summary>Changes</summary>
Like PhiNode, a branch is possibly hoistable if all of the parents are possible to hoist. The only difference is that we cannot hoisting to PreHeader as it might erase itself in getOrCreateHoistedBlock. This is why we have IsReplicatedBy check in hasExactlyReplicatedControlFlow.
In Phi, it is not a matter as we only change the incoming node from a phi node without overwrite its dominator.
Also, we fix a bug while we have nested hoisted node. In that case, we have to find the LCA of the dominators of all users.
---
Full diff: https://github.com/llvm/llvm-project/pull/225599.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/Scalar/LICM.cpp (+78-31)
- (modified) llvm/test/Transforms/LICM/hoist-phi.ll (+167-4)
``````````diff
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index cda59610ff4fc..50dafde187f25 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -677,6 +677,57 @@ class ControlFlowHoister {
MemorySSAUpdater &MSSAU)
: LI(LI), DT(DT), CurLoop(CurLoop), MSSAU(MSSAU) {}
+ static void erasePredecessorsReplicatedBy(
+ CondBrInst *BI, BasicBlock *BB,
+ SmallPtrSetImpl<BasicBlock *> &PredecessorBlocks) {
+ if (BI->getSuccessor(0) == BB) {
+ PredecessorBlocks.erase(BI->getParent());
+ PredecessorBlocks.erase(BI->getSuccessor(1));
+ } else if (BI->getSuccessor(1) == BB) {
+ PredecessorBlocks.erase(BI->getParent());
+ PredecessorBlocks.erase(BI->getSuccessor(0));
+ } else {
+ PredecessorBlocks.erase(BI->getSuccessor(0));
+ PredecessorBlocks.erase(BI->getSuccessor(1));
+ }
+ }
+
+ bool hasExactlyReplicatedControlFlow(CondBrInst *BI, BasicBlock *CommonSucc) {
+ // This can only be replicated only if other sucessor has single control
+ // flow. CommonSucc can be hoisted by others.
+ for (BasicBlock *Succ : successors(BI))
+ if (Succ != CommonSucc && !DT->dominates(BI, Succ))
+ return false;
+
+ BasicBlock *BB = BI->getParent();
+ if (BB == CommonSucc)
+ return false;
+ // There has to be a pending branch that BB is a successor of, or
+ // getOrCreateHoistedBlock() falls back to the preheader. The preheader is
+ // also the clone of CommonSucc at that point, so HoistTarget would be
+ // HoistCommonSucc and cloning the branch would erase the only edge into
+ // the loop.
+ auto IsReplicatedBy = [&](std::pair<CondBrInst *, BasicBlock *> Pair) {
+ return Pair.second == CommonSucc && DT->dominates(Pair.first, BB) &&
+ (Pair.first->getSuccessor(0) == BB ||
+ Pair.first->getSuccessor(1) == BB);
+ };
+ if (llvm::none_of(HoistableBranches, IsReplicatedBy))
+ return false;
+
+ SmallPtrSet<BasicBlock *, 8> PredecessorBlocks(llvm::from_range,
+ predecessors(CommonSucc));
+ // We don't give duplicate branch a chance as they are same as direct jump.
+ if (PredecessorBlocks.size() != pred_size(CommonSucc))
+ return false;
+ erasePredecessorsReplicatedBy(BI, CommonSucc, PredecessorBlocks);
+ for (auto &Pair : HoistableBranches)
+ if (Pair.second == CommonSucc)
+ erasePredecessorsReplicatedBy(Pair.first, CommonSucc,
+ PredecessorBlocks);
+ return PredecessorBlocks.empty();
+ }
+
void registerPossiblyHoistableBranch(CondBrInst *BI) {
// We can only hoist conditional branches with loop invariant operands.
if (!ControlFlowHoisting || !CurLoop->hasLoopInvariantOperands(BI))
@@ -725,11 +776,11 @@ class ControlFlowHoister {
// there will be some other path to the successor that will not be
// controlled by this branch so any phi we hoist would be controlled by the
// wrong condition. This also takes care of avoiding hoisting of loop back
- // edges.
- // TODO: In some cases this could be relaxed if the successor is dominated
- // by another block that's been hoisted and we can guarantee that the
- // control flow has been replicated exactly.
- if (CommonSucc && DT->dominates(BI, CommonSucc))
+ // edges. The requirement is relaxed when those other paths are themselves
+ // replicated by branches we are already going to hoist, see
+ // hasExactlyReplicatedControlFlow().
+ if (CommonSucc && (DT->dominates(BI, CommonSucc) ||
+ hasExactlyReplicatedControlFlow(BI, CommonSucc)))
HoistableBranches[BI] = CommonSucc;
}
@@ -749,22 +800,9 @@ class ControlFlowHoister {
// values.
if (PredecessorBlocks.size() != pred_size(BB))
return false;
- for (auto &Pair : HoistableBranches) {
- if (Pair.second == BB) {
- // Which blocks are predecessors via this branch depends on if the
- // branch is triangle-like or diamond-like.
- if (Pair.first->getSuccessor(0) == BB) {
- PredecessorBlocks.erase(Pair.first->getParent());
- PredecessorBlocks.erase(Pair.first->getSuccessor(1));
- } else if (Pair.first->getSuccessor(1) == BB) {
- PredecessorBlocks.erase(Pair.first->getParent());
- PredecessorBlocks.erase(Pair.first->getSuccessor(0));
- } else {
- PredecessorBlocks.erase(Pair.first->getSuccessor(0));
- PredecessorBlocks.erase(Pair.first->getSuccessor(1));
- }
- }
- }
+ for (auto &Pair : HoistableBranches)
+ if (Pair.second == BB)
+ erasePredecessorsReplicatedBy(Pair.first, BB, PredecessorBlocks);
// PredecessorBlocks will now be empty if for every predecessor of BB we
// found a hoistable branch source.
return PredecessorBlocks.empty();
@@ -1026,29 +1064,38 @@ bool llvm::hoistRegion(DomTreeNode *N, AAResults *AA, LoopInfo *LI,
// If we hoisted instructions to a conditional block they may not dominate
// their uses that weren't hoisted (such as phis where some operands are not
- // loop invariant). If so make them unconditional by moving them to their
- // immediate dominator. We iterate through the instructions in reverse order
- // which ensures that when we rehoist an instruction we rehoist its operands,
- // and also keep track of where in the block we are rehoisting to make sure
- // that we rehoist instructions before the instructions that use them.
+ // loop invariant). If so make them less conditional by moving them to a
+ // block that dominates all of their uses. We iterate through the
+ // instructions in reverse order which ensures that when we rehoist an
+ // instruction we rehoist its operands, and also keep track of where in the
+ // block we are rehoisting to make sure that we rehoist instructions before
+ // the instructions that use them.
Instruction *HoistPoint = nullptr;
if (ControlFlowHoisting) {
for (Instruction *I : reverse(HoistedInstructions)) {
if (!llvm::all_of(I->uses(),
[&](Use &U) { return DT->dominates(I, U); })) {
+ // Cloned control flow can be nested, so the immediate dominator of the
+ // block I was hoisted to is not necessarily enough.
BasicBlock *Dominator =
DT->getNode(I->getParent())->getIDom()->getBlock();
- if (!HoistPoint || !DT->dominates(HoistPoint->getParent(), Dominator)) {
- if (HoistPoint)
- assert(DT->dominates(Dominator, HoistPoint->getParent()) &&
- "New hoist point expected to dominate old hoist point");
- HoistPoint = Dominator->getTerminator();
+ for (Use &U : I->uses()) {
+ auto *User = cast<Instruction>(U.getUser());
+ BasicBlock *UserBB = isa<PHINode>(User)
+ ? cast<PHINode>(User)->getIncomingBlock(U)
+ : User->getParent();
+ Dominator = DT->findNearestCommonDominator(Dominator, UserBB);
}
+ if (!HoistPoint || !DT->dominates(HoistPoint->getParent(), Dominator))
+ HoistPoint = Dominator->getTerminator();
LLVM_DEBUG(dbgs() << "LICM rehoisting to "
<< HoistPoint->getParent()->getNameOrAsOperand()
<< ": " << *I << "\n");
moveInstructionBefore(*I, HoistPoint->getIterator(), *SafetyInfo, MSSAU,
SE);
+ assert(llvm::all_of(I->uses(),
+ [&](Use &U) { return DT->dominates(I, U); }) &&
+ "Rehoisting failed to make the instruction dominate its uses");
HoistPoint = I;
Changed = true;
}
diff --git a/llvm/test/Transforms/LICM/hoist-phi.ll b/llvm/test/Transforms/LICM/hoist-phi.ll
index bf999b98a1dac..71a3dd504a82c 100644
--- a/llvm/test/Transforms/LICM/hoist-phi.ll
+++ b/llvm/test/Transforms/LICM/hoist-phi.ll
@@ -100,20 +100,29 @@ end:
ret void
}
-; TODO: This is currently too complicated for us to be able to hoist the phi.
+; The branch in %if doesn't dominate %then, but the only other path to %then is
+; the one replicated by hoisting the branch in %loop, so both branches and the
+; three way phi can be hoisted.
; CHECK-LABEL: @three_way_phi
define void @three_way_phi(i32 %x, ptr %p) {
; CHECK-LABEL: entry:
; CHECK-DAG: %cmp1 = icmp sgt i32 %x, 0
; CHECK-DAG: %add = add i32 %x, 1
; CHECK-DAG: %cmp2 = icmp sgt i32 %add, 0
-; CHECK-ENABLED: br i1 %cmp1, label %[[IF_LICM:.*]], label %[[ELSE_LICM:.*]]
+; CHECK-DISABLED: %sub = sub i32 %x, 1
+; CHECK-ENABLED: br i1 %cmp1, label %[[IF_LICM:.*]], label %[[THEN_LICM:.*]]
; CHECK-ENABLED: [[IF_LICM]]:
-; CHECK-ENABLED: br label %[[THEN_LICM:.*]]
+; CHECK-ENABLED: br i1 %cmp2, label %[[IF_IF_LICM:.*]], label %[[THEN_LICM]]
+
+; CHECK-ENABLED: [[IF_IF_LICM]]:
+; CHECK-ENABLED: %sub = sub i32 %x, 1
+; CHECK-ENABLED: br label %[[THEN_LICM]]
; CHECK-ENABLED: [[THEN_LICM]]:
-; CHECK: %sub = sub i32 %x, 1
+; CHECK-ENABLED: %phi = phi i32 [ 0, %entry ], [ %add, %[[IF_LICM]] ], [ %sub, %[[IF_IF_LICM]] ]
+; CHECK-ENABLED: store i32 %phi, ptr %p
+; CHECK-ENABLED: %cmp3 = icmp ne i32 %phi, 0
; CHECK: br label %loop
entry:
@@ -142,6 +151,160 @@ end:
ret void
}
+; Same as @three_way_phi, but the branch in %loop is not loop invariant, so the
+; path from %loop to %then is not replicated and neither the branch in %if nor
+; the phi can be hoisted.
+; CHECK-LABEL: @three_way_phi_variant_branch
+define void @three_way_phi_variant_branch(i32 %x, ptr %p) {
+; CHECK-LABEL: entry:
+; CHECK-DAG: %add = add i32 %x, 1
+; CHECK-DAG: %cmp2 = icmp sgt i32 %add, 0
+; CHECK-DAG: %sub = sub i32 %x, 1
+; CHECK: br label %loop
+
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %then ]
+ %cmp1 = icmp sgt i32 %iv, 0
+ br i1 %cmp1, label %if, label %then
+
+if:
+ %add = add i32 %x, 1
+ %cmp2 = icmp sgt i32 %add, 0
+ br i1 %cmp2, label %if.if, label %then
+
+if.if:
+ %sub = sub i32 %x, 1
+ br label %then
+
+; CHECK-LABEL: then:
+; CHECK: %phi = phi i32 [ 0, %loop ], [ %add, %if ], [ %sub, %if.if ]
+then:
+ %phi = phi i32 [ 0, %loop ], [ %add, %if ], [ %sub, %if.if ]
+ store i32 %phi, ptr %p
+ %iv.next = add i32 %iv, 1
+ %cmp3 = icmp ne i32 %phi, 0
+ br i1 %cmp3, label %loop, label %end
+
+end:
+ ret void
+}
+
+; Same as @three_way_phi, but the branch in %loop is a diamond rather than a
+; triangle. The branch in %if converges at %then as well and %if is one of the
+; blocks the branch in %loop replicates, so both branches and the three way phi
+; can be hoisted.
+; CHECK-LABEL: @diamond_nested_phi
+define void @diamond_nested_phi(i32 %x, ptr %p) {
+; CHECK-LABEL: entry:
+; CHECK-DAG: %cmp1 = icmp sgt i32 %x, 0
+; CHECK-DAG: %add = add i32 %x, 1
+; CHECK-DAG: %cmp2 = icmp sgt i32 %add, 0
+; CHECK-DISABLED-DAG: %mul = mul i32 %x, 3
+; CHECK-DISABLED-DAG: %sub = sub i32 %x, 1
+; CHECK-ENABLED: br i1 %cmp1, label %[[IF_LICM:.*]], label %[[ELSE_LICM:.*]]
+
+; CHECK-ENABLED: [[IF_LICM]]:
+; CHECK-ENABLED: br i1 %cmp2, label %[[IF_IF_LICM:.*]], label %[[THEN_LICM:.*]]
+
+; CHECK-ENABLED: [[ELSE_LICM]]:
+; CHECK-ENABLED: %mul = mul i32 %x, 3
+; CHECK-ENABLED: br label %[[THEN_LICM]]
+
+; CHECK-ENABLED: [[IF_IF_LICM]]:
+; CHECK-ENABLED: %sub = sub i32 %x, 1
+; CHECK-ENABLED: br label %[[THEN_LICM]]
+
+; CHECK-ENABLED: [[THEN_LICM]]:
+; CHECK-ENABLED: %phi = phi i32 [ %add, %[[IF_LICM]] ], [ %sub, %[[IF_IF_LICM]] ], [ %mul, %[[ELSE_LICM]] ]
+; CHECK-ENABLED: store i32 %phi, ptr %p
+; CHECK: br label %loop
+
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [0, %entry], [%iv.next, %then]
+ %cmp1 = icmp sgt i32 %x, 0
+ br i1 %cmp1, label %if, label %else
+
+if:
+ %add = add i32 %x, 1
+ %cmp2 = icmp sgt i32 %add, 0
+ br i1 %cmp2, label %if.if, label %then
+
+if.if:
+ %sub = sub i32 %x, 1
+ br label %then
+
+else:
+ %mul = mul i32 %x, 3
+ br label %then
+
+; CHECK-DISABLED-LABEL: then:
+; CHECK-DISABLED: %phi = phi i32 [ %add, %if ], [ %sub, %if.if ], [ %mul, %else ]
+then:
+ %phi = phi i32 [ %add, %if ], [ %sub, %if.if ], [ %mul, %else ]
+ store i32 %phi, ptr %p
+ %iv.next = add i32 %iv, 1
+ %cmp3 = icmp slt i32 %iv.next, 200
+ br i1 %cmp3, label %loop, label %end
+
+end:
+ ret void
+}
+
+; The invariant part of %add.2 is reassociated into the preheader, which is the
+; convergence point of the cloned control flow. %xor is hoisted into the nested
+; cloned control flow first, so it has to be rehoisted more than one level up
+; for it to dominate its new use.
+; CHECK-LABEL: @rehoist_through_nested_control_flow
+define i32 @rehoist_through_nested_control_flow(i32 %a, i1 %c.1, i1 %c.2) {
+; CHECK-LABEL: bb:
+; CHECK: %xor = xor i32 %a, 1
+; CHECK-ENABLED: br i1 %c.1, label %[[LATCH_LICM:.*]], label %[[BODY_1_LICM:.*]]
+
+; CHECK-ENABLED: [[BODY_1_LICM]]:
+; CHECK-ENABLED: br i1 %c.2, label %[[LATCH_LICM]], label %[[BODY_2_LICM:.*]]
+
+; CHECK-ENABLED: [[BODY_2_LICM]]:
+; CHECK-ENABLED: br label %[[LATCH_LICM]]
+
+; CHECK-ENABLED: [[LATCH_LICM]]:
+; CHECK: %invariant.op = add i32 20, %xor
+; CHECK: br label %loop.header
+
+bb:
+ br label %loop.header
+
+loop.header:
+ %iv = phi i32 [ 6, %bb ], [ %iv.next, %loop.latch ]
+ %v = phi i32 [ 35902, %bb ], [ %p, %loop.latch ]
+ br i1 %c.1, label %loop.latch, label %body.1
+
+body.1:
+ %v.add = add i32 %v, 10
+ br i1 %c.2, label %loop.latch, label %body.2
+
+body.2:
+ %add.1 = add i32 %v.add, 20
+ %xor = xor i32 %a, 1
+ %add.2 = add i32 %add.1, %xor
+ br label %loop.latch
+
+loop.latch:
+ %p = phi i32 [ %v, %loop.header ], [ %v.add, %body.1 ], [ %add.2, %body.2 ]
+ %iv.next = add nuw nsw i32 %iv, 1
+ %ec = icmp ult i32 %iv, 181
+ br i1 %ec, label %loop.header, label %exit
+
+exit:
+ %e = phi i32 [ %p, %loop.latch ]
+ ret i32 %e
+}
+
; TODO: This is currently too complicated for us to be able to hoist the phi.
; CHECK-LABEL: @tree_phi
define void @tree_phi(i32 %x, ptr %p) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/225599
More information about the llvm-commits
mailing list