Skip to content
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

Jump to bottom

JIT: Remove BBF_NONE_QUIRK #99907

Merged
merged 1 commit into from
Mar 18, 2024
Merged

JIT: Remove BBF_NONE_QUIRK #99907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions src/coreclr/jit/block.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,6 @@ void BasicBlock::dspFlags() const
{BBF_HAS_ALIGN, "has-align"},
{BBF_HAS_MDARRAYREF, "mdarr"},
{BBF_NEEDS_GCPOLL, "gcpoll"},
{BBF_NONE_QUIRK, "q"},
};

bool first = true;
Expand Down Expand Up @@ -941,9 +940,6 @@ void BasicBlock::TransferTarget(BasicBlock* from)
SetCond(from->bbTrueEdge, from->bbFalseEdge);
break;
case BBJ_ALWAYS:
SetKindAndTargetEdge(BBJ_ALWAYS, from->bbTargetEdge);
CopyFlags(from, BBF_NONE_QUIRK);
break;
case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
case BBJ_EHCATCHRET:
Expand Down
8 changes: 2 additions & 6 deletions src/coreclr/jit/block.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,11 +456,7 @@ enum BasicBlockFlags : unsigned __int64
BBF_RECURSIVE_TAILCALL = MAKE_BBFLAG(37), // Block has recursive tailcall that may turn into a loop
BBF_NO_CSE_IN = MAKE_BBFLAG(38), // Block should kill off any incoming CSE
BBF_CAN_ADD_PRED = MAKE_BBFLAG(39), // Ok to add pred edge to this block, even when "safe" edge creation disabled
BBF_NONE_QUIRK = MAKE_BBFLAG(40), // Block was created as a BBJ_ALWAYS to the next block,
// and should be treated as if it falls through.
// This is just to reduce diffs from removing BBJ_NONE.
// (TODO: Remove this quirk after refactoring Compiler::fgFindInsertPoint)
BBF_HAS_VALUE_PROFILE = MAKE_BBFLAG(41), // Block has a node that needs a value probing
BBF_HAS_VALUE_PROFILE = MAKE_BBFLAG(40), // Block has a node that needs a value probing

// The following are sets of flags.

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

BBF_SPLIT_GAINED = BBF_DONT_REMOVE | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_PROF_WEIGHT | \
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,
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,

// Flags that must be propagated to a new block if code is copied from a block to a new block. These are flags that
// limit processing of a block if the code in question doesn't exist. This is conservative; we might not
Expand Down
24 changes: 2 additions & 22 deletions src/coreclr/jit/fgbasic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ bool Compiler::fgEnsureFirstBBisScratch()
noway_assert(fgLastBB != nullptr);

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

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

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

curr->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
curr->SetFlags(BBF_NONE_QUIRK);
assert(curr->JumpsToNext());

return newBlock;
Expand Down Expand Up @@ -4898,7 +4896,6 @@ BasicBlock* Compiler::fgSplitBlockBeforeTree(

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

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

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

#else // !FEATURE_EH_FUNCLETS
#else // !FEATURE_EH_FUNCLETS

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

// We have decided to insert the block(s) after fgLastBlock
fgMoveBlocksAfter(bStart, bLast, insertAfterBlk);

if (bPrev->KindIs(BBJ_ALWAYS) && bPrev->JumpsToNext())
{
bPrev->SetFlags(BBF_NONE_QUIRK);
}

if (bLast->KindIs(BBJ_ALWAYS) && bLast->JumpsToNext())
{
bLast->SetFlags(BBF_NONE_QUIRK);
}

#endif // !FEATURE_EH_FUNCLETS

goto DONE;
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/jit/fginline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,6 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
{
m_compiler->fgRemoveRefPred(block->GetTrueEdge());
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetFalseEdge());
block->SetFlags(BBF_NONE_QUIRK);
}
else
{
Expand Down Expand Up @@ -1536,11 +1535,6 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)

FlowEdge* const newEdge = fgAddRefPred(bottomBlock, block);
block->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);

if (block == InlineeCompiler->fgLastBB)
{
block->SetFlags(BBF_NONE_QUIRK);
}
}
}

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

topBlock->SetNext(InlineeCompiler->fgFirstBB);
topBlock->SetFlags(BBF_NONE_QUIRK);
InlineeCompiler->fgLastBB->SetNext(bottomBlock);

//
Expand Down
29 changes: 0 additions & 29 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,6 @@ PhaseStatus Compiler::fgPostImportationCleanup()
else
{
FlowEdge* const newEdge = fgAddRefPred(newTryEntry->Next(), newTryEntry);
newTryEntry->SetFlags(BBF_NONE_QUIRK);
newTryEntry->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge);
}

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

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

