Skip to content

JIT: Store unique switch successors in BBswtDesc #117423

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

Merged
merged 6 commits into from
Jul 16, 2025
Merged

JIT: Store unique switch successors in BBswtDesc #117423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5019a46
BBehfDesc -> BBJumpTable
amanasifkhalid Jul 3, 2025
c334fe2
BBswtDesc refactor
amanasifkhalid Jul 8, 2025
4cddaf0
SwitchCases -> SwitchSuccs
amanasifkhalid Jul 15, 2025
0173321
Remove SwitchCases iterator
amanasifkhalid Jul 15, 2025
126d4b3
Add comment
amanasifkhalid Jul 15, 2025
2507319
Merge branch 'main' into switch-desc-refactor
amanasifkhalid Jul 15, 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
15 changes: 8 additions & 7 deletions src/coreclr/jit/assertionprop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5633,8 +5633,8 @@ bool Compiler::optCreateJumpTableImpliedAssertions(BasicBlock* switchBb)
GenTree* switchTree = switchBb->lastStmt()->GetRootNode()->gtEffectiveVal();
assert(switchTree->OperIs(GT_SWITCH));

// bbsCount is uint32_t, but it's unlikely to be more than INT32_MAX.
noway_assert(switchBb->GetSwitchTargets()->bbsCount <= INT32_MAX);
// Case count is uint32_t, but it's unlikely to be more than INT32_MAX.
noway_assert(switchBb->GetSwitchTargets()->GetCaseCount() <= INT32_MAX);

ValueNum opVN = optConservativeNormalVN(switchTree->gtGetOp1());
if (opVN == ValueNumStore::NoVN)
Expand All @@ -5652,9 +5652,9 @@ bool Compiler::optCreateJumpTableImpliedAssertions(BasicBlock* switchBb)
int offset = 0;
vnStore->PeelOffsetsI32(&opVN, &offset);

int jumpCount = static_cast<int>(switchBb->GetSwitchTargets()->bbsCount);
FlowEdge** jumpTable = switchBb->GetSwitchTargets()->bbsDstTab;
bool hasDefault = switchBb->GetSwitchTargets()->bbsHasDefault;
int jumpCount = static_cast<int>(switchBb->GetSwitchTargets()->GetCaseCount());
FlowEdge** jumpTable = switchBb->GetSwitchTargets()->GetCases();
bool hasDefault = switchBb->GetSwitchTargets()->HasDefaultCase();

