Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
165 changes: 165 additions & 0 deletions src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1793,6 +1793,171 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block)

return true;
}
else if (block->GetSwitchTargets()->GetSuccCount() == 2 && block->GetSwitchTargets()->HasDefaultCase() &&
(block->IsLIR() || fgNodeThreading == NodeThreading::AllTrees))
Copy link
Member

Choose a reason for hiding this comment

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

Can we only do this for !block->IsLIR() I don't think it's beneficial to support LIR (Lower will handle such switches anyway). So we can simplify code by removing special cases for IsLIR()

{
// If all non-default cases jump to one target and the default jumps to a different target,
// replace the switch with an unsigned comparison against the max case index:
// GT_SWITCH(switchVal) -> GT_JTRUE(GT_LE/GT_GT(switchVal, caseCount - 2))
// The comparison direction is chosen to favor fall-through to the next block.
// When both targets are simple return blocks, fgFoldCondToReturnBlock can further
// convert this into branchless codegen like "cmp; setbe" instead of a jump table.

BBswtDesc* switchDesc = block->GetSwitchTargets();
unsigned caseCount = switchDesc->GetCaseCount();

// For small switches with 2 or fewer non-default cases (caseCount <= 3 including the default),
// the backend's switch lowering already generates efficient comparison chains. Converting
// early can enable if-conversion (cmov) that produces worse code for these cases.
if (caseCount <= 3)
Copy link
Member

Choose a reason for hiding this comment

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

Is it the case or just an assumption?

{
return modified;
}

FlowEdge* defaultEdge = switchDesc->GetDefaultCase();
BasicBlock* defaultDest = defaultEdge->getDestinationBlock();

// Check that all non-default cases share the same target, distinct from the default target.
FlowEdge* firstCaseEdge = switchDesc->GetCase(0);
BasicBlock* caseDest = firstCaseEdge->getDestinationBlock();

if (caseDest != defaultDest)
{
bool allCasesSameTarget = true;
for (unsigned i = 1; i < caseCount - 1; i++)
Copy link
Member

Choose a reason for hiding this comment

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

I think you only need to check defaultEdge->getDupCount() being 1 here

{
if (switchDesc->GetCase(i)->getDestinationBlock() != caseDest)
{
allCasesSameTarget = false;
break;
}
}

if (allCasesSameTarget)
{
GenTree* switchVal = switchTree->AsOp()->gtOp1;
noway_assert(genActualTypeIsIntOrI(switchVal->TypeGet()));

// If we are in LIR, remove the jump table from the block.
if (block->IsLIR())
{
GenTree* jumpTable = switchTree->AsOp()->gtOp2;
assert(jumpTable->OperIs(GT_JMPTABLE));
blockRange->Remove(jumpTable);
}

// The highest case index is caseCount - 2 (caseCount includes the default case).
// Using an unsigned GT comparison handles negative values correctly because they
// wrap to large unsigned values, making them greater than maxCaseIndex as expected.
const unsigned maxCaseIndex = caseCount - 2;
Copy link
Member

Choose a reason for hiding this comment

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

I think you can avoid this math and the explanation by using maxCaseIndex = firstCaseEdge->getDupCount() and then GT_LT instead of GT_LE


JITDUMP("\nConverting a switch (" FMT_BB
") where all non-default cases target the same block to a "
"conditional branch. Before:\n",
block->bbNum);
DISPNODE(switchTree);

switchTree->ChangeOper(GT_JTRUE);
GenTree* maxCaseNode = gtNewIconNode(maxCaseIndex, genActualType(switchVal->TypeGet()));

// Choose the comparison direction so the fall-through (false edge)
// targets the lexically next block when possible.
GenTree* condNode;
FlowEdge* trueEdge;
FlowEdge* falseEdge;

if (block->NextIs(defaultDest))
Copy link
Member

@EgorBo EgorBo Feb 19, 2026

Choose a reason for hiding this comment

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

We generally avoid relying on lexical order, you can delete this if and unconditionally use the GT_LE path. We used to do it, but it's no longer required.

{
// defaultDest is next: use GT_LE so false (out of range) falls through
// to defaultDest
condNode = gtNewOperNode(GT_LE, TYP_INT, switchVal, maxCaseNode);
trueEdge = firstCaseEdge;
falseEdge =
(switchDesc->GetSucc(0) == firstCaseEdge) ? switchDesc->GetSucc(1) : switchDesc->GetSucc(0);
}
else
{
// caseDest is next, or neither is next: use GT_GT so false (in range)
// falls through to caseDest, which is typically the hotter path
condNode = gtNewOperNode(GT_GT, TYP_INT, switchVal, maxCaseNode);
trueEdge =
(switchDesc->GetSucc(0) == firstCaseEdge) ? switchDesc->GetSucc(1) : switchDesc->GetSucc(0);
falseEdge = firstCaseEdge;
}

assert(trueEdge != nullptr);
assert(falseEdge != nullptr);

condNode->SetUnsigned();
switchTree->AsOp()->gtOp1 = condNode;
switchTree->AsOp()->gtOp1->gtFlags |= (GTF_RELOP_JMP_USED | GTF_DONT_CSE);

if (block->IsLIR())
{
blockRange->InsertAfter(switchVal, maxCaseNode, condNode);
LIR::ReadOnlyRange range(maxCaseNode, switchTree);
m_pLowering->LowerRange(block, range);
}
else if (fgNodeThreading == NodeThreading::AllTrees)
{
gtSetStmtInfo(switchStmt);
fgSetStmtSeq(switchStmt);
}

// Fix up dup counts: multiple switch cases originally pointed to the same
// successor, but the conditional branch has exactly one edge per target.
const unsigned trueDupCount = trueEdge->getDupCount();
const unsigned falseDupCount = falseEdge->getDupCount();

if (trueDupCount > 1)
Copy link
Member

Choose a reason for hiding this comment

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

I don't follow this logic. We basically only need to adjust the firstCaseEdge's dupCount - set it to 1. and adjust bbRefs of firstCaseEdge's destionation block.
The defaultEdge and its destionation block is unchanged.

{
trueEdge->decrementDupCount(trueDupCount - 1);
trueEdge->getDestinationBlock()->bbRefs -= (trueDupCount - 1);
}
if (falseDupCount > 1)
{
falseEdge->decrementDupCount(falseDupCount - 1);
falseEdge->getDestinationBlock()->bbRefs -= (falseDupCount - 1);
}

block->SetCond(trueEdge, falseEdge);

// The switch-to-cond conversion preserved edge likelihoods but
// successor block weights may be stale (they were set during import
// based on the original switch topology). Recompute them so that
// downstream passes like block compaction see correct weights.
if (block->hasProfileWeight())
{
if (caseDest->hasProfileWeight())
{
weight_t oldWeight = caseDest->bbWeight;
caseDest->setBBProfileWeight(caseDest->computeIncomingWeight());
JITDUMP("Updated " FMT_BB " (caseDest) profile weight from " FMT_WT " to " FMT_WT "\n",
caseDest->bbNum, oldWeight, caseDest->bbWeight);
}
if (defaultDest->hasProfileWeight())
{
weight_t oldWeight = defaultDest->bbWeight;
defaultDest->setBBProfileWeight(defaultDest->computeIncomingWeight());
JITDUMP("Updated " FMT_BB " (defaultDest) profile weight from " FMT_WT " to " FMT_WT "\n",
defaultDest->bbNum, oldWeight, defaultDest->bbWeight);
}
}

JITDUMP("After:\n");
DISPNODE(switchTree);

if (fgFoldCondToReturnBlock(block))
{
JITDUMP("Folded conditional return into branchless return. After:\n");
DISPNODE(switchTree);
}

return true;
}
}
}

