Skip to content

Commit 5fda474

Browse files
amanasifkhalidamanasifkhalid
authored
JIT: Remove BBF_NONE_QUIRK (#99907)
1 parent 3ab77bc commit 5fda474

17 files changed

Lines changed: 20 additions & 153 deletions

‎src/coreclr/jit/block.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,6 @@ void BasicBlock::dspFlags() const
583583
{BBF_HAS_ALIGN, "has-align"},
584584
{BBF_HAS_MDARRAYREF, "mdarr"},
585585
{BBF_NEEDS_GCPOLL, "gcpoll"},
586-
{BBF_NONE_QUIRK, "q"},
587586
};
588587

589588
bool first = true;
@@ -941,9 +940,6 @@ void BasicBlock::TransferTarget(BasicBlock* from)
941940
SetCond(from->bbTrueEdge, from->bbFalseEdge);
942941
break;
943942
case BBJ_ALWAYS:
944-
SetKindAndTargetEdge(BBJ_ALWAYS, from->bbTargetEdge);
945-
CopyFlags(from, BBF_NONE_QUIRK);
946-
break;
947943
case BBJ_CALLFINALLY:
948944
case BBJ_CALLFINALLYRET:
949945
case BBJ_EHCATCHRET:

‎src/coreclr/jit/block.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -456,11 +456,7 @@ enum BasicBlockFlags : unsigned __int64
456456
BBF_RECURSIVE_TAILCALL = MAKE_BBFLAG(37), // Block has recursive tailcall that may turn into a loop
457457
BBF_NO_CSE_IN = MAKE_BBFLAG(38), // Block should kill off any incoming CSE
458458
BBF_CAN_ADD_PRED = MAKE_BBFLAG(39), // Ok to add pred edge to this block, even when "safe" edge creation disabled
459-
BBF_NONE_QUIRK = MAKE_BBFLAG(40), // Block was created as a BBJ_ALWAYS to the next block,
460-
// and should be treated as if it falls through.
461-
// This is just to reduce diffs from removing BBJ_NONE.
462-
// (TODO: Remove this quirk after refactoring Compiler::fgFindInsertPoint)
463-
BBF_HAS_VALUE_PROFILE = MAKE_BBFLAG(41), // Block has a node that needs a value probing
459+
BBF_HAS_VALUE_PROFILE = MAKE_BBFLAG(40), // Block has a node that needs a value probing
464460

465461
// The following are sets of flags.
466462

@@ -486,7 +482,7 @@ enum BasicBlockFlags : unsigned __int64
486482
// TODO: Should BBF_RUN_RARELY be added to BBF_SPLIT_GAINED ?
487483

488484
BBF_SPLIT_GAINED = BBF_DONT_REMOVE | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_PROF_WEIGHT | \
489-
BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END | BBF_HAS_NULLCHECK | BBF_HAS_HISTOGRAM_PROFILE | BBF_HAS_VALUE_PROFILE | BBF_HAS_MDARRAYREF | BBF_NEEDS_GCPOLL | BBF_NONE_QUIRK,
485+
BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END | BBF_HAS_NULLCHECK | BBF_HAS_HISTOGRAM_PROFILE | BBF_HAS_VALUE_PROFILE | BBF_HAS_MDARRAYREF | BBF_NEEDS_GCPOLL,
490486

491487
// Flags that must be propagated to a new block if code is copied from a block to a new block. These are flags that
492488
// limit processing of a block if the code in question doesn't exist. This is conservative; we might not

‎src/coreclr/jit/fgbasic.cpp‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ bool Compiler::fgEnsureFirstBBisScratch()
239239
noway_assert(fgLastBB != nullptr);
240240

241241
// Set the expected flags
242-
block->SetFlags(BBF_INTERNAL | BBF_IMPORTED | BBF_NONE_QUIRK);
242+
block->SetFlags(BBF_INTERNAL | BBF_IMPORTED);
243243

244244
// This new first BB has an implicit ref, and no others.
245245
//
@@ -3503,7 +3503,6 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F
35033503
// Jump to the next block
35043504
jmpKind = BBJ_ALWAYS;
35053505
jmpAddr = nxtBBoffs;
3506-
bbFlags |= BBF_NONE_QUIRK;
35073506
}
35083507