for (int jmpTargetIdx = 0; jmpTargetIdx < jumpCount; jmpTargetIdx++)
{
Expand All @@ -5666,14 +5666,15 @@ bool Compiler::optCreateJumpTableImpliedAssertions(BasicBlock* switchBb)
int value = jmpTargetIdx - offset;

// We can only make "X == caseValue" assertions for blocks with a single edge from the switch.
BasicBlock* target = jumpTable[jmpTargetIdx]->getDestinationBlock();
FlowEdge* const edge = jumpTable[jmpTargetIdx];
BasicBlock* const target = edge->getDestinationBlock();
if (target->GetUniquePred(this) != switchBb)
{
// Target block is potentially reachable from multiple blocks (outside the switch).
continue;
}

if (fgGetPredForBlock(target, switchBb)->getDupCount() > 1)
if (edge->getDupCount() > 1)
{
// We have just one predecessor (BBJ_SWITCH), but there may be multiple edges (cases) per target.
continue;
Expand Down
31 changes: 19 additions & 12 deletions src/coreclr/jit/async.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,11 @@ BasicBlock* Compiler::InsertTryFinallyForContextRestore(BasicBlock* block, State
block->SetTargetEdge(fgAddRefPred(callFinally, block));
callFinally->SetTargetEdge(fgAddRefPred(finallyRet, callFinally));

BBehfDesc* ehfDesc = new (this, CMK_BasicBlock) BBehfDesc;
ehfDesc->bbeCount = 1;
ehfDesc->bbeSuccs = new (this, CMK_BasicBlock) FlowEdge* [1] {
FlowEdge** succs = new (this, CMK_BasicBlock) FlowEdge* [1] {
fgAddRefPred(callFinallyRet, finallyRet)
};
ehfDesc->bbeSuccs[0]->setLikelihood(1.0);
succs[0]->setLikelihood(1.0);
BBJumpTable* ehfDesc = new (this, CMK_BasicBlock) BBJumpTable(succs, 1);
finallyRet->SetEhfTargets(ehfDesc);

callFinallyRet->SetTargetEdge(fgAddRefPred(goToTailBlock, callFinallyRet));
Expand Down Expand Up @@ -2288,18 +2287,26 @@ void AsyncTransformation::CreateResumptionSwitch()

// Default case. TODO-CQ: Support bbsHasDefault = false before lowering.
m_resumptionBBs.push_back(m_resumptionBBs[0]);
BBswtDesc* swtDesc = new (m_comp, CMK_BasicBlock) BBswtDesc;
swtDesc->bbsCount = (unsigned)m_resumptionBBs.size();
swtDesc->bbsHasDefault = true;
swtDesc->bbsDstTab = new (m_comp, CMK_Async) FlowEdge*[m_resumptionBBs.size()];
const size_t numCases = m_resumptionBBs.size();
FlowEdge** const cases = new (m_comp, CMK_FlowEdge) FlowEdge*[numCases * 2];
FlowEdge** const succs = cases + numCases;
unsigned numUniqueSuccs = 0;

weight_t stateLikelihood = 1.0 / m_resumptionBBs.size();
for (size_t i = 0; i < m_resumptionBBs.size(); i++)
const weight_t stateLikelihood = 1.0 / m_resumptionBBs.size();
for (size_t i = 0; i < numCases; i++)
{
swtDesc->bbsDstTab[i] = m_comp->fgAddRefPred(m_resumptionBBs[i], switchBB);
swtDesc->bbsDstTab[i]->setLikelihood(stateLikelihood);
FlowEdge* const edge = m_comp->fgAddRefPred(m_resumptionBBs[i], switchBB);
edge->setLikelihood(stateLikelihood);
cases[i] = edge;

if (edge->getDupCount() == 1)
{
succs[numUniqueSuccs++] = edge;
}
}

BBswtDesc* const swtDesc =
new (m_comp, CMK_BasicBlock) BBswtDesc(cases, (unsigned)numCases, succs, numUniqueSuccs, true);
switchBB->SetSwitch(swtDesc);
}

Expand Down
120 changes: 30 additions & 90 deletions src/coreclr/jit/block.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,39 +586,10 @@ unsigned BasicBlock::dspPreds() const
void BasicBlock::dspSuccs(Compiler* compiler)
{
bool first = true;

// If this is a switch, we don't want to call `Succs(Compiler*)` because it will eventually call
// `GetSwitchDescMap()`, and that will have the side-effect of allocating the unique switch descriptor map
// and/or compute this switch block's unique succ set if it is not present. Debug output functions should
// never have an effect on codegen. We also don't want to assume the unique succ set is accurate, so we
// compute it ourselves here.
if (bbKind == BBJ_SWITCH)
{
// Create a set with all the successors.
unsigned bbNumMax = compiler->fgBBNumMax;
BitVecTraits bitVecTraits(bbNumMax + 1, compiler);
BitVec uniqueSuccBlocks(BitVecOps::MakeEmpty(&bitVecTraits));
for (BasicBlock* const bTarget : SwitchTargets())
{
BitVecOps::AddElemD(&bitVecTraits, uniqueSuccBlocks, bTarget->bbNum);
}
BitVecOps::Iter iter(&bitVecTraits, uniqueSuccBlocks);
unsigned bbNum = 0;
while (iter.NextElem(&bbNum))
{
// Note that we will output switch successors in increasing numerical bbNum order, which is
// not related to their order in the bbSwtTargets->bbsDstTab table.
printf("%s" FMT_BB, first ? "" : ",", bbNum);
first = false;
}
}
else
for (const BasicBlock* const succ : Succs(compiler))
{
for (const BasicBlock* const succ : Succs(compiler))
{
printf("%s" FMT_BB, first ? "" : ",", succ->bbNum);
first = false;
}
printf("%s" FMT_BB, first ? "" : ",", succ->bbNum);
first = false;
}
}

Expand Down Expand Up @@ -672,12 +643,9 @@ void BasicBlock::dspKind() const
}
else
{
const unsigned jumpCnt = bbEhfTargets->bbeCount;
FlowEdge** const jumpTab = bbEhfTargets->bbeSuccs;

for (unsigned i = 0; i < jumpCnt; i++)
for (unsigned i = 0; i < bbEhfTargets->GetSuccCount(); i++)
{
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(bbEhfTargets->GetSucc(i)));
}
}

Expand Down Expand Up @@ -736,20 +704,20 @@ void BasicBlock::dspKind() const
{
printf(" ->");

const unsigned jumpCnt = bbSwtTargets->bbsCount;
FlowEdge** const jumpTab = bbSwtTargets->bbsDstTab;
const unsigned jumpCnt = bbSwtTargets->GetCaseCount();
FlowEdge** const jumpTab = bbSwtTargets->GetCases();

for (unsigned i = 0; i < jumpCnt; i++)
{
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));

const bool isDefault = bbSwtTargets->bbsHasDefault && (i == jumpCnt - 1);
const bool isDefault = bbSwtTargets->HasDefaultCase() && (i == jumpCnt - 1);
if (isDefault)
{
printf("[def]");
}

const bool isDominant = bbSwtTargets->bbsHasDominantCase && (i == bbSwtTargets->bbsDominantCase);
const bool isDominant = bbSwtTargets->HasDominantCase() && (i == bbSwtTargets->GetDominantCase());
if (isDominant)
{
printf("[dom]");
Expand Down Expand Up @@ -1184,10 +1152,10 @@ unsigned BasicBlock::NumSucc() const
return 0;
}

return bbEhfTargets->bbeCount;
return bbEhfTargets->GetSuccCount();

case BBJ_SWITCH:
return bbSwtTargets->bbsCount;
return bbSwtTargets->GetCaseCount();

default:
unreached();
Expand Down Expand Up @@ -1229,10 +1197,10 @@ FlowEdge* BasicBlock::GetSuccEdge(unsigned i) const
}

