Skip to content

Commit

Permalink
JIT: Remove lexical dependencies in switch recognition (dotnet#110253)
Browse files Browse the repository at this point in the history
Fixes dotnet#107076. Part of dotnet#107749. Instead of relying on the "not equal" target of each test block being the next block, explicitly follow the chain of "not equal" targets.
  • Loading branch information
amanasifkhalid authored Nov 29, 2024
1 parent aba8199 commit 150d314
Showing 1 changed file with 20 additions and 10 deletions.
30 changes: 20 additions & 10 deletions src/coreclr/jit/switchrecognition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ PhaseStatus Compiler::optSwitchRecognition()
// Arguments:
// block - The block to check
// allowSideEffects - is variableNode allowed to have side-effects (COMMA)?
// trueEdge - [out] The successor edge taken if X == CNS
// falseEdge - [out] The successor edge taken if X != CNS
// trueTarget - [out] The successor visited if X == CNS
// falseTarget - [out] The successor visited if X != CNS
// isReversed - [out] True if the condition is reversed (GT_NE)
// variableNode - [out] The variable node (X in the example above)
// cns - [out] The constant value (CNS in the example above)
Expand Down Expand Up @@ -176,13 +176,12 @@ bool Compiler::optSwitchDetectAndConvert(BasicBlock* firstBlock)
weight_t falseLikelihood = firstBlock->GetFalseEdge()->getLikelihood();
const BasicBlock* prevBlock = firstBlock;

// Now walk the next blocks and see if they are basically the same type of test
for (const BasicBlock* currBb = firstBlock->Next(); currBb != nullptr; currBb = currBb->Next())
// Now walk the chain of test blocks, and see if they are basically the same type of test
for (BasicBlock *currBb = falseTarget, *currFalseTarget; currBb != nullptr; currBb = currFalseTarget)
{
GenTree* currVariableNode = nullptr;
ssize_t currCns = 0;
BasicBlock* currTrueTarget = nullptr;
BasicBlock* currFalseTarget = nullptr;

if (!currBb->hasSingleStmt())
{
Expand Down Expand Up @@ -323,9 +322,15 @@ bool Compiler::optSwitchConvert(

// Find the last block in the chain
const BasicBlock* lastBlock = firstBlock;
for (int i = 0; i < testsCount - 1; i++)
for (int i = 0; i < (testsCount - 1); i++)
{
lastBlock = lastBlock->Next();
const GenTree* rootNode = lastBlock->lastStmt()->GetRootNode();
assert(lastBlock->KindIs(BBJ_COND));
assert(rootNode->OperIs(GT_JTRUE));

// We only support reversed tests (GT_NE) in the last block of the chain.
assert(rootNode->gtGetOp1()->OperIs(GT_EQ));
lastBlock = lastBlock->GetFalseTarget();
}

BasicBlock* blockIfTrue = nullptr;
Expand Down Expand Up @@ -359,10 +364,13 @@ bool Compiler::optSwitchConvert(
// Unlink and remove the whole chain of conditional blocks
fgRemoveRefPred(falseEdge);
BasicBlock* blockToRemove = falseEdge->getDestinationBlock();
assert(firstBlock->NextIs(blockToRemove));
while (!lastBlock->NextIs(blockToRemove))
for (int i = 0; i < (testsCount - 1); i++)
{
blockToRemove = fgRemoveBlock(blockToRemove, true);
// We always follow the false target because reversed tests are only supported for the last block.
assert(blockToRemove->KindIs(BBJ_COND));
BasicBlock* const nextBlockToRemove = blockToRemove->GetFalseTarget();
fgRemoveBlock(blockToRemove, true);
blockToRemove = nextBlockToRemove;
}

const unsigned jumpCount = static_cast<unsigned>(maxValue - minValue + 1);
Expand Down Expand Up @@ -405,6 +413,8 @@ bool Compiler::optSwitchConvert(
}
}

assert(switchTrueEdge != nullptr);

// Link the 'default' case
FlowEdge* const switchDefaultEdge = fgAddRefPred(blockIfFalse, firstBlock);
jmpTab[jumpCount] = switchDefaultEdge;
Expand Down

0 comments on commit 150d314

Please sign in to comment.