Skip to content

Commit 98bdace

Browse files
amanasifkhalidamanasifkhalid
authored
JIT: Use successor edges instead of block targets for BBJ_SWITCH (#98671)
Part of #93020. Updates BBswtDesc::bbsDstTab to use successor edges instead of block targets for switch blocks. Also renames optRedirectBlock.
1 parent 6864ddf commit 98bdace

21 files changed

+208
-214
lines changed

‎src/coreclr/jit/block.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -722,12 +722,12 @@ void BasicBlock::dspKind() const
722722
{
723723
printf(" ->");
724724

725-
const unsigned jumpCnt = bbSwtTargets->bbsCount;
726-
BasicBlock** const jumpTab = bbSwtTargets->bbsDstTab;
725+
const unsigned jumpCnt = bbSwtTargets->bbsCount;
726+
FlowEdge** const jumpTab = bbSwtTargets->bbsDstTab;
727727

728728
for (unsigned i = 0; i < jumpCnt; i++)
729729
{
730-
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));
730+
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock()));
731731

732732
const bool isDefault = bbSwtTargets->bbsHasDefault && (i == jumpCnt - 1);
733733
if (isDefault)
@@ -1217,7 +1217,7 @@ BasicBlock* BasicBlock::GetSucc(unsigned i) const
12171217
return bbEhfTargets->bbeSuccs[i]->getDestinationBlock();
12181218

12191219
case BBJ_SWITCH:
1220-
return bbSwtTargets->bbsDstTab[i];
1220+
return bbSwtTargets->bbsDstTab[i]->getDestinationBlock();
12211221

12221222
default:
12231223
unreached();
@@ -1774,7 +1774,7 @@ BBswtDesc::BBswtDesc(Compiler* comp, const BBswtDesc* other)
17741774
{
17751775
// Allocate and fill in a new dst tab
17761776
//
1777-
bbsDstTab = new (comp, CMK_BasicBlock) BasicBlock*[bbsCount];
1777+
bbsDstTab = new (comp, CMK_FlowEdge) FlowEdge*[bbsCount];
17781778
for (unsigned i = 0; i < bbsCount; i++)
17791779
{
17801780
bbsDstTab[i] = other->bbsDstTab[i];

‎src/coreclr/jit/block.h‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1840,8 +1840,8 @@ class BasicBlockRangeList
18401840
//
18411841
struct BBswtDesc
18421842
{
1843-
BasicBlock** bbsDstTab; // case label table address
1844-
unsigned bbsCount; // count of cases (includes 'default' if bbsHasDefault)
1843+
FlowEdge** bbsDstTab; // case label table address
1844+
unsigned bbsCount; // count of cases (includes 'default' if bbsHasDefault)
18451845

18461846
// Case number and likelihood of most likely case
18471847
// (only known with PGO, only valid if bbsHasDominantCase is true)
@@ -1867,7 +1867,7 @@ struct BBswtDesc
18671867
bbsCount--;
18681868
}
18691869

1870-
BasicBlock* getDefault()
1870+
FlowEdge* getDefault()
18711871
{
18721872
assert(bbsHasDefault);
18731873
assert(bbsCount > 0);
@@ -1994,8 +1994,10 @@ inline BasicBlock::BBSuccList::BBSuccList(const BasicBlock* block)
19941994
// We don't use the m_succs in-line data for switches; use the existing jump table in the block.
19951995
assert(block->bbSwtTargets != nullptr);
19961996
assert(block->bbSwtTargets->bbsDstTab != nullptr);
1997-
m_begin = block->bbSwtTargets->bbsDstTab;
1998-
m_end = block->bbSwtTargets->bbsDstTab + block->bbSwtTargets->bbsCount;
1997+
m_beginEdge = block->bbSwtTargets->bbsDstTab;
1998+
m_endEdge = block->bbSwtTargets->bbsDstTab + block->bbSwtTargets->bbsCount;
1999+
2000+
iterateEdges = true;
19992001
break;
20002002

20012003
default:

‎src/coreclr/jit/codegenarm.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,17 +650,18 @@ void CodeGen::genJumpTable(GenTree* treeNode)
650650
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
651651
assert(treeNode->OperGet() == GT_JMPTABLE);
652652

653-
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
654-
BasicBlock** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
655-
unsigned jmpTabBase;
653+
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
654+
FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
655+
unsigned jmpTabBase;
656656

657657
jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, false);
658658

659659
JITDUMP("\n J_M%03u_DS%02u LABEL DWORD\n", compiler->compMethodID, jmpTabBase);
660660

661661
for (unsigned i = 0; i < jumpCount; i++)
662662
{
663-
BasicBlock* target = *jumpTable++;
663+
BasicBlock* target = (*jumpTable)->getDestinationBlock();
664+
jumpTable++;
664665
noway_assert(target->HasFlag(BBF_HAS_LABEL));
665666

666667
JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);

‎src/coreclr/jit/codegenarm64.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3753,10 +3753,10 @@ void CodeGen::genJumpTable(GenTree* treeNode)
37533753
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
37543754
assert(treeNode->OperGet() == GT_JMPTABLE);
37553755

3756-
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
3757-
BasicBlock** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
3758-
unsigned jmpTabOffs;
3759-
unsigned jmpTabBase;
3756+
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
3757+
FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
3758+
unsigned jmpTabOffs;
3759+
unsigned jmpTabBase;
37603760

37613761
jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true);
37623762

@@ -3766,7 +3766,8 @@ void CodeGen::genJumpTable(GenTree* treeNode)
37663766

37673767
for (unsigned i = 0; i < jumpCount; i++)
37683768
{
3769-
BasicBlock* target = *jumpTable++;
3769+
BasicBlock* target = (*jumpTable)->getDestinationBlock();
3770+
jumpTable++;
37703771
noway_assert(target->HasFlag(BBF_HAS_LABEL));
37713772

37723773
JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);

‎src/coreclr/jit/codegenloongarch64.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2930,10 +2930,10 @@ void CodeGen::genJumpTable(GenTree* treeNode)
29302930
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
29312931
assert(treeNode->OperGet() == GT_JMPTABLE);
29322932

2933-
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
2934-
BasicBlock** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
2935-
unsigned jmpTabOffs;
2936-
unsigned jmpTabBase;
2933+
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
2934+
FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
2935+
unsigned jmpTabOffs;
2936+
unsigned jmpTabBase;
29372937

29382938
jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true);
29392939