return modified;
}

Expand Down
48 changes: 48 additions & 0 deletions src/tests/JIT/opt/OptSwitchRecognition/optSwitchRecognition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,52 @@ private static int RecSwitchSkipBitTest(int arch)
[InlineData(6, 4)]
[InlineData(10, 1)]
public static void TestRecSwitchSkipBitTest(int arg1, int expected) => Assert.Equal(expected, RecSwitchSkipBitTest(arg1));

// Test that consecutive equality comparisons (comparison chain) produce the same result
// as pattern matching. The switch recognition should convert the chain to a switch, and
// then fgOptimizeSwitchBranches should simplify it to an unsigned range comparison.
[MethodImpl(MethodImplOptions.NoInlining)]
private static bool IsLetterCategoryCompare(int uc)
{
return uc == 0
|| uc == 1
|| uc == 2
|| uc == 3
|| uc == 4;
}

[Theory]
[InlineData(-1, false)]
[InlineData(0, true)]
[InlineData(1, true)]
[InlineData(2, true)]
[InlineData(3, true)]
[InlineData(4, true)]
[InlineData(5, false)]
[InlineData(100, false)]
[InlineData(int.MinValue, false)]
[InlineData(int.MaxValue, false)]
public static void TestSwitchToRangeCheck(int arg1, bool expected) => Assert.Equal(expected, IsLetterCategoryCompare(arg1));

