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: simple redundant compare optimization #43811

Merged
merged 2 commits into from
Nov 4, 2020
Merged

JIT: simple redundant compare optimization #43811

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc57631
JIT: simple redundant compare optimization
AndyAyersMS May 2, 2020
9d27468
update header comment per feedback
AndyAyersMS Nov 3, 2020
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
147 changes: 147 additions & 0 deletions src/coreclr/src/jit/assertionprop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3215,6 +3215,153 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen
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
Copy link
Contributor

Choose a reason for hiding this comment

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

You should update the method header to the standard header and under Note: add comment saying that we also do this special case optimization.

// 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