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.

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

JIT: Clean up fgReplaceJumpTarget call sites #111006

Merged
merged 3 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 1 addition & 16 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5086,22 +5086,7 @@ BasicBlock* Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable)
for (BasicBlock* const predBlock : block->PredBlocksEditing())
{
/* change all jumps/refs to the removed block */
switch (predBlock->GetKind())
{
default:
noway_assert(!"Unexpected bbKind in fgRemoveBlock()");
break;

case BBJ_COND:
case BBJ_CALLFINALLY:
case BBJ_CALLFINALLYRET:
case BBJ_ALWAYS:
case BBJ_EHCATCHRET:
case BBJ_SWITCH:
case BBJ_EHFINALLYRET:
fgReplaceJumpTarget(predBlock, block, succBlock);
break;
}
fgReplaceJumpTarget(predBlock, block, succBlock);
}

fgUnlinkBlockForRemoval(block);
Expand Down
48 changes: 7 additions & 41 deletions src/coreclr/jit/fgehopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2329,7 +2329,6 @@ PhaseStatus Compiler::fgTailMergeThrows()
// The second pass modifies flow so that predecessors of
// non-canonical throw blocks now transfer control to the
// appropriate canonical block.
unsigned numCandidates = 0;

// First pass
//
Expand Down Expand Up @@ -2393,7 +2392,6 @@ PhaseStatus Compiler::fgTailMergeThrows()
// Yes, this one can be optimized away...
JITDUMP(" in " FMT_BB " can be dup'd to canonical " FMT_BB "\n", block->bbNum, canonicalBlock->bbNum);
blockMap.Set(block, canonicalBlock);
numCandidates++;
}
else
{
Expand All @@ -2403,9 +2401,8 @@ PhaseStatus Compiler::fgTailMergeThrows()
}
}

assert(numCandidates <= optNoReturnCallCount);

// Bail if no candidates were found
const unsigned numCandidates = blockMap.GetCount();
if (numCandidates == 0)
{
JITDUMP("\n*************** no throws can be tail merged, sorry\n");
Expand All @@ -2417,56 +2414,25 @@ PhaseStatus Compiler::fgTailMergeThrows()
// Second pass.
//
// We walk the map rather than the block list, to save a bit of time.
unsigned updateCount = 0;

for (BlockToBlockMap::Node* const iter : BlockToBlockMap::KeyValueIteration(&blockMap))
{
BasicBlock* const nonCanonicalBlock = iter->GetKey();
BasicBlock* const canonicalBlock = iter->GetValue();
FlowEdge* nextPredEdge = nullptr;
bool updated = false;

// Walk pred list of the non canonical block, updating flow to target
// the canonical block instead.
for (BasicBlock* const predBlock : nonCanonicalBlock->PredBlocksEditing())
{
switch (predBlock->GetKind())
{
case BBJ_ALWAYS:
case BBJ_COND:
case BBJ_SWITCH:
{
JITDUMP("*** " FMT_BB " now branching to " FMT_BB "\n", predBlock->bbNum, canonicalBlock->bbNum);
fgReplaceJumpTarget(predBlock, nonCanonicalBlock, canonicalBlock);
updated = true;
}
break;

default:
// We don't expect other kinds of preds, and it is safe to ignore them
// as flow is still correct, just not as optimized as it could be.
break;
}
JITDUMP("*** " FMT_BB " now branching to " FMT_BB "\n", predBlock->bbNum, canonicalBlock->bbNum);
fgReplaceJumpTarget(predBlock, nonCanonicalBlock, canonicalBlock);
}

if (updated)
{
updateCount++;
}
}

if (updateCount == 0)
{
return PhaseStatus::MODIFIED_NOTHING;
}

// TODO: Update the count of noreturn call sites -- this feeds a heuristic in morph
// to determine if these noreturn calls should be tail called.
//
// Updating the count does not lead to better results, so deferring for now.
// Update the count of noreturn call sites
//
JITDUMP("Made %u updates\n", updateCount);
assert(updateCount < optNoReturnCallCount);
JITDUMP("Made %u updates\n", numCandidates);
assert(numCandidates < optNoReturnCallCount);
optNoReturnCallCount -= numCandidates;

// If we altered flow, reset fgModified. Given where we sit in the
// phase list, flow-dependent side data hasn't been built yet, so
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/redundantbranchopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1709,7 +1709,7 @@ bool Compiler::optJumpThreadCore(JumpThreadInfo& jti)
BasicBlock* const ambBlock = jti.m_ambiguousVNBlock;
if ((ambBlock != nullptr) && jti.m_block->KindIs(BBJ_COND) && (jti.m_block->GetUniquePred(this) == ambBlock))
{
JITDUMP(FMT_BB " has just one remaining predcessor " FMT_BB "\n", jti.m_block->bbNum, ambBlock->bbNum);
JITDUMP(FMT_BB " has just one remaining predecessor " FMT_BB "\n", jti.m_block->bbNum, ambBlock->bbNum);

Statement* const stmt = jti.m_block->lastStmt();
assert(stmt != nullptr);
Expand Down
Loading