Skip to content

Commit c806bf6

Browse files
amanasifkhalidamanasifkhalid
authored
JIT: Add helper method for updating bbTargetEdge (#99362)
Part of #93020. Adds fgRedirectTargetEdge, which updates bbTargetEdge's block target while maintaining the rest of the edge's state to simplify edge profile maintenance (and avoid additional allocations).
1 parent 709097d commit c806bf6

File tree

12 files changed

+149
-119
lines changed

12 files changed

+149
-119
lines changed

‎src/coreclr/jit/compiler.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5912,6 +5912,12 @@ class Compiler
59125912
template <bool initializingPreds = false>
59135913
FlowEdge* fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowEdge* oldEdge = nullptr);
59145914

5915+
private:
5916+
FlowEdge** fgGetPredInsertPoint(BasicBlock* blockPred, BasicBlock* newTarget);
5917+
5918+
public:
5919+
void fgRedirectTargetEdge(BasicBlock* block, BasicBlock* newTarget);
5920+
59155921
void fgFindBasicBlocks();
59165922

59175923
bool fgCheckEHCanInsertAfterBlock(BasicBlock* blk, unsigned regionIndex, bool putInTryRegion);

‎src/coreclr/jit/fgbasic.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,7 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas
646646
case BBJ_EHFILTERRET:
647647
case BBJ_LEAVE: // This function can be called before import, so we still have BBJ_LEAVE
648648
{
649+
// TODO: Use fgRedirectTargetEdge once pred edge iterators are resilient to bbPreds being modified.
649650
assert(block->TargetIs(oldTarget));
650651
fgRemoveRefPred(block->GetTargetEdge());
651652
FlowEdge* const newEdge = fgAddRefPred(newTarget, block, block->GetTargetEdge());
@@ -4192,9 +4193,7 @@ void Compiler::fgFixEntryFlowForOSR()
41924193
//
41934194
fgEnsureFirstBBisScratch();
41944195
assert(fgFirstBB->KindIs(BBJ_ALWAYS) && fgFirstBB->JumpsToNext());
4195-
fgRemoveRefPred(fgFirstBB->GetTargetEdge());
4196-
FlowEdge* const newEdge = fgAddRefPred(fgOSREntryBB, fgFirstBB);
4197-
fgFirstBB->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
4196+
fgRedirectTargetEdge(fgFirstBB, fgOSREntryBB);
41984197

41994198
// We don't know the right weight for this block, since
42004199
// execution of the method was interrupted within the

‎src/coreclr/jit/fgehopt.cpp

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,8 @@ PhaseStatus Compiler::fgRemoveEmptyFinally()
168168
fgRemoveBlock(leaveBlock, /* unreachable */ true);
169169

170170
// Ref count updates.
171-
fgRemoveRefPred(currentBlock->GetTargetEdge());
172-
FlowEdge* const newEdge = fgAddRefPred(postTryFinallyBlock, currentBlock);
173-
174-
currentBlock->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
171+
fgRedirectTargetEdge(currentBlock, postTryFinallyBlock);
172+
currentBlock->SetKind(BBJ_ALWAYS);
175173
currentBlock->RemoveFlags(BBF_RETLESS_CALL); // no longer a BBJ_CALLFINALLY
176174

177175
// Cleanup the postTryFinallyBlock
@@ -1136,12 +1134,11 @@ PhaseStatus Compiler::fgCloneFinally()
11361134
fgRemoveBlock(leaveBlock, /* unreachable */ true);
11371135

11381136
// Ref count updates.
1139-
fgRemoveRefPred(currentBlock->GetTargetEdge());
1140-
FlowEdge* const newEdge = fgAddRefPred(firstCloneBlock, currentBlock);
1137+
fgRedirectTargetEdge(currentBlock, firstCloneBlock);
11411138

11421139
// This call returns to the expected spot, so retarget it to branch to the clone.
11431140
currentBlock->RemoveFlags(BBF_RETLESS_CALL); // no longer a BBJ_CALLFINALLY
1144-
currentBlock->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
1141+
currentBlock->SetKind(BBJ_ALWAYS);
11451142

11461143
// Make sure iteration isn't going off the deep end.
11471144
assert(leaveBlock != endCallFinallyRangeBlock);
@@ -1758,9 +1755,7 @@ bool Compiler::fgRetargetBranchesToCanonicalCallFinally(BasicBlock* block,
17581755
canonicalCallFinally->bbNum);
17591756

17601757
assert(callFinally->bbRefs > 0);
1761-
fgRemoveRefPred(block->GetTargetEdge());
1762-
FlowEdge* const newEdge = fgAddRefPred(canonicalCallFinally, block);
1763-
block->SetTargetEdge(newEdge);
1758+
fgRedirectTargetEdge(block, canonicalCallFinally);
17641759

17651760
// Update profile counts
17661761
//
@@ -2132,16 +2127,11 @@ void Compiler::fgTailMergeThrowsJumpToHelper(BasicBlock* predBlock,
21322127
{
21332128
JITDUMP("*** " FMT_BB " now branching to " FMT_BB "\n", predBlock->bbNum, canonicalBlock->bbNum);
21342129

2135-
FlowEdge* const newEdge = fgAddRefPred(canonicalBlock, predBlock, predEdge);
2136-
21372130
if (predBlock->KindIs(BBJ_ALWAYS))
21382131
{
2139-
// Remove the old flow
2132+
// Update flow to new target
21402133
assert(predBlock->TargetIs(nonCanonicalBlock));
2141-
fgRemoveRefPred(predBlock->GetTargetEdge());
2142-
2143-
// Wire up the new flow
2144-
predBlock->SetTargetEdge(newEdge);
2134+
fgRedirectTargetEdge(predBlock, canonicalBlock);
21452135
}
21462136
else
21472137
{
@@ -2151,6 +2141,7 @@ void Compiler::fgTailMergeThrowsJumpToHelper(BasicBlock* predBlock,
21512141
fgRemoveRefPred(predBlock->GetTrueEdge());
21522142

21532143
// Wire up the new flow
2154-
predBlock->SetTrueEdge(newEdge);
2144+
FlowEdge* const trueEdge = fgAddRefPred(canonicalBlock, predBlock, predEdge);
2145+
predBlock->SetTrueEdge(trueEdge);
21552146
}
21562147
}

‎src/coreclr/jit/fgflow.cpp

Lines changed: 74 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,16 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE
117117
// dependency on this order. Note also that we don't allow duplicates in the list; we maintain a DupCount
118118
// count of duplication. This also necessitates walking the flow list for every edge we add.
119119
//
120-
FlowEdge* flow = nullptr;
121-
FlowEdge** listp = &block->bbPreds;
120+
FlowEdge* flow = nullptr;
121+
FlowEdge** listp;
122122

123123
if (initializingPreds)
124124
{
125125
// List is sorted order and we're adding references in
126126
// increasing blockPred->bbNum order. The only possible
127127
// dup list entry is the last one.
128128
//
129+
listp = &block->bbPreds;
129130
FlowEdge* flowLast = block->bbLastPred;
130131
if (flowLast != nullptr)
131132
{
@@ -143,10 +144,7 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE
143144
{
144145
// References are added randomly, so we have to search.
145146
//
146-
while ((*listp != nullptr) && ((*listp)->getSourceBlock()->bbNum < blockPred->bbNum))
147-
{
148-
listp = (*listp)->getNextPredEdgeRef();
149-
}
147+
listp = fgGetPredInsertPoint(blockPred, block);
150148

151149
if ((*listp != nullptr) && ((*listp)->getSourceBlock() == blockPred))
152150
{
@@ -381,6 +379,76 @@ void Compiler::fgRemoveBlockAsPred(BasicBlock* block)
381379
}
382380
}
383381

382+
//------------------------------------------------------------------------
383+
// fgGetPredInsertPoint: Searches newTarget->bbPreds for where to insert an edge from blockPred.
384+
//
385+
// Arguments:
386+
// blockPred -- The block we want to make a predecessor of newTarget (it could already be one).
387+
// newTarget -- The block whose pred list we will search.
388+
//
389+
// Return Value:
390+
// Returns a pointer to the next pointer of an edge in newTarget's pred list.
391+
// A new edge from blockPred to newTarget can be inserted here
392+
// without disrupting bbPreds' sorting invariant.
393+
//
394+
FlowEdge** Compiler::fgGetPredInsertPoint(BasicBlock* blockPred, BasicBlock* newTarget)
395+
{
396+
assert(blockPred != nullptr);
397+
assert(newTarget != nullptr);
398+
assert(fgPredsComputed);
399+
400+
FlowEdge** listp = &newTarget->bbPreds;
401+
402+
// Search pred list for insertion point
403+
//
404+
while ((*listp != nullptr) && ((*listp)->getSourceBlock()->bbNum < blockPred->bbNum))
405+
{
406+
listp = (*listp)->getNextPredEdgeRef();
407+
}
408+
409+
return listp;
410+
}
411+
412+
//------------------------------------------------------------------------
413+
// fgRedirectTargetEdge: Sets block->bbTargetEdge's target block to newTarget,
414+
// updating pred lists as necessary.
415+
//
416+
// Arguments:
417+
// block -- The block we want to make a predecessor of newTarget.
418+
// It could be one already, in which case nothing changes.
419+
// newTarget -- The new successor of block.
420+
//
421+
void Compiler::fgRedirectTargetEdge(BasicBlock* block, BasicBlock* newTarget)
422+
{
423+
assert(block != nullptr);
424+
assert(newTarget != nullptr);
425+
426+
FlowEdge* edge = block->GetTargetEdge();
427+
assert(edge->getDupCount() == 1);
428+
429+
// Update oldTarget's pred list.
430+
// We could call fgRemoveRefPred, but since we're removing the one and only ref from block to oldTarget,
431+
// fgRemoveAllRefPreds is slightly more efficient (one fewer branch, doesn't update edge's dup count, etc).
432+
//
433+
BasicBlock* oldTarget = edge->getDestinationBlock();
434+
fgRemoveAllRefPreds(oldTarget, block);
435+
436+
// Splice edge into new target block's pred list
437+
//
438+
FlowEdge** predListPtr = fgGetPredInsertPoint(block, newTarget);
439+
edge->setNextPredEdge(*predListPtr);
440+
edge->setDestinationBlock(newTarget);
441+
*predListPtr = edge;
442+
443+
// Pred list of target should (still) be ordered
444+
//
445+
assert(newTarget->checkPredListOrder());
446+
447+
// Edge should still have only one ref
448+
assert(edge->getDupCount() == 1);
449+
newTarget->bbRefs++;
450+
}
451+
384452
Compiler::SwitchUniqueSuccSet Compiler::GetDescriptorForSwitch(BasicBlock* switchBlk)
385453
{
386454
assert(switchBlk->KindIs(BBJ_SWITCH));

‎src/coreclr/jit/fginline.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1550,11 +1550,9 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
15501550
// Insert inlinee's blocks into inliner's block list.
15511551
assert(topBlock->KindIs(BBJ_ALWAYS));
15521552
assert(topBlock->TargetIs(bottomBlock));
1553-
fgRemoveRefPred(topBlock->GetTargetEdge());
1554-
FlowEdge* const newEdge = fgAddRefPred(InlineeCompiler->fgFirstBB, topBlock, topBlock->GetTargetEdge());
1553+
fgRedirectTargetEdge(topBlock, InlineeCompiler->fgFirstBB);
15551554

15561555
topBlock->SetNext(InlineeCompiler->fgFirstBB);
1557-
topBlock->SetTargetEdge(newEdge);
15581556
topBlock->SetFlags(BBF_NONE_QUIRK);
15591557
InlineeCompiler->fgLastBB->SetNext(bottomBlock);
15601558

‎src/coreclr/jit/fgopt.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -834,9 +834,7 @@ PhaseStatus Compiler::fgPostImportationCleanup()
834834

835835
if (entryJumpTarget != osrEntry)
836836
{
837-
fgRemoveRefPred(fgFirstBB->GetTargetEdge());
838-
FlowEdge* const newEdge = fgAddRefPred(entryJumpTarget, fgFirstBB, fgFirstBB->GetTargetEdge());
839-
fgFirstBB->SetTargetEdge(newEdge);
837+
fgRedirectTargetEdge(fgFirstBB, entryJumpTarget);
840838

841839
JITDUMP("OSR: redirecting flow from method entry " FMT_BB " to OSR entry " FMT_BB
842840
" via step blocks.\n",
@@ -1570,9 +1568,7 @@ bool Compiler::fgOptimizeBranchToEmptyUnconditional(BasicBlock* block, BasicBloc
15701568
case BBJ_ALWAYS:
15711569
case BBJ_CALLFINALLYRET:
15721570
{
1573-
fgRemoveRefPred(block->GetTargetEdge());
1574-
FlowEdge* const newEdge = fgAddRefPred(bDest->GetTarget(), block, block->GetTargetEdge());
1575-
block->SetTargetEdge(newEdge);
1571+
fgRedirectTargetEdge(block, bDest->GetTarget());
15761572
break;
15771573
}
15781574

‎src/coreclr/jit/helperexpansion.cpp

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,6 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm
372372
// Update preds in all new blocks
373373
//
374374
assert(prevBb->KindIs(BBJ_ALWAYS));
375-
fgRemoveRefPred(prevBb->GetTargetEdge());
376375

377376
{
378377
FlowEdge* const newEdge = fgAddRefPred(block, fastPathBb);
@@ -389,8 +388,7 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm
389388
if (needsSizeCheck)
390389
{
391390
// sizeCheckBb is the first block after prevBb
392-
FlowEdge* const newEdge = fgAddRefPred(sizeCheckBb, prevBb);
393-
prevBb->SetTargetEdge(newEdge);
391+
fgRedirectTargetEdge(prevBb, sizeCheckBb);
394392

395393
// sizeCheckBb flows into nullcheckBb in case if the size check passes
396394
{
@@ -406,8 +404,7 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm
406404
else
407405
{
408406
// nullcheckBb is the first block after prevBb
409-
FlowEdge* const newEdge = fgAddRefPred(nullcheckBb, prevBb);
410-
prevBb->SetTargetEdge(newEdge);
407+
fgRedirectTargetEdge(prevBb, nullcheckBb);
411408

412409
// No size check, nullcheckBb jumps to fast path
413410
// fallbackBb is only reachable from nullcheckBb (jump destination)
@@ -742,9 +739,7 @@ bool Compiler::fgExpandThreadLocalAccessForCallNativeAOT(BasicBlock** pBlock, St
742739
// fallback will just execute first time
743740
fallbackBb->bbSetRunRarely();
744741

745-
fgRemoveRefPred(prevBb->GetTargetEdge());
746-
FlowEdge* const newEdge = fgAddRefPred(tlsRootNullCondBB, prevBb);
747-
prevBb->SetTargetEdge(newEdge);
742+
fgRedirectTargetEdge(prevBb, tlsRootNullCondBB);
748743

749744
// All blocks are expected to be in the same EH region
750745
assert(BasicBlock::sameEHRegion(prevBb, block));
@@ -1089,12 +1084,7 @@ bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock** pBlock, Statement*
10891084
// Update preds in all new blocks
10901085
//
10911086
assert(prevBb->KindIs(BBJ_ALWAYS));
1092-
fgRemoveRefPred(prevBb->GetTargetEdge());
1093-
1094-
{
1095-
FlowEdge* const newEdge = fgAddRefPred(maxThreadStaticBlocksCondBB, prevBb);
1096-
prevBb->SetTargetEdge(newEdge);
1097-
}
1087+
fgRedirectTargetEdge(prevBb, maxThreadStaticBlocksCondBB);
10981088

10991089
{
11001090
FlowEdge* const trueEdge = fgAddRefPred(fallbackBb, maxThreadStaticBlocksCondBB);
@@ -1462,8 +1452,10 @@ bool Compiler::fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, G
14621452
// Update preds in all new blocks
14631453
//
14641454

1465-
// Unlink block and prevBb
1466-
fgRemoveRefPred(prevBb->GetTargetEdge());
1455+
// Redirect prevBb from block to isInitedBb
1456+
fgRedirectTargetEdge(prevBb, isInitedBb);
1457+
prevBb->SetFlags(BBF_NONE_QUIRK);
1458+
assert(prevBb->JumpsToNext());
14671459

14681460
{
14691461
// Block has two preds now: either isInitedBb or helperCallBb
@@ -1473,15 +1465,6 @@ bool Compiler::fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, G
14731465
helperCallBb->SetFlags(BBF_NONE_QUIRK);
14741466
}
14751467

1476-
{
1477-
// prevBb always flows into isInitedBb
1478-
assert(prevBb->KindIs(BBJ_ALWAYS));
1479-
FlowEdge* const newEdge = fgAddRefPred(isInitedBb, prevBb);
1480-
prevBb->SetTargetEdge(newEdge);
1481-
prevBb->SetFlags(BBF_NONE_QUIRK);
1482-
assert(prevBb->JumpsToNext());
1483-
}
1484-
14851468
{
14861469
// Both fastPathBb and helperCallBb have a single common pred - isInitedBb
14871470
FlowEdge* const trueEdge = fgAddRefPred(block, isInitedBb);
@@ -1792,17 +1775,10 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock,
17921775
//
17931776
// Update preds in all new blocks
17941777
//
1795-
// block is no longer a predecessor of prevBb
1796-
fgRemoveRefPred(prevBb->GetTargetEdge());
1797-
1798-
{
1799-
// prevBb flows into lengthCheckBb
1800-
assert(prevBb->KindIs(BBJ_ALWAYS));
1801-
FlowEdge* const newEdge = fgAddRefPred(lengthCheckBb, prevBb);
1802-
prevBb->SetTargetEdge(newEdge);
1803-
prevBb->SetFlags(BBF_NONE_QUIRK);
1804-
assert(prevBb->JumpsToNext());
1805-
}
1778+
// Redirect prevBb to lengthCheckBb
1779+
fgRedirectTargetEdge(prevBb, lengthCheckBb);
1780+
prevBb->SetFlags(BBF_NONE_QUIRK);
1781+
assert(prevBb->JumpsToNext());
18061782

18071783
{
18081784
// lengthCheckBb has two successors: block and fastpathBb
@@ -2511,12 +2487,7 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt,
25112487
}
25122488
}
25132489

2514-
fgRemoveRefPred(firstBb->GetTargetEdge());
2515-
2516-
{
2517-
FlowEdge* const newEdge = fgAddRefPred(nullcheckBb, firstBb);
2518-
firstBb->SetTargetEdge(newEdge);
2519-
}
2490+
fgRedirectTargetEdge(firstBb, nullcheckBb);
25202491

25212492
{
25222493
FlowEdge* const trueEdge = fgAddRefPred(lastBb, nullcheckBb);

0 commit comments

Comments
 (0)