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: Run optRedundantBranches twice and try to compute more accurate doms #70907

Merged
merged 18 commits into from
Jul 2, 2022
Merged
Show file tree
Hide file tree
Changes from 13 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
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5178,6 +5178,8 @@ class Compiler

BasicBlock* fgEndBBAfterMainFunction();

BasicBlock* fgGetDomSpeculatively(const BasicBlock* block);

void fgUnlinkRange(BasicBlock* bBeg, BasicBlock* bEnd);

void fgRemoveBlock(BasicBlock* block, bool unreachable);
Expand Down
47 changes: 47 additions & 0 deletions src/coreclr/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3116,6 +3116,53 @@ BasicBlock* Compiler::fgLastBBInMainFunction()
return fgLastBB;
}

//------------------------------------------------------------------------------
// fgGetDomSpeculatively: Try determine a more accurate dominator than cached bbIDom
//
// Arguments:
// block - Basic block to get a dominator for
//
// Return Value:
// Basic block that dominates this block
//
BasicBlock* Compiler::fgGetDomSpeculatively(const BasicBlock* block)
{
assert(fgDomsComputed);
int reachablePreds = 0;
BasicBlock* lastReachablePred = nullptr;

// Check if we have unreachable preds
for (const flowList* predEdge : block->PredEdges())
{
BasicBlock* predBlock = predEdge->getBlock();
if (predBlock == block)
{
// can a block have a predEdge to itself?
return block->bbIDom;
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}

// We check pred's count of InEdges - it's quite conservative.
// We, probably, could use fgReachable(fgFirstBb, pred) here to detect unreachable preds
if (predBlock->countOfInEdges() > 0)
{
lastReachablePred = predBlock;
reachablePreds++;
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
if (reachablePreds > 1)
{
// Too many "reachable" preds - return cached result
return block->bbIDom;
}
}
}

if (reachablePreds == 1)
{
assert(lastReachablePred != nullptr);
return lastReachablePred;
}
return block->bbIDom;
}

/*****************************************************************************************************
*
* Function to return the first basic block after the main part of the function. With funclets, it is
Expand Down
49 changes: 48 additions & 1 deletion src/coreclr/jit/redundantbranchopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,29 @@ PhaseStatus Compiler::optRedundantBranches()
if (block->bbJumpKind == BBJ_COND)
{
madeChanges |= m_compiler->optRedundantRelop(block);

BasicBlock* bbNext = block->bbNext;
BasicBlock* bbJump = block->bbJumpDest;

madeChanges |= m_compiler->optRedundantBranch(block);

// It's possible that either bbNext or bbJump were unlinked and it's proven
// to be profitable to pay special attention to their successors.
if (madeChanges && (bbNext != nullptr) && (bbNext->countOfInEdges() == 0))
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
for (BasicBlock* succ : bbNext->Succs())
{
m_compiler->optRedundantBranch(succ);
}
}

if (madeChanges && (bbJump != nullptr) && (bbJump->countOfInEdges() == 0))
{
for (BasicBlock* succ : bbJump->Succs())
{
m_compiler->optRedundantBranch(succ);
}
}
}
}
};
Expand Down Expand Up @@ -293,8 +315,27 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)
relopValue == 0 ? "false" : "true");
}

while ((relopValue == -1) && (domBlock != nullptr))
bool trySpeculativeDom = false;
while ((relopValue == -1) && !trySpeculativeDom)
{
if (domBlock == nullptr)
{
// It's possible that bbIDom is not up to date at this point due to recent BB modifications
// so let's try to quickly calculate new one
domBlock = fgGetDomSpeculatively(block);
if (domBlock == block->bbIDom)
{
// We already checked this one
break;
}
trySpeculativeDom = true;
}

if (domBlock == nullptr)
{
break;
}

// Check the current dominator
//
if (domBlock->bbJumpKind == BBJ_COND)
Expand Down Expand Up @@ -351,6 +392,12 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)

if (trueReaches && falseReaches && rii.canInferFromTrue && rii.canInferFromFalse)
{
// JIT-TP: it didn't produce diffs so let's skip it
if (trySpeculativeDom)
{
break;
}

// Both dominating compare outcomes reach the current block so we can't infer the
// value of the relop.
//
Expand Down