35093508
assert(jmpKind != BBJ_COUNT);
@@ -4806,7 +4805,6 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr)
48064805
newBlock->TransferTarget(curr);
48074806

48084807
curr->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
4809-
curr->SetFlags(BBF_NONE_QUIRK);
48104808
assert(curr->JumpsToNext());
48114809

48124810
return newBlock;
@@ -4898,7 +4896,6 @@ BasicBlock* Compiler::fgSplitBlockBeforeTree(
48984896

48994897
// prevBb should flow into block
49004898
assert(prevBb->KindIs(BBJ_ALWAYS) && prevBb->JumpsToNext() && prevBb->NextIs(block));
4901-
prevBb->SetFlags(BBF_NONE_QUIRK);
49024899

49034900
return block;
49044901
}
@@ -5031,9 +5028,7 @@ BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ)
50315028
// an immediately following block of a BBJ_SWITCH (which has
50325029
// no fall-through path). For this case, simply insert a new
50335030
// fall-through block after 'curr'.
5034-
// TODO-NoFallThrough: Once false target can diverge from bbNext, this will be unnecessary for BBJ_COND
50355031
newBlock = fgNewBBafter(BBJ_ALWAYS, curr, true /* extendRegion */);
5036-
newBlock->SetFlags(BBF_NONE_QUIRK);
50375032
}
50385033
else
50395034
{
@@ -5484,10 +5479,6 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
54845479
JITDUMP("Added an unconditional jump to " FMT_BB " after block " FMT_BB "\n", jmpBlk->GetTarget()->bbNum,
54855480
bSrc->bbNum);
54865481
}
5487-
else if (bSrc->KindIs(BBJ_ALWAYS) && bSrc->HasInitializedTarget() && bSrc->JumpsToNext())
5488-
{
5489-
bSrc->SetFlags(BBF_NONE_QUIRK);
5490-
}
54915482

