From 244722775b34c7fa811548d7f28702549a2cc75b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 25 Nov 2023 23:07:05 +0100 Subject: [PATCH 01/26] Optimize "implied relops" in assertprop --- src/coreclr/jit/assertionprop.cpp | 121 ++++++++++++++++++++++++++++++ src/coreclr/jit/compiler.h | 1 + 2 files changed, 122 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index fc3908b1d43f4..f6cf28f04021c 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3936,6 +3936,123 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* return optAssertionPropLocal_RelOp(assertions, tree, stmt); } +enum RangeCheckResult +{ + Unknown, + Never, + Always +}; + +//------------------------------------------------------------------------ +// Given two range checks, determine if the 2nd one is redundant or not. +// It is assumed, that the boths range checks are for the same X on the left side of the comparisons. +// e.g. "x > 100 && x > 10" -> Always +// +// Arguments: +// oper1 - the first range check operator +// bound1 - the first range check constant bound +// oper2 - the second range check operator +// bound2 - the second range check constant bound +// +// Returns: +// "Always" if the 2nd range check is always "true", "Never" if it is never "true" +// "Unknown" if we can't determine if it is redundant or not. +// +RangeCheckResult GetRangeCheckResult(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) +{ + // TODO: normalize GT_GE and GT_LE to GT_GT and GT_LT respectively (with adjusted bounds) + // TODO: Handle other cases (e.g. "Never" cases, etc). + + if (oper1 == oper2) + { + // x > 100 + // x > 10 + + // x <= 100 + // x <= 10 + + if (bound1 >= bound2) + { + return Always; + } + } + return Unknown; +} + +//------------------------------------------------------------------------ +// optAssertionProp: try and optimize a constant range check via assertion propagation +// e.g. "x > 100" where "x" has an assertion "x < 10" can be optimized to "false" +// +// Arguments: +// assertions - set of live assertions +// tree - tree to possibly optimize +// stmt - statement containing the tree +// +// Returns: +// The modified tree, or nullptr if no assertion prop took place. +// +GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt) +{ + assert(tree->OperIs(GT_LT, GT_LE, GT_GE, GT_GT) && tree->gtGetOp2()->IsCnsIntOrI()); + if (optLocalAssertionProp || !varTypeIsIntegral(tree) || BitVecOps::IsEmpty(apTraits, assertions) || + tree->IsUnsigned()) + { + return nullptr; + } + + const GenTree* op1 = tree->gtGetOp1(); + const GenTreeIntCon* op2 = tree->gtGetOp2()->AsIntCon(); + + const ValueNum op1VN = vnStore->VNConservativeNormalValue(op1->gtVNPair); + BitVecOps::Iter iter(apTraits, assertions); + unsigned index = 0; + while (iter.NextElem(&index)) + { + AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); + // OAK_[NOT]_EQUAL assertion with op1 being O1K_CONSTANT_LOOP_BND + // representing "(X relop CNS) ==/!= 0" assertion. + if (!curAssertion->IsConstantBound()) + { + continue; + } + + ValueNumStore::ConstantBoundInfo info; + vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info); + + if (info.cmpOpVN != op1VN || info.isUnsigned) + { + continue; + } + + // Root assertion has to be either: + // (X relop CNS) == 0 + // (X relop CNS) != 0 + if ((curAssertion->op2.kind != O2K_CONST_INT) || (curAssertion->op2.u1.iconVal != 0)) + { + continue; + } + + genTreeOps cmpOper = static_cast(info.cmpOper); + + // Normalize "(X relop CNS) == false" to "(X reversed_relop CNS) == true" + if (curAssertion->assertionKind == OAK_EQUAL) + { + cmpOper = GenTree::ReverseRelop(cmpOper); + } + + RangeCheckResult result = GetRangeCheckResult(cmpOper, info.constVal, tree->OperGet(), op2->IconValue()); + if (result == Unknown) + { + continue; + } + + tree->BashToConst(result == Never ? 0 : 1); + GenTree* newTree = fgMorphTree(tree); + return optAssertionProp_Update(newTree, tree, stmt); + } + return nullptr; +} + //------------------------------------------------------------------------ // optAssertionProp: try and optimize a relop via assertion propagation // @@ -3995,6 +4112,10 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen // Else check if we have an equality check involving a local or an indir if (!tree->OperIs(GT_EQ, GT_NE)) { + if (op2->IsCnsIntOrI() && tree->OperIs(GT_LT, GT_LE, GT_GE, GT_GT)) + { + return optAssertionPropGlobal_ConstRangeCheck(assertions, tree, stmt); + } return nullptr; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0f2d6ec1c1a00..0365d236f8887 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7761,6 +7761,7 @@ class Compiler GenTree* optAssertionProp_Comma(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); GenTree* optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); GenTree* optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); + GenTree* optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); GenTree* optAssertionPropLocal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); GenTree* optAssertionProp_Update(GenTree* newTree, GenTree* tree, Statement* stmt); GenTree* optNonNullAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCall* call); From a6e0867a6fdbc1b7cd3cfe5014bd1074b421ade9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 25 Nov 2023 23:54:25 +0100 Subject: [PATCH 02/26] Improve impl --- src/coreclr/jit/assertionprop.cpp | 93 ++++++++++++++++++++++++++----- 1 file changed, 80 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index f6cf28f04021c..4788a0901f57d 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3958,24 +3958,79 @@ enum RangeCheckResult // "Always" if the 2nd range check is always "true", "Never" if it is never "true" // "Unknown" if we can't determine if it is redundant or not. // -RangeCheckResult GetRangeCheckResult(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) +RangeCheckResult GetRangeCheckResult(genTreeOps oper1, int bound1, genTreeOps oper2, int bound2) { - // TODO: normalize GT_GE and GT_LE to GT_GT and GT_LT respectively (with adjusted bounds) - // TODO: Handle other cases (e.g. "Never" cases, etc). - - if (oper1 == oper2) + struct Int32Range { - // x > 100 - // x > 10 + int startIncl; + int endIncl; + }; - // x <= 100 - // x <= 10 + Int32Range range1 = {INT_MIN, INT_MAX}; + Int32Range range2 = {INT_MIN, INT_MAX}; - if (bound1 >= bound2) + auto setRange = [](genTreeOps oper, int bound, Int32Range* range) -> bool { + switch (oper) { - return Always; + case GT_LT: + // x < cns -> [INT_MIN, cns - 1] + if (bound == INT_MIN) + { + // overflows + return true; + } + range->endIncl = bound - 1; + break; + case GT_LE: + // x <= cns -> [INT_MIN, cns] + range->endIncl = bound; + break; + case GT_GT: + // x > cns -> [cns + 1, INT_MAX] + if (bound == INT_MAX) + { + // overflows + return true; + } + range->startIncl = bound + 1; + break; + case GT_GE: + // x >= cns -> [cns, INT_MAX] + range->startIncl = bound; + break; + default: + unreached(); } + return false; // doesn't overflow + }; + + const bool overflows1 = setRange(oper1, bound1, &range1); + const bool overflows2 = setRange(oper2, bound2, &range2); + if (overflows1 || overflows2) + { + return Unknown; } + + if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) + { + // [100 .. INT_MAX] + // [INT_MIN .. 10] + + // Ranges never intersect + return Never; + } + + // check whether range2 is fully contained in range1: + if ((range1.startIncl <= range2.startIncl) && (range2.endIncl <= range1.endIncl)) + { + // [100 .. INT_MAX] + // [110 .. INT_MAX] + + // range2 is fully contained in range1 + return Always; + } + + // Ranges intersect, but we can't determine if the 2nd range is redundant or not. return Unknown; } @@ -4000,6 +4055,12 @@ GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP asser return nullptr; } + // Bail out if tree is not side effect free. + if ((tree->gtFlags & GTF_SIDE_EFFECT) != 0) + { + return nullptr; + } + const GenTree* op1 = tree->gtGetOp1(); const GenTreeIntCon* op2 = tree->gtGetOp2()->AsIntCon(); @@ -4040,10 +4101,16 @@ GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP asser cmpOper = GenTree::ReverseRelop(cmpOper); } - RangeCheckResult result = GetRangeCheckResult(cmpOper, info.constVal, tree->OperGet(), op2->IconValue()); + ssize_t cns2 = op2->IconValue(); + if (!FitsIn(cns2)) + { + return nullptr; + } + + RangeCheckResult result = GetRangeCheckResult(cmpOper, info.constVal, tree->OperGet(), (int)cns2); if (result == Unknown) { - continue; + return nullptr; } tree->BashToConst(result == Never ? 0 : 1); From 579c05e1017e2f429d9c8738512b01774079f2d9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 00:31:09 +0100 Subject: [PATCH 03/26] Clean up --- src/coreclr/jit/assertionprop.cpp | 73 +++++++++++++++++++------------ 1 file changed, 45 insertions(+), 28 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 4788a0901f57d..21aa618f0cc36 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3936,30 +3936,34 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* return optAssertionPropLocal_RelOp(assertions, tree, stmt); } -enum RangeCheckResult +enum RangeStatus { Unknown, - Never, - Always + NeverIntersects, + AlwaysIncluded }; //------------------------------------------------------------------------ -// Given two range checks, determine if the 2nd one is redundant or not. -// It is assumed, that the boths range checks are for the same X on the left side of the comparisons. -// e.g. "x > 100 && x > 10" -> Always +// IsRange2ImpliedByRange1: Given two range checks, determine if the 2nd one is redundant or not. +// It is assumed, that the both range checks are for the same X on LHS of the comparison operators. +// e.g. "x > 100 && x > 10" -> AlwaysIncluded // // Arguments: -// oper1 - the first range check operator -// bound1 - the first range check constant bound -// oper2 - the second range check operator -// bound2 - the second range check constant bound +// oper1 - the first range check operator [X *oper1* bound1 ] +// bound1 - the first range check constant bound [X oper1 *bound1*] +// oper2 - the second range check operator [X *oper2* bound2 ] +// bound2 - the second range check constant bound [X oper2 *bound2*] // // Returns: -// "Always" if the 2nd range check is always "true", "Never" if it is never "true" -// "Unknown" if we can't determine if it is redundant or not. +// "AlwaysIncluded" means that the 2nd range check is always "true" +// "NeverIntersects" means that the 2nd range check is never "true" +// "Unknown" means that we can't determine if the 2nd range check is redundant or not. // -RangeCheckResult GetRangeCheckResult(genTreeOps oper1, int bound1, genTreeOps oper2, int bound2) +RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps oper2, int bound2) { + assert((oper1 == GT_LT) || (oper1 == GT_LE) || (oper1 == GT_GT) || (oper1 == GT_GE)); + assert((oper2 == GT_LT) || (oper2 == GT_LE) || (oper2 == GT_GT) || (oper2 == GT_GE)); + struct Int32Range { int startIncl; @@ -3969,6 +3973,7 @@ RangeCheckResult GetRangeCheckResult(genTreeOps oper1, int bound1, genTreeOps op Int32Range range1 = {INT_MIN, INT_MAX}; Int32Range range2 = {INT_MIN, INT_MAX}; + // Update ranges based on inputs auto setRange = [](genTreeOps oper, int bound, Int32Range* range) -> bool { switch (oper) { @@ -4008,26 +4013,29 @@ RangeCheckResult GetRangeCheckResult(genTreeOps oper1, int bound1, genTreeOps op const bool overflows2 = setRange(oper2, bound2, &range2); if (overflows1 || overflows2) { + // Conservatively give up if we faced an overflow during normalization of ranges. + // to be always inclusive. return Unknown; } + // If ranges never intersect, then the 2nd range is never "true" if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) { - // [100 .. INT_MAX] - // [INT_MIN .. 10] - - // Ranges never intersect - return Never; + // E.g.: + // + // range1: [100 .. INT_MAX] + // range2: [INT_MIN .. 10] + return NeverIntersects; } - // check whether range2 is fully contained in range1: - if ((range1.startIncl <= range2.startIncl) && (range2.endIncl <= range1.endIncl)) + // Check if range1 is fully included into range2 + if ((range2.startIncl <= range1.startIncl) && (range1.endIncl <= range2.endIncl)) { + // E.g.: + // // [100 .. INT_MAX] - // [110 .. INT_MAX] - - // range2 is fully contained in range1 - return Always; + // [10 .. INT_MAX] + return AlwaysIncluded; } // Ranges intersect, but we can't determine if the 2nd range is redundant or not. @@ -4080,7 +4088,9 @@ GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP asser ValueNumStore::ConstantBoundInfo info; vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info); - if (info.cmpOpVN != op1VN || info.isUnsigned) + // Make sure VN is the same as op1's VN. + // TODO: don't give up on unsigned comparisons. + if ((info.cmpOpVN != op1VN) || info.isUnsigned) { continue; } @@ -4101,20 +4111,27 @@ GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP asser cmpOper = GenTree::ReverseRelop(cmpOper); } - ssize_t cns2 = op2->IconValue(); + // We only handle int32 ranges for now (limited by O1K_CONSTANT_LOOP_BND) + const ssize_t cns2 = op2->IconValue(); if (!FitsIn(cns2)) { return nullptr; } - RangeCheckResult result = GetRangeCheckResult(cmpOper, info.constVal, tree->OperGet(), (int)cns2); + const RangeStatus result = IsRange2ImpliedByRange1(cmpOper, info.constVal, tree->OperGet(), (int)cns2); if (result == Unknown) { + // We can't determine if the range check is redundant or not. return nullptr; } - tree->BashToConst(result == Never ? 0 : 1); + JITDUMP("Fold a comparison against a constant:\n") + DISPTREE(tree) + JITDUMP("\ninto\n") + tree->BashToConst(result == NeverIntersects ? 0 : 1); GenTree* newTree = fgMorphTree(tree); + DISPTREE(newTree) + JITDUMP("\nbased on assertions.\n") return optAssertionProp_Update(newTree, tree, stmt); } return nullptr; From 7171a5dd1df245d704fcbdbfbf4e5cc215e2ae03 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 00:47:24 +0100 Subject: [PATCH 04/26] Clean up --- src/coreclr/jit/assertionprop.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 21aa618f0cc36..b2bcb53a17b74 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3961,9 +3961,6 @@ enum RangeStatus // RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps oper2, int bound2) { - assert((oper1 == GT_LT) || (oper1 == GT_LE) || (oper1 == GT_GT) || (oper1 == GT_GE)); - assert((oper2 == GT_LT) || (oper2 == GT_LE) || (oper2 == GT_GT) || (oper2 == GT_GE)); - struct Int32Range { int startIncl; @@ -4033,8 +4030,8 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope { // E.g.: // - // [100 .. INT_MAX] - // [10 .. INT_MAX] + // range1: [100 .. INT_MAX] + // range2: [10 .. INT_MAX] return AlwaysIncluded; } @@ -4043,8 +4040,9 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope } //------------------------------------------------------------------------ -// optAssertionProp: try and optimize a constant range check via assertion propagation -// e.g. "x > 100" where "x" has an assertion "x < 10" can be optimized to "false" +// optAssertionPropGlobal_ConstRangeCheck: try and optimize a constant range check via +// assertion propagation e.g.: +// "x > 100" where "x" has an assertion "x < 10" can be optimized to "false" // // Arguments: // assertions - set of live assertions @@ -4115,14 +4113,14 @@ GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP asser const ssize_t cns2 = op2->IconValue(); if (!FitsIn(cns2)) { - return nullptr; + continue; } const RangeStatus result = IsRange2ImpliedByRange1(cmpOper, info.constVal, tree->OperGet(), (int)cns2); if (result == Unknown) { // We can't determine if the range check is redundant or not. - return nullptr; + continue; } JITDUMP("Fold a comparison against a constant:\n") From 62a5f353fc8d68b7d0612bc6533fc85f376c1b5a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 00:57:28 +0100 Subject: [PATCH 05/26] Add GT_EQ --- src/coreclr/jit/assertionprop.cpp | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index b2bcb53a17b74..a38c9da209c3a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4000,6 +4000,11 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope // x >= cns -> [cns, INT_MAX] range->startIncl = bound; break; + case GT_EQ: + // x == cns -> [cns, cns] + range->startIncl = bound; + range->endIncl = bound; + break; default: unreached(); } @@ -4054,9 +4059,8 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope // GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt) { - assert(tree->OperIs(GT_LT, GT_LE, GT_GE, GT_GT) && tree->gtGetOp2()->IsCnsIntOrI()); - if (optLocalAssertionProp || !varTypeIsIntegral(tree) || BitVecOps::IsEmpty(apTraits, assertions) || - tree->IsUnsigned()) + assert(tree->OperIs(GT_LT, GT_LE, GT_GE, GT_GT, GT_EQ) && tree->gtGetOp2()->IsCnsIntOrI()); + if (optLocalAssertionProp || !varTypeIsIntegral(tree) || BitVecOps::IsEmpty(apTraits, assertions)) { return nullptr; } @@ -4087,8 +4091,13 @@ GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP asser vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info); // Make sure VN is the same as op1's VN. + if (info.cmpOpVN != op1VN) + { + continue; + } + // TODO: don't give up on unsigned comparisons. - if ((info.cmpOpVN != op1VN) || info.isUnsigned) + if (info.isUnsigned || tree->IsUnsigned()) { continue; } @@ -4191,13 +4200,18 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen return optAssertionProp_Update(newTree, tree, stmt); } - // Else check if we have an equality check involving a local or an indir - if (!tree->OperIs(GT_EQ, GT_NE)) + if (op2->IsCnsIntOrI() && tree->OperIs(GT_EQ, GT_LT, GT_LE, GT_GE, GT_GT)) { - if (op2->IsCnsIntOrI() && tree->OperIs(GT_LT, GT_LE, GT_GE, GT_GT)) + newTree = optAssertionPropGlobal_ConstRangeCheck(assertions, tree, stmt); + if (newTree != nullptr) { - return optAssertionPropGlobal_ConstRangeCheck(assertions, tree, stmt); + return newTree; } + } + + // Else check if we have an equality check involving a local or an indir + if (!tree->OperIs(GT_EQ, GT_NE)) + { return nullptr; } From 7489fad49447c85e5cae5be02ffc6eb7d1ef7c42 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 01:10:21 +0100 Subject: [PATCH 06/26] add GT_NE as well --- src/coreclr/jit/assertionprop.cpp | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index a38c9da209c3a..130f9b4182cec 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4005,12 +4005,18 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope range->startIncl = bound; range->endIncl = bound; break; + case GT_NE: + // special cased below (we can't represent it as a range) + break; default: unreached(); } return false; // doesn't overflow }; + // oper1 is never GT_NE or GT_EQ, see GetConstantBoundInfo + assert((oper1 != GT_NE) && (oper1 != GT_EQ)); + const bool overflows1 = setRange(oper1, bound1, &range1); const bool overflows2 = setRange(oper2, bound2, &range2); if (overflows1 || overflows2) @@ -4020,6 +4026,22 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope return Unknown; } + assert(oper1 != GT_NE); // oper1 is coming from an assertion, so it can't be GT_NE/GT_EQ + if (oper2 == GT_NE) + { + // "x > 100 && x != 10", the 2nd range check is redundant + if ((bound2 < range1.startIncl) || (bound2 > range1.endIncl)) + { + return AlwaysIncluded; + } + if (range1.startIncl == bound2 && range1.endIncl == bound2) + { + // x == 100 && x != 100 + return NeverIntersects; + } + return Unknown; + } + // If ranges never intersect, then the 2nd range is never "true" if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) { @@ -4059,7 +4081,7 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope // GenTree* Compiler::optAssertionPropGlobal_ConstRangeCheck(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt) { - assert(tree->OperIs(GT_LT, GT_LE, GT_GE, GT_GT, GT_EQ) && tree->gtGetOp2()->IsCnsIntOrI()); + assert(tree->OperIs(GT_EQ, GT_NE, GT_LT, GT_LE, GT_GE, GT_GT) && tree->gtGetOp2()->IsCnsIntOrI()); if (optLocalAssertionProp || !varTypeIsIntegral(tree) || BitVecOps::IsEmpty(apTraits, assertions)) { return nullptr; @@ -4200,7 +4222,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen return optAssertionProp_Update(newTree, tree, stmt); } - if (op2->IsCnsIntOrI() && tree->OperIs(GT_EQ, GT_LT, GT_LE, GT_GE, GT_GT)) + if (op2->IsCnsIntOrI() && tree->OperIs(GT_EQ, GT_NE, GT_LT, GT_LE, GT_GE, GT_GT)) { newTree = optAssertionPropGlobal_ConstRangeCheck(assertions, tree, stmt); if (newTree != nullptr) From a1041025943cc7fa87ce5255615a76f250a640e1 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 01:13:46 +0100 Subject: [PATCH 07/26] clean up --- src/coreclr/jit/assertionprop.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 130f9b4182cec..3f056553c496f 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4014,9 +4014,6 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope return false; // doesn't overflow }; - // oper1 is never GT_NE or GT_EQ, see GetConstantBoundInfo - assert((oper1 != GT_NE) && (oper1 != GT_EQ)); - const bool overflows1 = setRange(oper1, bound1, &range1); const bool overflows2 = setRange(oper2, bound2, &range2); if (overflows1 || overflows2) @@ -4027,16 +4024,17 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope } assert(oper1 != GT_NE); // oper1 is coming from an assertion, so it can't be GT_NE/GT_EQ + // only oper2 can be so, since it has no meaningful Int32Range data we check it separately. if (oper2 == GT_NE) { - // "x > 100 && x != 10", the 2nd range check is redundant if ((bound2 < range1.startIncl) || (bound2 > range1.endIncl)) { + // "x > 100 && x != 10", the 2nd range check is always true return AlwaysIncluded; } if (range1.startIncl == bound2 && range1.endIncl == bound2) { - // x == 100 && x != 100 + // "x == 100 && x != 100", the 2nd range check is never true return NeverIntersects; } return Unknown; From ffa7d1a559ee6dd26e300565cb75b83ea7f343e9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 10:41:59 +0100 Subject: [PATCH 08/26] Test commit --- src/coreclr/jit/assertionprop.cpp | 2 +- src/coreclr/jit/gentree.h | 2 +- src/coreclr/jit/morph.cpp | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 3f056553c496f..cd24e67236544 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4032,7 +4032,7 @@ RangeStatus IsRange2ImpliedByRange1(genTreeOps oper1, int bound1, genTreeOps ope // "x > 100 && x != 10", the 2nd range check is always true return AlwaysIncluded; } - if (range1.startIncl == bound2 && range1.endIncl == bound2) + if ((range1.startIncl == bound2) && (range1.endIncl == bound2)) { // "x == 100 && x != 100", the 2nd range check is never true return NeverIntersects; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index ff45d2def4168..8f81f11840a49 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -2104,7 +2104,7 @@ struct GenTree void ClearUnsigned() { - assert(OperIs(GT_ADD, GT_SUB, GT_CAST) || OperIsMul()); + assert(OperIs(GT_ADD, GT_SUB, GT_CAST, GT_LE, GT_LT, GT_GT, GT_GE) || OperIsMul()); gtFlags &= ~GTF_UNSIGNED; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 73a8474324d39..8c6ce324226bf 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -9135,6 +9135,12 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA case GT_LE: case GT_GE: case GT_GT: + if (varTypeIsIntegral(op1) && varTypeIsIntegral(op2) && op1->IsNeverNegative(this) && + op2->IsNeverNegative(this)) + { + // Some branch optimizations don't work well with unsigned relops + tree->ClearUnsigned(); + } if (!optValnumCSE_phase && (op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST))) { From 3f82e12d05c3cf4e07040fa213fbcd6520fd66f6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 12:21:38 +0100 Subject: [PATCH 09/26] Implement in redundant-branch phase --- src/coreclr/jit/redundantbranchopts.cpp | 168 ++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 071f8bdb5251c..6fc0fddc5c977 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -152,6 +152,142 @@ struct RelopImplicationRule bool reverse; }; +enum ImpliedRangeCheckStatus +{ + Unknown, + NeverIntersects, + AlwaysIncluded +}; + +//------------------------------------------------------------------------ +// IsRange2ImpliedByRange1: Given two range checks, determine if the 2nd one is redundant or not. +// It is assumed, that the both range checks are for the same X on LHS of the comparison operators. +// e.g. "x > 100 && x > 10" -> AlwaysIncluded +// +// Arguments: +// oper1 - the first range check operator [X *oper1* bound1 ] +// bound1 - the first range check constant bound [X oper1 *bound1*] +// oper2 - the second range check operator [X *oper2* bound2 ] +// bound2 - the second range check constant bound [X oper2 *bound2*] +// +// Returns: +// "AlwaysIncluded" means that the 2nd range check is always "true" +// "NeverIntersects" means that the 2nd range check is never "true" +// "Unknown" means that we can't determine if the 2nd range check is redundant or not. +// +ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) +{ + struct IntegralRange + { + ssize_t startIncl; + ssize_t endIncl; + }; + + IntegralRange range1 = {INTPTR_MIN, INTPTR_MAX}; + IntegralRange range2 = {INTPTR_MIN, INTPTR_MAX}; + + // Update ranges based on inputs + auto setRange = [](genTreeOps oper, ssize_t bound, IntegralRange* range) -> bool { + switch (oper) + { + case GT_LT: + // x < cns -> [INTPTR_MIN, cns - 1] + if (bound == INTPTR_MIN) + { + // overflows + return false; + } + range->endIncl = bound - 1; + return true; + + case GT_LE: + // x <= cns -> [INTPTR_MIN, cns] + range->endIncl = bound; + return true; + + case GT_GT: + // x > cns -> [cns + 1, INTPTR_MAX] + if (bound == INTPTR_MAX) + { + // overflows + return false; + } + range->startIncl = bound + 1; + return true; + + case GT_GE: + // x >= cns -> [cns, INTPTR_MAX] + range->startIncl = bound; + return true; + + case GT_EQ: + // x == cns -> [cns, cns] + range->startIncl = bound; + range->endIncl = bound; + return true; + + case GT_NE: + // special cased below (we can't represent it as a range) + return true; + + default: + // unsupported operator + return false; + } + }; + + const bool success1 = setRange(oper1, bound1, &range1); + const bool success2 = setRange(oper2, bound2, &range2); + if (!success1 || !success2) + { + return Unknown; + } + + // NE is special since we can't represent it as a range, let's only handle it if it's the 2nd operand + // for simplicity (driven by jit-diffs). + if (oper1 == GT_NE) + { + return Unknown; + } + if (oper2 == GT_NE) + { + if ((bound2 < range1.startIncl) || (bound2 > range1.endIncl)) + { + // "x > 100 && x != 10", the 2nd range check is always true + return AlwaysIncluded; + } + if ((range1.startIncl == bound2) && (range1.endIncl == bound2)) + { + // "x == 100 && x != 100", the 2nd range check is never true + return NeverIntersects; + } + return Unknown; + } + + // If ranges never intersect, then the 2nd range is never "true" + if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) + { + // E.g.: + // + // range1: [100 .. INT_MAX] + // range2: [INT_MIN .. 10] + return NeverIntersects; + } + + // Check if range1 is fully included into range2 + if ((range2.startIncl <= range1.startIncl) && (range1.endIncl <= range2.endIncl)) + { + // E.g.: + // + // range1: [100 .. INT_MAX] + // range2: [10 .. INT_MAX] + return AlwaysIncluded; + } + + // Ranges intersect, but we can't determine if the 2nd range is redundant or not. + return Unknown; +} + //------------------------------------------------------------------------ // s_implicationRules: rule table for unrelated relops // @@ -338,6 +474,38 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) } } } + // Given R(x, cns1) and R*(x, cns2) see if we can infer R* from R. + else if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) && + vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && + varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1]))) + { + const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); + const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); + + // We currently don't handle VNF_relop_UN funcs here, they'll be ignored. + const genTreeOps treeOper = static_cast(treeApp.m_func); + const genTreeOps domOper = static_cast(domApp.m_func); + + bool canInferFromTrue = true; + ImpliedRangeCheckStatus result = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); + if ((result == Unknown) && GenTree::OperIsCompare(domOper)) + { + // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". + result = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + canInferFromTrue = false; + } + + // TODO: handle NeverIntersects case. + if (result == AlwaysIncluded) + { + rii->canInfer = true; + rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; + rii->canInferFromTrue = canInferFromTrue; + rii->canInferFromFalse = !canInferFromTrue; + rii->reverseSense = false; + return; + } + } } // See if dominating compare is a compound comparison that might From eff4f9d9ef97cb9cd331f943ace8113d0a642a46 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 12:28:46 +0100 Subject: [PATCH 10/26] jit-format --- src/coreclr/jit/redundantbranchopts.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 6fc0fddc5c977..297f0d7a7578c 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -474,13 +474,14 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) } } } + // Given R(x, cns1) and R*(x, cns2) see if we can infer R* from R. - else if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) && - vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && - varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1]))) + if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) && + vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && + varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1]))) { - const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); - const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); + const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); + const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); // We currently don't handle VNF_relop_UN funcs here, they'll be ignored. const genTreeOps treeOper = static_cast(treeApp.m_func); From cbe3461772b021d41101888bc51cd2a0df050000 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 12:52:53 +0100 Subject: [PATCH 11/26] Fix build --- src/coreclr/jit/redundantbranchopts.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 297f0d7a7578c..950a46c638ecc 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -183,16 +183,16 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 ssize_t endIncl; }; - IntegralRange range1 = {INTPTR_MIN, INTPTR_MAX}; - IntegralRange range2 = {INTPTR_MIN, INTPTR_MAX}; + IntegralRange range1 = {INT_PTR_MIN, INT_PTR_MAX}; + IntegralRange range2 = {INT_PTR_MIN, INT_PTR_MAX}; // Update ranges based on inputs auto setRange = [](genTreeOps oper, ssize_t bound, IntegralRange* range) -> bool { switch (oper) { case GT_LT: - // x < cns -> [INTPTR_MIN, cns - 1] - if (bound == INTPTR_MIN) + // x < cns -> [INT_PTR_MIN, cns - 1] + if (bound == INT_PTR_MIN) { // overflows return false; @@ -201,13 +201,13 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 return true; case GT_LE: - // x <= cns -> [INTPTR_MIN, cns] + // x <= cns -> [INT_PTR_MIN, cns] range->endIncl = bound; return true; case GT_GT: - // x > cns -> [cns + 1, INTPTR_MAX] - if (bound == INTPTR_MAX) + // x > cns -> [cns + 1, INT_PTR_MAX] + if (bound == INT_PTR_MAX) { // overflows return false; @@ -216,7 +216,7 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 return true; case GT_GE: - // x >= cns -> [cns, INTPTR_MAX] + // x >= cns -> [cns, INT_PTR_MAX] range->startIncl = bound; return true; From 069279c7be4e316c60a248bcb8388ce100465d2d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 13:58:02 +0100 Subject: [PATCH 12/26] Fix build --- src/coreclr/jit/redundantbranchopts.cpp | 146 +++++++----------------- src/coreclr/jit/valuenum.h | 21 +++- 2 files changed, 58 insertions(+), 109 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 950a46c638ecc..54889730bd932 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -152,30 +152,7 @@ struct RelopImplicationRule bool reverse; }; -enum ImpliedRangeCheckStatus -{ - Unknown, - NeverIntersects, - AlwaysIncluded -}; - -//------------------------------------------------------------------------ -// IsRange2ImpliedByRange1: Given two range checks, determine if the 2nd one is redundant or not. -// It is assumed, that the both range checks are for the same X on LHS of the comparison operators. -// e.g. "x > 100 && x > 10" -> AlwaysIncluded -// -// Arguments: -// oper1 - the first range check operator [X *oper1* bound1 ] -// bound1 - the first range check constant bound [X oper1 *bound1*] -// oper2 - the second range check operator [X *oper2* bound2 ] -// bound2 - the second range check constant bound [X oper2 *bound2*] -// -// Returns: -// "AlwaysIncluded" means that the 2nd range check is always "true" -// "NeverIntersects" means that the 2nd range check is never "true" -// "Unknown" means that we can't determine if the 2nd range check is redundant or not. -// -ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) +bool IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) { struct IntegralRange { @@ -183,16 +160,16 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 ssize_t endIncl; }; - IntegralRange range1 = {INT_PTR_MIN, INT_PTR_MAX}; - IntegralRange range2 = {INT_PTR_MIN, INT_PTR_MAX}; + IntegralRange range1 = { SSIZE_T_MIN, SSIZE_T_MAX}; + IntegralRange range2 = { SSIZE_T_MIN, SSIZE_T_MAX }; // Update ranges based on inputs auto setRange = [](genTreeOps oper, ssize_t bound, IntegralRange* range) -> bool { switch (oper) { case GT_LT: - // x < cns -> [INT_PTR_MIN, cns - 1] - if (bound == INT_PTR_MIN) + // x < cns -> [SSIZE_T_MIN, cns - 1] + if (bound == SSIZE_T_MIN) { // overflows return false; @@ -201,13 +178,13 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 return true; case GT_LE: - // x <= cns -> [INT_PTR_MIN, cns] + // x <= cns -> [SSIZE_T_MIN, cns] range->endIncl = bound; return true; case GT_GT: - // x > cns -> [cns + 1, INT_PTR_MAX] - if (bound == INT_PTR_MAX) + // x > cns -> [cns + 1, SSIZE_T_MAX] + if (bound == SSIZE_T_MAX) { // overflows return false; @@ -216,20 +193,10 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 return true; case GT_GE: - // x >= cns -> [cns, INT_PTR_MAX] + // x >= cns -> [cns, SSIZE_T_MAX] range->startIncl = bound; return true; - case GT_EQ: - // x == cns -> [cns, cns] - range->startIncl = bound; - range->endIncl = bound; - return true; - - case GT_NE: - // special cased below (we can't represent it as a range) - return true; - default: // unsupported operator return false; @@ -240,38 +207,7 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 const bool success2 = setRange(oper2, bound2, &range2); if (!success1 || !success2) { - return Unknown; - } - - // NE is special since we can't represent it as a range, let's only handle it if it's the 2nd operand - // for simplicity (driven by jit-diffs). - if (oper1 == GT_NE) - { - return Unknown; - } - if (oper2 == GT_NE) - { - if ((bound2 < range1.startIncl) || (bound2 > range1.endIncl)) - { - // "x > 100 && x != 10", the 2nd range check is always true - return AlwaysIncluded; - } - if ((range1.startIncl == bound2) && (range1.endIncl == bound2)) - { - // "x == 100 && x != 100", the 2nd range check is never true - return NeverIntersects; - } - return Unknown; - } - - // If ranges never intersect, then the 2nd range is never "true" - if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) - { - // E.g.: - // - // range1: [100 .. INT_MAX] - // range2: [INT_MIN .. 10] - return NeverIntersects; + return false; } // Check if range1 is fully included into range2 @@ -279,13 +215,11 @@ ImpliedRangeCheckStatus IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1 { // E.g.: // - // range1: [100 .. INT_MAX] - // range2: [10 .. INT_MAX] - return AlwaysIncluded; + // range1: [100 .. SSIZE_T_MAX] + // range2: [10 .. SSIZE_T_MAX] + return true; } - - // Ranges intersect, but we can't determine if the 2nd range is redundant or not. - return Unknown; + return false; } //------------------------------------------------------------------------ @@ -443,7 +377,8 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // VNFunc const domFunc = domApp.m_func; VNFuncApp treeApp; - if (inRange && ValueNumStore::VNFuncIsComparison(domFunc) && vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) + bool domFuncIsUnsignedCmp = false; + if (inRange && ValueNumStore::VNFuncIsComparison(domFunc, &domFuncIsUnsignedCmp) && vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) { if (((treeApp.m_args[0] == domApp.m_args[0]) && (treeApp.m_args[1] == domApp.m_args[1])) || ((treeApp.m_args[0] == domApp.m_args[1]) && (treeApp.m_args[1] == domApp.m_args[0]))) @@ -480,31 +415,34 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1]))) { - const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); - const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); - - // We currently don't handle VNF_relop_UN funcs here, they'll be ignored. - const genTreeOps treeOper = static_cast(treeApp.m_func); - const genTreeOps domOper = static_cast(domApp.m_func); - - bool canInferFromTrue = true; - ImpliedRangeCheckStatus result = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); - if ((result == Unknown) && GenTree::OperIsCompare(domOper)) + // We currently don't handle VNF_relop_UN funcs here + bool treeFuncIsUnsignedCmp = false; + if (!domFuncIsUnsignedCmp && ValueNumStore::VNFuncIsComparison(treeApp.m_func, &treeFuncIsUnsignedCmp) && !treeFuncIsUnsignedCmp) { - // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". - result = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); - canInferFromTrue = false; - } + const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); + const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); + const genTreeOps treeOper = static_cast(treeApp.m_func); + const genTreeOps domOper = static_cast(domApp.m_func); + + bool canInferFromTrue = true; + bool implied = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); + if (!implied) + { + // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". + implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + canInferFromTrue = false; + } - // TODO: handle NeverIntersects case. - if (result == AlwaysIncluded) - { - rii->canInfer = true; - rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; - rii->canInferFromTrue = canInferFromTrue; - rii->canInferFromFalse = !canInferFromTrue; - rii->reverseSense = false; - return; + // TODO: handle NeverIntersects case. + if (implied) + { + rii->canInfer = true; + rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; + rii->canInferFromTrue = canInferFromTrue; + rii->canInferFromFalse = !canInferFromTrue; + rii->reverseSense = false; + return; + } } } } diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index e6ffb137584bb..61bab4d2e6e95 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -1044,7 +1044,7 @@ class ValueNumStore static VNFunc SwapRelop(VNFunc vnf); // Returns "true" iff "vnf" is a comparison (and thus binary) operator. - static bool VNFuncIsComparison(VNFunc vnf); + static bool VNFuncIsComparison(VNFunc vnf, bool* isUnsigned = nullptr); // 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. @@ -2082,17 +2082,28 @@ inline bool ValueNumStore::VNFuncIsCommutative(VNFunc vnf) return (s_vnfOpAttribs[vnf] & VNFOA_Commutative) != 0; } -inline bool ValueNumStore::VNFuncIsComparison(VNFunc vnf) +inline bool ValueNumStore::VNFuncIsComparison(VNFunc vnf, bool* isUnsigned) { if (vnf >= VNF_Boundary) { // For integer types we have unsigned comparisons, and // for floating point types these are the unordered variants. // - return ((vnf == VNF_LT_UN) || (vnf == VNF_LE_UN) || (vnf == VNF_GE_UN) || (vnf == VNF_GT_UN)); + if (((vnf == VNF_LT_UN) || (vnf == VNF_LE_UN) || (vnf == VNF_GE_UN) || (vnf == VNF_GT_UN))) + { + if (isUnsigned != nullptr) + { + *isUnsigned = true; + } + return true; + } + return false; + } + if (isUnsigned != nullptr) + { + *isUnsigned = false; } - genTreeOps gtOp = genTreeOps(vnf); - return GenTree::OperIsCompare(gtOp) != 0; + return GenTree::OperIsCompare(static_cast(vnf)) != 0; } template <> From 2c18ec45f5b30fd844664cec9a3c549def9ac448 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 14:00:59 +0100 Subject: [PATCH 13/26] test --- src/coreclr/jit/redundantbranchopts.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 54889730bd932..2e9befddc717c 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -426,12 +426,12 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) bool canInferFromTrue = true; bool implied = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); - if (!implied) - { - // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". - implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); - canInferFromTrue = false; - } + //if (!implied) + //{ + // // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". + // implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + // canInferFromTrue = false; + //} // TODO: handle NeverIntersects case. if (implied) From 541ada48770cfbb20a2d509622199d832e9935ad Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 14:02:01 +0100 Subject: [PATCH 14/26] jit-format --- src/coreclr/jit/redundantbranchopts.cpp | 30 +++++++++++++------------ 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 2e9befddc717c..b01f40a4b813a 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -160,8 +160,8 @@ bool IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t endIncl; }; - IntegralRange range1 = { SSIZE_T_MIN, SSIZE_T_MAX}; - IntegralRange range2 = { SSIZE_T_MIN, SSIZE_T_MAX }; + IntegralRange range1 = {SSIZE_T_MIN, SSIZE_T_MAX}; + IntegralRange range2 = {SSIZE_T_MIN, SSIZE_T_MAX}; // Update ranges based on inputs auto setRange = [](genTreeOps oper, ssize_t bound, IntegralRange* range) -> bool { @@ -377,8 +377,9 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // VNFunc const domFunc = domApp.m_func; VNFuncApp treeApp; - bool domFuncIsUnsignedCmp = false; - if (inRange && ValueNumStore::VNFuncIsComparison(domFunc, &domFuncIsUnsignedCmp) && vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) + bool domFuncIsUnsignedCmp = false; + if (inRange && ValueNumStore::VNFuncIsComparison(domFunc, &domFuncIsUnsignedCmp) && + vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) { if (((treeApp.m_args[0] == domApp.m_args[0]) && (treeApp.m_args[1] == domApp.m_args[1])) || ((treeApp.m_args[0] == domApp.m_args[1]) && (treeApp.m_args[1] == domApp.m_args[0]))) @@ -417,16 +418,17 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) { // We currently don't handle VNF_relop_UN funcs here bool treeFuncIsUnsignedCmp = false; - if (!domFuncIsUnsignedCmp && ValueNumStore::VNFuncIsComparison(treeApp.m_func, &treeFuncIsUnsignedCmp) && !treeFuncIsUnsignedCmp) + if (!domFuncIsUnsignedCmp && ValueNumStore::VNFuncIsComparison(treeApp.m_func, &treeFuncIsUnsignedCmp) && + !treeFuncIsUnsignedCmp) { - const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); - const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); + const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); + const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); const genTreeOps treeOper = static_cast(treeApp.m_func); - const genTreeOps domOper = static_cast(domApp.m_func); + const genTreeOps domOper = static_cast(domApp.m_func); bool canInferFromTrue = true; - bool implied = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); - //if (!implied) + bool implied = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); + // if (!implied) //{ // // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". // implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); @@ -436,11 +438,11 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // TODO: handle NeverIntersects case. if (implied) { - rii->canInfer = true; - rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; - rii->canInferFromTrue = canInferFromTrue; + rii->canInfer = true; + rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; + rii->canInferFromTrue = canInferFromTrue; rii->canInferFromFalse = !canInferFromTrue; - rii->reverseSense = false; + rii->reverseSense = false; return; } } From 5245290f478e2693691720f958c42cb8ce79163c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 26 Nov 2023 17:46:00 +0100 Subject: [PATCH 15/26] Test commit --- src/coreclr/jit/redundantbranchopts.cpp | 30 +++++++++++++++---------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index b01f40a4b813a..e810701219689 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -197,6 +197,12 @@ bool IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, range->startIncl = bound; return true; + case GT_EQ: + // x == cns -> [cns, cns] + range->startIncl = bound; + range->endIncl = bound; + return true; + default: // unsupported operator return false; @@ -210,15 +216,16 @@ bool IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, return false; } - // Check if range1 is fully included into range2 - if ((range2.startIncl <= range1.startIncl) && (range1.endIncl <= range2.endIncl)) + // If ranges never intersect, then the 2nd range is never "true" + if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) { // E.g.: // - // range1: [100 .. SSIZE_T_MAX] - // range2: [10 .. SSIZE_T_MAX] + // range1: [100 .. INT_MAX] + // range2: [INT_MIN .. 10] return true; } + return false; } @@ -327,8 +334,6 @@ static const RelopImplicationRule s_implicationRules[] = // We don't get all the cases here we could. Still to do: // * two unsigned compares, same operands // * mixture of signed/unsigned compares, same operands -// * mixture of compares, one operand same, other operands different constants -// x > 1 ==> x >= 0 // void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) { @@ -428,12 +433,13 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) bool canInferFromTrue = true; bool implied = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); - // if (!implied) - //{ - // // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". - // implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); - // canInferFromTrue = false; - //} + + if (!implied) + { + // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". + implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + canInferFromTrue = false; + } // TODO: handle NeverIntersects case. if (implied) From 9418be5e5b193b01e733a368129dd4ae758dc49e Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Nov 2023 16:17:18 +0100 Subject: [PATCH 16/26] test fix --- src/coreclr/jit/redundantbranchopts.cpp | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 23fee53ac8f86..a80a62413f764 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -428,26 +428,16 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) { const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); - const genTreeOps treeOper = static_cast(treeApp.m_func); const genTreeOps domOper = static_cast(domApp.m_func); + const genTreeOps treeOper = static_cast(treeApp.m_func); - bool canInferFromTrue = true; - bool implied = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); - - if (!implied) - { - // Reverse the dominating compare and try again, if it succeeds, we can infer from "false". - implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); - canInferFromTrue = false; - } - - // TODO: handle NeverIntersects case. + bool implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); if (implied) { rii->canInfer = true; rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; - rii->canInferFromTrue = canInferFromTrue; - rii->canInferFromFalse = !canInferFromTrue; + rii->canInferFromTrue = false; + rii->canInferFromFalse = true; rii->reverseSense = false; return; } From efbb400315a103655061a6de597a449a9e4eab18 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Nov 2023 16:38:26 +0100 Subject: [PATCH 17/26] Better fix --- src/coreclr/jit/redundantbranchopts.cpp | 35 ++++++++++++++++++++----- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index a80a62413f764..2ee82d576f439 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -152,7 +152,14 @@ struct RelopImplicationRule bool reverse; }; -bool IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) +enum Range2Status +{ + Unknown, + AlwaysFalse, + AlwaysTrue +}; + +Range2Status IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) { struct IntegralRange { @@ -213,7 +220,7 @@ bool IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, const bool success2 = setRange(oper2, bound2, &range2); if (!success1 || !success2) { - return false; + return Range2Status::Unknown; } // If ranges never intersect, then the 2nd range is never "true" @@ -223,10 +230,20 @@ bool IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, // // range1: [100 .. INT_MAX] // range2: [INT_MIN .. 10] - return true; + return Range2Status::AlwaysFalse; } - return false; + // If range1 is a subset of range2, then the 2nd range is always "true" + if ((range1.startIncl >= range2.startIncl) && (range1.endIncl <= range2.endIncl)) + { + // E.g.: + // + // range1: [100 .. INT_MAX] + // range2: [0 .. INT_MAX] + return Range2Status::AlwaysTrue; + } + + return Range2Status::Unknown; } //------------------------------------------------------------------------ @@ -431,14 +448,18 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) const genTreeOps domOper = static_cast(domApp.m_func); const genTreeOps treeOper = static_cast(treeApp.m_func); - bool implied = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); - if (implied) + // We only handle "canInferFromFalse" here, so we need to reverse domOper so then when it's "false" we + // check whether + // we can check whether treeOper is always "true" or "false" (or bail out if it's unknown). + Range2Status treeOperStatus = + IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + if (treeOperStatus != Range2Status::Unknown) { rii->canInfer = true; rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; rii->canInferFromTrue = false; rii->canInferFromFalse = true; - rii->reverseSense = false; + rii->reverseSense = treeOperStatus == Range2Status::AlwaysTrue; return; } } From 714c0019698a0080f16d1818510f38fd44340df4 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Nov 2023 18:46:51 +0100 Subject: [PATCH 18/26] Final clean up --- src/coreclr/jit/redundantbranchopts.cpp | 134 ++++++++++++++++++------ 1 file changed, 100 insertions(+), 34 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 2ee82d576f439..f2c7c10870d05 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -159,6 +159,27 @@ enum Range2Status AlwaysTrue }; +//------------------------------------------------------------------------ +// IsRange2ImpliedByRange1: given two constant range checks: +// +// if (X oper1 bound1) +// { +// if (X oper2 bound2) +// { +// +// determine if the second range check is implied by the dominating first one. +// +// Arguments: +// oper1 - the first comparison operator +// bound1 - the first constant bound +// oper2 - the second comparison operator +// bound2 - the second constant bound +// +// Returns: +// Unknown - the second check is not implied by the first one +// AlwaysFalse - the second check is implied by the first one and is always false +// AlwaysTrue - the second check is implied by the first one and is always true +// Range2Status IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) { struct IntegralRange @@ -210,39 +231,49 @@ Range2Status IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOp range->endIncl = bound; return true; + // TODO: re-think this function in order to handle GT_NE + // and unsigned comparisons. + default: // unsupported operator return false; } }; - const bool success1 = setRange(oper1, bound1, &range1); - const bool success2 = setRange(oper2, bound2, &range2); - if (!success1 || !success2) - { - return Range2Status::Unknown; - } - - // If ranges never intersect, then the 2nd range is never "true" - if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) + if (setRange(oper1, bound1, &range1) && setRange(oper2, bound2, &range2)) { - // E.g.: - // - // range1: [100 .. INT_MAX] - // range2: [INT_MIN .. 10] - return Range2Status::AlwaysFalse; - } + // If ranges never intersect, then the 2nd range is never "true" + if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) + { + // E.g.: + // + // range1: [100 .. INT_MAX] + // range2: [INT_MIN .. 10] + // + // or in other words: + // + // if (x >= 100) + // if (x <= 10) // always false + // + return Range2Status::AlwaysFalse; + } - // If range1 is a subset of range2, then the 2nd range is always "true" - if ((range1.startIncl >= range2.startIncl) && (range1.endIncl <= range2.endIncl)) - { - // E.g.: - // - // range1: [100 .. INT_MAX] - // range2: [0 .. INT_MAX] - return Range2Status::AlwaysTrue; + // If range1 is a subset of range2, then the 2nd range is always "true" + if ((range1.startIncl >= range2.startIncl) && (range1.endIncl <= range2.endIncl)) + { + // E.g.: + // + // range1: [100 .. INT_MAX] + // range2: [10 .. INT_MAX] + // + // or in other words: + // + // if (x >= 100) + // if (x >= 10) // always true + // + return Range2Status::AlwaysTrue; + } } - return Range2Status::Unknown; } @@ -434,6 +465,7 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) } // Given R(x, cns1) and R*(x, cns2) see if we can infer R* from R. + // We assume cns1 and cns2 are always on the RHS of the compare. if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) && vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1]))) @@ -443,22 +475,56 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) if (!domFuncIsUnsignedCmp && ValueNumStore::VNFuncIsComparison(treeApp.m_func, &treeFuncIsUnsignedCmp) && !treeFuncIsUnsignedCmp) { - const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); - const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); - const genTreeOps domOper = static_cast(domApp.m_func); + // Dominating "X relop CNS" + const genTreeOps domOper = static_cast(domApp.m_func); + const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); + + // Dominated "X relop CNS" const genTreeOps treeOper = static_cast(treeApp.m_func); + const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); + + // Example: + // + // void Test(int x) + // { + // if (x > 100) + // if (x > 10) + // Console.WriteLine("Taken!"); + // } + // + + // Corresponding BB layout: + // + // BB1: + // if (x <= 100) + // goto BB4 + // + // BB2: + // // x is known to be > 100 here + // if (x <= 10) // never true + // goto BB4 + // + // BB3: + // Console.WriteLine("Taken!"); + // + // BB4: + // return; + + Range2Status treeOperStatus = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); + bool canInferFromTrue = true; + if (treeOperStatus == Range2Status::Unknown) + { + // Try again for "canInferFromFalse" case - can we infer "treeOper" if "domOper" is false? + canInferFromTrue = false; + treeOperStatus = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + } - // We only handle "canInferFromFalse" here, so we need to reverse domOper so then when it's "false" we - // check whether - // we can check whether treeOper is always "true" or "false" (or bail out if it's unknown). - Range2Status treeOperStatus = - IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); if (treeOperStatus != Range2Status::Unknown) { rii->canInfer = true; rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; - rii->canInferFromTrue = false; - rii->canInferFromFalse = true; + rii->canInferFromTrue = canInferFromTrue; + rii->canInferFromFalse = !canInferFromTrue; rii->reverseSense = treeOperStatus == Range2Status::AlwaysTrue; return; } From d796e79f9d9181994c13c7ab28ace682e1c34032 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Nov 2023 18:50:49 +0100 Subject: [PATCH 19/26] Rename function --- src/coreclr/jit/redundantbranchopts.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index f2c7c10870d05..04a7021b1ca10 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -152,7 +152,7 @@ struct RelopImplicationRule bool reverse; }; -enum Range2Status +enum RelopResult { Unknown, AlwaysFalse, @@ -160,7 +160,7 @@ enum Range2Status }; //------------------------------------------------------------------------ -// IsRange2ImpliedByRange1: given two constant range checks: +// IsCmp2ImpliedByCmp1: given two constant range checks: // // if (X oper1 bound1) // { @@ -180,7 +180,7 @@ enum Range2Status // AlwaysFalse - the second check is implied by the first one and is always false // AlwaysTrue - the second check is implied by the first one and is always true // -Range2Status IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) +RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) { struct IntegralRange { @@ -255,7 +255,7 @@ Range2Status IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOp // if (x >= 100) // if (x <= 10) // always false // - return Range2Status::AlwaysFalse; + return RelopResult::AlwaysFalse; } // If range1 is a subset of range2, then the 2nd range is always "true" @@ -271,10 +271,10 @@ Range2Status IsRange2ImpliedByRange1(genTreeOps oper1, ssize_t bound1, genTreeOp // if (x >= 100) // if (x >= 10) // always true // - return Range2Status::AlwaysTrue; + return RelopResult::AlwaysTrue; } } - return Range2Status::Unknown; + return RelopResult::Unknown; } //------------------------------------------------------------------------ @@ -510,22 +510,22 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // BB4: // return; - Range2Status treeOperStatus = IsRange2ImpliedByRange1(domOper, domCns, treeOper, treeCns); - bool canInferFromTrue = true; - if (treeOperStatus == Range2Status::Unknown) + RelopResult treeOperStatus = IsCmp2ImpliedByCmp1(domOper, domCns, treeOper, treeCns); + bool canInferFromTrue = true; + if (treeOperStatus == RelopResult::Unknown) { // Try again for "canInferFromFalse" case - can we infer "treeOper" if "domOper" is false? canInferFromTrue = false; - treeOperStatus = IsRange2ImpliedByRange1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); + treeOperStatus = IsCmp2ImpliedByCmp1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); } - if (treeOperStatus != Range2Status::Unknown) + if (treeOperStatus != RelopResult::Unknown) { rii->canInfer = true; rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; rii->canInferFromTrue = canInferFromTrue; rii->canInferFromFalse = !canInferFromTrue; - rii->reverseSense = treeOperStatus == Range2Status::AlwaysTrue; + rii->reverseSense = treeOperStatus == RelopResult::AlwaysTrue; return; } } From 5f6673086a201c8b7584434527d98f168cdafdf0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Nov 2023 19:37:16 +0100 Subject: [PATCH 20/26] Revert previous changes --- src/coreclr/jit/redundantbranchopts.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 04a7021b1ca10..26523d9e1d441 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -510,21 +510,16 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // BB4: // return; - RelopResult treeOperStatus = IsCmp2ImpliedByCmp1(domOper, domCns, treeOper, treeCns); - bool canInferFromTrue = true; - if (treeOperStatus == RelopResult::Unknown) - { - // Try again for "canInferFromFalse" case - can we infer "treeOper" if "domOper" is false? - canInferFromTrue = false; - treeOperStatus = IsCmp2ImpliedByCmp1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); - } - + // Check whether the dominating compare being "false" implies the dominated compare is known + // to be either "true" or "false". + RelopResult treeOperStatus = + IsCmp2ImpliedByCmp1(GenTree::ReverseRelop(domOper), domCns, treeOper, treeCns); if (treeOperStatus != RelopResult::Unknown) { rii->canInfer = true; rii->vnRelation = ValueNumStore::VN_RELATION_KIND::VRK_Inferred; - rii->canInferFromTrue = canInferFromTrue; - rii->canInferFromFalse = !canInferFromTrue; + rii->canInferFromTrue = false; + rii->canInferFromFalse = true; rii->reverseSense = treeOperStatus == RelopResult::AlwaysTrue; return; } From 94776af9ab4f5d60601017ccf2724e6dc16748e8 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Nov 2023 19:42:57 +0100 Subject: [PATCH 21/26] Clean up --- src/coreclr/jit/redundantbranchopts.cpp | 9 +++----- src/coreclr/jit/valuenum.h | 29 +++++++++++++------------ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 26523d9e1d441..ebc91ce105db6 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -430,9 +430,7 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // VNFunc const domFunc = domApp.m_func; VNFuncApp treeApp; - bool domFuncIsUnsignedCmp = false; - if (inRange && ValueNumStore::VNFuncIsComparison(domFunc, &domFuncIsUnsignedCmp) && - vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) + if (inRange && ValueNumStore::VNFuncIsComparison(domFunc) && vnStore->GetVNFunc(rii->treeNormVN, &treeApp)) { if (((treeApp.m_args[0] == domApp.m_args[0]) && (treeApp.m_args[1] == domApp.m_args[1])) || ((treeApp.m_args[0] == domApp.m_args[1]) && (treeApp.m_args[1] == domApp.m_args[0]))) @@ -471,9 +469,8 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1]))) { // We currently don't handle VNF_relop_UN funcs here - bool treeFuncIsUnsignedCmp = false; - if (!domFuncIsUnsignedCmp && ValueNumStore::VNFuncIsComparison(treeApp.m_func, &treeFuncIsUnsignedCmp) && - !treeFuncIsUnsignedCmp) + if (ValueNumStore::VNFuncIsSignedComparison(domApp.m_func) && + ValueNumStore::VNFuncIsSignedComparison(treeApp.m_func)) { // Dominating "X relop CNS" const genTreeOps domOper = static_cast(domApp.m_func); diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 61bab4d2e6e95..8a8b8426e7796 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -1044,7 +1044,10 @@ class ValueNumStore static VNFunc SwapRelop(VNFunc vnf); // Returns "true" iff "vnf" is a comparison (and thus binary) operator. - static bool VNFuncIsComparison(VNFunc vnf, bool* isUnsigned = nullptr); + static bool VNFuncIsComparison(VNFunc vnf); + + // Returns "true" iff "vnf" is a signed comparison (and thus binary) operator. + static bool VNFuncIsSignedComparison(VNFunc vnf); // 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. @@ -2082,28 +2085,26 @@ inline bool ValueNumStore::VNFuncIsCommutative(VNFunc vnf) return (s_vnfOpAttribs[vnf] & VNFOA_Commutative) != 0; } -inline bool ValueNumStore::VNFuncIsComparison(VNFunc vnf, bool* isUnsigned) +inline bool ValueNumStore::VNFuncIsComparison(VNFunc vnf) { if (vnf >= VNF_Boundary) { // For integer types we have unsigned comparisons, and // for floating point types these are the unordered variants. // - if (((vnf == VNF_LT_UN) || (vnf == VNF_LE_UN) || (vnf == VNF_GE_UN) || (vnf == VNF_GT_UN))) - { - if (isUnsigned != nullptr) - { - *isUnsigned = true; - } - return true; - } - return false; + return ((vnf == VNF_LT_UN) || (vnf == VNF_LE_UN) || (vnf == VNF_GE_UN) || (vnf == VNF_GT_UN)); } - if (isUnsigned != nullptr) + genTreeOps gtOp = genTreeOps(vnf); + return GenTree::OperIsCompare(gtOp) != 0; +} + +inline bool ValueNumStore::VNFuncIsSignedComparison(VNFunc vnf) +{ + if (vnf >= VNF_Boundary) { - *isUnsigned = false; + return false; } - return GenTree::OperIsCompare(static_cast(vnf)) != 0; + return GenTree::OperIsCompare(genTreeOps(vnf)) != 0; } template <> From 6bdd596acec244c156e0647791722cf1979d369f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Nov 2023 21:36:46 +0100 Subject: [PATCH 22/26] Handle GT_NE --- src/coreclr/jit/redundantbranchopts.cpp | 62 ++++++++++++++++++++----- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index ebc91ce105db6..d3c09ffd0d9f0 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -184,10 +184,21 @@ RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps ope { struct IntegralRange { - ssize_t startIncl; - ssize_t endIncl; + ssize_t startIncl; // inclusive + ssize_t endIncl; // inclusive + + bool Intersects(const IntegralRange other) const + { + return (startIncl <= other.endIncl) && (other.startIncl <= endIncl); + } + + bool Contains(const IntegralRange other) const + { + return (startIncl <= other.startIncl) && (other.endIncl <= endIncl); + } }; + // Start with the widest possible ranges IntegralRange range1 = {SSIZE_T_MIN, SSIZE_T_MAX}; IntegralRange range2 = {SSIZE_T_MIN, SSIZE_T_MAX}; @@ -226,14 +237,13 @@ RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps ope return true; case GT_EQ: + case GT_NE: // x == cns -> [cns, cns] + // NE is special-cased below range->startIncl = bound; range->endIncl = bound; return true; - // TODO: re-think this function in order to handle GT_NE - // and unsigned comparisons. - default: // unsupported operator return false; @@ -242,13 +252,43 @@ RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps ope if (setRange(oper1, bound1, &range1) && setRange(oper2, bound2, &range2)) { + // Special handling of GT_NE: + if ((oper1 == GT_NE) || (oper2 == GT_NE)) + { + // if (x != 100) + // if (x != 100) // always true + if (oper1 == oper2) + { + return bound1 == bound2 ? RelopResult::AlwaysTrue : RelopResult::Unknown; + } + + // if (x == 100) + // if (x != 100) // always false + // + // if (x == 100) + // if (x != 101) // always true + if (oper1 == GT_EQ) + { + return bound1 == bound2 ? RelopResult::AlwaysFalse : RelopResult::AlwaysTrue; + } + + // if (x > 100) + // if (x != 10) // always true + if ((oper2 == GT_NE) && !range1.Intersects(range2)) + { + return AlwaysTrue; + } + + return RelopResult::Unknown; + } + // If ranges never intersect, then the 2nd range is never "true" - if ((range1.startIncl > range2.endIncl) || (range2.startIncl > range1.endIncl)) + if (!range1.Intersects(range2)) { // E.g.: // - // range1: [100 .. INT_MAX] - // range2: [INT_MIN .. 10] + // range1: [100 .. SSIZE_T_MAX] + // range2: [SSIZE_T_MIN .. 10] // // or in other words: // @@ -259,12 +299,12 @@ RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps ope } // If range1 is a subset of range2, then the 2nd range is always "true" - if ((range1.startIncl >= range2.startIncl) && (range1.endIncl <= range2.endIncl)) + if (range2.Contains(range1)) { // E.g.: // - // range1: [100 .. INT_MAX] - // range2: [10 .. INT_MAX] + // range1: [100 .. SSIZE_T_MAX] + // range2: [10 .. SSIZE_T_MAX] // // or in other words: // From b1c69f06f1b8f37cd79d3a8065909ab40fe3d483 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 3 Dec 2023 18:45:05 +0100 Subject: [PATCH 23/26] test --- src/coreclr/jit/redundantbranchopts.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 05342f21314a7..25f7a7bc268c9 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -511,6 +511,12 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) if (ValueNumStore::VNFuncIsSignedComparison(domApp.m_func) && ValueNumStore::VNFuncIsSignedComparison(treeApp.m_func)) { + // CI test + assert(varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[0]))); + assert(vnStore->TypeOfVN(treeApp.m_args[0]) == vnStore->TypeOfVN(domApp.m_args[0])); + assert(vnStore->TypeOfVN(treeApp.m_args[1]) == vnStore->TypeOfVN(domApp.m_args[1])); + assert(vnStore->TypeOfVN(treeApp.m_args[0]) == vnStore->TypeOfVN(domApp.m_args[1])); + // Dominating "X relop CNS" const genTreeOps domOper = static_cast(domApp.m_func); const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); From c8136b0871eb8aeb73da975c56b4a38e7b2f2da2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 3 Dec 2023 21:45:31 +0100 Subject: [PATCH 24/26] Check type of domApp.m_args[0] (same as treeApp.m_args[0]) --- src/coreclr/jit/redundantbranchopts.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 25f7a7bc268c9..4a30244558cbe 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -505,18 +505,13 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // We assume cns1 and cns2 are always on the RHS of the compare. if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) && vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && - varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1]))) + varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1])) && + varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[0]))) { // We currently don't handle VNF_relop_UN funcs here if (ValueNumStore::VNFuncIsSignedComparison(domApp.m_func) && ValueNumStore::VNFuncIsSignedComparison(treeApp.m_func)) { - // CI test - assert(varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[0]))); - assert(vnStore->TypeOfVN(treeApp.m_args[0]) == vnStore->TypeOfVN(domApp.m_args[0])); - assert(vnStore->TypeOfVN(treeApp.m_args[1]) == vnStore->TypeOfVN(domApp.m_args[1])); - assert(vnStore->TypeOfVN(treeApp.m_args[0]) == vnStore->TypeOfVN(domApp.m_args[1])); - // Dominating "X relop CNS" const genTreeOps domOper = static_cast(domApp.m_func); const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); From 302430e9e45e5069fe75ed4f4a83f1cc22a46f9c Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 4 Dec 2023 02:03:28 +0100 Subject: [PATCH 25/26] Update redundantbranchopts.cpp --- src/coreclr/jit/redundantbranchopts.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 4a30244558cbe..07137838794b6 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -505,8 +505,8 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) // We assume cns1 and cns2 are always on the RHS of the compare. if ((treeApp.m_args[0] == domApp.m_args[0]) && vnStore->IsVNConstant(treeApp.m_args[1]) && vnStore->IsVNConstant(domApp.m_args[1]) && varTypeIsIntOrI(vnStore->TypeOfVN(treeApp.m_args[1])) && - varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[1])) && - varTypeIsIntOrI(vnStore->TypeOfVN(domApp.m_args[0]))) + vnStore->TypeOfVN(domApp.m_args[0]) == vnStore->TypeOfVN(treeApp.m_args[1]) && + vnStore->TypeOfVN(domApp.m_args[1]) == vnStore->TypeOfVN(treeApp.m_args[1])) { // We currently don't handle VNF_relop_UN funcs here if (ValueNumStore::VNFuncIsSignedComparison(domApp.m_func) && From 2d8a6df482906f6e6359c03cfc31575f0a9eb18a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 27 Dec 2023 23:41:07 +0100 Subject: [PATCH 26/26] Replace ssize_t with target_ssize_t --- src/coreclr/jit/redundantbranchopts.cpp | 35 ++++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 4a08cb9c24f01..e5fe4d8990f10 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -182,12 +182,12 @@ enum RelopResult // AlwaysFalse - the second check is implied by the first one and is always false // AlwaysTrue - the second check is implied by the first one and is always true // -RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps oper2, ssize_t bound2) +RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, target_ssize_t bound1, genTreeOps oper2, target_ssize_t bound2) { struct IntegralRange { - ssize_t startIncl; // inclusive - ssize_t endIncl; // inclusive + target_ssize_t startIncl; // inclusive + target_ssize_t endIncl; // inclusive bool Intersects(const IntegralRange other) const { @@ -200,17 +200,20 @@ RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps ope } }; + constexpr target_ssize_t minValue = TARGET_POINTER_SIZE == 4 ? INT32_MIN : INT64_MIN; + constexpr target_ssize_t maxValue = TARGET_POINTER_SIZE == 4 ? INT32_MAX : INT64_MAX; + // Start with the widest possible ranges - IntegralRange range1 = {SSIZE_T_MIN, SSIZE_T_MAX}; - IntegralRange range2 = {SSIZE_T_MIN, SSIZE_T_MAX}; + IntegralRange range1 = {minValue, maxValue}; + IntegralRange range2 = {minValue, maxValue}; // Update ranges based on inputs - auto setRange = [](genTreeOps oper, ssize_t bound, IntegralRange* range) -> bool { + auto setRange = [](genTreeOps oper, target_ssize_t bound, IntegralRange* range) -> bool { switch (oper) { case GT_LT: - // x < cns -> [SSIZE_T_MIN, cns - 1] - if (bound == SSIZE_T_MIN) + // x < cns -> [minValue, cns - 1] + if (bound == minValue) { // overflows return false; @@ -219,13 +222,13 @@ RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps ope return true; case GT_LE: - // x <= cns -> [SSIZE_T_MIN, cns] + // x <= cns -> [minValue, cns] range->endIncl = bound; return true; case GT_GT: - // x > cns -> [cns + 1, SSIZE_T_MAX] - if (bound == SSIZE_T_MAX) + // x > cns -> [cns + 1, maxValue] + if (bound == maxValue) { // overflows return false; @@ -234,7 +237,7 @@ RelopResult IsCmp2ImpliedByCmp1(genTreeOps oper1, ssize_t bound1, genTreeOps ope return true; case GT_GE: - // x >= cns -> [cns, SSIZE_T_MAX] + // x >= cns -> [cns, maxValue] range->startIncl = bound; return true; @@ -516,12 +519,12 @@ void Compiler::optRelopImpliesRelop(RelopImplicationInfo* rii) ValueNumStore::VNFuncIsSignedComparison(treeApp.m_func)) { // Dominating "X relop CNS" - const genTreeOps domOper = static_cast(domApp.m_func); - const ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); + const genTreeOps domOper = static_cast(domApp.m_func); + const target_ssize_t domCns = vnStore->CoercedConstantValue(domApp.m_args[1]); // Dominated "X relop CNS" - const genTreeOps treeOper = static_cast(treeApp.m_func); - const ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); + const genTreeOps treeOper = static_cast(treeApp.m_func); + const target_ssize_t treeCns = vnStore->CoercedConstantValue(treeApp.m_args[1]); // Example: //