@@ -2943,7 +2943,8 @@ void CodeGen::genJumpTable(GenTree* treeNode)
29432943

29442944
for (unsigned i = 0; i < jumpCount; i++)
29452945
{
2946-
BasicBlock* target = *jumpTable++;
2946+
BasicBlock* target = (*jumpTable)->getDestinationBlock();
2947+
jumpTable++;
29472948
noway_assert(target->HasFlag(BBF_HAS_LABEL));
29482949

29492950
JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);

‎src/coreclr/jit/codegenriscv64.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2852,10 +2852,10 @@ void CodeGen::genJumpTable(GenTree* treeNode)
28522852
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
28532853
assert(treeNode->OperGet() == GT_JMPTABLE);
28542854

2855-
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
2856-
BasicBlock** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
2857-
unsigned jmpTabOffs;
2858-
unsigned jmpTabBase;
2855+
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
2856+
FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
2857+
unsigned jmpTabOffs;
2858+
unsigned jmpTabBase;
28592859

28602860
jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true);
28612861

@@ -2865,7 +2865,7 @@ void CodeGen::genJumpTable(GenTree* treeNode)
28652865

28662866
for (unsigned i = 0; i < jumpCount; i++)
28672867
{
2868-
BasicBlock* target = *jumpTable++;
2868+
BasicBlock* target = (*jumpTable)->getDestinationBlock();
28692869
noway_assert(target->HasFlag(BBF_HAS_LABEL));
28702870

28712871
JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);

‎src/coreclr/jit/codegenxarch.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4356,10 +4356,10 @@ void CodeGen::genJumpTable(GenTree* treeNode)
43564356
noway_assert(compiler->compCurBB->KindIs(BBJ_SWITCH));
43574357
assert(treeNode->OperGet() == GT_JMPTABLE);
43584358

4359-
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
4360-
BasicBlock** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
4361-
unsigned jmpTabOffs;
4362-
unsigned jmpTabBase;
4359+
unsigned jumpCount = compiler->compCurBB->GetSwitchTargets()->bbsCount;
4360+
FlowEdge** jumpTable = compiler->compCurBB->GetSwitchTargets()->bbsDstTab;
4361+
unsigned jmpTabOffs;
4362+
unsigned jmpTabBase;
43634363

43644364
jmpTabBase = GetEmitter()->emitBBTableDataGenBeg(jumpCount, true);
43654365

@@ -4369,7 +4369,8 @@ void CodeGen::genJumpTable(GenTree* treeNode)
43694369

