Skip to content

Commit

Permalink
JIT: Remove Compiler::fgIsBetterFallThrough (dotnet#97222)
Browse files Browse the repository at this point in the history
  • Loading branch information
amanasifkhalid authored and tmds committed Jan 23, 2024
1 parent 5a7aa9d commit cf2487c
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 99 deletions.
2 changes: 0 additions & 2 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5927,8 +5927,6 @@ class Compiler

void fgFindBasicBlocks();

bool fgIsBetterFallThrough(BasicBlock* bCur, BasicBlock* bAlt);

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

BasicBlock* fgFindInsertPoint(unsigned regionIndex,
Expand Down
100 changes: 3 additions & 97 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6378,83 +6378,6 @@ void Compiler::fgInsertBBafter(BasicBlock* insertAfterBlk, BasicBlock* newBlk)
insertAfterBlk->SetNext(newBlk);
}

// We have two edges (bAlt => bCur) and (bCur => bNext).
//
// Returns true if the weight of (bAlt => bCur)
// is greater than the weight of (bCur => bNext).
// We compare the edge weights if we have valid edge weights
// otherwise we compare blocks weights.
//
bool Compiler::fgIsBetterFallThrough(BasicBlock* bCur, BasicBlock* bAlt)
{
// bCur can't be NULL and must be a fall through bbKind
noway_assert(bCur != nullptr);
noway_assert(bCur->bbFallsThrough() || (bCur->KindIs(BBJ_ALWAYS) && bCur->JumpsToNext()));
noway_assert(bAlt != nullptr);

if (bAlt->KindIs(BBJ_ALWAYS))
{
// If bAlt doesn't jump to bCur, it can't be a better fall through than bCur
if (!bAlt->TargetIs(bCur))
{
return false;
}
}
else if (bAlt->KindIs(BBJ_COND))
{
// If bAlt doesn't potentially jump to bCur, it can't be a better fall through than bCur
if (!bAlt->TrueTargetIs(bCur))
{
return false;
}
}
else
{
// We only handle the cases when bAlt is a BBJ_ALWAYS or a BBJ_COND
return false;
}

// BBJ_CALLFINALLY shouldn't have a flow edge into the next (BBJ_CALLFINALLYRET) block
if (bCur->isBBCallFinallyPair())
{
return false;
}

// Currently bNext is the fall through for bCur
// TODO-NoFallThrough: Allow bbFalseTarget to diverge from bbNext for BBJ_COND
assert(!bCur->KindIs(BBJ_COND) || bCur->NextIs(bCur->GetFalseTarget()));
BasicBlock* bNext = bCur->Next();
noway_assert(bNext != nullptr);

// We will set result to true if bAlt is a better fall through than bCur
bool result;
if (fgHaveValidEdgeWeights)
{
// We will compare the edge weight for our two choices
FlowEdge* edgeFromAlt = fgGetPredForBlock(bCur, bAlt);
FlowEdge* edgeFromCur = fgGetPredForBlock(bNext, bCur);
noway_assert(edgeFromCur != nullptr);
noway_assert(edgeFromAlt != nullptr);

result = (edgeFromAlt->edgeWeightMin() > edgeFromCur->edgeWeightMax());
}
else
{
if (bAlt->KindIs(BBJ_ALWAYS))
{
// Our result is true if bAlt's weight is more than bCur's weight
result = (bAlt->bbWeight > bCur->bbWeight);
}
else
{
noway_assert(bAlt->KindIs(BBJ_COND));
// Our result is true if bAlt's weight is more than twice bCur's weight
result = (bAlt->bbWeight > (2 * bCur->bbWeight));
}
}
return result;
}

//------------------------------------------------------------------------
// Finds the block closest to endBlk in the range [startBlk..endBlk) after which a block can be
// inserted easily. Note that endBlk cannot be returned; its predecessor is the last block that can
Expand Down Expand Up @@ -6663,32 +6586,15 @@ BasicBlock* Compiler::fgFindInsertPoint(unsigned regionIndex,
}
}

// Look for an insert location:
// 1. We want blocks that don't end with a fall through,
// 2. Also, when blk equals nearBlk we may want to insert here.
// Look for an insert location. We want blocks that don't end with a fall through.
// Quirk: Manually check for BBJ_COND fallthrough behavior
const bool blkFallsThrough =
blk->bbFallsThrough() && (!blk->KindIs(BBJ_COND) || blk->NextIs(blk->GetFalseTarget()));
const bool blkJumpsToNext = blk->KindIs(BBJ_ALWAYS) && blk->HasFlag(BBF_NONE_QUIRK) && blk->JumpsToNext();
if ((!blkFallsThrough && !blkJumpsToNext) || (blk == nearBlk))
if (!blkFallsThrough && !blkJumpsToNext)
{
bool updateBestBlk = true; // We will probably update the bestBlk

// If blk falls through then we must decide whether to use the nearBlk
// hint
if (blkFallsThrough || blkJumpsToNext)
{
noway_assert(blk == nearBlk);
if (jumpBlk != nullptr)
{
updateBestBlk = fgIsBetterFallThrough(blk, jumpBlk);
}
else
{
updateBestBlk = false;
}
}

// If we already have a best block, see if the 'runRarely' flags influences
// our choice. If we want a runRarely insertion point, and the existing best
// block is run rarely but the current block isn't run rarely, then don't
Expand All @@ -6697,7 +6603,7 @@ BasicBlock* Compiler::fgFindInsertPoint(unsigned regionIndex,
// want a non-rarely-run block), but bestBlock->isRunRarely() is true. In that
// case, we should update the block, also. Probably what we want is:
// (bestBlk->isRunRarely() != runRarely) && (blk->isRunRarely() == runRarely)
if (updateBestBlk && (bestBlk != nullptr) && runRarely && bestBlk->isRunRarely() && !blk->isRunRarely())
if ((bestBlk != nullptr) && runRarely && bestBlk->isRunRarely() && !blk->isRunRarely())
{
updateBestBlk = false;
}
Expand Down

0 comments on commit cf2487c

Please sign in to comment.