Skip to content

[InstCombine] Add cast support in simplifyUsingControlFlow #142263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[InstCombine] Add cast support in simplifyUsingControlFlow #142263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1178db4
[InstCombine] Change value to avoid change of purpose of test with ne…
andjo403 May 31, 2025
b9cd63e
[InstCombine] Add cast support in simplifyUsingControlFlow
andjo403 Jun 7, 2025
c5d037c
fixup! [InstCombine] Add cast support in simplifyUsingControlFlow
andjo403 Jun 19, 2025
f3ade79
fixup! [InstCombine] Add cast support in simplifyUsingControlFlow
andjo403 Jun 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 85 additions & 38 deletions llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,8 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN,
// ... ...
// \ /
// phi [v1] [v2]
if (!PN.getType()->isIntegerTy())
return nullptr;
// Make sure all inputs are constants.
if (!all_of(PN.operands(), [](Value *V) { return isa<ConstantInt>(V); }))
return nullptr;
Expand All @@ -1301,6 +1303,7 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN,

// Determine which value the condition of the idom has for which successor.
LLVMContext &Context = PN.getContext();
unsigned PNBitWidth = PN.getType()->getScalarSizeInBits();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to bail out on vector phi nodes. In the future, constant splats will be represented as ConstantInt. Although it is not a real problem since SuccForValue.find(Input) always returns nil...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch have added a early bail out if the phi node is not a integer type

auto *IDom = DT.getNode(BB)->getIDom()->getBlock();
Value *Cond;
SmallDenseMap<ConstantInt *, BasicBlock *, 8> SuccForValue;
Expand All @@ -1314,65 +1317,109 @@ static Value *simplifyUsingControlFlow(InstCombiner &Self, PHINode &PN,
return nullptr;

Cond = BI->getCondition();

if (Cond->getType() != PN.getType())
return nullptr;

AddSucc(ConstantInt::getTrue(Context), BI->getSuccessor(0));
AddSucc(ConstantInt::getFalse(Context), BI->getSuccessor(1));
} else if (auto *SI = dyn_cast<SwitchInst>(IDom->getTerminator())) {
Cond = SI->getCondition();
unsigned CondBitWidth = Cond->getType()->getScalarSizeInBits();
++SuccCount[SI->getDefaultDest()];
for (auto Case : SI->cases())
AddSucc(Case.getCaseValue(), Case.getCaseSuccessor());
for (auto Case : SI->cases()) {
ConstantInt *CaseValue = Case.getCaseValue();
if (CondBitWidth > PNBitWidth) {
CaseValue = ConstantInt::get(
Context, Case.getCaseValue()->getValue().trunc(PNBitWidth));
}
AddSucc(CaseValue, Case.getCaseSuccessor());
}
} else {
return nullptr;
}

if (Cond->getType() != PN.getType())
return nullptr;
unsigned CondBitWidth = Cond->getType()->getScalarSizeInBits();

