Skip to content

Commit

Permalink
JIT: some enhancements to redundant branch opts
Browse files Browse the repository at this point in the history 
Handle cases where the dominating compare is the reverse of the compare
we're trying to optimize. For example, if `(x > y)` dominates `(y <= x)`
we may be able to optimize the dominated compare.

Addresses aspects of dotnet#48115.
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed Oct 21, 2021
1 parent 3c91041 commit e446e7e
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7453,7 +7453,7 @@ class Compiler
//
PhaseStatus optRedundantBranches();
bool optRedundantBranch(BasicBlock* const block);
bool optJumpThread(BasicBlock* const block, BasicBlock* const domBlock);
bool optJumpThread(BasicBlock* const block, BasicBlock* const domBlock, bool domIsSameRelop);
bool optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock);

#if ASSERTION_PROP
Expand Down
46 changes: 28 additions & 18 deletions src/coreclr/jit/redundantbranchopts.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,20 +131,23 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)
// We can use liberal VNs as bounds checks are not yet
// manifest explicitly as relops.
//
ValueNum domCmpVN = domCmpTree->GetVN(VNK_Liberal);
ValueNum domCmpVN = domCmpTree->GetVN(VNK_Liberal);
ValueNum domCmpRevVN = vnStore->GetReversedRelopVN(domCmpVN);

// 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_Liberal))
if ((domCmpVN == tree->GetVN(VNK_Liberal)) ||
((domCmpRevVN != ValueNumStore::NoVN) && (domCmpRevVN == tree->GetVN(VNK_Liberal))))
{
// The compare in "tree" is redundant.
// Is there a unique path from the dominating compare?
//
JITDUMP("\nDominator " FMT_BB " of " FMT_BB " has relop with same liberal VN:\n", domBlock->bbNum,
block->bbNum);
const bool domIsSameRelop = domCmpVN == tree->GetVN(VNK_Liberal);
JITDUMP("\nDominator " FMT_BB " of " FMT_BB " has relop with %s liberal VN\n", domBlock->bbNum,
block->bbNum, domIsSameRelop ? "the same" : "a reverse");
DISPTREE(domCmpTree);
JITDUMP(" Redundant compare; current relop:\n");
DISPTREE(tree);
Expand All @@ -162,7 +165,7 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)
// However we may be able to update the flow from block's predecessors so they
// bypass block and instead transfer control to jump's successors (aka jump threading).
//
const bool wasThreaded = optJumpThread(block, domBlock);
const bool wasThreaded = optJumpThread(block, domBlock, domIsSameRelop);

if (wasThreaded)
{
Expand All @@ -171,20 +174,20 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)
}
else if (trueReaches)
{
// Taken jump in dominator reaches, fall through doesn't; relop must be true.
// Taken jump in dominator reaches, fall through doesn't; relop must be true/false.
//
JITDUMP("Jump successor " FMT_BB " of " FMT_BB " reaches, relop must be true\n",
domBlock->bbJumpDest->bbNum, domBlock->bbNum);
relopValue = 1;
JITDUMP("Jump successor " FMT_BB " of " FMT_BB " reaches, relop must be %s\n",
domBlock->bbJumpDest->bbNum, domBlock->bbNum, domIsSameRelop ? "true" : "false");
relopValue = domIsSameRelop ? 1 : 0;
break;
}
else if (falseReaches)
{
// Fall through from dominator reaches, taken jump doesn't; relop must be false.
// Fall through from dominator reaches, taken jump doesn't; relop must be false/true.
//
JITDUMP("Fall through successor " FMT_BB " of " FMT_BB " reaches, relop must be false\n",
domBlock->bbNext->bbNum, domBlock->bbNum);
relopValue = 0;
JITDUMP("Fall through successor " FMT_BB " of " FMT_BB " reaches, relop must be %s\n",
domBlock->bbNext->bbNum, domBlock->bbNum, domIsSameRelop ? "false" : "true");
relopValue = domIsSameRelop ? 0 : 1;
break;
}
else
Expand Down Expand Up @@ -259,6 +262,8 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)
// Arguments:
// block - block with branch to optimize
// domBlock - a dominating block that has an equivalent branch
// domIsSameRelop - if true, dominating block does the same compare;
// if false, dominating block does a reverse compare
//
// Returns:
// True if the branch was optimized.
Expand All @@ -273,7 +278,7 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)
// / \ | |
// C D C D
//
bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock)
bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock, bool domIsSameRelop)
{
assert(block->bbJumpKind == BBJ_COND);
assert(domBlock->bbJumpKind == BBJ_COND);
Expand Down Expand Up @@ -370,8 +375,13 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock
// are correlated exclusively with a true value for block's relop, and which
// are correlated exclusively with a false value (aka true preds and false preds).
//
// To do this we try and follow the flow from domBlock to block; any block pred
// reachable from domBlock's true edge is a true pred, and vice versa.
// To do this we try and follow the flow from domBlock to block. When domIsSameRelop
// is true, any block pred reachable from domBlock's true edge is a true pred of block,
// and any block pred reachable from domBlock's false edge is a false pred of block.
//
// If domIsSameRelop is false, then the roles of the of the paths from domBlock swap:
// any block pred reachable from domBlock's true edge is a false pred of block,
// and any block pred reachable from domBlock's false edge is a true pred of block.
//
// However, there are some exceptions:
//
Expand Down Expand Up @@ -400,8 +410,8 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock
int numTruePreds = 0;
int numFalsePreds = 0;
BasicBlock* fallThroughPred = nullptr;
BasicBlock* const trueSuccessor = domBlock->bbJumpDest;
BasicBlock* const falseSuccessor = domBlock->bbNext;
BasicBlock* const trueSuccessor = domIsSameRelop ? domBlock->bbJumpDest : domBlock->bbNext;
BasicBlock* const falseSuccessor = domIsSameRelop ? domBlock->bbNext : domBlock->bbJumpDest;
BasicBlock* const trueTarget = block->bbJumpDest;
BasicBlock* const falseTarget = block->bbNext;
BlockSet truePreds = BlockSetOps::MakeEmpty(this);
Expand Down
81 changes: 81 additions & 0 deletions src/coreclr/jit/valuenum.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4450,6 +4450,87 @@ bool ValueNumStore::IsVNHandle(ValueNum vn)
return c->m_attribs == CEA_Handle;
}