if (block->KindIs(BBJ_ALWAYS))
{
// Propagate BBF_NONE_QUIRK flag
block->CopyFlags(bNext, BBF_NONE_QUIRK);
}
else
{
// It's no longer a BBJ_ALWAYS; remove the BBF_NONE_QUIRK flag.
block->RemoveFlags(BBF_NONE_QUIRK);
}

#if DEBUG
if (verbose && 0)
{
Expand Down Expand Up @@ -2613,9 +2601,6 @@ void Compiler::fgRemoveConditionalJump(BasicBlock* block)
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetTrueEdge());
assert(block->TargetIs(target));

// TODO-NoFallThrough: Set BBF_NONE_QUIRK only when false target is the next block
block->SetFlags(BBF_NONE_QUIRK);

/* Update bbRefs and bbNum - Conditional predecessors to the same
* block are counted twice so we have to remove one of them */

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

// If bStartPrev is a BBJ_ALWAYS to some block after bStart, unlinking bStart can move
// bStartPrev's jump destination up, making bStartPrev jump to the next block for now.
// This can lead us to make suboptimal decisions in Compiler::fgFindInsertPoint,
// so make sure the BBF_NONE_QUIRK flag is unset for bStartPrev beforehand.
// TODO: Remove quirk.
if (bStartPrev->KindIs(BBJ_ALWAYS) && (bStartPrevJumpsToNext != bStartPrev->JumpsToNext()))
{
bStartPrev->RemoveFlags(BBF_NONE_QUIRK);
}

if (insertAfterBlk == nullptr)
{
// Find new location for the unlinked block(s)
Expand Down Expand Up @@ -4840,10 +4815,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
if (bDest == bNext)
{
// Skip jump optimizations, and try to compact block and bNext later
if (!block->isBBCallFinallyPairTail())
{
block->SetFlags(BBF_NONE_QUIRK);
}
bDest = nullptr;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/fgprofile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ void BlockCountInstrumentor::RelocateProbes()
if (criticalPreds.Height() > 0)
{
BasicBlock* const intermediary = m_comp->fgNewBBbefore(BBJ_ALWAYS, block, /* extendRegion */ true);
intermediary->SetFlags(BBF_IMPORTED | BBF_MARKED | BBF_NONE_QUIRK);
intermediary->SetFlags(BBF_IMPORTED | BBF_MARKED);
intermediary->inheritWeight(block);
FlowEdge* const newEdge = m_comp->fgAddRefPred(block, intermediary);
intermediary->SetTargetEdge(newEdge);
Expand Down Expand Up @@ -1679,7 +1679,7 @@ void EfficientEdgeCountInstrumentor::RelocateProbes()
if (criticalPreds.Height() > 0)
{
BasicBlock* intermediary = m_comp->fgNewBBbefore(BBJ_ALWAYS, block, /* extendRegion */ true);
intermediary->SetFlags(BBF_IMPORTED | BBF_NONE_QUIRK);
intermediary->SetFlags(BBF_IMPORTED);
intermediary->inheritWeight(block);
FlowEdge* const newEdge = m_comp->fgAddRefPred(block, intermediary);
intermediary->SetTargetEdge(newEdge);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2775,7 +2775,7 @@ void Compiler::fgInsertFuncletPrologBlock(BasicBlock* block)
/* Allocate a new basic block */

BasicBlock* newHead = BasicBlock::New(this);
newHead->SetFlags(BBF_INTERNAL | BBF_NONE_QUIRK);
newHead->SetFlags(BBF_INTERNAL);
newHead->inheritWeight(block);
newHead->bbRefs = 0;

Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/jit/helperexpansion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,6 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stm
FlowEdge* const newEdge = fgAddRefPred(block, fallbackBb);
fallbackBb->SetTargetEdge(newEdge);
assert(fallbackBb->JumpsToNext());
fallbackBb->SetFlags(BBF_NONE_QUIRK);
}

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

// Redirect prevBb from block to isInitedBb
fgRedirectTargetEdge(prevBb, isInitedBb);
prevBb->SetFlags(BBF_NONE_QUIRK);
assert(prevBb->JumpsToNext());

{
// Block has two preds now: either isInitedBb or helperCallBb
FlowEdge* const newEdge = fgAddRefPred(block, helperCallBb);
helperCallBb->SetTargetEdge(newEdge);
assert(helperCallBb->JumpsToNext());
helperCallBb->SetFlags(BBF_NONE_QUIRK);
}

{
Expand Down Expand Up @@ -1790,7 +1787,6 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock,
//
// Redirect prevBb to lengthCheckBb
fgRedirectTargetEdge(prevBb, lengthCheckBb);
prevBb->SetFlags(BBF_NONE_QUIRK);
assert(prevBb->JumpsToNext());

{
Expand All @@ -1810,7 +1806,6 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock,
FlowEdge* const newEdge = fgAddRefPred(block, fastpathBb);
fastpathBb->SetTargetEdge(newEdge);
assert(fastpathBb->JumpsToNext());
fastpathBb->SetFlags(BBF_NONE_QUIRK);
}

//
Expand Down
20 changes: 1 addition & 19 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2052,7 +2052,7 @@ BasicBlock* Compiler::impPushCatchArgOnStack(BasicBlock* hndBlk, CORINFO_CLASS_H
// Create extra basic block for the spill
//
BasicBlock* newBlk = fgNewBBbefore(BBJ_ALWAYS, hndBlk, /* extendRegion */ true);
newBlk->SetFlags(BBF_IMPORTED | BBF_DONT_REMOVE | BBF_NONE_QUIRK);
newBlk->SetFlags(BBF_IMPORTED | BBF_DONT_REMOVE);
newBlk->inheritWeight(hndBlk);
newBlk->bbCodeOffs = hndBlk->bbCodeOffs;

Expand Down Expand Up @@ -7233,10 +7233,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
fgRemoveRefPred(block->GetFalseEdge());
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetTrueEdge());

// TODO-NoFallThrough: Once false target can diverge from bbNext, it may not make sense to
// set BBF_NONE_QUIRK
block->SetFlags(BBF_NONE_QUIRK);

jumpToNextOptimization = true;
}
else if (block->KindIs(BBJ_ALWAYS) && block->JumpsToNext())
Expand Down Expand Up @@ -7310,15 +7306,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
}
else
{
// TODO-NoFallThrough: Update once false target can diverge from bbNext
assert(block->NextIs(block->GetFalseTarget()));
JITDUMP("\nThe block jumps to the next " FMT_BB "\n", block->Next()->bbNum);
fgRemoveRefPred(block->GetTrueEdge());
block->SetKindAndTargetEdge(BBJ_ALWAYS, block->GetFalseEdge());

// TODO-NoFallThrough: Once false target can diverge from bbNext, it may not make sense
// to set BBF_NONE_QUIRK
block->SetFlags(BBF_NONE_QUIRK);
}
}

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

// TODO-NoFallThrough: Once false target can diverge from bbNext, it may not make sense to
// set BBF_NONE_QUIRK
block->SetFlags(BBF_NONE_QUIRK);

jumpToNextOptimization = true;
}
else if (block->KindIs(BBJ_ALWAYS) && block->JumpsToNext())
Expand Down Expand Up @@ -7591,11 +7578,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
}

