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

JIT: introduce redundant branch opt phase #46237

Merged
merged 1 commit into from
Dec 19, 2020
Merged

JIT: introduce redundant branch opt phase #46237

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
1 change: 1 addition & 0 deletions src/coreclr/jit/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ set( JIT_SOURCES
phase.cpp
rangecheck.cpp
rationalize.cpp
redundantbranchopts.cpp
regalloc.cpp
register_arg_convention.cpp
regset.cpp
Expand Down
152 changes: 0 additions & 152 deletions src/coreclr/jit/assertionprop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3211,7 +3211,6 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree*

//------------------------------------------------------------------------
// optAssertionProp: try and optimize a relop via assertion propagation
// (and dominator based redundant branch elimination)
//
// Arguments:
// assertions - set of live assertions
Expand All @@ -3221,163 +3220,12 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree*
// Returns:
// The modified tree, or nullptr if no assertion prop took place.
//
// Notes:
// Redundant branch elimination doesn't rely on assertions currently,
// it is done here out of convenience.
//
GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt)
{
GenTree* newTree = tree;
GenTree* op1 = tree->AsOp()->gtOp1;
GenTree* op2 = tree->AsOp()->gtOp2;

// First, walk up the dom tree and see if any dominating block has branched on
// exactly this tree's VN...
//
BasicBlock* prevBlock = compCurBB;
BasicBlock* domBlock = compCurBB->bbIDom;
int relopValue = -1;

while (domBlock != nullptr)
{
if (prevBlock == compCurBB)
{
JITDUMP("\nVN relop, checking " FMT_BB " for redundancy\n", compCurBB->bbNum);
}

// Check the current dominator
//
JITDUMP(" ... checking dom " FMT_BB "\n", domBlock->bbNum);

if (domBlock->bbJumpKind == BBJ_COND)
{
Statement* const domJumpStmt = domBlock->lastStmt();
GenTree* const domJumpTree = domJumpStmt->GetRootNode();
assert(domJumpTree->OperIs(GT_JTRUE));
GenTree* const domCmpTree = domJumpTree->AsOp()->gtGetOp1();

if (domCmpTree->OperKind() & GTK_RELOP)
{
ValueNum domCmpVN = domCmpTree->GetVN(VNK_Conservative);

// Note we could also infer the tree relop's value from similar relops higher in the dom tree.
// For example, (x >= 0) dominating (x > 0), or (x < 0) dominating (x > 0).
//
// That is left as a future enhancement.
//
if (domCmpVN == tree->GetVN(VNK_Conservative))
{
// Thes compare in "tree" is redundant.
// Is there a unique path from the dominating compare?
JITDUMP(" Redundant compare; current relop:\n");
DISPTREE(tree);
JITDUMP(" dominating relop in " FMT_BB " with same VN:\n", domBlock->bbNum);
DISPTREE(domCmpTree);

BasicBlock* trueSuccessor = domBlock->bbJumpDest;
BasicBlock* falseSuccessor = domBlock->bbNext;

const bool trueReaches = fgReachable(trueSuccessor, compCurBB);
const bool falseReaches = fgReachable(falseSuccessor, compCurBB);

if (trueReaches && falseReaches)
{
// Both dominating compare outcomes reach the current block so we can't infer the
// value of the relop.
//
// If the dominating compare is close to the current compare, this may be a missed
// opportunity to tail duplicate.
//
JITDUMP("Both successors of " FMT_BB " reach, can't optimize\n", domBlock->bbNum);

if ((trueSuccessor->GetUniqueSucc() == compCurBB) ||
(falseSuccessor->GetUniqueSucc() == compCurBB))
{
JITDUMP("Perhaps we should have tail duplicated " FMT_BB "\n", compCurBB->bbNum);
}
}
else if (trueReaches)
{
// Taken jump in dominator reaches, fall through doesn't; relop must be true.
//
JITDUMP("Jump successor " FMT_BB " of " FMT_BB " reaches, relop must be true\n",
domBlock->bbJumpDest->bbNum, domBlock->bbNum);
relopValue = 1;
break;
}
else if (falseReaches)
{
// Fall through from dominator reaches, taken jump doesn't; relop must be false.
//
JITDUMP("Fall through successor " FMT_BB " of " FMT_BB " reaches, relop must be false\n",
domBlock->bbNext->bbNum, domBlock->bbNum);
relopValue = 0;
break;
}
else
{
// No apparent path from the dominating BB.
//
// If domBlock or compCurBB is in an EH handler we may fail to find a path.
// Just ignore those cases.
//
// No point in looking further up the tree.
//
break;
}
}
}
}

// Keep looking higher up in the tree
//
prevBlock = domBlock;
domBlock = domBlock->bbIDom;
}

// Did we determine the relop value via dominance checks? If so, optimize.
//
if (relopValue == -1)
{
JITDUMP("Failed to find a suitable dominating compare, so we won't optimize\n");
}
else
{
// Bail out if tree is has certain side effects
//
// Note we really shouldn't get here if the tree has non-exception effects,
// as they should have impacted the value number.
//
if ((tree->gtFlags & GTF_SIDE_EFFECT) != 0)
{
// Bail if there is a non-exception effect.
//
if ((tree->gtFlags & GTF_SIDE_EFFECT) != GTF_EXCEPT)
{
JITDUMP("Current relop has non-exception side effects, so we won't optimize\n");
return nullptr;
}

// Be conservative if there is an exception effect and we're in an EH region
// as we might not model the full extent of EH flow.
//
if (compCurBB->hasTryIndex())
{
JITDUMP("Current relop has exception side effect and is in a try, so we won't optimize\n");
return nullptr;
}
}

JITDUMP("\nVN relop based redundant branch opt in " FMT_BB ":\n", compCurBB->bbNum);

tree->ChangeOperConst(GT_CNS_INT);
tree->AsIntCon()->gtIconVal = relopValue;

newTree = fgMorphTree(tree);
DISPTREE(newTree);
return optAssertionProp_Update(newTree, tree, stmt);
}

// Look for assertions of the form (tree EQ/NE 0)
AssertionIndex index = optGlobalAssertionIsEqualOrNotEqualZero(assertions, tree);

Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4826,6 +4826,7 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
bool doValueNum = true;
bool doLoopHoisting = true;
bool doCopyProp = true;
bool doBranchOpt = true;
bool doAssertionProp = true;
bool doRangeAnalysis = true;
int iterations = 1;
Expand All @@ -4836,6 +4837,7 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
doValueNum = doSsa && (JitConfig.JitDoValueNumber() != 0);
doLoopHoisting = doValueNum && (JitConfig.JitDoLoopHoisting() != 0);
doCopyProp = doValueNum && (JitConfig.JitDoCopyProp() != 0);
doBranchOpt = doValueNum && (JitConfig.JitDoRedundantBranchOpts() != 0);
doAssertionProp = doValueNum && (JitConfig.JitDoAssertionProp() != 0);
doRangeAnalysis = doAssertionProp && (JitConfig.JitDoRangeAnalysis() != 0);

Expand Down Expand Up @@ -4882,6 +4884,11 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
DoPhase(this, PHASE_VN_COPY_PROP, &Compiler::optVnCopyProp);
}