//------------------------------------------------------------------------
// GetReversedRelopVN: return value number for reversed comparsion
//
// Arguments:
// vn - vn to reverse
//
// Returns:
// vn for reversed comparsion, or NoVN.
//
// Note:
// If "vn" corresponds to (x > y)
// then reverse("vn") corresponds to (x <= y)
//
ValueNum ValueNumStore::GetReversedRelopVN(ValueNum vn)
{
if (vn == NoVN)
{
return NoVN;
}

// Pull out any exception set.
//
ValueNum valueVN;
ValueNum excepVN;
VNUnpackExc(vn, &valueVN, &excepVN);

// Verify we have a binary func application
//
VNFuncApp funcAttr;
if (!GetVNFunc(valueVN, &funcAttr))
{
return NoVN;
}

if (funcAttr.m_arity != 2)
{
return NoVN;
}

// Map to the reverse VNFunc
//
VNFunc reverseOp;

if (funcAttr.m_func >= VNF_Boundary)
{
switch (funcAttr.m_func)
{
case VNF_LT_UN:
reverseOp = VNF_GE_UN;
break;
case VNF_LE_UN:
reverseOp = VNF_GT_UN;
break;
case VNF_GE_UN:
reverseOp = VNF_LT_UN;
break;
case VNF_GT_UN:
reverseOp = VNF_LE_UN;
break;
default:
return NoVN;
}
}
else
{
const genTreeOps op = (genTreeOps)funcAttr.m_func;

if (!GenTree::OperIsRelop(op))
{
return NoVN;
}

reverseOp = (VNFunc)GenTree::ReverseRelop(op);
}

// Create the resulting VN
//
ValueNum reverseValueVN = VNForFunc(TYP_INT, reverseOp, funcAttr.m_args[0], funcAttr.m_args[1]);
return VNWithExc(reverseValueVN, excepVN);
}

bool ValueNumStore::IsVNConstantBound(ValueNum vn)
{
// Do we have "var < 100"?
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/jit/valuenum.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,12 @@ class ValueNumStore
// Returns true iff the VN represents a handle constant.
bool IsVNHandle(ValueNum vn);

// Given a relop VN, get the VN for the "reversed" relop, which has the opposite value
// eg given VN(x < y), return VN(y <=x)
//
// If vn is not a relop, return NoVN.
ValueNum GetReversedRelopVN(ValueNum vn);

// Convert a vartype_t to the value number's storage type for that vartype_t.
// For example, ValueNum of type TYP_LONG are stored in a map of INT64 variables.
// Lang is the language (C++) type for the corresponding vartype_t.
Expand Down

0 comments on commit e446e7e

Please sign in to comment.