case BBJ_EHFINALLYRET:
return bbEhfTargets->bbeSuccs[i];
return bbEhfTargets->GetSucc(i);

case BBJ_SWITCH:
return bbSwtTargets->bbsDstTab[i];
return bbSwtTargets->GetCase(i);

default:
unreached();
Expand Down Expand Up @@ -1288,7 +1256,7 @@ unsigned BasicBlock::NumSucc(Compiler* comp)
return 0;
}

return bbEhfTargets->bbeCount;
return bbEhfTargets->GetSuccCount();

case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
Expand All @@ -1309,10 +1277,7 @@ unsigned BasicBlock::NumSucc(Compiler* comp)
}

case BBJ_SWITCH:
{
Compiler::SwitchUniqueSuccSet sd = comp->GetDescriptorForSwitch(this);
return sd.numDistinctSuccs;
}
return bbSwtTargets->GetSuccCount();

default:
unreached();
Expand Down Expand Up @@ -1343,8 +1308,7 @@ FlowEdge* BasicBlock::GetSuccEdge(unsigned i, Compiler* comp)

case BBJ_EHFINALLYRET:
assert(bbEhfTargets != nullptr);
assert(i < bbEhfTargets->bbeCount);
return bbEhfTargets->bbeSuccs[i];
return bbEhfTargets->GetSucc(i);

case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
Expand All @@ -1366,11 +1330,7 @@ FlowEdge* BasicBlock::GetSuccEdge(unsigned i, Compiler* comp)
}