// Test with non-zero-based consecutive values
[MethodImpl(MethodImplOptions.NoInlining)]
private static bool IsInRange10To14(int val)
{
return val == 10
|| val == 11
|| val == 12
|| val == 13
|| val == 14;
}

[Theory]
[InlineData(9, false)]
[InlineData(10, true)]
[InlineData(11, true)]
[InlineData(12, true)]
[InlineData(13, true)]
[InlineData(14, true)]
[InlineData(15, false)]
[InlineData(-1, false)]
public static void TestSwitchToRangeCheckNonZeroBased(int arg1, bool expected) => Assert.Equal(expected, IsInRange10To14(arg1));
Comment on lines +169 to +189
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

This “non-zero-based consecutive values” test uses 10..14, but SwitchRecognition forces minValue=0 when maxValue <= SWITCH_MAX_DISTANCE, so the recognized switch will likely cover 0..14 with many entries targeting the false block. That shape is not eligible for the new switch→range-check simplification (which requires all non-default cases to share one target). If the intent is to exercise the new optimization, consider using a range where maxValue > SWITCH_MAX_DISTANCE (e.g., 100..104) so recognition keeps a non-zero minValue and emits a dense 0..4 switch after subtracting.

Suggested change
// Test with non-zero-based consecutive values
[MethodImpl(MethodImplOptions.NoInlining)]
private static bool IsInRange10To14(int val)
{
return val == 10
|| val == 11
|| val == 12
|| val == 13
|| val == 14;
}
[Theory]
[InlineData(9, false)]
[InlineData(10, true)]
[InlineData(11, true)]
[InlineData(12, true)]
[InlineData(13, true)]
[InlineData(14, true)]
[InlineData(15, false)]
[InlineData(-1, false)]
public static void TestSwitchToRangeCheckNonZeroBased(int arg1, bool expected) => Assert.Equal(expected, IsInRange10To14(arg1));
// Test with non-zero-based consecutive values (range 100 to 104)
[MethodImpl(MethodImplOptions.NoInlining)]
private static bool IsInRange100To104(int val)
{
return val == 100
|| val == 101
|| val == 102
|| val == 103
|| val == 104;
}
[Theory]
[InlineData(99, false)]
[InlineData(100, true)]
[InlineData(101, true)]
[InlineData(102, true)]
[InlineData(103, true)]
[InlineData(104, true)]
[InlineData(105, false)]
[InlineData(-1, false)]
public static void TestSwitchToRangeCheckNonZeroBased(int arg1, bool expected) => Assert.Equal(expected, IsInRange100To104(arg1));

Copilot uses AI. Check for mistakes.
}
Loading