// Check that edges outgoing from the idom's terminators dominate respective
// inputs of the Phi.
std::optional<bool> Invert;
for (auto Pair : zip(PN.incoming_values(), PN.blocks())) {
auto *Input = cast<ConstantInt>(std::get<0>(Pair));
BasicBlock *Pred = std::get<1>(Pair);
auto IsCorrectInput = [&](ConstantInt *Input) {
// The input needs to be dominated by the corresponding edge of the idom.
// This edge cannot be a multi-edge, as that would imply that multiple
// different condition values follow the same edge.
auto It = SuccForValue.find(Input);
return It != SuccForValue.end() && SuccCount[It->second] == 1 &&
DT.dominates(BasicBlockEdge(IDom, It->second),
BasicBlockEdge(Pred, BB));
};

// Depending on the constant, the condition may need to be inverted.
bool NeedsInvert;
if (IsCorrectInput(Input))
NeedsInvert = false;
else if (IsCorrectInput(cast<ConstantInt>(ConstantExpr::getNot(Input))))
NeedsInvert = true;
else
return nullptr;
auto CheckInputs = [&](Instruction::CastOps CastType) -> std::optional<bool> {
std::optional<bool> Invert;
for (auto Pair : zip(PN.incoming_values(), PN.blocks())) {
auto *Input = cast<ConstantInt>(std::get<0>(Pair));
BasicBlock *Pred = std::get<1>(Pair);

auto IsCorrectInput = [&](ConstantInt *Input) {
if (CondBitWidth < PNBitWidth) {
if ((CastType == Instruction::SExt &&
!Input->getValue().isSignedIntN(CondBitWidth)) ||
(CastType == Instruction::ZExt &&
!Input->getValue().isIntN(CondBitWidth)))
return false;

Input =
ConstantInt::get(Context, Input->getValue().trunc(CondBitWidth));
}
// The input needs to be dominated by the corresponding edge of the
// idom. This edge cannot be a multi-edge, as that would imply that
// multiple different condition values follow the same edge.
auto It = SuccForValue.find(Input);
return It != SuccForValue.end() && SuccCount[It->second] == 1 &&
DT.dominates(BasicBlockEdge(IDom, It->second),
BasicBlockEdge(Pred, BB));
};

// Depending on the constant, the condition may need to be inverted.
bool NeedsInvert;
if (IsCorrectInput(Input))
NeedsInvert = false;
else if (IsCorrectInput(cast<ConstantInt>(ConstantExpr::getNot(Input))))
NeedsInvert = true;
else
return std::nullopt;

// Make sure the inversion requirement is always the same.
if (Invert && *Invert != NeedsInvert)
return nullptr;
// Make sure the inversion requirement is always the same.
if (Invert && *Invert != NeedsInvert)
return std::nullopt;

Invert = NeedsInvert;
Invert = NeedsInvert;
}
return Invert;
};

Instruction::CastOps CastType =
CondBitWidth == PNBitWidth ? Instruction::BitCast
: CondBitWidth < PNBitWidth ? Instruction::ZExt
: Instruction::Trunc;

auto Result = CheckInputs(CastType);
if (!Result && CondBitWidth < PNBitWidth) {
CastType = Instruction::SExt;
Result = CheckInputs(CastType);
}
if (!Result)
return nullptr;
bool Invert = *Result;

if (!*Invert)
if (!Invert && CastType == Instruction::BitCast)
return Cond;

// This Phi is actually opposite to branching condition of IDom. We invert
// the condition that will potentially open up some opportunities for
// sinking.
auto InsertPt = BB->getFirstInsertionPt();
if (InsertPt != BB->end()) {
Self.Builder.SetInsertPoint(&*BB, InsertPt);
return Self.Builder.CreateNot(Cond);
if (InsertPt == BB->end())
return nullptr;

Self.Builder.SetInsertPoint(&*BB, InsertPt);

if (CastType != Instruction::BitCast) {
Cond = Self.Builder.CreateCast(CastType, Cond, PN.getType());
if (!Invert)
return Cond;
}

return nullptr;
// This Phi is actually opposite to branching condition of IDom. We invert
// the condition that will potentially open up some opportunities for
// sinking.
return Self.Builder.CreateNot(Cond);
}

// Fold iv = phi(start, iv.next = iv2.next op start)
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/DebugInfo/X86/instcombine-fold-cast-into-phi.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
;; user (otherwise the dbg use becomes poison after the original phi is
;; deleted). Check the new phi inherits the DebugLoc.

; CHECK: %[[phi:.*]] = phi i8 [ 1, %{{.*}} ], [ 0, %{{.*}} ], !dbg ![[dbg:[0-9]+]]
; CHECK: %[[phi:.*]] = phi i8 [ 2, %{{.*}} ], [ 0, %{{.*}} ], !dbg ![[dbg:[0-9]+]]
; CHECK: #dbg_value(i8 %[[phi]], ![[#]], !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)
; CHECK: ![[dbg]] = !DILocation(line: 123,

Expand All @@ -19,7 +19,7 @@ if.then: ; preds = entry
br label %if.end

if.end: ; preds = %if.then, %entry
%p.0 = phi i32 [ 1, %if.then ], [ 0, %entry ], !dbg !13
%p.0 = phi i32 [ 2, %if.then ], [ 0, %entry ], !dbg !13
call void @llvm.dbg.value(metadata i32 %p.0, metadata !4, metadata !DIExpression()), !dbg !13
%x = trunc i32 %p.0 to i8
%callff = call float @ff(i8 %x)
Expand Down
73 changes: 55 additions & 18 deletions llvm/test/Transforms/InstCombine/simple_phi_condition.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ define i32 @test_phi_to_zext(i8 noundef %0) {
; CHECK: sw.1:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ 1, [[SW_1]] ], [ 0, [[SW_0]] ], [ 255, [[SW_255]] ]
; CHECK-NEXT: [[DOT0:%.*]] = zext i8 [[TMP0]] to i32
; CHECK-NEXT: ret i32 [[DOT0]]
;
entry:
Expand Down Expand Up @@ -713,7 +713,8 @@ define i32 @test_phi_to_zext_inverted(i8 noundef %0) {
; CHECK: sw.1:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ -256, [[SW_255]] ], [ -2, [[SW_1]] ], [ -1, [[SW_0]] ]
; CHECK-NEXT: [[TMP1:%.*]] = zext i8 [[TMP0]] to i32
; CHECK-NEXT: [[DOT0:%.*]] = xor i32 [[TMP1]], -1
; CHECK-NEXT: ret i32 [[DOT0]]
;
entry:
Expand All @@ -740,27 +741,44 @@ merge:
ret i32 %.0
}

define i8 @test_multiple_predecessors_phi_to_zext(i1 %cond, i1 %cond2) {
define i8 @test_multiple_predecessors_phi_to_zext(i2 %cond, i2 %cond2) {
; CHECK-LABEL: @test_multiple_predecessors_phi_to_zext(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]]
; CHECK-NEXT: switch i2 [[COND:%.*]], label [[DEFAULT:%.*]] [
; CHECK-NEXT: i2 1, label [[IF_TRUE:%.*]]
; CHECK-NEXT: i2 0, label [[IF_FALSE:%.*]]
; CHECK-NEXT: ]
; CHECK: if.true:
; CHECK-NEXT: br i1 [[COND2:%.*]], label [[IF2_TRUE:%.*]], label [[IF2_FALSE:%.*]]
; CHECK-NEXT: switch i2 [[COND2:%.*]], label [[DEFAULT]] [
; CHECK-NEXT: i2 1, label [[IF2_TRUE:%.*]]
; CHECK-NEXT: i2 0, label [[IF2_FALSE:%.*]]
; CHECK-NEXT: ]
; CHECK: default:
; CHECK-NEXT: unreachable
; CHECK: if.false:
; CHECK-NEXT: br label [[MERGE:%.*]]
; CHECK: if2.true:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: if2.false:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 0, [[IF_FALSE]] ], [ 1, [[IF2_TRUE]] ], [ 1, [[IF2_FALSE]] ]
; CHECK-NEXT: [[RET:%.*]] = zext i2 [[COND]] to i8
; CHECK-NEXT: ret i8 [[RET]]
;
entry:
br i1 %cond, label %if.true, label %if.false
switch i2 %cond, label %default [
i2 1, label %if.true
i2 0, label %if.false
]

if.true:
br i1 %cond2, label %if2.true, label %if2.false
switch i2 %cond2, label %default [
i2 1, label %if2.true
i2 0, label %if2.false
]

default:
unreachable

if.false:
br label %merge
Expand Down Expand Up @@ -793,7 +811,7 @@ define i32 @test_phi_to_sext(i8 noundef %0) {
; CHECK: sw.1:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ 1, [[SW_1]] ], [ 0, [[SW_0]] ], [ -1, [[SW_255]] ]
; CHECK-NEXT: [[DOT0:%.*]] = sext i8 [[TMP0]] to i32
; CHECK-NEXT: ret i32 [[DOT0]]
;
entry:
Expand Down Expand Up @@ -837,7 +855,8 @@ define i32 @test_phi_to_sext_inverted(i8 noundef %0) {
; CHECK: sw.1:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[DOT0:%.*]] = phi i32 [ -2, [[SW_1]] ], [ -1, [[SW_0]] ], [ 0, [[SW_255]] ]
; CHECK-NEXT: [[TMP1:%.*]] = xor i8 [[TMP0]], -1
; CHECK-NEXT: [[DOT0:%.*]] = sext i8 [[TMP1]] to i32
; CHECK-NEXT: ret i32 [[DOT0]]
;
entry:
Expand All @@ -864,27 +883,44 @@ merge:
ret i32 %.0
}