54925483
return jmpBlk;
54935484
}
@@ -5944,7 +5935,7 @@ BasicBlock* Compiler::fgRelocateEHRange(unsigned regionIndex, FG_RELOCATE_TYPE r
59445935
// Because this relies on ebdEnclosingTryIndex and ebdEnclosingHndIndex
59455936
#endif // DEBUG
59465937

5947-
#else // !FEATURE_EH_FUNCLETS
5938+
#else // !FEATURE_EH_FUNCLETS
59485939

59495940
for (XTnum = 0, HBtab = compHndBBtab; XTnum < compHndBBtabCount; XTnum++, HBtab++)
59505941
{
@@ -5994,17 +5985,6 @@ BasicBlock* Compiler::fgRelocateEHRange(unsigned regionIndex, FG_RELOCATE_TYPE r
59945985

59955986
// We have decided to insert the block(s) after fgLastBlock
59965987
fgMoveBlocksAfter(bStart, bLast, insertAfterBlk);
5997-
5998-
if (bPrev->KindIs(BBJ_ALWAYS) && bPrev->JumpsToNext())
5999-
{
6000-
bPrev->SetFlags(BBF_NONE_QUIRK);
6001-
}
6002-
6003-
if (bLast->KindIs(BBJ_ALWAYS) && bLast->JumpsToNext())
6004-
{
6005-
bLast->SetFlags(BBF_NONE_QUIRK);
6006-
}
6007-
60085988
#endif // !FEATURE_EH_FUNCLETS
60095989

60105990
goto DONE;

‎src/coreclr/jit/fginline.cpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,6 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
677677
{
678678
m_compiler->fgRemoveRefPred(block->GetTrueEdge());
679679
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetFalseEdge());
680-
block->SetFlags(BBF_NONE_QUIRK);
681680
}
682681
else
683682
{
@@ -1536,11 +1535,6 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
15361535

15371536
FlowEdge* const newEdge = fgAddRefPred(bottomBlock, block);
15381537
block->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
1539-
1540-
if (block == InlineeCompiler->fgLastBB)
1541-
{
1542-
block->SetFlags(BBF_NONE_QUIRK);
1543-
}
15441538
}
15451539
}
15461540

@@ -1554,7 +1548,6 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
15541548
fgRedirectTargetEdge(topBlock, InlineeCompiler->fgFirstBB);
15551549

15561550
topBlock->SetNext(InlineeCompiler->fgFirstBB);
1557-
topBlock->SetFlags(BBF_NONE_QUIRK);
15581551
InlineeCompiler->fgLastBB->SetNext(bottomBlock);
15591552

15601553
//

‎src/coreclr/jit/fgopt.cpp‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,6 @@ PhaseStatus Compiler::fgPostImportationCleanup()
620620
else
621621
{
622622
FlowEdge* const newEdge = fgAddRefPred(newTryEntry->Next(), newTryEntry);
623-
newTryEntry->SetFlags(BBF_NONE_QUIRK);
624623
newTryEntry->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
625624
}
626625

@@ -1309,17 +1308,6 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
13091308

13101309
assert(block->KindIs(bNext->GetKind()));
13111310

1312-
if (block->KindIs(BBJ_ALWAYS))
1313-
{
1314-
// Propagate BBF_NONE_QUIRK flag
1315-
block->CopyFlags(bNext, BBF_NONE_QUIRK);
1316-
}
1317-
else
1318-
{
1319-
// It's no longer a BBJ_ALWAYS; remove the BBF_NONE_QUIRK flag.
1320-
block->RemoveFlags(BBF_NONE_QUIRK);
1321-
}
1322-
13231311
#if DEBUG
13241312
if (verbose && 0)
13251313
{
@@ -2613,9 +2601,6 @@ void Compiler::fgRemoveConditionalJump(BasicBlock* block)
26132601
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetTrueEdge());
26142602
assert(block->TargetIs(target));
26152603

2616-
// TODO-NoFallThrough: Set BBF_NONE_QUIRK only when false target is the next block
2617-
block->SetFlags(BBF_NONE_QUIRK);
2618-
26192604
/* Update bbRefs and bbNum - Conditional predecessors to the same
26202605
* block are counted twice so we have to remove one of them */
26212606

@@ -4314,16 +4299,6 @@ bool Compiler::fgReorderBlocks(bool useProfile)
43144299
const bool bStartPrevJumpsToNext = bStartPrev->KindIs(BBJ_ALWAYS) && bStartPrev->JumpsToNext();
43154300
fgUnlinkRange(bStart, bEnd);
43164301

4317-
// If bStartPrev is a BBJ_ALWAYS to some block after bStart, unlinking bStart can move
4318-
// bStartPrev's jump destination up, making bStartPrev jump to the next block for now.
4319-
// This can lead us to make suboptimal decisions in Compiler::fgFindInsertPoint,
4320-
// so make sure the BBF_NONE_QUIRK flag is unset for bStartPrev beforehand.
4321-
// TODO: Remove quirk.
4322-
if (bStartPrev->KindIs(BBJ_ALWAYS) && (bStartPrevJumpsToNext != bStartPrev->JumpsToNext()))
4323-
{
4324-
bStartPrev->RemoveFlags(BBF_NONE_QUIRK);
4325-
}
4326-
43274302
if (insertAfterBlk == nullptr)
43284303
{
43294304
// Find new location for the unlinked block(s)
@@ -4840,10 +4815,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
48404815
if (bDest == bNext)
48414816
{
48424817
// Skip jump optimizations, and try to compact block and bNext later
4843-
if (!block->isBBCallFinallyPairTail())
4844-
{
4845-
block->SetFlags(BBF_NONE_QUIRK);
4846-
}
48474818
bDest = nullptr;
48484819
}
48494820
}

‎src/coreclr/jit/fgprofile.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ void BlockCountInstrumentor::RelocateProbes()
508508
if (criticalPreds.Height() > 0)
509509
{
510510
BasicBlock* const intermediary = m_comp->fgNewBBbefore(BBJ_ALWAYS, block, /* extendRegion */ true);
511-
intermediary->SetFlags(BBF_IMPORTED | BBF_MARKED | BBF_NONE_QUIRK);
511+
intermediary->SetFlags(BBF_IMPORTED | BBF_MARKED);
512512
intermediary->inheritWeight(block);
513513
FlowEdge* const newEdge = m_comp->fgAddRefPred(block, intermediary);
514514
intermediary->SetTargetEdge(newEdge);
@@ -1679,7 +1679,7 @@ void EfficientEdgeCountInstrumentor::RelocateProbes()
16791679
if (criticalPreds.Height() > 0)
16801680
{
16811681
BasicBlock* intermediary = m_comp->fgNewBBbefore(BBJ_ALWAYS, block, /* extendRegion */ true);
1682-
intermediary->SetFlags(BBF_IMPORTED | BBF_NONE_QUIRK);
1682+
intermediary->SetFlags(BBF_IMPORTED);
16831683
intermediary->inheritWeight(block);
16841684
FlowEdge* const newEdge = m_comp->fgAddRefPred(block, intermediary);
16851685
intermediary->SetTargetEdge(newEdge);

‎src/coreclr/jit/flowgraph.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2775,7 +2775,7 @@ void Compiler::fgInsertFuncletPrologBlock(BasicBlock* block)
27752775
/* Allocate a new basic block */
27762776

27772777
BasicBlock* newHead = BasicBlock::New(this);
2778-
newHead->SetFlags(BBF_INTERNAL | BBF_NONE_QUIRK);
2778+
newHead->SetFlags(BBF_INTERNAL);
27792779
newHead->inheritWeight(block);
27802780
newHead->bbRefs = 0;
27812781

‎src/coreclr/jit/helperexpansion.cpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,6 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm
382382
FlowEdge* const newEdge = fgAddRefPred(block, fallbackBb);
383383
fallbackBb->SetTargetEdge(newEdge);
384384
assert(fallbackBb->JumpsToNext());
385-
fallbackBb->SetFlags(BBF_NONE_QUIRK);
386385
}
387386

388387
if (needsSizeCheck)
@@ -1465,15 +1464,13 @@ bool Compiler::fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, G
14651464

14661465
// Redirect prevBb from block to isInitedBb
14671466
fgRedirectTargetEdge(prevBb, isInitedBb);
1468-
prevBb->SetFlags(BBF_NONE_QUIRK);
14691467
assert(prevBb->JumpsToNext());
14701468

14711469
{
14721470
// Block has two preds now: either isInitedBb or helperCallBb
14731471
FlowEdge* const newEdge = fgAddRefPred(block, helperCallBb);
14741472
helperCallBb->SetTargetEdge(newEdge);
14751473
assert(helperCallBb->JumpsToNext());
1476-
helperCallBb->SetFlags(BBF_NONE_QUIRK);
14771474
}
14781475

14791476
{
@@ -1790,7 +1787,6 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock,
17901787
//
17911788
// Redirect prevBb to lengthCheckBb
17921789
fgRedirectTargetEdge(prevBb, lengthCheckBb);
1793-
prevBb->SetFlags(BBF_NONE_QUIRK);
17941790
assert(prevBb->JumpsToNext());
17951791

17961792
{
@@ -1810,7 +1806,6 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock,
18101806
FlowEdge* const newEdge = fgAddRefPred(block, fastpathBb);
18111807
fastpathBb->SetTargetEdge(newEdge);
18121808
assert(fastpathBb->JumpsToNext());
1813-
fastpathBb->SetFlags(BBF_NONE_QUIRK);
18141809
}
18151810

18161811
//

‎src/coreclr/jit/importer.cpp‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2052,7 +2052,7 @@ BasicBlock* Compiler::impPushCatchArgOnStack(BasicBlock* hndBlk, CORINFO_CLASS_H
20522052
// Create extra basic block for the spill
20532053
//
20542054
BasicBlock* newBlk = fgNewBBbefore(BBJ_ALWAYS, hndBlk, /* extendRegion */ true);
2055-
newBlk->SetFlags(BBF_IMPORTED | BBF_DONT_REMOVE | BBF_NONE_QUIRK);
2055+
newBlk->SetFlags(BBF_IMPORTED | BBF_DONT_REMOVE);
20562056
newBlk->inheritWeight(hndBlk);
20572057
newBlk->bbCodeOffs = hndBlk->bbCodeOffs;
20582058

@@ -7233,10 +7233,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
72337233
fgRemoveRefPred(block->GetFalseEdge());
72347234
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetTrueEdge());
72357235

7236-
// TODO-NoFallThrough: Once false target can diverge from bbNext, it may not make sense to
7237-
// set BBF_NONE_QUIRK
7238-
block->SetFlags(BBF_NONE_QUIRK);
7239-
72407236
jumpToNextOptimization = true;
72417237
}
72427238
else if (block->KindIs(BBJ_ALWAYS) && block->JumpsToNext())
@@ -7310,15 +7306,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
73107306
}
73117307
else
73127308
{
7313-
// TODO-NoFallThrough: Update once false target can diverge from bbNext
73147309
assert(block->NextIs(block->GetFalseTarget()));
73157310
JITDUMP("\nThe block jumps to the next " FMT_BB "\n", block->Next()->bbNum);
73167311
fgRemoveRefPred(block->GetTrueEdge());
73177312
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetFalseEdge());
7318-
7319-
// TODO-NoFallThrough: Once false target can diverge from bbNext, it may not make sense
7320-
// to set BBF_NONE_QUIRK
7321-
block->SetFlags(BBF_NONE_QUIRK);
73227313
}
73237314
}
73247315

