Skip to content

Merge return true/false blocks #94387

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

Merged
EgorBo merged 1 commit into from
Nov 9, 2023
Merged

Merge return true/false blocks #94387

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
170 changes: 101 additions & 69 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6729,69 +6729,7 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
// If return value is true, retry.
// May also add to retryBlocks.
//
auto tailMerge = [&](BasicBlock* block) -> bool {

if (block->countOfInEdges() < 2)
{
// Nothing to merge here
return false;
}

predInfo.Reset();

// Find the subset of preds that reach along non-critical edges
// and populate predInfo.
//
for (BasicBlock* const predBlock : block->PredBlocks())
{
if (predBlock->GetUniqueSucc() != block)
{
continue;
}

if (!BasicBlock::sameEHRegion(block, predBlock))
{
continue;
}

Statement* lastStmt = predBlock->lastStmt();

// Block might be empty.
//
if (lastStmt == nullptr)
{
continue;
}

// Walk back past any GT_NOPs.
//
Statement* const firstStmt = predBlock->firstStmt();
while (lastStmt->GetRootNode()->OperIs(GT_NOP))
{
if (lastStmt == firstStmt)
{
// predBlock is evidently all GT_NOP.
//
lastStmt = nullptr;
break;
}

lastStmt = lastStmt->GetPrevStmt();
}

// Block might be effectively empty.
//
if (lastStmt == nullptr)
{
continue;
}

// We don't expect to see PHIs but watch for them anyways.
//
assert(!lastStmt->IsPhiDefnStmt());
predInfo.Emplace(predBlock, lastStmt);
}

auto tailMergePreds = [&](BasicBlock* commSucc) -> bool {
Copy link
Copy Markdown
Member

@jakobbotsch jakobbotsch Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to convert these to functions instead of massive lambdas... but that's a no-diff refactoring to be done separately.

// Are there enough preds to make it interesting?
//
if (predInfo.Height() < 2)
Expand Down Expand Up @@ -6842,9 +6780,9 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
// We have some number of preds that have identical last statements.
// If all preds of block have a matching last stmt, move that statement to the start of block.
//
if (matchedPredInfo.Height() == (int)block->countOfInEdges())
if ((commSucc != nullptr) && (matchedPredInfo.Height() == (int)commSucc->countOfInEdges()))
{
JITDUMP("All preds of " FMT_BB " end with the same tree, moving\n", block->bbNum);
JITDUMP("All preds of " FMT_BB " end with the same tree, moving\n", commSucc->bbNum);
JITDUMPEXEC(gtDispStmt(matchedPredInfo.TopRef(0).m_stmt));

for (int j = 0; j < matchedPredInfo.Height(); j++)
Expand All @@ -6860,8 +6798,8 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
//
if (j == 0)
{
fgInsertStmtAtBeg(block, stmt);
block->bbFlags |= predBlock->bbFlags & BBF_COPY_PROPAGATE;
fgInsertStmtAtBeg(commSucc, stmt);
commSucc->bbFlags |= predBlock->bbFlags & BBF_COPY_PROPAGATE;
}

madeChanges = true;
Expand All @@ -6876,7 +6814,16 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
// Pick one block as the victim -- preferably a block with just one
// statement or one that falls through to block (or both).
//
JITDUMP("A set of %d preds of " FMT_BB " end with the same tree\n", matchedPredInfo.Height(), block->bbNum);
if (commSucc != nullptr)
{
JITDUMP("A set of %d preds of " FMT_BB " end with the same tree\n", matchedPredInfo.Height(),
commSucc->bbNum);
}
else
{
JITDUMP("A set of %d return blocks end with the same tree\n", matchedPredInfo.Height());
}

JITDUMPEXEC(gtDispStmt(matchedPredInfo.TopRef(0).m_stmt));

BasicBlock* crossJumpVictim = nullptr;
Expand Down Expand Up @@ -6977,7 +6924,10 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
//
predBlock->SetJumpKindAndTarget(BBJ_ALWAYS, crossJumpTarget DEBUG_ARG(this));

fgRemoveRefPred(block, predBlock);
if (commSucc != nullptr)
{
fgRemoveRefPred(commSucc, predBlock);
}
fgAddRefPred(crossJumpTarget, predBlock);
}

Expand All @@ -7001,6 +6951,71 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
return false;
};

auto tailMerge = [&](BasicBlock* block) -> bool {
if (block->countOfInEdges() < 2)
{
// Nothing to merge here
return false;
}

predInfo.Reset();

// Find the subset of preds that reach along non-critical edges
// and populate predInfo.
//
for (BasicBlock* const predBlock : block->PredBlocks())
{
if (predBlock->GetUniqueSucc() != block)
{
continue;
}

if (!BasicBlock::sameEHRegion(block, predBlock))
{
continue;
}

Statement* lastStmt = predBlock->lastStmt();

// Block might be empty.
//
if (lastStmt == nullptr)
{
continue;
}

// Walk back past any GT_NOPs.
//
Statement* const firstStmt = predBlock->firstStmt();
while (lastStmt->GetRootNode()->OperIs(GT_NOP))
{
if (lastStmt == firstStmt)
{
// predBlock is evidently all GT_NOP.
//
lastStmt = nullptr;
break;
}

lastStmt = lastStmt->GetPrevStmt();
}

// Block might be effectively empty.
//
if (lastStmt == nullptr)
{
continue;
}

// We don't expect to see PHIs but watch for them anyways.
//
assert(!lastStmt->IsPhiDefnStmt());
predInfo.Emplace(predBlock, lastStmt);
}

return tailMergePreds(block);
};

auto iterateTailMerge = [&](BasicBlock* block) -> void {

int numOpts = 0;
Expand All @@ -7016,13 +7031,30 @@ PhaseStatus Compiler::fgHeadTailMerge(bool early)
}
};

ArrayStack<BasicBlock*> retBlocks(getAllocator(CMK_ArrayStack));

// Visit each block
//
for (BasicBlock* const block : Blocks())
{
iterateTailMerge(block);

// TODO: consider removing hasSingleStmt(), it should find more opportunities
// (with size and TP regressions)
if (block->KindIs(BBJ_RETURN) && block->hasSingleStmt() && (block != genReturnBB))
{
retBlocks.Push(block);
}
}

predInfo.Reset();
for (int i = 0; i < retBlocks.Height(); i++)
{
predInfo.Push(PredInfo(retBlocks.Bottom(i), retBlocks.Bottom(i)->lastStmt()));
}

tailMergePreds(nullptr);

// Work through any retries
//
while (retryBlocks.Height() > 0)
Expand Down