define i8 @test_multiple_predecessors_phi_to_sext(i1 %cond, i1 %cond2) {
define i8 @test_multiple_predecessors_phi_to_sext(i2 %cond, i2 %cond2) {
; CHECK-LABEL: @test_multiple_predecessors_phi_to_sext(
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]]
; CHECK-NEXT: switch i2 [[COND:%.*]], label [[DEFAULT:%.*]] [
; CHECK-NEXT: i2 -1, label [[IF_TRUE:%.*]]
; CHECK-NEXT: i2 0, label [[IF_FALSE:%.*]]
; CHECK-NEXT: ]
; CHECK: if.true:
; CHECK-NEXT: br i1 [[COND2:%.*]], label [[IF2_TRUE:%.*]], label [[IF2_FALSE:%.*]]
; CHECK-NEXT: switch i2 [[COND2:%.*]], label [[DEFAULT]] [
; CHECK-NEXT: i2 -1, label [[IF2_TRUE:%.*]]
; CHECK-NEXT: i2 0, label [[IF2_FALSE:%.*]]
; CHECK-NEXT: ]
; CHECK: default:
; CHECK-NEXT: unreachable
; CHECK: if.false:
; CHECK-NEXT: br label [[MERGE:%.*]]
; CHECK: if2.true:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: if2.false:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[RET:%.*]] = phi i8 [ 0, [[IF_FALSE]] ], [ -1, [[IF2_TRUE]] ], [ -1, [[IF2_FALSE]] ]
; CHECK-NEXT: [[RET:%.*]] = sext i2 [[COND]] to i8
; CHECK-NEXT: ret i8 [[RET]]
;
entry:
br i1 %cond, label %if.true, label %if.false
switch i2 %cond, label %default [
i2 3, label %if.true
i2 0, label %if.false
]