assert(foundVal);
if (block->JumpsToNext())
{
block->SetFlags(BBF_NONE_QUIRK);
}

#ifdef DEBUG
if (verbose)
{
Expand Down
6 changes: 1 addition & 5 deletions src/coreclr/jit/indirectcalltransformer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,7 @@ class IndirectCallTransformer
{
assert(checkIdx == 0);

checkBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, currBlock);
checkBlock->SetFlags(BBF_NONE_QUIRK);
checkBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, currBlock);
GenTree* fatPointerMask = new (compiler, GT_CNS_INT) GenTreeIntCon(TYP_I_IMPL, FAT_POINTER_MASK);
GenTree* fptrAddressCopy = compiler->gtCloneExpr(fptrAddress);
GenTree* fatPointerAnd = compiler->gtNewOperNode(GT_AND, TYP_I_IMPL, fptrAddressCopy, fatPointerMask);
Expand Down Expand Up @@ -407,7 +406,6 @@ class IndirectCallTransformer
virtual void CreateElse()
{
elseBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, thenBlock);
elseBlock->SetFlags(BBF_NONE_QUIRK);

GenTree* fixedFptrAddress = GetFixedFptrAddress();
GenTree* actualCallAddress = compiler->gtNewIndir(pointerType, fixedFptrAddress);
Expand Down Expand Up @@ -1061,7 +1059,6 @@ class IndirectCallTransformer
assert(checkBlock->KindIs(BBJ_ALWAYS));
FlowEdge* const checkThenEdge = compiler->fgAddRefPred(thenBlock, checkBlock);
checkBlock->SetTargetEdge(checkThenEdge);
checkBlock->SetFlags(BBF_NONE_QUIRK);
assert(checkBlock->JumpsToNext());

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

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