case BBJ_SWITCH:
{
Compiler::SwitchUniqueSuccSet sd = comp->GetDescriptorForSwitch(this);
assert(i < sd.numDistinctSuccs); // Range check.
return sd.nonDuplicates[i];
}
return bbSwtTargets->GetSucc(i);

default:
unreached();
Expand Down Expand Up @@ -1636,7 +1596,7 @@ BasicBlock* BasicBlock::New(Compiler* compiler, BBKinds kind)
return block;
}

BasicBlock* BasicBlock::New(Compiler* compiler, BBehfDesc* ehfTargets)
BasicBlock* BasicBlock::New(Compiler* compiler, BBJumpTable* ehfTargets)
{
BasicBlock* block = BasicBlock::New(compiler);
block->SetEhf(ehfTargets);
Expand Down Expand Up @@ -1758,21 +1718,6 @@ bool BasicBlock::hasEHBoundaryOut() const
return KindIs(BBJ_EHFILTERRET, BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHCATCHRET);
}

//------------------------------------------------------------------------
// BBswtDesc copy ctor: copy a switch descriptor, but don't set up the jump table
//
// Arguments:
// other - existing switch descriptor to copy (except for its jump table)
//
BBswtDesc::BBswtDesc(const BBswtDesc* other)
: bbsDstTab(nullptr)
, bbsCount(other->bbsCount)
, bbsDominantCase(other->bbsDominantCase)
, bbsHasDefault(other->bbsHasDefault)
, bbsHasDominantCase(other->bbsHasDominantCase)
{
}

//------------------------------------------------------------------------
// BBswtDesc copy ctor: copy a switch descriptor
//
Expand All @@ -1781,38 +1726,33 @@ BBswtDesc::BBswtDesc(const BBswtDesc* other)
// other - existing switch descriptor to copy
//
BBswtDesc::BBswtDesc(Compiler* comp, const BBswtDesc* other)
: bbsDstTab(nullptr)
, bbsCount(other->bbsCount)
: BBJumpTable(new(comp, CMK_FlowEdge) FlowEdge*[other->succCount + other->caseCount], other -> succCount)
, caseCount(other->caseCount)
, cases(succs + succCount)
, bbsDominantCase(other->bbsDominantCase)
, bbsHasDefault(other->bbsHasDefault)
, bbsHasDominantCase(other->bbsHasDominantCase)
{
// Allocate and fill in a new dst tab
// Fill in the new tables
//
bbsDstTab = new (comp, CMK_FlowEdge) FlowEdge*[bbsCount];
for (unsigned i = 0; i < bbsCount; i++)
{
bbsDstTab[i] = other->bbsDstTab[i];
}
memcpy(succs, other->succs, sizeof(FlowEdge*) * succCount);
memcpy(cases, other->cases, sizeof(FlowEdge*) * caseCount);
}

//------------------------------------------------------------------------
// BBehfDesc copy ctor: copy a EHFINALLYRET descriptor
// BBJumpTable copy ctor: copy a N-successor block descriptor
//
// Arguments:
// comp - compiler instance
// other - existing descriptor to copy
//
BBehfDesc::BBehfDesc(Compiler* comp, const BBehfDesc* other)
: bbeCount(other->bbeCount)
BBJumpTable::BBJumpTable(Compiler* comp, const BBJumpTable* other)
: succs(new(comp, CMK_FlowEdge) FlowEdge*[other->succCount])
, succCount(other->succCount)
{
// Allocate and fill in a new dst tab
// Fill in the new jump table
//
bbeSuccs = new (comp, CMK_FlowEdge) FlowEdge*[bbeCount];
for (unsigned i = 0; i < bbeCount; i++)
{
bbeSuccs[i] = other->bbeSuccs[i];
}
memcpy(succs, other->succs, sizeof(FlowEdge*) * succCount);
}

//------------------------------------------------------------------------
Expand Down
Loading
Loading