diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5d40dd03d2978..442e249abf91f 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -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 diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 4afefd6e2a10a..d55564e542543 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -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); @@ -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) { @@ -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 @@ -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. @@ -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); @@ -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: // @@ -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); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 73c78b8b1d8a2..ed436e5c86254 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -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"? diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 5c521fe5a8cbf..fb3d89824fdf9 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -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.