if (doBranchOpt)
Copy link
Member

Choose a reason for hiding this comment

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

Earlier, we would do redundant branch elimination after CSE and now we do it before. Is that the source of diff?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

{
DoPhase(this, PHASE_OPTIMIZE_BRANCHES, &Compiler::optRedundantBranches);
}

#if FEATURE_ANYCSE
// Remove common sub-expressions
//
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6828,6 +6828,11 @@ class Compiler
BasicBlock* basicBlock);
#endif

// Redundant branch opts
//
PhaseStatus optRedundantBranches();
bool optRedundantBranch(BasicBlock* const block);

#if ASSERTION_PROP
/**************************************************************************
* Value/Assertion propagation
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compphases.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_VALNUM_CSES, "Optimize Valnum CSEs",
#endif

CompPhaseNameMacro(PHASE_VN_COPY_PROP, "VN based copy prop", "CP-PROP", false, -1, false)
CompPhaseNameMacro(PHASE_OPTIMIZE_BRANCHES, "Redundant branch opts", "OPT-BR", false, -1, false)
#if ASSERTION_PROP
CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN, "Assertion prop", "AST-PROP", false, -1, false)
#endif
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/jitconfigvalues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ CONFIG_INTEGER(JitDoCopyProp, W("JitDoCopyProp"), 1) // Perform copy propagati
CONFIG_INTEGER(JitDoEarlyProp, W("JitDoEarlyProp"), 1) // Perform Early Value Propagation
CONFIG_INTEGER(JitDoLoopHoisting, W("JitDoLoopHoisting"), 1) // Perform loop hoisting on loop invariant values
CONFIG_INTEGER(JitDoRangeAnalysis, W("JitDoRangeAnalysis"), 1) // Perform range check analysis
CONFIG_INTEGER(JitDoRedundantBranchOpts, W("JitDoRedundantBranchOpts"), 1) // Perform redundant branch optimizations
CONFIG_INTEGER(JitDoSsa, W("JitDoSsa"), 1) // Perform Static Single Assignment (SSA) numbering on the variables
CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numbering on method expressions

Expand Down
Loading