Skip to content

Commit

Permalink
JIT: introduce redundant branch opt phase (#46237)
Browse files Browse the repository at this point in the history 
Move the redundant branch elimination out of assertion prop into its
own phase, in anticipation of future enhancements. Run it a bit earlier
(before CSE).
  • Loading branch information
@AndyAyersMS
AndyAyersMS authored Dec 19, 2020
1 parent 6dd7b2d commit c6d988f
Show file tree
Hide file tree
Showing 7 changed files with 236 additions and 152 deletions.
1 change: 1 addition & 0 deletions src/coreclr/jit/CMakeLists.txt
View file
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
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
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)
{
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
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
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
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

0 comments on commit c6d988f

Please sign in to comment.