@@ -7495,10 +7486,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
74957486
fgRemoveRefPred(block->GetFalseEdge());
74967487
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetTrueEdge());
74977488

7498-
// TODO-NoFallThrough: Once false target can diverge from bbNext, it may not make sense to
7499-
// set BBF_NONE_QUIRK
7500-
block->SetFlags(BBF_NONE_QUIRK);
7501-
75027489
jumpToNextOptimization = true;
75037490
}
75047491
else if (block->KindIs(BBJ_ALWAYS) && block->JumpsToNext())
@@ -7591,11 +7578,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
75917578
}
75927579

75937580
assert(foundVal);
7594-
if (block->JumpsToNext())
7595-
{
7596-
block->SetFlags(BBF_NONE_QUIRK);
7597-
}
7598-
75997581
#ifdef DEBUG
76007582
if (verbose)
76017583
{

‎src/coreclr/jit/indirectcalltransformer.cpp‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,7 @@ class IndirectCallTransformer
377377
{
378378
assert(checkIdx == 0);
379379

380-
checkBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, currBlock);
381-
checkBlock->SetFlags(BBF_NONE_QUIRK);
380+
checkBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, currBlock);
382381
GenTree* fatPointerMask = new (compiler, GT_CNS_INT) GenTreeIntCon(TYP_I_IMPL, FAT_POINTER_MASK);
383382
GenTree* fptrAddressCopy = compiler->gtCloneExpr(fptrAddress);
384383
GenTree* fatPointerAnd = compiler->gtNewOperNode(GT_AND, TYP_I_IMPL, fptrAddressCopy, fatPointerMask);
@@ -407,7 +406,6 @@ class IndirectCallTransformer
407406
virtual void CreateElse()
408407
{
409408
elseBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, thenBlock);
410-
elseBlock->SetFlags(BBF_NONE_QUIRK);
411409

412410
GenTree* fixedFptrAddress = GetFixedFptrAddress();
413411
GenTree* actualCallAddress = compiler->gtNewIndir(pointerType, fixedFptrAddress);
@@ -1061,7 +1059,6 @@ class IndirectCallTransformer
10611059
assert(checkBlock->KindIs(BBJ_ALWAYS));
10621060
FlowEdge* const checkThenEdge = compiler->fgAddRefPred(thenBlock, checkBlock);
10631061
checkBlock->SetTargetEdge(checkThenEdge);
1064-
checkBlock->SetFlags(BBF_NONE_QUIRK);
10651062
assert(checkBlock->JumpsToNext());
10661063

10671064
// SetTargetEdge() gave checkThenEdge a (correct) likelihood of 1.0.
@@ -1084,7 +1081,6 @@ class IndirectCallTransformer
10841081
{
10851082
elseBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, thenBlock);
10861083
elseBlock->CopyFlags(currBlock, BBF_SPLIT_GAINED);
1087-
elseBlock->SetFlags(BBF_NONE_QUIRK);
10881084

10891085
// We computed the "then" likelihood in CreateThen, so we
10901086
// just use that to figure out the "else" likelihood.

0 commit comments

Comments
 (0)