From e446e7e326bbb3981d17ad8069e9fbaaa10921bd Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 20 Oct 2021 18:43:37 -0700 Subject: [PATCH 1/4] JIT: some enhancements to redundant branch opts 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 #48115. --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/redundantbranchopts.cpp | 46 ++++++++------ src/coreclr/jit/valuenum.cpp | 81 +++++++++++++++++++++++++ src/coreclr/jit/valuenum.h | 6 ++ 4 files changed, 116 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5d40dd03d29786..442e249abf91fb 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 4afefd6e2a10af..d55564e5425433 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 73c78b8b1d8a2f..ed436e5c86254b 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 5c521fe5a8cbf5..fb3d89824fdf92 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. From e3d5f5ccbc1ae7627e7a02b985dbee51fd3f163f Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 21 Oct 2021 15:26:31 -0700 Subject: [PATCH 2/4] revise --- src/coreclr/jit/redundantbranchopts.cpp | 6 +++--- src/coreclr/jit/valuenum.cpp | 13 +++++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index d55564e5425433..2a5a635f0cbbb6 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -131,6 +131,7 @@ bool Compiler::optRedundantBranch(BasicBlock* const block) // We can use liberal VNs as bounds checks are not yet // manifest explicitly as relops. // + ValueNum treeVN = tree->GetVN(VNK_Liberal); ValueNum domCmpVN = domCmpTree->GetVN(VNK_Liberal); ValueNum domCmpRevVN = vnStore->GetReversedRelopVN(domCmpVN); @@ -139,13 +140,12 @@ bool Compiler::optRedundantBranch(BasicBlock* const block) // // That is left as a future enhancement. // - if ((domCmpVN == tree->GetVN(VNK_Liberal)) || - ((domCmpRevVN != ValueNumStore::NoVN) && (domCmpRevVN == tree->GetVN(VNK_Liberal)))) + if ((domCmpVN == treeVN) || ((domCmpRevVN != ValueNumStore::NoVN) && (domCmpRevVN == treeVN))) { // The compare in "tree" is redundant. // Is there a unique path from the dominating compare? // - const bool domIsSameRelop = domCmpVN == tree->GetVN(VNK_Liberal); + const bool domIsSameRelop = (domCmpVN == treeVN); 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); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index ed436e5c86254b..43ee02779abe51 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4451,7 +4451,7 @@ bool ValueNumStore::IsVNHandle(ValueNum vn) } //------------------------------------------------------------------------ -// GetReversedRelopVN: return value number for reversed comparsion +// GetReversedRelopVN: return value number for reversed comparison // // Arguments: // vn - vn to reverse @@ -4460,9 +4460,11 @@ bool ValueNumStore::IsVNHandle(ValueNum vn) // vn for reversed comparsion, or NoVN. // // Note: -// If "vn" corresponds to (x > y) +// If "vn" corresponds to (x > y) // then reverse("vn") corresponds to (x <= y) // +// Will return NoVN for all float comparisons. +// ValueNum ValueNumStore::GetReversedRelopVN(ValueNum vn) { if (vn == NoVN) @@ -4489,6 +4491,13 @@ ValueNum ValueNumStore::GetReversedRelopVN(ValueNum vn) return NoVN; } + // Don't try and model float compares. + // + if (varTypeIsFloating(TypeOfVN(funcAttr.m_args[0]))) + { + return NoVN; + } + // Map to the reverse VNFunc // VNFunc reverseOp; From 4b8f1981759551f858fb80f0ebe168e03931be7e Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 22 Oct 2021 13:20:39 -0700 Subject: [PATCH 3/4] handle swapped compares (and reversed swapped compares) --- src/coreclr/jit/redundantbranchopts.cpp | 30 ++++-- src/coreclr/jit/valuenum.cpp | 129 ++++++++++++++++++------ src/coreclr/jit/valuenum.h | 13 ++- 3 files changed, 129 insertions(+), 43 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 2a5a635f0cbbb6..de565428d1016a 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -128,26 +128,40 @@ bool Compiler::optRedundantBranch(BasicBlock* const block) if (domCmpTree->OperKind() & GTK_RELOP) { - // We can use liberal VNs as bounds checks are not yet + // We can use liberal VNs here, as bounds checks are not yet // manifest explicitly as relops. // - ValueNum treeVN = tree->GetVN(VNK_Liberal); - ValueNum domCmpVN = domCmpTree->GetVN(VNK_Liberal); - ValueNum domCmpRevVN = vnStore->GetReversedRelopVN(domCmpVN); + // Look for an exact match and also try the various swapped/reversed forms. + // + const ValueNum treeVN = tree->GetVN(VNK_Liberal); + const ValueNum domCmpVN = domCmpTree->GetVN(VNK_Liberal); + const ValueNum domCmpSwpVN = + vnStore->GetRelatedRelop(domCmpVN, ValueNumStore::VN_RELATION_KIND::VRK_Swap); + const ValueNum domCmpRevVN = + vnStore->GetRelatedRelop(domCmpVN, ValueNumStore::VN_RELATION_KIND::VRK_Reverse); + const ValueNum domCmpSwpRevVN = + vnStore->GetRelatedRelop(domCmpVN, ValueNumStore::VN_RELATION_KIND::VRK_SwapReverse); + + const bool matchCmp = ((domCmpVN != ValueNumStore::NoVN) && (domCmpVN == treeVN)); + const bool matchSwp = ((domCmpSwpVN != ValueNumStore::NoVN) && (domCmpSwpVN == treeVN)); + const bool matchRev = ((domCmpRevVN != ValueNumStore::NoVN) && (domCmpRevVN == treeVN)); + const bool matchSwpRev = ((domCmpSwpRevVN != ValueNumStore::NoVN) && (domCmpSwpRevVN == treeVN)); + const bool domIsSameRelop = matchCmp || matchSwp; + const bool domIsRevRelop = matchRev || matchSwpRev; // 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 == treeVN) || ((domCmpRevVN != ValueNumStore::NoVN) && (domCmpRevVN == treeVN))) + if (domIsSameRelop || domIsRevRelop) { // The compare in "tree" is redundant. // Is there a unique path from the dominating compare? // - const bool domIsSameRelop = (domCmpVN == treeVN); - JITDUMP("\nDominator " FMT_BB " of " FMT_BB " has relop with %s liberal VN\n", domBlock->bbNum, - block->bbNum, domIsSameRelop ? "the same" : "a reverse"); + JITDUMP("\nDominator " FMT_BB " of " FMT_BB " has %srelop with %s liberal VN\n", domBlock->bbNum, + block->bbNum, matchSwp || matchSwpRev ? "swapped " : "", + domIsSameRelop ? "the same" : "a reverse"); DISPTREE(domCmpTree); JITDUMP(" Redundant compare; current relop:\n"); DISPTREE(tree); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 43ee02779abe51..c38ff3c5a57fe4 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4451,21 +4451,24 @@ bool ValueNumStore::IsVNHandle(ValueNum vn) } //------------------------------------------------------------------------ -// GetReversedRelopVN: return value number for reversed comparison +// GetRelatedRelop: return value number for reversed/swapped comparison // // Arguments: -// vn - vn to reverse +// vn - vn to base things on +// vnk - whether the new vn should swap, reverse, or both // // Returns: -// vn for reversed comparsion, or NoVN. +// vn for reversed/swapped comparsion, or NoVN. // // Note: -// If "vn" corresponds to (x > y) -// then reverse("vn") corresponds to (x <= y) +// If "vn" corresponds to (x > y), the resulting VN correponds to +// VRK_Swap (y < x) +// VRK_Reverse (x <= y) +// VRK_SwapReverse (y >= x) // // Will return NoVN for all float comparisons. // -ValueNum ValueNumStore::GetReversedRelopVN(ValueNum vn) +ValueNum ValueNumStore::GetRelatedRelop(ValueNum vn, VN_RELATION_KIND vrk) { if (vn == NoVN) { @@ -4498,46 +4501,108 @@ ValueNum ValueNumStore::GetReversedRelopVN(ValueNum vn) return NoVN; } - // Map to the reverse VNFunc + const bool reverse = (vrk == VN_RELATION_KIND::VRK_Reverse) || (vrk == VN_RELATION_KIND::VRK_SwapReverse); + const bool swap = (vrk == VN_RELATION_KIND::VRK_Swap) || (vrk == VN_RELATION_KIND::VRK_SwapReverse); + + // Set up the new function // - VNFunc reverseOp; + VNFunc newFunc = funcAttr.m_func; - if (funcAttr.m_func >= VNF_Boundary) + // Swap the predicate, if so asked. + // + if (swap) { - switch (funcAttr.m_func) + if (newFunc >= VNF_Boundary) { - 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: + switch (newFunc) + { + case VNF_LT_UN: + newFunc = VNF_GT_UN; + break; + case VNF_LE_UN: + newFunc = VNF_GE_UN; + break; + case VNF_GE_UN: + newFunc = VNF_LE_UN; + break; + case VNF_GT_UN: + newFunc = VNF_LT_UN; + break; + default: + return NoVN; + } + } + else + { + const genTreeOps op = (genTreeOps)newFunc; + + if (!GenTree::OperIsRelop(op)) + { return NoVN; + } + + newFunc = (VNFunc)GenTree::SwapRelop(op); } } - else - { - const genTreeOps op = (genTreeOps)funcAttr.m_func; - if (!GenTree::OperIsRelop(op)) + // Reverse the predicate, if so asked. + // + if (reverse) + { + if (newFunc >= VNF_Boundary) { - return NoVN; + switch (newFunc) + { + case VNF_LT_UN: + newFunc = VNF_GE_UN; + break; + case VNF_LE_UN: + newFunc = VNF_GT_UN; + break; + case VNF_GE_UN: + newFunc = VNF_LT_UN; + break; + case VNF_GT_UN: + newFunc = VNF_LE_UN; + break; + default: + return NoVN; + } } + else + { + const genTreeOps op = (genTreeOps)newFunc; - reverseOp = (VNFunc)GenTree::ReverseRelop(op); + if (!GenTree::OperIsRelop(op)) + { + return NoVN; + } + + newFunc = (VNFunc)GenTree::ReverseRelop(op); + } } - // Create the resulting VN + // Create the resulting VN, swapping arguments if needed. // - ValueNum reverseValueVN = VNForFunc(TYP_INT, reverseOp, funcAttr.m_args[0], funcAttr.m_args[1]); - return VNWithExc(reverseValueVN, excepVN); + ValueNum newVN = VNForFunc(TYP_INT, newFunc, funcAttr.m_args[swap ? 1 : 0], funcAttr.m_args[swap ? 0 : 1]); + ValueNum result = VNWithExc(newVN, excepVN); + +#ifdef DEBUG + if (m_pComp->verbose) + { + printf("%s of ", swap ? (reverse ? "swap-reverse" : "swap") : "reverse"); + m_pComp->vnPrint(vn, 1); + printf(" => "); + m_pComp->vnPrint(newVN, 1); + if (result != newVN) + { + m_pComp->vnPrint(result, 1); + } + printf("\n"); + } +#endif + + return result; } bool ValueNumStore::IsVNConstantBound(ValueNum vn) diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index fb3d89824fdf92..03974f821efbae 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -842,11 +842,18 @@ 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) + // Given VN(x > y), return VN(y > x), VN(x <= y) or VN(y >= x) // // If vn is not a relop, return NoVN. - ValueNum GetReversedRelopVN(ValueNum vn); + // + enum class VN_RELATION_KIND + { + VRK_Swap, // (y > x) + VRK_Reverse, // (x <= y) + VRK_SwapReverse // (y >= x) + }; + + ValueNum GetRelatedRelop(ValueNum vn, VN_RELATION_KIND vrk); // 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. From 280834d2994696fb4c5e62838fe56088d4d5832f Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 22 Oct 2021 18:40:09 -0700 Subject: [PATCH 4/4] feedback --- src/coreclr/jit/redundantbranchopts.cpp | 6 ++++-- src/coreclr/jit/valuenum.cpp | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index de565428d1016a..c0a5e419920770 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -149,8 +149,10 @@ bool Compiler::optRedundantBranch(BasicBlock* const block) const bool domIsSameRelop = matchCmp || matchSwp; const bool domIsRevRelop = matchRev || matchSwpRev; - // 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). + // Note we could also infer the tree relop's value from relops higher in the dom tree + // that involve the same operands but are not swaps or reverses. + // + // For example, (x >= 0) dominating (x > 0). // // That is left as a future enhancement. // diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index c38ff3c5a57fe4..d3a17814a1f490 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -4455,13 +4455,13 @@ bool ValueNumStore::IsVNHandle(ValueNum vn) // // Arguments: // vn - vn to base things on -// vnk - whether the new vn should swap, reverse, or both +// vrk - whether the new vn should swap, reverse, or both // // Returns: // vn for reversed/swapped comparsion, or NoVN. // // Note: -// If "vn" corresponds to (x > y), the resulting VN correponds to +// If "vn" corresponds to (x > y), the resulting VN corresponds to // VRK_Swap (y < x) // VRK_Reverse (x <= y) // VRK_SwapReverse (y >= x)