if.true:
br i1 %cond2, label %if2.true, label %if2.false
switch i2 %cond2, label %default [
i2 3, label %if2.true
i2 0, label %if2.false
]

default:
unreachable

if.false:
br label %merge
Expand Down Expand Up @@ -995,7 +1031,7 @@ define i8 @test_phi_to_trunc(i32 %0) {
; CHECK: sw.255:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[DOT0:%.*]] = phi i8 [ -1, [[SW_255]] ], [ 1, [[SW_1]] ], [ 0, [[ENTRY:%.*]] ]
; CHECK-NEXT: [[DOT0:%.*]] = trunc i32 [[TMP0]] to i8
; CHECK-NEXT: ret i8 [[DOT0]]
;
entry:
Expand Down Expand Up @@ -1034,7 +1070,8 @@ define i8 @test_phi_to_trunc_inverted(i32 %0) {
; CHECK: sw.255:
; CHECK-NEXT: br label [[MERGE]]
; CHECK: merge:
; CHECK-NEXT: [[DOT0:%.*]] = phi i8 [ 0, [[SW_255]] ], [ -2, [[SW_1]] ], [ -1, [[ENTRY:%.*]] ]
; CHECK-NEXT: [[TMP1:%.*]] = trunc i32 [[TMP0]] to i8
; CHECK-NEXT: [[DOT0:%.*]] = xor i8 [[TMP1]], -1
; CHECK-NEXT: ret i8 [[DOT0]]
;
entry:
Expand Down
Loading