[llvm] [StructurizeCFG] bug fix in zero cost hoist (PR #157969)
Vigneshwar Jayakumar via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 11 07:33:01 PDT 2025
https://github.com/VigneshwarJ updated https://github.com/llvm/llvm-project/pull/157969
>From df43d2998a68a92240aafb286048a175d86e7f3c Mon Sep 17 00:00:00 2001
From: vigneshwar jayakumar <vigneshwar.jayakumar at amd.com>
Date: Wed, 10 Sep 2025 18:05:34 -0500
Subject: [PATCH 1/2] [StructurizeCFG] bug fix in zero cost hoist
This fixes a bug where zero cost instruction was hoisted to nearest
common dominator but the hoisted instruction's operands didn't dominate
the common dominator causing poison values.
---
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | 31 ++++++------
.../StructurizeCFG/hoist-zerocost.ll | 50 +++++++++++++++++++
2 files changed, 67 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
index e05625344ee29..72f96e1a27f0b 100644
--- a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
+++ b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
@@ -308,6 +308,9 @@ class StructurizeCFG {
void hoistZeroCostElseBlockPhiValues(BasicBlock *ElseBB, BasicBlock *ThenBB);
+ bool isHoistableInstruction(Instruction *I, BasicBlock *BB,
+ BasicBlock *HoistTo);
+
void orderNodes();
void analyzeLoops(RegionNode *N);
@@ -415,11 +418,21 @@ class StructurizeCFGLegacyPass : public RegionPass {
} // end anonymous namespace
+char StructurizeCFGLegacyPass::ID = 0;
+
+INITIALIZE_PASS_BEGIN(StructurizeCFGLegacyPass, "structurizecfg",
+ "Structurize the CFG", false, false)
+INITIALIZE_PASS_DEPENDENCY(UniformityInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(RegionInfoPass)
+INITIALIZE_PASS_END(StructurizeCFGLegacyPass, "structurizecfg",
+ "Structurize the CFG", false, false)
+
/// Checks whether an instruction is zero cost instruction and checks if the
/// operands are from different BB. If so, this instruction can be coalesced
/// if its hoisted to predecessor block. So, this returns true.
-static bool isHoistableInstruction(Instruction *I, BasicBlock *BB,
- const TargetTransformInfo *TTI) {
+bool StructurizeCFG::isHoistableInstruction(Instruction *I, BasicBlock *BB,
+ BasicBlock *HoistTo) {
if (I->getParent() != BB || isa<PHINode>(I))
return false;
@@ -435,7 +448,7 @@ static bool isHoistableInstruction(Instruction *I, BasicBlock *BB,
// Check if any operands are instructions defined in the same block.
for (auto &Op : I->operands()) {
if (auto *OpI = dyn_cast<Instruction>(Op)) {
- if (OpI->getParent() == BB)
+ if (OpI->getParent() == BB || !DT->dominates(OpI->getParent(), HoistTo))
return false;
}
}
@@ -443,16 +456,6 @@ static bool isHoistableInstruction(Instruction *I, BasicBlock *BB,
return true;
}
-char StructurizeCFGLegacyPass::ID = 0;
-
-INITIALIZE_PASS_BEGIN(StructurizeCFGLegacyPass, "structurizecfg",
- "Structurize the CFG", false, false)
-INITIALIZE_PASS_DEPENDENCY(UniformityInfoWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(RegionInfoPass)
-INITIALIZE_PASS_END(StructurizeCFGLegacyPass, "structurizecfg",
- "Structurize the CFG", false, false)
-
/// Structurization can introduce unnecessary VGPR copies due to register
/// coalescing interference. For example, if the Else block has a zero-cost
/// instruction and the Then block modifies the VGPR value, only one value is
@@ -478,7 +481,7 @@ void StructurizeCFG::hoistZeroCostElseBlockPhiValues(BasicBlock *ElseBB,
for (PHINode &Phi : ElseSucc->phis()) {
Value *ElseVal = Phi.getIncomingValueForBlock(ElseBB);
auto *Inst = dyn_cast<Instruction>(ElseVal);
- if (!Inst || !isHoistableInstruction(Inst, ElseBB, TTI))
+ if (!Inst || !isHoistableInstruction(Inst, ElseBB, CommonDominator))
continue;
Inst->removeFromParent();
Inst->insertInto(CommonDominator, Term->getIterator());
diff --git a/llvm/test/Transforms/StructurizeCFG/hoist-zerocost.ll b/llvm/test/Transforms/StructurizeCFG/hoist-zerocost.ll
index d084e199ceb89..b118f8189d716 100644
--- a/llvm/test/Transforms/StructurizeCFG/hoist-zerocost.ll
+++ b/llvm/test/Transforms/StructurizeCFG/hoist-zerocost.ll
@@ -209,3 +209,53 @@ merge:
store i32 %phi, ptr %ptr
ret void
}
+
+define void @test_nested_if_2 (i32 %val,ptr %gep, i1 %cond) {
+; CHECK-LABEL: define void @test_nested_if_2(
+; CHECK-SAME: i32 [[VAL:%.*]], ptr [[GEP:%.*]], i1 [[COND:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: [[COND_INV:%.*]] = xor i1 [[COND]], true
+; CHECK-NEXT: [[T825:%.*]] = icmp eq i32 [[VAL]], 0
+; CHECK-NEXT: [[T825_INV:%.*]] = xor i1 [[T825]], true
+; CHECK-NEXT: br i1 [[T825]], label %[[IF:.*]], label %[[FLOW:.*]]
+; CHECK: [[IF]]:
+; CHECK-NEXT: [[LOADED:%.*]] = load [[PAIR:%.*]], ptr [[GEP]], align 4
+; CHECK-NEXT: br label %[[FLOW]]
+; CHECK: [[FLOW1:.*]]:
+; CHECK-NEXT: [[TMP0:%.*]] = phi i1 [ false, %[[ELSE:.*]] ], [ [[TMP2:%.*]], %[[FLOW]] ]
+; CHECK-NEXT: br i1 [[TMP0]], label %[[IF_2:.*]], label %[[EXIT:.*]]
+; CHECK: [[IF_2]]:
+; CHECK-NEXT: [[IF_VALUE:%.*]] = extractvalue [[PAIR]] [[TMP1:%.*]], 0
+; CHECK-NEXT: br label %[[EXIT]]
+; CHECK: [[FLOW]]:
+; CHECK-NEXT: [[TMP1]] = phi [[PAIR]] [ [[LOADED]], %[[IF]] ], [ poison, %[[ENTRY]] ]
+; CHECK-NEXT: [[TMP2]] = phi i1 [ true, %[[IF]] ], [ false, %[[ENTRY]] ]
+; CHECK-NEXT: [[TMP3:%.*]] = phi i1 [ [[COND_INV]], %[[IF]] ], [ [[T825_INV]], %[[ENTRY]] ]
+; CHECK-NEXT: br i1 [[TMP3]], label %[[ELSE]], label %[[FLOW1]]
+; CHECK: [[ELSE]]:
+; CHECK-NEXT: br label %[[FLOW1]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: [[T_SINK168:%.*]] = phi i32 [ 0, %[[FLOW1]] ], [ [[IF_VALUE]], %[[IF_2]] ]
+; CHECK-NEXT: store i32 [[T_SINK168]], ptr [[GEP]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ %t825 = icmp eq i32 %val, 0
+ br i1 %t825, label %if, label %else
+
+if:
+ %loaded = load %pair, ptr %gep
+ br i1 %cond, label %if_2, label %else
+
+if_2:
+ %if_value = extractvalue %pair %loaded, 0
+ br label %exit
+
+else:
+ br label %exit
+
+exit:
+ %phi = phi i32 [ %if_value, %if_2 ], [ 0, %else ]
+ store i32 %phi,ptr %gep
+ ret void
+}
>From 6575ceb2a822664b3a39484e068e9eafb5bffcde Mon Sep 17 00:00:00 2001
From: vigneshwar jayakumar <vigneshwar.jayakumar at amd.com>
Date: Thu, 11 Sep 2025 09:31:57 -0500
Subject: [PATCH 2/2] review
---
llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
index 72f96e1a27f0b..2ee91a9b40026 100644
--- a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
+++ b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
@@ -445,10 +445,11 @@ bool StructurizeCFG::isHoistableInstruction(Instruction *I, BasicBlock *BB,
if (CostVal != 0)
return false;
- // Check if any operands are instructions defined in the same block.
+ // Check if all operands are available at the hoisting destination.
for (auto &Op : I->operands()) {
if (auto *OpI = dyn_cast<Instruction>(Op)) {
- if (OpI->getParent() == BB || !DT->dominates(OpI->getParent(), HoistTo))
+ // Operand must dominate the hoisting destination.
+ if (!DT->dominates(OpI->getParent(), HoistTo))
return false;
}
}
More information about the llvm-commits
mailing list