43704370
for (unsigned i = 0; i < jumpCount; i++)
43714371
{
4372-
BasicBlock* target = *jumpTable++;
4372+
BasicBlock* target = (*jumpTable)->getDestinationBlock();
4373+
jumpTable++;
43734374
noway_assert(target->HasFlag(BBF_HAS_LABEL));
43744375

43754376
JITDUMP(" DD L_M%03u_" FMT_BB "\n", compiler->compMethodID, target->bbNum);

‎src/coreclr/jit/compiler.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6861,7 +6861,7 @@ class Compiler
68616861
bool optExtractInitTestIncr(
68626862
BasicBlock** pInitBlock, BasicBlock* bottom, BasicBlock* top, GenTree** ppInit, GenTree** ppTest, GenTree** ppIncr);
68636863

6864-
void optRedirectBlock(BasicBlock* blk,
6864+
void optSetMappedBlockTargets(BasicBlock* blk,
68656865
BasicBlock* newBlk,
68666866
BlockToBlockMap* redirectMap);
68676867

‎src/coreclr/jit/fgbasic.cpp‎

Lines changed: 37 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -382,36 +382,26 @@ void Compiler::fgChangeSwitchBlock(BasicBlock* oldSwitchBlock, BasicBlock* newSw
382382
assert(fgPredsComputed);
383383

384384
// Walk the switch's jump table, updating the predecessor for each branch.
385-
for (BasicBlock* const bJump : oldSwitchBlock->SwitchTargets())
386-
{
387-
noway_assert(bJump != nullptr);
388-
389-
// Note that if there are duplicate branch targets in the switch jump table,
390-
// fgRemoveRefPred()/fgAddRefPred() will do the right thing: the second and
391-
// subsequent duplicates will simply subtract from and add to the duplicate
392-
// count (respectively).
393-
//
394-
// However this does the "wrong" thing with respect to edge profile
395-
// data; the old edge is not returned by fgRemoveRefPred until it has
396-
// a dup count of 0, and the fgAddRefPred only uses the optional
397-
// old edge arg when the new edge is first created.
398-
//
399-
// Remove the old edge [oldSwitchBlock => bJump]
400-
//
401-
assert(bJump->countOfInEdges() > 0);
402-
FlowEdge* const oldEdge = fgRemoveRefPred(bJump, oldSwitchBlock);
385+
BBswtDesc* swtDesc = oldSwitchBlock->GetSwitchTargets();
403386

404-
//
405-
// Create the new edge [newSwitchBlock => bJump]
406-
//
407-
FlowEdge* const newEdge = fgAddRefPred(bJump, newSwitchBlock);
387+
for (unsigned i = 0; i < swtDesc->bbsCount; i++)
388+
{
389+
FlowEdge* succEdge = swtDesc->bbsDstTab[i];
390+
assert(succEdge != nullptr);
408391

409-
// Handle the profile update, once we get our hands on the old edge.
410-
//
411-
if (oldEdge != nullptr)
392+
if (succEdge->getSourceBlock() != oldSwitchBlock)
412393
{
413-
assert(!newEdge->hasLikelihood());
414-
newEdge->setLikelihood(oldEdge->getLikelihood());
394+
// swtDesc can have duplicate targets, so we may have updated this edge already
395+
//
396+
assert(succEdge->getSourceBlock() == newSwitchBlock);
397+
assert(succEdge->getDupCount() > 1);
398+
}
399+
else
400+
{
401+
// Redirect edge's source block from oldSwitchBlock to newSwitchBlock,
402+
// and keep successor block's pred list in order
403+
//
404+
fgReplacePred(succEdge, newSwitchBlock);
415405
}
416406
}
417407

@@ -709,29 +699,17 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas
709699

710700
case BBJ_SWITCH:
711701
{
712-
unsigned const jumpCnt = block->GetSwitchTargets()->bbsCount;
713-
BasicBlock** const jumpTab = block->GetSwitchTargets()->bbsDstTab;
714-
bool changed = false;
702+
unsigned const jumpCnt = block->GetSwitchTargets()->bbsCount;
703+
FlowEdge** const jumpTab = block->GetSwitchTargets()->bbsDstTab;
704+
bool changed = false;
715705

716706
for (unsigned i = 0; i < jumpCnt; i++)
717707
{
718-
if (jumpTab[i] == oldTarget)
708+
if (jumpTab[i]->getDestinationBlock() == oldTarget)
719709
{
720-
jumpTab[i] = newTarget;
721-
changed = true;
722-
FlowEdge* const oldEdge = fgRemoveRefPred(oldTarget, block);
723-
FlowEdge* const newEdge = fgAddRefPred(newTarget, block, oldEdge);
724-
725-
// Handle the profile update, once we get our hands on the old edge.
726-
// (see notes in fgChangeSwitchBlock for why this extra step is necessary)
727-
//
728-
// We do it slightly differently here so we don't lose the old
729-
// edge weight propagation that would sometimes happen
730-
//
731-
if ((oldEdge != nullptr) && !newEdge->hasLikelihood())
732-
{
733-
newEdge->setLikelihood(oldEdge->getLikelihood());
734-
}
710+
fgRemoveRefPred(jumpTab[i]);
711+
jumpTab[i] = fgAddRefPred(newTarget, block, jumpTab[i]);
712+
changed = true;
735713
}
736714
}
737715

@@ -3041,23 +3019,23 @@ void Compiler::fgLinkBasicBlocks()
30413019

30423020
case BBJ_SWITCH:
30433021
{
3044-
unsigned jumpCnt = curBBdesc->GetSwitchTargets()->bbsCount;
3045-
BasicBlock** jumpPtr = curBBdesc->GetSwitchTargets()->bbsDstTab;
3022+
unsigned jumpCnt = curBBdesc->GetSwitchTargets()->bbsCount;
3023+
FlowEdge** jumpPtr = curBBdesc->GetSwitchTargets()->bbsDstTab;
30463024

30473025
do
30483026
{
3049-
BasicBlock* jumpDest = fgLookupBB((unsigned)*(size_t*)jumpPtr);
3050-
*jumpPtr = jumpDest;
3051-
fgAddRefPred<initializingPreds>(jumpDest, curBBdesc);
3052-
if ((*jumpPtr)->bbNum <= curBBdesc->bbNum)
3027+
BasicBlock* jumpDest = fgLookupBB((unsigned)*(size_t*)jumpPtr);
3028+
FlowEdge* const newEdge = fgAddRefPred<initializingPreds>(jumpDest, curBBdesc);
3029+
*jumpPtr = newEdge;
3030+
if (jumpDest->bbNum <= curBBdesc->bbNum)
30533031
{
3054-
fgMarkBackwardJump(*jumpPtr, curBBdesc);
3032+
fgMarkBackwardJump(jumpDest, curBBdesc);
30553033
}
30563034
} while (++jumpPtr, --jumpCnt);
30573035

30583036
/* Default case of CEE_SWITCH (next block), is at end of jumpTab[] */
30593037

3060-
noway_assert(curBBdesc->NextIs(*(jumpPtr - 1)));
3038+
noway_assert(curBBdesc->NextIs((*(jumpPtr - 1))->getDestinationBlock()));
30613039
break;
30623040
}
30633041

@@ -3220,8 +3198,8 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
32203198
unsigned jmpBase;
32213199
unsigned jmpCnt; // # of switch cases (excluding default)
32223200

3223-
BasicBlock** jmpTab;
3224-
BasicBlock** jmpPtr;
3201+
FlowEdge** jmpTab;
3202+
FlowEdge** jmpPtr;
32253203

32263204
/* Allocate the switch descriptor */
32273205

@@ -3238,7 +3216,7 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
32383216

32393217
/* Allocate the jump table */
32403218

3241-
jmpPtr = jmpTab = new (this, CMK_BasicBlock) BasicBlock*[jmpCnt + 1];
3219+
jmpPtr = jmpTab = new (this, CMK_FlowEdge) FlowEdge*[jmpCnt + 1];
32423220

32433221
/* Fill in the jump table */
32443222

@@ -3248,12 +3226,12 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
32483226
codeAddr += 4;
32493227

32503228
// store the offset in the pointer. We change these in fgLinkBasicBlocks().
3251-
*jmpPtr++ = (BasicBlock*)(size_t)(jmpBase + jmpDist);
3229+
*jmpPtr++ = (FlowEdge*)(size_t)(jmpBase + jmpDist);
32523230
}
32533231

32543232
/* Append the default label to the target table */
32553233

3256-
*jmpPtr++ = (BasicBlock*)(size_t)jmpBase;
3234+
*jmpPtr++ = (FlowEdge*)(size_t)jmpBase;
32573235

32583236
/* Make sure we found the right number of labels */
32593237

‎src/coreclr/jit/fgdiagnostic.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,7 +1101,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
11011101
{
11021102
fprintf(fgxFile, "\n switchCases=\"%d\"", edge->getDupCount());
11031103
}
1104-
if (bSource->GetSwitchTargets()->getDefault() == bTarget)
1104+
if (bSource->GetSwitchTargets()->getDefault()->getDestinationBlock() == bTarget)
11051105
{
11061106
fprintf(fgxFile, "\n switchDefault=\"true\"");
11071107
}
@@ -2066,12 +2066,12 @@ void Compiler::fgTableDispBasicBlock(const BasicBlock* block,
20662066

20672067
const BBswtDesc* const jumpSwt = block->GetSwitchTargets();
20682068
const unsigned jumpCnt = jumpSwt->bbsCount;
2069-
BasicBlock** const jumpTab = jumpSwt->bbsDstTab;
2069+
FlowEdge** const jumpTab = jumpSwt->bbsDstTab;
20702070

20712071
for (unsigned i = 0; i < jumpCnt; i++)
20722072
{
20732073
printedBlockWidth += 1 /* space/comma */;
2074-
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));
2074+
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock()));
20752075

20762076
const bool isDefault = jumpSwt->bbsHasDefault && (i == jumpCnt - 1);
20772077
if (isDefault)

0 commit comments

Comments
 (0)