From 6cc213de688f7edc9eaee342016661bfd5c31ae2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 03:20:14 +0100 Subject: [PATCH 01/14] cleanup --- src/coreclr/jit/assertionprop.cpp | 20 ++--- src/coreclr/jit/compiler.h | 60 +++++++++----- src/coreclr/jit/rangecheck.cpp | 90 ++++---------------- src/coreclr/jit/valuenum.cpp | 133 ++++++++++-------------------- src/coreclr/jit/valuenum.h | 49 +---------- 5 files changed, 106 insertions(+), 246 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index fbd0bc96fcedcb..f6a4984005f848 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -781,14 +781,6 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde printf("VN " FMT_VN "", curAssertion.GetOp1().GetVN()); break; - case O1K_BOUND_OPER_BND: - printf("Oper_Bnd " FMT_VN "", curAssertion.GetOp1().GetVN()); - break; - - case O1K_BOUND_LOOP_BND: - printf("Loop_Bnd " FMT_VN "", curAssertion.GetOp1().GetVN()); - break; - case O1K_EXACT_TYPE: printf("ExactType " FMT_VN "", curAssertion.GetOp1().GetVN()); break; @@ -872,10 +864,6 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde printf("MT(%s)", eeGetClassName(reinterpret_cast(iconVal))); } } - else if (curAssertion.GetOp1().KindIs(O1K_BOUND_OPER_BND, O1K_BOUND_LOOP_BND)) - { - vnStore->vnDump(this, curAssertion.GetOp2().GetVN()); - } else if (curAssertion.GetOp2().IsNullConstant()) { printf("null"); @@ -910,7 +898,11 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde break; case O2K_CHECKED_BOUND: - printf("VN " FMT_VN "", curAssertion.GetOp2().GetVN()); + printf("Checked_Bnd " FMT_VN "", curAssertion.GetOp2().GetVN()); + break; + + case O2K_CHECKED_BOUND_BINOP: + printf("Checked_Bnd_BinOp " FMT_VN "", curAssertion.GetOp2().GetVN()); break; default: @@ -1458,8 +1450,6 @@ void Compiler::optDebugCheckAssertion(const AssertionDsc& assertion) const case O1K_EXACT_TYPE: case O1K_SUBTYPE: case O1K_VN: - case O1K_BOUND_OPER_BND: - case O1K_BOUND_LOOP_BND: assert(!optLocalAssertionProp); break; default: diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e164ac1b2085c6..e6ac69f7d0c0e0 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7697,8 +7697,6 @@ class Compiler { O1K_LCLVAR, O1K_VN, - O1K_BOUND_OPER_BND, - O1K_BOUND_LOOP_BND, O1K_EXACT_TYPE, O1K_SUBTYPE // NOTE: as of today, only LCLVAR is used by both Local and Global assertion prop @@ -7711,9 +7709,14 @@ class Compiler O2K_LCLVAR_COPY, O2K_CONST_INT, O2K_CONST_DOUBLE, - O2K_CHECKED_BOUND, // Array/Span length (or similar) - O2K_ZEROOBJ, // zeroed struct - O2K_SUBRANGE // IntegralRange + + // These two are just hints that Op2.GetVN() is either a checked bound + // or a binary operation on top of a checked bound and a constant. + O2K_CHECKED_BOUND, + O2K_CHECKED_BOUND_BINOP, + + O2K_ZEROOBJ, + O2K_SUBRANGE }; struct AssertionDsc @@ -7827,7 +7830,8 @@ class Compiler ValueNum GetVN() const { - assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ, O2K_CHECKED_BOUND)); + assert( + KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ, O2K_CHECKED_BOUND, O2K_CHECKED_BOUND_BINOP)); assert(m_vn != ValueNumStore::NoVN); return m_vn; } @@ -7908,11 +7912,11 @@ class Compiler bool IsCheckedBoundArithBound() const { - return (CanPropEqualOrNotEqual() && GetOp1().KindIs(O1K_BOUND_OPER_BND)); + return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP); } bool IsCheckedBoundBound() const { - return (CanPropEqualOrNotEqual() && GetOp1().KindIs(O1K_BOUND_LOOP_BND)); + return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && GetOp2().KindIs(O2K_CHECKED_BOUND); } bool IsCopyAssertion() const @@ -8132,6 +8136,7 @@ class Compiler return true; case O2K_CHECKED_BOUND: + case O2K_CHECKED_BOUND_BINOP: return GetOp2().GetVN() == that.GetOp2().GetVN(); case O2K_LCLVAR_COPY: @@ -8338,25 +8343,40 @@ class Compiler { assert(relopVN != ValueNumStore::NoVN); + VNFuncApp funcApp; + comp->vnStore->GetVNFunc(relopVN, &funcApp); + + VNFunc oper = funcApp.m_func; + ValueNum op1VN = funcApp.m_args[0]; + ValueNum op2VN = funcApp.m_args[1]; + + AssertionDsc dsc = {}; if (withArith) { - assert(comp->vnStore->IsVNCompareCheckedBoundArith(relopVN)); + if (comp->vnStore->IsVNCheckedBoundArith(op1VN) && !comp->vnStore->IsVNCheckedBoundArith(op2VN)) + { + std::swap(op1VN, op2VN); + oper = comp->vnStore->SwapRelop(oper); + } + assert(comp->vnStore->IsVNCheckedBoundArith(op2VN)); + dsc.m_op2.m_kind = O2K_CHECKED_BOUND_BINOP; } else { - assert(comp->vnStore->IsVNCompareCheckedBound(relopVN)); + if (comp->vnStore->IsVNCheckedBound(op1VN) && !comp->vnStore->IsVNCheckedBound(op2VN)) + { + std::swap(op1VN, op2VN); + oper = comp->vnStore->SwapRelop(oper); + } + assert(comp->vnStore->IsVNCheckedBound(op2VN)); + dsc.m_op2.m_kind = O2K_CHECKED_BOUND; } - AssertionDsc dsc = {}; - dsc.m_assertionKind = OAK_NOT_EQUAL; - // TODO-Cleanup: Rename O1K_BOUND_OPER_BND and O1K_BOUND_LOOP_BND to something more meaningful - // Also, consider removing O1K_BOUND_LOOP_BND entirely and use O1K_BOUND_OPER_BND for both cases. - // O1K_BOUND_LOOP_BND is basically "i < bnd +/- 0 != 0". Unless it regresses TP. - dsc.m_op1.m_kind = withArith ? O1K_BOUND_OPER_BND : O1K_BOUND_LOOP_BND; - dsc.m_op1.m_vn = relopVN; - dsc.m_op2.m_kind = O2K_CONST_INT; - dsc.m_op2.m_vn = comp->vnStore->VNZeroForType(TYP_INT); - dsc.m_op2.m_icon.m_iconVal = 0; + dsc.m_assertionKind = FromVMFunc(oper); + dsc.m_op1.m_kind = O1K_VN; + dsc.m_op1.m_vn = op1VN; + dsc.m_op2.m_vn = op2VN; + return dsc; } diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 1525b2eeae1e19..9a8619fdb67f97 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -909,57 +909,32 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, genTreeOps cmpOper = GT_NONE; bool isUnsigned = false; + ValueNum checkedBndVN; + int checkedBndCns; // Current assertion is of the form (i < len - cns) != 0 - if (canUseCheckedBounds && curAssertion.IsCheckedBoundArithBound()) + if (canUseCheckedBounds && curAssertion.IsCheckedBoundArithBound() && + (normalLclVN == curAssertion.GetOp1().GetVN() && + comp->vnStore->IsVNCheckedBoundArith(curAssertion.GetOp2().GetVN(), &checkedBndVN, &checkedBndCns))) { - ValueNumStore::CompareCheckedBoundArithInfo info; - - // Get i, len, cns and < as "info." - comp->vnStore->GetCompareCheckedBoundArithInfo(curAssertion.GetOp1().GetVN(), &info); - - // If we don't have the same variable we are comparing against, bail. - if (normalLclVN != info.cmpOp) - { - continue; - } - - if ((info.arrOper != GT_ADD) && (info.arrOper != GT_SUB)) - { - continue; - } - - // If the operand that operates on the bound is not constant, then done. - if (!comp->vnStore->IsVNInt32Constant(info.arrOp)) - { - continue; - } - - int cons = comp->vnStore->ConstantValue(info.arrOp); - limit = Limit(Limit::keBinOpArray, info.vnBound, info.arrOper == GT_SUB ? -cons : cons); - cmpOper = (genTreeOps)info.cmpOper; + limit = Limit(Limit::keBinOpArray, checkedBndVN, checkedBndCns); + cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); } // Current assertion is of the form (i < len) != 0 - else if (canUseCheckedBounds && curAssertion.IsCheckedBoundBound()) + else if (canUseCheckedBounds && curAssertion.IsCheckedBoundBound() && + (normalLclVN == curAssertion.GetOp1().GetVN() || normalLclVN == curAssertion.GetOp2().GetVN())) { - ValueNumStore::CompareCheckedBoundArithInfo info; - - // Get the info as "i", "<" and "len" - comp->vnStore->GetCompareCheckedBound(curAssertion.GetOp1().GetVN(), &info); - // If we don't have the same variable we are comparing against, bail. - if (normalLclVN == info.cmpOp) - { - cmpOper = (genTreeOps)info.cmpOper; - limit = Limit(Limit::keBinOpArray, info.vnBound, 0); - } - else if (normalLclVN == info.vnBound) + if (normalLclVN == curAssertion.GetOp1().GetVN()) { - cmpOper = GenTree::SwapRelop((genTreeOps)info.cmpOper); - limit = Limit(Limit::keBinOpArray, info.cmpOp, 0); + cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); + limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetVN(), 0); } else { - continue; + assert(normalLclVN == curAssertion.GetOp2().GetVN()); + cmpOper = + GenTree::SwapRelop(Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned)); + limit = Limit(Limit::keBinOpArray, curAssertion.GetOp1().GetVN(), 0); } } // Current assertion is of the form (i < 100) != 0 @@ -1085,17 +1060,6 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, } assert(limit.IsBinOpArray() || limit.IsConstant()); - - // TODO-Cleanup: remove this once these assertions are no longer existed. - // All relop-related assertions should be "op1VN relop op2VN" instead of "relopVN ==/!= 0" - bool legacyAssertionKind = - curAssertion.GetOp1().KindIs(Compiler::O1K_BOUND_LOOP_BND, Compiler::O1K_BOUND_OPER_BND); - - // Make sure the assertion is of the form != 0 or == 0 if it isn't a constant assertion. - if (legacyAssertionKind && (curAssertion.GetOp2().GetVN() != comp->vnStore->VNZeroForType(TYP_INT))) - { - continue; - } #ifdef DEBUG if (comp->verbose) { @@ -1124,28 +1088,6 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, arrLenVN = ValueNumStore::NoVN; } - // During assertion prop we add assertions of the form: - // - // (i < length) == 0 - // (i < length) != 0 - // (i < 100) == 0 - // (i < 100) != 0 - // i == 100 - // - // At this point, we have detected that either op1.vn is (i < length) or (i < length + cns) or - // (i < 100) and the op2.vn is 0 or that op1.vn is i and op2.vn is a known constant. - // - // Now, let us check if we are == 0 (i.e., op1 assertion is false) or != 0 (op1 assertion - // is true.). - // - // If we have a non-constant assertion of the form == 0 (i.e., equals false), then reverse relop. - // The relop has to be reversed because we have: (i < length) is false which is the same - // as (i >= length). - if (curAssertion.KindIs(Compiler::OAK_EQUAL) && legacyAssertionKind) - { - cmpOper = GenTree::ReverseRelop(cmpOper); - } - assert(cmpOper != GT_NONE); // Bounds are inclusive, so add -1 for upper bound when "<". But make sure we won't underflow. diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index fcc2db1eb0e495..1a066c1e330809 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7173,66 +7173,54 @@ bool ValueNumStore::IsVNCompareCheckedBound(ValueNum vn) return true; } -void ValueNumStore::GetCompareCheckedBound(ValueNum vn, CompareCheckedBoundArithInfo* info) +bool ValueNumStore::IsVNCheckedBoundArith(ValueNum vn, ValueNum* pCheckedBndVN, int* addCns) { - assert(IsVNCompareCheckedBound(vn)); - - // Do we have var < a.len? - VNFuncApp funcAttr; - GetVNFunc(vn, &funcAttr); - - bool isOp1CheckedBound = IsVNCheckedBound(funcAttr.m_args[1]); - if (isOp1CheckedBound) - { - info->cmpOper = funcAttr.m_func; - info->cmpOp = funcAttr.m_args[0]; - info->vnBound = funcAttr.m_args[1]; - } - else - { - info->cmpOper = GenTree::SwapRelop((genTreeOps)funcAttr.m_func); - info->cmpOp = funcAttr.m_args[1]; - info->vnBound = funcAttr.m_args[0]; - } -} - -bool ValueNumStore::IsVNCheckedBoundArith(ValueNum vn) -{ - // Do we have "a.len +or- var" - if (vn == NoVN) + int cns; + VNFuncApp funcApp; + if (GetVNFunc(vn, &funcApp) && ((funcApp.m_func == VNF_ADD) || (funcApp.m_func == VNF_SUB))) { - return false; - } - - VNFuncApp funcAttr; - - return GetVNFunc(vn, &funcAttr) && // vn is a func. - (funcAttr.m_func == (VNFunc)GT_ADD || funcAttr.m_func == (VNFunc)GT_SUB) && // the func is +/- - (IsVNCheckedBound(funcAttr.m_args[0]) || IsVNCheckedBound(funcAttr.m_args[1])); // either op1 or op2 is a.len -} + // ADD is commutative, so check if "cns + checkedBndVN" first + if ((funcApp.m_func == VNF_ADD) && IsVNCheckedBound(funcApp.m_args[1]) && + IsVNIntegralConstant(funcApp.m_args[0], &cns)) + { + if (pCheckedBndVN != nullptr) + { + *pCheckedBndVN = funcApp.m_args[1]; + } + if (addCns != nullptr) + { + *addCns = cns; + } + return true; + } -void ValueNumStore::GetCheckedBoundArithInfo(ValueNum vn, CompareCheckedBoundArithInfo* info) -{ - // Do we have a.len +/- var? - assert(IsVNCheckedBoundArith(vn)); - VNFuncApp funcArith; - GetVNFunc(vn, &funcArith); + // Otherwise, check if we have "checkedBndVN -/+ cns" + if (IsVNCheckedBound(funcApp.m_args[0]) && IsVNIntegralConstant(funcApp.m_args[1], &cns)) + { + // Normalize "checkedBndVN - cns" into "checkedBndVN + (-cns)" to make it easier for the caller to handle + // both cases. + if (funcApp.m_func == VNF_SUB) + { + if (cns == INT32_MIN) + { + // Avoid possible overflow from negating cns. + return false; + } + cns = -cns; + } - bool isOp1CheckedBound = IsVNCheckedBound(funcArith.m_args[1]); - if (isOp1CheckedBound) - { - info->arrOper = funcArith.m_func; - info->arrOp = funcArith.m_args[0]; - info->vnBound = funcArith.m_args[1]; - info->arrOpLHS = true; - } - else - { - info->arrOper = funcArith.m_func; - info->arrOp = funcArith.m_args[1]; - info->vnBound = funcArith.m_args[0]; - info->arrOpLHS = false; + if (pCheckedBndVN != nullptr) + { + *pCheckedBndVN = funcApp.m_args[0]; + } + if (addCns != nullptr) + { + *addCns = cns; + } + return true; + } } + return false; } bool ValueNumStore::IsVNCompareCheckedBoundArith(ValueNum vn) @@ -7265,29 +7253,6 @@ bool ValueNumStore::IsVNCompareCheckedBoundArith(ValueNum vn) return true; } -void ValueNumStore::GetCompareCheckedBoundArithInfo(ValueNum vn, CompareCheckedBoundArithInfo* info) -{ - assert(IsVNCompareCheckedBoundArith(vn)); - - VNFuncApp funcAttr; - GetVNFunc(vn, &funcAttr); - - // Check whether op0 or op1 is checked bound arithmetic. - bool isOp1CheckedBoundArith = IsVNCheckedBoundArith(funcAttr.m_args[1]); - if (isOp1CheckedBoundArith) - { - info->cmpOper = funcAttr.m_func; - info->cmpOp = funcAttr.m_args[0]; - GetCheckedBoundArithInfo(funcAttr.m_args[1], info); - } - else - { - info->cmpOper = GenTree::SwapRelop((genTreeOps)funcAttr.m_func); - info->cmpOp = funcAttr.m_args[1]; - GetCheckedBoundArithInfo(funcAttr.m_args[0], info); - } -} - ValueNum ValueNumStore::GetArrForLenVn(ValueNum vn) { if (vn == NoVN) @@ -10331,18 +10296,6 @@ void ValueNumStore::vnDump(Compiler* comp, ValueNum vn, bool isPtr) unreached(); } } - else if (IsVNCompareCheckedBound(vn)) - { - CompareCheckedBoundArithInfo info; - GetCompareCheckedBound(vn, &info); - info.dump(this); - } - else if (IsVNCompareCheckedBoundArith(vn)) - { - CompareCheckedBoundArithInfo info; - GetCompareCheckedBoundArithInfo(vn, &info); - info.dump(this); - } else if (IsVNFunc(vn)) { VNFuncApp funcApp; diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index e62122b3cccce1..de6e2caf22ec89 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -1088,42 +1088,6 @@ class ValueNumStore } }; - struct CompareCheckedBoundArithInfo - { - // (vnBound - 1) > vnOp - // (vnBound arrOper arrOp) cmpOper cmpOp - ValueNum vnBound; - unsigned arrOper; - ValueNum arrOp; - bool arrOpLHS; // arrOp is on the left side of cmpOp expression - unsigned cmpOper; - ValueNum cmpOp; - CompareCheckedBoundArithInfo() - : vnBound(NoVN) - , arrOper(GT_NONE) - , arrOp(NoVN) - , arrOpLHS(false) - , cmpOper(GT_NONE) - , cmpOp(NoVN) - { - } -#ifdef DEBUG - void dump(ValueNumStore* vnStore) - { - vnStore->vnDump(vnStore->m_compiler, cmpOp); - printf(" "); - printf(vnStore->VNFuncName((VNFunc)cmpOper)); - printf(" "); - vnStore->vnDump(vnStore->m_compiler, vnBound); - if (arrOper != GT_NONE) - { - printf(vnStore->VNFuncName((VNFunc)arrOper)); - vnStore->vnDump(vnStore->m_compiler, arrOp); - } - } -#endif - }; - // Check if "vn" is "new [] (type handle, size)" bool IsVNNewArr(ValueNum vn, VNFuncApp* funcApp); @@ -1151,21 +1115,12 @@ class ValueNumStore // If "vn" is of the form "var < len" or "len <= var" return true. bool IsVNCompareCheckedBound(ValueNum vn); - // If "vn" is checked bound, then populate the "info" fields for the boundVn, cmpOp, cmpOper. - void GetCompareCheckedBound(ValueNum vn, CompareCheckedBoundArithInfo* info); - - // If "vn" is of the form "len +/- var" return true. - bool IsVNCheckedBoundArith(ValueNum vn); - - // If "vn" is checked bound arith, then populate the "info" fields for arrOper, arrVn, arrOp. - void GetCheckedBoundArithInfo(ValueNum vn, CompareCheckedBoundArithInfo* info); + // If "vn" is of the form "len + cns" return true. + bool IsVNCheckedBoundArith(ValueNum vn, ValueNum* pCheckedBndVN = nullptr, int* addCns = nullptr); // If "vn" is of the form "var < len +/- k" return true. bool IsVNCompareCheckedBoundArith(ValueNum vn); - // If "vn" is checked bound arith, then populate the "info" fields for cmpOp, cmpOper. - void GetCompareCheckedBoundArithInfo(ValueNum vn, CompareCheckedBoundArithInfo* info); - // Returns the flags on the current handle. GTF_ICON_SCOPE_HDL for example. GenTreeFlags GetHandleFlags(ValueNum vn); From 3a828f9ff24ae81a6e6a95a5cba4331e46c3c2c4 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 16:02:57 +0100 Subject: [PATCH 02/14] x --- src/coreclr/jit/assertionprop.cpp | 23 ++++++++++++----------- src/coreclr/jit/compiler.h | 28 +++++++++++++++++++++++----- src/coreclr/jit/rangecheck.cpp | 7 ++----- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index f6a4984005f848..626bfef1562b1a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1601,23 +1601,24 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) ValueNum relopVN = vnStore->VNConservativeNormalValue(relop->gtVNPair); - // Cases where op1 holds the lhs of the condition and op2 holds the bound arithmetic. - // Loop condition like: "i < bnd +/-k" - // Assertion: "i < bnd +/- k != 0" - if (vnStore->IsVNCompareCheckedBoundArith(relopVN)) + + // Cases where op1 holds the lhs of the condition op2 holds the bound. + // Loop condition like "i < bnd" + // Assertion: "i < bnd != 0" + if (vnStore->IsVNCompareCheckedBound(relopVN)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBoundArith(this, relopVN, /*withArith*/ true); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBoundArith(this, relopVN, /*withArith*/ false); AssertionIndex index = optAddAssertion(dsc); optCreateComplementaryAssertion(index); return index; } - // Cases where op1 holds the lhs of the condition op2 holds the bound. - // Loop condition like "i < bnd" - // Assertion: "i < bnd != 0" - if (vnStore->IsVNCompareCheckedBound(relopVN)) + // Cases where op1 holds the lhs of the condition and op2 holds the bound arithmetic. + // Loop condition like: "i < bnd +/-k" + // Assertion: "i < bnd +/- k != 0" + if (vnStore->IsVNCompareCheckedBoundArith(relopVN)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBoundArith(this, relopVN, /*withArith*/ false); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBoundArith(this, relopVN, /*withArith*/ true); AssertionIndex index = optAddAssertion(dsc); optCreateComplementaryAssertion(index); return index; @@ -3892,7 +3893,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, // Example: currentTree is "X >= Y" and we have an assertion "X >= Y" (or its inverse "X < Y"). // if (curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == op1VN) && - (curAssertion.GetOp2().GetVN() == op2VN)) + (curAssertion.GetOp2().GetVN() == op2VN) && !curAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP)) { bool isUnsigned; genTreeOps assertionOper = AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e6ac69f7d0c0e0..b7a7cd2adbcf44 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7815,6 +7815,13 @@ class Compiler return m_icon.m_iconVal; } + int GetCheckedBoundConstant() const + { + assert(KindIs(O2K_CHECKED_BOUND_BINOP, O2K_CHECKED_BOUND)); + assert(m_icon.m_iconVal == 0 || KindIs(O2K_CHECKED_BOUND_BINOP)); + return (int)m_icon.m_iconVal; + } + IntegralRange GetIntegralRange() const { assert(KindIs(O2K_SUBRANGE)); @@ -7916,7 +7923,9 @@ class Compiler } bool IsCheckedBoundBound() const { - return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && GetOp2().KindIs(O2K_CHECKED_BOUND); + return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && + (GetOp2().KindIs(O2K_CHECKED_BOUND) || + (GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP) && GetOp2().GetCheckedBoundConstant() == 0)); } bool IsCopyAssertion() const @@ -7962,7 +7971,9 @@ class Compiler bool IsBoundsCheckNoThrow() const { // O1K_VN (idx) u< O2K_VN (len) - return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && GetOp2().KindIs(O2K_CHECKED_BOUND); + return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && + (GetOp2().KindIs(O2K_CHECKED_BOUND) || + (GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP) && GetOp2().GetCheckedBoundConstant() == 0)); } // Convert VNFunc to optAssertionKind @@ -8136,9 +8147,12 @@ class Compiler return true; case O2K_CHECKED_BOUND: - case O2K_CHECKED_BOUND_BINOP: return GetOp2().GetVN() == that.GetOp2().GetVN(); + case O2K_CHECKED_BOUND_BINOP: + return GetOp2().GetVN() == that.GetOp2().GetVN() && + GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant(); + case O2K_LCLVAR_COPY: return GetOp2().GetLclNum() == that.GetOp2().GetLclNum(); @@ -8358,8 +8372,12 @@ class Compiler std::swap(op1VN, op2VN); oper = comp->vnStore->SwapRelop(oper); } - assert(comp->vnStore->IsVNCheckedBoundArith(op2VN)); + + int cns; + bool found = comp->vnStore->IsVNCheckedBoundArith(op2VN, &dsc.m_op2.m_vn, &cns); + assert(found); dsc.m_op2.m_kind = O2K_CHECKED_BOUND_BINOP; + dsc.m_op2.m_icon.m_iconVal = cns; } else { @@ -8370,12 +8388,12 @@ class Compiler } assert(comp->vnStore->IsVNCheckedBound(op2VN)); dsc.m_op2.m_kind = O2K_CHECKED_BOUND; + dsc.m_op2.m_vn = op2VN; } dsc.m_assertionKind = FromVMFunc(oper); dsc.m_op1.m_kind = O1K_VN; dsc.m_op1.m_vn = op1VN; - dsc.m_op2.m_vn = op2VN; return dsc; } diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 9a8619fdb67f97..bb3ece5b3e2b93 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -909,14 +909,11 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, genTreeOps cmpOper = GT_NONE; bool isUnsigned = false; - ValueNum checkedBndVN; - int checkedBndCns; // Current assertion is of the form (i < len - cns) != 0 if (canUseCheckedBounds && curAssertion.IsCheckedBoundArithBound() && - (normalLclVN == curAssertion.GetOp1().GetVN() && - comp->vnStore->IsVNCheckedBoundArith(curAssertion.GetOp2().GetVN(), &checkedBndVN, &checkedBndCns))) + (normalLclVN == curAssertion.GetOp1().GetVN())) { - limit = Limit(Limit::keBinOpArray, checkedBndVN, checkedBndCns); + limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetVN(), curAssertion.GetOp2().GetCheckedBoundConstant()); cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); } // Current assertion is of the form (i < len) != 0 From 49b1346b0af3a4f508a95c9715b5dc4ed10658ef Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 17:54:25 +0100 Subject: [PATCH 03/14] clean up --- src/coreclr/jit/assertionprop.cpp | 98 ++++++++++++++++++++++--------- src/coreclr/jit/compiler.h | 94 +++++++++-------------------- src/coreclr/jit/rangecheck.cpp | 37 +++++------- src/coreclr/jit/valuenum.cpp | 93 ++++++----------------------- src/coreclr/jit/valuenum.h | 8 +-- 5 files changed, 132 insertions(+), 198 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 626bfef1562b1a..6612c1040a3acc 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -897,12 +897,16 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde IntegralRange::Print(curAssertion.GetOp2().GetIntegralRange()); break; - case O2K_CHECKED_BOUND: - printf("Checked_Bnd " FMT_VN "", curAssertion.GetOp2().GetVN()); - break; - - case O2K_CHECKED_BOUND_BINOP: - printf("Checked_Bnd_BinOp " FMT_VN "", curAssertion.GetOp2().GetVN()); + case O2K_CHECKED_BOUND_ADD_CNS: + if (curAssertion.GetOp2().GetCheckedBoundConstant() != 0) + { + printf("(Checked_Bnd_BinOp " FMT_VN " + %d)", curAssertion.GetOp2().GetVN(), + curAssertion.GetOp2().GetCheckedBoundConstant()); + } + else + { + printf("Checked_Bnd_BinOp " FMT_VN "", curAssertion.GetOp2().GetVN()); + } break; default: @@ -1599,33 +1603,67 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) return NO_ASSERTION_INDEX; } - ValueNum relopVN = vnStore->VNConservativeNormalValue(relop->gtVNPair); - - - // Cases where op1 holds the lhs of the condition op2 holds the bound. - // Loop condition like "i < bnd" - // Assertion: "i < bnd != 0" - if (vnStore->IsVNCompareCheckedBound(relopVN)) + ValueNum relopVN = optConservativeNormalVN(relop); + VNFuncApp relopFuncApp; + if (!vnStore->GetVNFunc(relopVN, &relopFuncApp)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBoundArith(this, relopVN, /*withArith*/ false); - AssertionIndex index = optAddAssertion(dsc); - optCreateComplementaryAssertion(index); - return index; + return NO_ASSERTION_INDEX; } - // Cases where op1 holds the lhs of the condition and op2 holds the bound arithmetic. - // Loop condition like: "i < bnd +/-k" - // Assertion: "i < bnd +/- k != 0" - if (vnStore->IsVNCompareCheckedBoundArith(relopVN)) + VNFunc relopFunc = relopFuncApp.m_func; + ValueNum op1VN = relopFuncApp.m_args[0]; + ValueNum op2VN = relopFuncApp.m_args[1]; + + // Comparisons involving checked bounds + if ((relopFunc == VNF_LE) || (relopFunc == VNF_LT) || (relopFunc == VNF_GE) || (relopFunc == VNF_GT) || + (relopFunc == VNF_GT)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBoundArith(this, relopVN, /*withArith*/ true); - AssertionIndex index = optAddAssertion(dsc); - optCreateComplementaryAssertion(index); - return index; + // "CheckedBnd X" + if (vnStore->IsVNCheckedBound(op1VN)) + { + // Move the checked bound to the right side for simplicity + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0); + AssertionIndex index = optAddAssertion(dsc); + optCreateComplementaryAssertion(index); + return index; + } + + // "X CheckedBnd" + if (vnStore->IsVNCheckedBound(op2VN)) + { + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0); + AssertionIndex index = optAddAssertion(dsc); + optCreateComplementaryAssertion(index); + return index; + } + + ValueNum checkedBnd; + int cns; + + // "(CheckedBnd + CNS) X" + if (vnStore->IsVNCheckedBoundAddConst(op1VN, &checkedBnd, &cns)) + { + // Move the (CheckedBnd + CNS) part to the right side for simplicity + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, cns); + AssertionIndex index = optAddAssertion(dsc); + optCreateComplementaryAssertion(index); + return index; + } + + // "X (CheckedBnd + CNS)" + if (vnStore->IsVNCheckedBoundAddConst(op2VN, &checkedBnd, &cns)) + { + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, cns); + AssertionIndex index = optAddAssertion(dsc); + optCreateComplementaryAssertion(index); + return index; + } } - // Loop condition like "(uint)i < (uint)bnd" or equivalent - // Assertion: "no throw" since this condition guarantees that i is both >= 0 and < bnd (on the appropriate edge) + // Pretty much the same as above but for unsigned comparisons. + // We're only interested in "X u< CheckedBnd" or it's inverse "CheckedBnd u>= X". ValueNumStore::UnsignedCompareCheckedBoundInfo unsignedCompareBnd; if (vnStore->IsVNUnsignedCompareCheckedBound(relopVN, &unsignedCompareBnd)) { @@ -3893,7 +3931,11 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, // Example: currentTree is "X >= Y" and we have an assertion "X >= Y" (or its inverse "X < Y"). // if (curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == op1VN) && - (curAssertion.GetOp2().GetVN() == op2VN) && !curAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP)) + (curAssertion.GetOp2().GetVN() == op2VN) && + // O2K_CHECKED_BOUND_ADD_CNS means op2 is decomposed into op2.vn being checked bound + // and op2.cns being the constant offset. We assemble the original relop VN if we want. + // For now, just skip such assertions here. + !curAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) { bool isUnsigned; genTreeOps assertionOper = AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b7a7cd2adbcf44..231d93de51e9d1 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7710,10 +7710,7 @@ class Compiler O2K_CONST_INT, O2K_CONST_DOUBLE, - // These two are just hints that Op2.GetVN() is either a checked bound - // or a binary operation on top of a checked bound and a constant. - O2K_CHECKED_BOUND, - O2K_CHECKED_BOUND_BINOP, + O2K_CHECKED_BOUND_ADD_CNS, O2K_ZEROOBJ, O2K_SUBRANGE @@ -7817,8 +7814,8 @@ class Compiler int GetCheckedBoundConstant() const { - assert(KindIs(O2K_CHECKED_BOUND_BINOP, O2K_CHECKED_BOUND)); - assert(m_icon.m_iconVal == 0 || KindIs(O2K_CHECKED_BOUND_BINOP)); + assert(KindIs(O2K_CHECKED_BOUND_ADD_CNS)); + assert(FitsIn(m_icon.m_iconVal)); return (int)m_icon.m_iconVal; } @@ -7837,8 +7834,7 @@ class Compiler ValueNum GetVN() const { - assert( - KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ, O2K_CHECKED_BOUND, O2K_CHECKED_BOUND_BINOP)); + assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ, O2K_CHECKED_BOUND_ADD_CNS)); assert(m_vn != ValueNumStore::NoVN); return m_vn; } @@ -7917,15 +7913,11 @@ class Compiler return m_op2; } - bool IsCheckedBoundArithBound() const + // Is it "X relop (len + cns)" form? + // NOTE: the cns can be zero, so this implies "X relop len" form. + bool IsCheckedBoundWithConst() const { - return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP); - } - bool IsCheckedBoundBound() const - { - return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && - (GetOp2().KindIs(O2K_CHECKED_BOUND) || - (GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP) && GetOp2().GetCheckedBoundConstant() == 0)); + return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS); } bool IsCopyAssertion() const @@ -7972,12 +7964,11 @@ class Compiler { // O1K_VN (idx) u< O2K_VN (len) return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && - (GetOp2().KindIs(O2K_CHECKED_BOUND) || - (GetOp2().KindIs(O2K_CHECKED_BOUND_BINOP) && GetOp2().GetCheckedBoundConstant() == 0)); + (GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS) && GetOp2().GetCheckedBoundConstant() == 0); } // Convert VNFunc to optAssertionKind - static optAssertionKind FromVMFunc(VNFunc func) + static optAssertionKind FromVNFunc(VNFunc func) { switch (func) { @@ -8146,12 +8137,9 @@ class Compiler case O2K_ZEROOBJ: return true; - case O2K_CHECKED_BOUND: - return GetOp2().GetVN() == that.GetOp2().GetVN(); - - case O2K_CHECKED_BOUND_BINOP: + case O2K_CHECKED_BOUND_ADD_CNS: return GetOp2().GetVN() == that.GetOp2().GetVN() && - GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant(); + GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant(); case O2K_LCLVAR_COPY: return GetOp2().GetLclNum() == that.GetOp2().GetLclNum(); @@ -8347,54 +8335,26 @@ class Compiler dsc.m_assertionKind = OAK_LT_UN; dsc.m_op1.m_kind = O1K_VN; dsc.m_op1.m_vn = idxVN; - dsc.m_op2.m_kind = O2K_CHECKED_BOUND; + dsc.m_op2.m_kind = O2K_CHECKED_BOUND_ADD_CNS; dsc.m_op2.m_vn = lenVN; + + dsc.m_op2.m_icon.m_iconVal = 0; return dsc; } - // Create "i < bnd +/- k != 0" or just "i < bnd != 0" assertion - static AssertionDsc CreateCompareCheckedBoundArith(const Compiler* comp, ValueNum relopVN, bool withArith) + // Create "i (bnd + cns)" assertion + static AssertionDsc CreateCompareCheckedBound(VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns) { - assert(relopVN != ValueNumStore::NoVN); - - VNFuncApp funcApp; - comp->vnStore->GetVNFunc(relopVN, &funcApp); - - VNFunc oper = funcApp.m_func; - ValueNum op1VN = funcApp.m_args[0]; - ValueNum op2VN = funcApp.m_args[1]; - - AssertionDsc dsc = {}; - if (withArith) - { - if (comp->vnStore->IsVNCheckedBoundArith(op1VN) && !comp->vnStore->IsVNCheckedBoundArith(op2VN)) - { - std::swap(op1VN, op2VN); - oper = comp->vnStore->SwapRelop(oper); - } - - int cns; - bool found = comp->vnStore->IsVNCheckedBoundArith(op2VN, &dsc.m_op2.m_vn, &cns); - assert(found); - dsc.m_op2.m_kind = O2K_CHECKED_BOUND_BINOP; - dsc.m_op2.m_icon.m_iconVal = cns; - } - else - { - if (comp->vnStore->IsVNCheckedBound(op1VN) && !comp->vnStore->IsVNCheckedBound(op2VN)) - { - std::swap(op1VN, op2VN); - oper = comp->vnStore->SwapRelop(oper); - } - assert(comp->vnStore->IsVNCheckedBound(op2VN)); - dsc.m_op2.m_kind = O2K_CHECKED_BOUND; - dsc.m_op2.m_vn = op2VN; - } - - dsc.m_assertionKind = FromVMFunc(oper); - dsc.m_op1.m_kind = O1K_VN; - dsc.m_op1.m_vn = op1VN; + assert(op1VN != ValueNumStore::NoVN); + assert(checkedBndVN != ValueNumStore::NoVN); + AssertionDsc dsc = {}; + dsc.m_assertionKind = FromVNFunc(relop); + dsc.m_op1.m_kind = O1K_VN; + dsc.m_op1.m_vn = op1VN; + dsc.m_op2.m_kind = O2K_CHECKED_BOUND_ADD_CNS; + dsc.m_op2.m_vn = checkedBndVN; + dsc.m_op2.m_icon.m_iconVal = cns; return dsc; } @@ -8424,7 +8384,7 @@ class Compiler } AssertionDsc dsc = {}; - dsc.m_assertionKind = FromVMFunc(func); + dsc.m_assertionKind = FromVNFunc(func); dsc.m_op1.m_kind = O1K_VN; dsc.m_op1.m_vn = op1; dsc.m_op2.m_kind = O2K_CONST_INT; diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index bb3ece5b3e2b93..bb4e05847804d1 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -909,32 +909,30 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, genTreeOps cmpOper = GT_NONE; bool isUnsigned = false; - // Current assertion is of the form (i < len - cns) != 0 - if (canUseCheckedBounds && curAssertion.IsCheckedBoundArithBound() && - (normalLclVN == curAssertion.GetOp1().GetVN())) + // Current assertion is of the form "i (checkedBndVN + cns)" + if (canUseCheckedBounds && curAssertion.IsCheckedBoundWithConst()) { - limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetVN(), curAssertion.GetOp2().GetCheckedBoundConstant()); - cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); - } - // Current assertion is of the form (i < len) != 0 - else if (canUseCheckedBounds && curAssertion.IsCheckedBoundBound() && - (normalLclVN == curAssertion.GetOp1().GetVN() || normalLclVN == curAssertion.GetOp2().GetVN())) - { - // If we don't have the same variable we are comparing against, bail. if (normalLclVN == curAssertion.GetOp1().GetVN()) { cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); - limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetVN(), 0); + limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetVN(), + curAssertion.GetOp2().GetCheckedBoundConstant()); } - else + // Check if it's "checkedBndVN1 (checkedBndVN2 + 0)" and normalLclVN is checkedBndVN2 + else if ((normalLclVN == curAssertion.GetOp2().GetVN()) && + (curAssertion.GetOp2().GetCheckedBoundConstant() == 0)) { - assert(normalLclVN == curAssertion.GetOp2().GetVN()); cmpOper = GenTree::SwapRelop(Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned)); limit = Limit(Limit::keBinOpArray, curAssertion.GetOp1().GetVN(), 0); } + else + { + continue; + } + assert(!isUnsigned); } - // Current assertion is of the form (i < 100) != 0 + // Current assertion is of the form "i cns" else if (curAssertion.IsRelop() && curAssertion.GetOp2().KindIs(Compiler::O2K_CONST_INT) && (curAssertion.GetOp1().GetVN() == normalLclVN) && FitsIn(curAssertion.GetOp2().GetIntConstant())) { @@ -942,14 +940,9 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); limit = Limit(Limit::keConstant, static_cast(curAssertion.GetOp2().GetIntConstant())); } - // Current assertion is of the form i == 100 - else if (curAssertion.IsConstantInt32Assertion()) + // Current assertion is of the form "i == cns" or "i != cns" + else if (curAssertion.IsConstantInt32Assertion() && (curAssertion.GetOp1().GetVN() == normalLclVN)) { - if (curAssertion.GetOp1().GetVN() != normalLclVN) - { - continue; - } - // Ignore GC values/NULL caught by IsConstantInt32Assertion assertion (may happen on 32bit) if (varTypeIsGC(comp->vnStore->TypeOfVN(curAssertion.GetOp2().GetVN()))) { diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 1a066c1e330809..e95e6e8c883580 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7147,50 +7147,31 @@ bool ValueNumStore::IsVNUnsignedCompareCheckedBound(ValueNum vn, UnsignedCompare return false; } -bool ValueNumStore::IsVNCompareCheckedBound(ValueNum vn) -{ - // Do we have "var < len"? - if (vn == NoVN) - { - return false; - } - - VNFuncApp funcAttr; - if (!GetVNFunc(vn, &funcAttr)) - { - return false; - } - if (funcAttr.m_func != (VNFunc)GT_LE && funcAttr.m_func != (VNFunc)GT_GE && funcAttr.m_func != (VNFunc)GT_LT && - funcAttr.m_func != (VNFunc)GT_GT) - { - return false; - } - if (!IsVNCheckedBound(funcAttr.m_args[0]) && !IsVNCheckedBound(funcAttr.m_args[1])) - { - return false; - } - - return true; -} - -bool ValueNumStore::IsVNCheckedBoundArith(ValueNum vn, ValueNum* pCheckedBndVN, int* addCns) +//------------------------------------------------------------------------ +// IsVNCheckedBoundAddConst: Checks if the specified vn represents an expression +// of the form "checkedBndVN +/- cns" or its variations. +// +// Arguments: +// vn - Value number to inspect +// checkedBndVN - [Out] checked bound VN in the expression +// addCns - [Out] constant being added to/subtracted from the checked bound VN +// +// Returns: +// true if the specified vn represents an expression of the form "checkedBndVN +/- cns" or its variations, false +// otherwise. +// +bool ValueNumStore::IsVNCheckedBoundAddConst(ValueNum vn, ValueNum* checkedBndVN, int* addCns) { int cns; VNFuncApp funcApp; if (GetVNFunc(vn, &funcApp) && ((funcApp.m_func == VNF_ADD) || (funcApp.m_func == VNF_SUB))) { - // ADD is commutative, so check if "cns + checkedBndVN" first + // ADD is commutative, so check an unusual "cns + checkedBndVN" shape first if ((funcApp.m_func == VNF_ADD) && IsVNCheckedBound(funcApp.m_args[1]) && IsVNIntegralConstant(funcApp.m_args[0], &cns)) { - if (pCheckedBndVN != nullptr) - { - *pCheckedBndVN = funcApp.m_args[1]; - } - if (addCns != nullptr) - { - *addCns = cns; - } + *checkedBndVN = funcApp.m_args[1]; + *addCns = cns; return true; } @@ -7209,50 +7190,14 @@ bool ValueNumStore::IsVNCheckedBoundArith(ValueNum vn, ValueNum* pCheckedBndVN, cns = -cns; } - if (pCheckedBndVN != nullptr) - { - *pCheckedBndVN = funcApp.m_args[0]; - } - if (addCns != nullptr) - { - *addCns = cns; - } + *checkedBndVN = funcApp.m_args[0]; + *addCns = cns; return true; } } return false; } -bool ValueNumStore::IsVNCompareCheckedBoundArith(ValueNum vn) -{ - // Do we have: "var < a.len - var" - if (vn == NoVN) - { - return false; - } - - VNFuncApp funcAttr; - if (!GetVNFunc(vn, &funcAttr)) - { - return false; - } - - // Suitable comparator. - if (funcAttr.m_func != (VNFunc)GT_LE && funcAttr.m_func != (VNFunc)GT_GE && funcAttr.m_func != (VNFunc)GT_LT && - funcAttr.m_func != (VNFunc)GT_GT) - { - return false; - } - - // Either the op0 or op1 is arr len arithmetic. - if (!IsVNCheckedBoundArith(funcAttr.m_args[0]) && !IsVNCheckedBoundArith(funcAttr.m_args[1])) - { - return false; - } - - return true; -} - ValueNum ValueNumStore::GetArrForLenVn(ValueNum vn) { if (vn == NoVN) diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index de6e2caf22ec89..c1e655108a1686 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -1112,14 +1112,8 @@ class ValueNumStore // If "vn" is of the form "(uint)var < (uint)len" (or equivalent) return true. bool IsVNUnsignedCompareCheckedBound(ValueNum vn, UnsignedCompareCheckedBoundInfo* info); - // If "vn" is of the form "var < len" or "len <= var" return true. - bool IsVNCompareCheckedBound(ValueNum vn); - // If "vn" is of the form "len + cns" return true. - bool IsVNCheckedBoundArith(ValueNum vn, ValueNum* pCheckedBndVN = nullptr, int* addCns = nullptr); - - // If "vn" is of the form "var < len +/- k" return true. - bool IsVNCompareCheckedBoundArith(ValueNum vn); + bool IsVNCheckedBoundAddConst(ValueNum vn, ValueNum* checkedBndVN, int* addCns); // Returns the flags on the current handle. GTF_ICON_SCOPE_HDL for example. GenTreeFlags GetHandleFlags(ValueNum vn); From 8ef1880de5d00710f4f37d7d8d8d2142fb207f86 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 18:06:24 +0100 Subject: [PATCH 04/14] format --- src/coreclr/jit/assertionprop.cpp | 41 +++++++++++++++---------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 6612c1040a3acc..cceb8188baeb7f 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1614,28 +1614,27 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) ValueNum op1VN = relopFuncApp.m_args[0]; ValueNum op2VN = relopFuncApp.m_args[1]; - // Comparisons involving checked bounds - if ((relopFunc == VNF_LE) || (relopFunc == VNF_LT) || (relopFunc == VNF_GE) || (relopFunc == VNF_GT) || - (relopFunc == VNF_GT)) + // Signed comparisons involving checked bounds + if ((relopFunc == VNF_LE) || (relopFunc == VNF_LT) || (relopFunc == VNF_GE) || (relopFunc == VNF_GT)) { // "CheckedBnd X" if (vnStore->IsVNCheckedBound(op1VN)) { // Move the checked bound to the right side for simplicity - relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0); - AssertionIndex index = optAddAssertion(dsc); - optCreateComplementaryAssertion(index); - return index; + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0); + AssertionIndex idx = optAddAssertion(dsc); + optCreateComplementaryAssertion(idx); + return idx; } // "X CheckedBnd" if (vnStore->IsVNCheckedBound(op2VN)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0); - AssertionIndex index = optAddAssertion(dsc); - optCreateComplementaryAssertion(index); - return index; + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0); + AssertionIndex idx = optAddAssertion(dsc); + optCreateComplementaryAssertion(idx); + return idx; } ValueNum checkedBnd; @@ -1645,20 +1644,20 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) if (vnStore->IsVNCheckedBoundAddConst(op1VN, &checkedBnd, &cns)) { // Move the (CheckedBnd + CNS) part to the right side for simplicity - relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, cns); - AssertionIndex index = optAddAssertion(dsc); - optCreateComplementaryAssertion(index); - return index; + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, cns); + AssertionIndex idx = optAddAssertion(dsc); + optCreateComplementaryAssertion(idx); + return idx; } // "X (CheckedBnd + CNS)" if (vnStore->IsVNCheckedBoundAddConst(op2VN, &checkedBnd, &cns)) { AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, cns); - AssertionIndex index = optAddAssertion(dsc); - optCreateComplementaryAssertion(index); - return index; + AssertionIndex idx = optAddAssertion(dsc); + optCreateComplementaryAssertion(idx); + return idx; } } @@ -3932,7 +3931,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, // if (curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == op1VN) && (curAssertion.GetOp2().GetVN() == op2VN) && - // O2K_CHECKED_BOUND_ADD_CNS means op2 is decomposed into op2.vn being checked bound + // O2K_CHECKED_BOUND_ADD_CNS means op2 is decomposed into op2.vn being checked bound // and op2.cns being the constant offset. We assemble the original relop VN if we want. // For now, just skip such assertions here. !curAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) From fd5f72f00fea41db7b4f55b5c4bc8b7dcd82250f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 18:56:26 +0100 Subject: [PATCH 05/14] cleanup --- src/coreclr/jit/assertionprop.cpp | 133 ++++++++++++++++++------------ src/coreclr/jit/compiler.h | 33 ++------ src/coreclr/jit/rangecheck.cpp | 8 +- src/coreclr/jit/valuenum.cpp | 40 --------- src/coreclr/jit/valuenum.h | 17 ++-- 5 files changed, 106 insertions(+), 125 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index cceb8188baeb7f..899faa7179e553 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1550,6 +1550,12 @@ void Compiler::optCreateComplementaryAssertion(AssertionIndex assertionIndex) AssertionDsc reversed = candidateAssertion.Reverse(); optMapComplementary(optAddAssertion(reversed), assertionIndex); } + else if (candidateAssertion.KindIs(OAK_LT, OAK_LT_UN, OAK_LE, OAK_LE_UN) && + candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) + { + // Assertions such as "X > checkedBndVN" aren't very useful. + return; + } else if (AssertionDsc::IsReversible(candidateAssertion.GetKind())) { AssertionDsc reversed = candidateAssertion.Reverse(); @@ -1610,55 +1616,69 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) return NO_ASSERTION_INDEX; } + bool isUnsignedRelop; + if (relopFuncApp.FuncIs(VNF_LE, VNF_LT, VNF_GE, VNF_GT)) + { + isUnsignedRelop = false; + } + else if (relopFuncApp.FuncIs(VNF_LE_UN, VNF_LT_UN, VNF_GE_UN, VNF_GT_UN)) + { + isUnsignedRelop = true; + } + else + { + // Not a relop we're interested in. + return NO_ASSERTION_INDEX; + } + VNFunc relopFunc = relopFuncApp.m_func; ValueNum op1VN = relopFuncApp.m_args[0]; ValueNum op2VN = relopFuncApp.m_args[1]; - // Signed comparisons involving checked bounds - if ((relopFunc == VNF_LE) || (relopFunc == VNF_LT) || (relopFunc == VNF_GE) || (relopFunc == VNF_GT)) + // For now, we don't have consumers for assertions derived from non-int32 comparisons + if ((genActualType(vnStore->TypeOfVN(op1VN)) != TYP_INT) || (genActualType(vnStore->TypeOfVN(op2VN)) != TYP_INT)) { - // "CheckedBnd X" - if (vnStore->IsVNCheckedBound(op1VN)) - { - // Move the checked bound to the right side for simplicity - relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0); - AssertionIndex idx = optAddAssertion(dsc); - optCreateComplementaryAssertion(idx); - return idx; - } + return NO_ASSERTION_INDEX; + } - // "X CheckedBnd" - if (vnStore->IsVNCheckedBound(op2VN)) - { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0); - AssertionIndex idx = optAddAssertion(dsc); - optCreateComplementaryAssertion(idx); - return idx; - } + // "CheckedBnd X" + if (!isUnsignedRelop && vnStore->IsVNCheckedBound(op1VN)) + { + // Move the checked bound to the right side for simplicity + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionIndex idx = optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0)); + optCreateComplementaryAssertion(idx); + return idx; + } - ValueNum checkedBnd; - int cns; + // "X CheckedBnd" + if (!isUnsignedRelop && vnStore->IsVNCheckedBound(op2VN)) + { + AssertionIndex idx = optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0)); + optCreateComplementaryAssertion(idx); + return idx; + } - // "(CheckedBnd + CNS) X" - if (vnStore->IsVNCheckedBoundAddConst(op1VN, &checkedBnd, &cns)) - { - // Move the (CheckedBnd + CNS) part to the right side for simplicity - relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, cns); - AssertionIndex idx = optAddAssertion(dsc); - optCreateComplementaryAssertion(idx); - return idx; - } + // "(CheckedBnd + CNS) X" + ValueNum checkedBnd; + int checkedBndCns; + if (!isUnsignedRelop && vnStore->IsVNCheckedBoundAddConst(op1VN, &checkedBnd, &checkedBndCns)) + { + // Move the (CheckedBnd + CNS) part to the right side for simplicity + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionIndex idx = + optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, checkedBndCns)); + optCreateComplementaryAssertion(idx); + return idx; + } - // "X (CheckedBnd + CNS)" - if (vnStore->IsVNCheckedBoundAddConst(op2VN, &checkedBnd, &cns)) - { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, cns); - AssertionIndex idx = optAddAssertion(dsc); - optCreateComplementaryAssertion(idx); - return idx; - } + // "X (CheckedBnd + CNS)" + if (!isUnsignedRelop && vnStore->IsVNCheckedBoundAddConst(op2VN, &checkedBnd, &checkedBndCns)) + { + AssertionIndex idx = + optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, checkedBndCns)); + optCreateComplementaryAssertion(idx); + return idx; } // Pretty much the same as above but for unsigned comparisons. @@ -1681,12 +1701,21 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) } // Create "X relop CNS" assertion (both signed and unsigned relops) - if (vnStore->IsVNConstantBound(relopVN) || vnStore->IsVNConstantBoundUnsigned(relopVN)) + // Ignore non-positive constants for unsigned relops as they don't add any useful information. + ssize_t cns; + if (vnStore->IsVNIntegralConstant(op1VN, &cns) && (!isUnsignedRelop || (cns > 0))) { - AssertionDsc dsc = AssertionDsc::CreateConstantLoopBound(this, relopVN); - AssertionIndex index = optAddAssertion(dsc); - optCreateComplementaryAssertion(index); - return index; + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionIndex idx = optAddAssertion(AssertionDsc::CreateConstantBound(this, relopFunc, op2VN, op1VN)); + optCreateComplementaryAssertion(idx); + return idx; + } + + if (vnStore->IsVNIntegralConstant(op2VN, &cns) && (!isUnsignedRelop || (cns > 0))) + { + AssertionIndex idx = optAddAssertion(AssertionDsc::CreateConstantBound(this, relopFunc, op1VN, op2VN)); + optCreateComplementaryAssertion(idx); + return idx; } return NO_ASSERTION_INDEX; @@ -3931,7 +3960,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, // if (curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == op1VN) && (curAssertion.GetOp2().GetVN() == op2VN) && - // O2K_CHECKED_BOUND_ADD_CNS means op2 is decomposed into op2.vn being checked bound + // O2K_CHECKED_BOUND_ADD_CNS means op2 is decomposed into op2.vn being checked bound // and op2.cns being the constant offset. We assemble the original relop VN if we want. // For now, just skip such assertions here. !curAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) @@ -5259,16 +5288,16 @@ bool Compiler::optCreateJumpTableImpliedAssertions(BasicBlock* switchBb) if (vnStore->IsVNNeverNegative(opVN)) { // Create "X >= value" assertion (both operands are never negative) - ValueNum relop = vnStore->VNForFunc(TYP_INT, VNF_GE, opVN, vnStore->VNForIntCon(value)); - AssertionDsc dsc = AssertionDsc::CreateConstantLoopBound(this, relop); - newAssertIdx = optAddAssertion(dsc); + AssertionDsc dsc = + AssertionDsc::CreateConstantBound(this, VNF_GE, opVN, vnStore->VNForIntCon(value)); + newAssertIdx = optAddAssertion(dsc); } else { // Create "X u>= value" assertion - ValueNum relop = vnStore->VNForFunc(TYP_INT, VNF_GE_UN, opVN, vnStore->VNForIntCon(value)); - AssertionDsc dsc = AssertionDsc::CreateConstantLoopBound(this, relop); - newAssertIdx = optAddAssertion(dsc); + AssertionDsc dsc = + AssertionDsc::CreateConstantBound(this, VNF_GE_UN, opVN, vnStore->VNForIntCon(value)); + newAssertIdx = optAddAssertion(dsc); } } else diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 231d93de51e9d1..9636c829a9d464 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8360,36 +8360,21 @@ class Compiler // Create "i < constant" or "i u< constant" assertion // TODO-Cleanup: Rename it as it's not necessarily a loop bound - static AssertionDsc CreateConstantLoopBound(const Compiler* comp, ValueNum relopVN) + static AssertionDsc CreateConstantBound(const Compiler* comp, VNFunc relop, ValueNum op1VN, ValueNum cnsVN) { - assert(comp->vnStore->IsVNConstantBound(relopVN) || comp->vnStore->IsVNConstantBoundUnsigned(relopVN)); - - VNFuncApp funcApp; - comp->vnStore->GetVNFunc(relopVN, &funcApp); + assert(op1VN != ValueNumStore::NoVN); - VNFunc func = funcApp.m_func; - ValueNum op1 = funcApp.m_args[0]; - ValueNum op2 = funcApp.m_args[1]; - - // if op1 is a constant, then swap the operands and the operator - if (comp->vnStore->IsVNInt32Constant(op1) && !comp->vnStore->IsVNInt32Constant(op2)) - { - std::swap(op1, op2); - func = comp->vnStore->SwapRelop(func); - } - else - { - bool op2IsCns = comp->vnStore->IsVNInt32Constant(op2); - assert(op2IsCns); - } + ssize_t cns; + bool op2IsCns = comp->vnStore->IsVNIntegralConstant(cnsVN, &cns); + assert(op2IsCns); AssertionDsc dsc = {}; - dsc.m_assertionKind = FromVNFunc(func); + dsc.m_assertionKind = FromVNFunc(relop); dsc.m_op1.m_kind = O1K_VN; - dsc.m_op1.m_vn = op1; + dsc.m_op1.m_vn = op1VN; dsc.m_op2.m_kind = O2K_CONST_INT; - dsc.m_op2.m_vn = op2; - dsc.m_op2.m_icon.m_iconVal = comp->vnStore->CoercedConstantValue(op2); + dsc.m_op2.m_vn = cnsVN; + dsc.m_op2.m_icon.m_iconVal = cns; return dsc; } }; diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index bb4e05847804d1..1a2f2daeddceb4 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -936,9 +936,11 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, else if (curAssertion.IsRelop() && curAssertion.GetOp2().KindIs(Compiler::O2K_CONST_INT) && (curAssertion.GetOp1().GetVN() == normalLclVN) && FitsIn(curAssertion.GetOp2().GetIntConstant())) { - isUnsigned = false; - cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); - limit = Limit(Limit::keConstant, static_cast(curAssertion.GetOp2().GetIntConstant())); + cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); + limit = Limit(Limit::keConstant, static_cast(curAssertion.GetOp2().GetIntConstant())); + + // Make sure we don't deal with negative constants for unsigned comparisons + assert((curAssertion.GetOp2().GetIntConstant() > 0) || !isUnsigned); } // Current assertion is of the form "i == cns" or "i != cns" else if (curAssertion.IsConstantInt32Assertion() && (curAssertion.GetOp1().GetVN() == normalLclVN)) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index e95e6e8c883580..7270f95c823871 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -7010,46 +7010,6 @@ bool ValueNumStore::IsVNRelop(ValueNum vn) } } -bool ValueNumStore::IsVNConstantBound(ValueNum vn) -{ - VNFuncApp funcApp; - if ((vn != NoVN) && GetVNFunc(vn, &funcApp)) - { - if ((funcApp.m_func == (VNFunc)GT_LE) || (funcApp.m_func == (VNFunc)GT_GE) || - (funcApp.m_func == (VNFunc)GT_LT) || (funcApp.m_func == (VNFunc)GT_GT)) - { - const bool op1IsConst = IsVNInt32Constant(funcApp.m_args[0]); - const bool op2IsConst = IsVNInt32Constant(funcApp.m_args[1]); - - // Technically, we can allow both to be constants, - // but such relops are expected to be constant folded anyway. - return op1IsConst != op2IsConst; - } - } - return false; -} - -bool ValueNumStore::IsVNConstantBoundUnsigned(ValueNum vn) -{ - VNFuncApp funcApp; - if (GetVNFunc(vn, &funcApp)) - { - switch (funcApp.m_func) - { - case VNF_LT_UN: - case VNF_LE_UN: - case VNF_GE_UN: - case VNF_GT_UN: - // Technically, we can allow both to be constants, - // but such relops are expected to be constant folded anyway. - return IsVNPositiveInt32Constant(funcApp.m_args[0]) != IsVNPositiveInt32Constant(funcApp.m_args[1]); - default: - break; - } - } - return false; -} - //------------------------------------------------------------------------ // IsVNPositiveInt32Constant: returns true iff vn is a known Int32 constant that is greater then 0 // diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index c1e655108a1686..228e0352076db5 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -186,6 +186,17 @@ struct VNFuncApp unsigned m_arity; ValueNum* m_args; + bool FuncIs(VNFunc func) const + { + return m_func == func; + } + + template + bool FuncIs(VNFunc func, T... rest) const + { + return FuncIs(func) || FuncIs(rest...); + } + bool Equals(const VNFuncApp& funcApp) { if (m_func != funcApp.m_func) @@ -1103,12 +1114,6 @@ class ValueNumStore // If "vn" is VN(a.Length) or VN(a.GetLength(n)) then return VN(a); NoVN if VN(a) can't be determined. ValueNum GetArrForLenVn(ValueNum vn); - // Return true with any Relop except for == and != and one operand has to be a 32-bit integer constant. - bool IsVNConstantBound(ValueNum vn); - - // If "vn" is of the form "(uint)var relop cns" for any relop except for == and != - bool IsVNConstantBoundUnsigned(ValueNum vn); - // If "vn" is of the form "(uint)var < (uint)len" (or equivalent) return true. bool IsVNUnsignedCompareCheckedBound(ValueNum vn, UnsignedCompareCheckedBoundInfo* info); From 9be84a98e8cd1fba6b8214d9ee1b0778051546ca Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 19:20:23 +0100 Subject: [PATCH 06/14] cleanup --- src/coreclr/jit/assertionprop.cpp | 41 +++++++++++++++---------------- src/coreclr/jit/compiler.h | 11 +++++---- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 899faa7179e553..d235416fef2066 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -898,15 +898,8 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde break; case O2K_CHECKED_BOUND_ADD_CNS: - if (curAssertion.GetOp2().GetCheckedBoundConstant() != 0) - { - printf("(Checked_Bnd_BinOp " FMT_VN " + %d)", curAssertion.GetOp2().GetVN(), - curAssertion.GetOp2().GetCheckedBoundConstant()); - } - else - { - printf("Checked_Bnd_BinOp " FMT_VN "", curAssertion.GetOp2().GetVN()); - } + printf("(Checked_Bnd_BinOp " FMT_VN " + %d)", curAssertion.GetOp2().GetVN(), + curAssertion.GetOp2().GetCheckedBoundConstant()); break; default: @@ -1613,6 +1606,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) VNFuncApp relopFuncApp; if (!vnStore->GetVNFunc(relopVN, &relopFuncApp)) { + // We're expecting a relop here return NO_ASSERTION_INDEX; } @@ -1628,6 +1622,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) else { // Not a relop we're interested in. + // Assertions for NE/EQ are handled elsewhere. return NO_ASSERTION_INDEX; } @@ -1635,9 +1630,9 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) ValueNum op1VN = relopFuncApp.m_args[0]; ValueNum op2VN = relopFuncApp.m_args[1]; - // For now, we don't have consumers for assertions derived from non-int32 comparisons if ((genActualType(vnStore->TypeOfVN(op1VN)) != TYP_INT) || (genActualType(vnStore->TypeOfVN(op2VN)) != TYP_INT)) { + // For now, we don't have consumers for assertions derived from non-int32 comparisons return NO_ASSERTION_INDEX; } @@ -1646,7 +1641,8 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) { // Move the checked bound to the right side for simplicity relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionIndex idx = optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0)); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0); + AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; } @@ -1654,7 +1650,8 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // "X CheckedBnd" if (!isUnsignedRelop && vnStore->IsVNCheckedBound(op2VN)) { - AssertionIndex idx = optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0)); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0); + AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; } @@ -1665,9 +1662,9 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) if (!isUnsignedRelop && vnStore->IsVNCheckedBoundAddConst(op1VN, &checkedBnd, &checkedBndCns)) { // Move the (CheckedBnd + CNS) part to the right side for simplicity - relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionIndex idx = - optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, checkedBndCns)); + relopFunc = ValueNumStore::SwapRelop(relopFunc); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, checkedBndCns); + AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; } @@ -1675,14 +1672,14 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // "X (CheckedBnd + CNS)" if (!isUnsignedRelop && vnStore->IsVNCheckedBoundAddConst(op2VN, &checkedBnd, &checkedBndCns)) { - AssertionIndex idx = - optAddAssertion(AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, checkedBndCns)); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, checkedBndCns); + AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; } - // Pretty much the same as above but for unsigned comparisons. - // We're only interested in "X u< CheckedBnd" or it's inverse "CheckedBnd u>= X". + // Loop condition like "(uint)i < (uint)bnd" or equivalent + // Assertion: "no throw" since this condition guarantees that i is both >= 0 and < bnd (on the appropriate edge) ValueNumStore::UnsignedCompareCheckedBoundInfo unsignedCompareBnd; if (vnStore->IsVNUnsignedCompareCheckedBound(relopVN, &unsignedCompareBnd)) { @@ -1706,14 +1703,16 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) if (vnStore->IsVNIntegralConstant(op1VN, &cns) && (!isUnsignedRelop || (cns > 0))) { relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionIndex idx = optAddAssertion(AssertionDsc::CreateConstantBound(this, relopFunc, op2VN, op1VN)); + AssertionDsc dsc = AssertionDsc::CreateConstantBound(this, relopFunc, op2VN, op1VN); + AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; } if (vnStore->IsVNIntegralConstant(op2VN, &cns) && (!isUnsignedRelop || (cns > 0))) { - AssertionIndex idx = optAddAssertion(AssertionDsc::CreateConstantBound(this, relopFunc, op1VN, op2VN)); + AssertionDsc dsc = AssertionDsc::CreateConstantBound(this, relopFunc, op1VN, op2VN); + AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 9636c829a9d464..ab5a44d9d8b936 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7709,9 +7709,8 @@ class Compiler O2K_LCLVAR_COPY, O2K_CONST_INT, O2K_CONST_DOUBLE, - - O2K_CHECKED_BOUND_ADD_CNS, - + O2K_CHECKED_BOUND_ADD_CNS, // "checkedBndVN + cns" where op2.vn holds the "checkedBndVN" + // and op2.iconVal holds the "cns". O2K_ZEROOBJ, O2K_SUBRANGE }; @@ -7812,6 +7811,7 @@ class Compiler return m_icon.m_iconVal; } + // For "checkedBndVN + cns" form, return the "cns" part. int GetCheckedBoundConstant() const { assert(KindIs(O2K_CHECKED_BOUND_ADD_CNS)); @@ -7964,7 +7964,9 @@ class Compiler { // O1K_VN (idx) u< O2K_VN (len) return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && - (GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS) && GetOp2().GetCheckedBoundConstant() == 0); + (GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS) && + // We really only want to cover "idx < len" form here, which is represented as "idx < (len + 0)". + GetOp2().GetCheckedBoundConstant() == 0); } // Convert VNFunc to optAssertionKind @@ -8359,7 +8361,6 @@ class Compiler } // Create "i < constant" or "i u< constant" assertion - // TODO-Cleanup: Rename it as it's not necessarily a loop bound static AssertionDsc CreateConstantBound(const Compiler* comp, VNFunc relop, ValueNum op1VN, ValueNum cnsVN) { assert(op1VN != ValueNumStore::NoVN); From b3a51c773035b7a85ccaac8fca3b39fcf311c7d3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 20:24:56 +0100 Subject: [PATCH 07/14] introduce GetCheckedBound --- src/coreclr/jit/assertionprop.cpp | 10 +++++----- src/coreclr/jit/compiler.h | 29 +++++++++++++++++++---------- src/coreclr/jit/rangecheck.cpp | 6 +++--- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index d235416fef2066..69bc642e4a83a1 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -898,7 +898,7 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde break; case O2K_CHECKED_BOUND_ADD_CNS: - printf("(Checked_Bnd_BinOp " FMT_VN " + %d)", curAssertion.GetOp2().GetVN(), + printf("(Checked_Bnd_BinOp " FMT_VN " + %d)", curAssertion.GetOp2().GetCheckedBound(), curAssertion.GetOp2().GetCheckedBoundConstant()); break; @@ -3586,7 +3586,7 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, // array[idx] = 42; // array.Length is known to be non-negative and non-zero here // - if (curAssertion.IsBoundsCheckNoThrow() && (curAssertion.GetOp2().GetVN() == treeVN)) + if (curAssertion.IsBoundsCheckNoThrow() && (curAssertion.GetOp2().GetCheckedBound() == treeVN)) { *isKnownNonNegative = true; *isKnownNonZero = true; @@ -3958,11 +3958,10 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, // Example: currentTree is "X >= Y" and we have an assertion "X >= Y" (or its inverse "X < Y"). // if (curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == op1VN) && - (curAssertion.GetOp2().GetVN() == op2VN) && // O2K_CHECKED_BOUND_ADD_CNS means op2 is decomposed into op2.vn being checked bound // and op2.cns being the constant offset. We assemble the original relop VN if we want. // For now, just skip such assertions here. - !curAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) + !curAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS) && (curAssertion.GetOp2().GetVN() == op2VN)) { bool isUnsigned; genTreeOps assertionOper = AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); @@ -4973,7 +4972,8 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree } // Do we have a previous range check involving the same 'vnLen' upper bound? - if (curAssertion.GetOp2().GetVN() == vnStore->VNConservativeNormalValue(arrBndsChk->GetArrayLength()->gtVNPair)) + if (curAssertion.GetOp2().GetCheckedBound() == + vnStore->VNConservativeNormalValue(arrBndsChk->GetArrayLength()->gtVNPair)) { // Do we have the exact same lower bound 'vnIdx'? // a[i] followed by a[i] diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index ab5a44d9d8b936..f91e4c6eacf1ff 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7811,14 +7811,6 @@ class Compiler return m_icon.m_iconVal; } - // For "checkedBndVN + cns" form, return the "cns" part. - int GetCheckedBoundConstant() const - { - assert(KindIs(O2K_CHECKED_BOUND_ADD_CNS)); - assert(FitsIn(m_icon.m_iconVal)); - return (int)m_icon.m_iconVal; - } - IntegralRange GetIntegralRange() const { assert(KindIs(O2K_SUBRANGE)); @@ -7834,7 +7826,24 @@ class Compiler ValueNum GetVN() const { - assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ, O2K_CHECKED_BOUND_ADD_CNS)); + assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ)); + assert(m_vn != ValueNumStore::NoVN); + return m_vn; + } + + // For "checkedBndVN + cns" form, return the "cns" part. + int GetCheckedBoundConstant() const + { + assert(KindIs(O2K_CHECKED_BOUND_ADD_CNS)); + assert(FitsIn(m_icon.m_iconVal)); + return (int)m_icon.m_iconVal; + } + + // For "checkedBndVN + cns" form, return the "checkedBndVN" part. + // We intentionally don't allow to use it via GetVN() to avoid confusion. + ValueNum GetCheckedBound() const + { + assert(KindIs(O2K_CHECKED_BOUND_ADD_CNS)); assert(m_vn != ValueNumStore::NoVN); return m_vn; } @@ -8140,7 +8149,7 @@ class Compiler return true; case O2K_CHECKED_BOUND_ADD_CNS: - return GetOp2().GetVN() == that.GetOp2().GetVN() && + return GetOp2().GetCheckedBound() == that.GetOp2().GetCheckedBound() && GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant(); case O2K_LCLVAR_COPY: diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 1a2f2daeddceb4..8e64b9ad9a903b 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -915,11 +915,11 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, if (normalLclVN == curAssertion.GetOp1().GetVN()) { cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); - limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetVN(), + limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetCheckedBound(), curAssertion.GetOp2().GetCheckedBoundConstant()); } // Check if it's "checkedBndVN1 (checkedBndVN2 + 0)" and normalLclVN is checkedBndVN2 - else if ((normalLclVN == curAssertion.GetOp2().GetVN()) && + else if ((normalLclVN == curAssertion.GetOp2().GetCheckedBound()) && (curAssertion.GetOp2().GetCheckedBoundConstant() == 0)) { cmpOper = @@ -997,7 +997,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, { // IsBoundsCheckNoThrow is "op1VN (Idx) LT_UN op2VN (Len)" ValueNum indexVN = curAssertion.GetOp1().GetVN(); - ValueNum lenVN = curAssertion.GetOp2().GetVN(); + ValueNum lenVN = curAssertion.GetOp2().GetCheckedBound(); if (normalLclVN == indexVN) { if (canUseCheckedBounds) From 0f38f18412ecf3ff6e38a1d819d52b9575fe25f2 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 21:19:43 +0100 Subject: [PATCH 08/14] clean up --- src/coreclr/jit/assertionprop.cpp | 26 ++++++++++++++++---------- src/coreclr/jit/compiler.h | 26 +++++++++++++++++--------- src/coreclr/jit/rangecheck.cpp | 4 ++-- src/coreclr/jit/valuenum.h | 1 + 4 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 69bc642e4a83a1..afc14d6c92f574 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -902,6 +902,10 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde curAssertion.GetOp2().GetCheckedBoundConstant()); break; + case O2K_NEVER_NEGATIVE: + printf("(Never_Negative " FMT_VN ")", curAssertion.GetOp2().GetVN()); + break; + default: unreached(); break; @@ -1544,7 +1548,7 @@ void Compiler::optCreateComplementaryAssertion(AssertionIndex assertionIndex) optMapComplementary(optAddAssertion(reversed), assertionIndex); } else if (candidateAssertion.KindIs(OAK_LT, OAK_LT_UN, OAK_LE, OAK_LE_UN) && - candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) + candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS, O2K_NEVER_NEGATIVE)) { // Assertions such as "X > checkedBndVN" aren't very useful. return; @@ -1641,7 +1645,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) { // Move the checked bound to the right side for simplicity relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op2VN, op1VN, 0); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; @@ -1650,7 +1654,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // "X CheckedBnd" if (!isUnsignedRelop && vnStore->IsVNCheckedBound(op2VN)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op1VN, op2VN, 0); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; @@ -1663,7 +1667,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) { // Move the (CheckedBnd + CNS) part to the right side for simplicity relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, checkedBndCns); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op2VN, checkedBnd, checkedBndCns); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; @@ -1672,7 +1676,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // "X (CheckedBnd + CNS)" if (!isUnsignedRelop && vnStore->IsVNCheckedBoundAddConst(op2VN, &checkedBnd, &checkedBndCns)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, checkedBndCns); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op1VN, checkedBnd, checkedBndCns); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; @@ -1686,7 +1690,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) ValueNum idxVN = vnStore->VNNormalValue(unsignedCompareBnd.vnIdx); ValueNum lenVN = vnStore->VNNormalValue(unsignedCompareBnd.vnBound); - AssertionDsc dsc = AssertionDsc::CreateNoThrowArrBnd(this, idxVN, lenVN); + AssertionDsc dsc = AssertionDsc::CreateNoThrowArrBnd(idxVN, lenVN); AssertionIndex index = optAddAssertion(dsc); if (unsignedCompareBnd.cmpOper == VNF_GE_UN) { @@ -1982,7 +1986,10 @@ void Compiler::optAssertionGen(GenTree* tree) } else { - assertionInfo = optAddAssertion(AssertionDsc::CreateNoThrowArrBnd(this, idxVN, lenVN)); + // GT_BOUNDS_CHECK node provides the following contract: + // * idxVN < lenVN + // * lenVN is non-negative + assertionInfo = optAddAssertion(AssertionDsc::CreateNoThrowArrBnd(idxVN, lenVN)); } } break; @@ -3586,7 +3593,7 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, // array[idx] = 42; // array.Length is known to be non-negative and non-zero here // - if (curAssertion.IsBoundsCheckNoThrow() && (curAssertion.GetOp2().GetCheckedBound() == treeVN)) + if (curAssertion.IsBoundsCheckNoThrow() && (curAssertion.GetOp2().GetVN() == treeVN)) { *isKnownNonNegative = true; *isKnownNonZero = true; @@ -4972,8 +4979,7 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree } // Do we have a previous range check involving the same 'vnLen' upper bound? - if (curAssertion.GetOp2().GetCheckedBound() == - vnStore->VNConservativeNormalValue(arrBndsChk->GetArrayLength()->gtVNPair)) + if (curAssertion.GetOp2().GetVN() == vnStore->VNConservativeNormalValue(arrBndsChk->GetArrayLength()->gtVNPair)) { // Do we have the exact same lower bound 'vnIdx'? // a[i] followed by a[i] diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f91e4c6eacf1ff..ae70fe317acb85 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7709,8 +7709,12 @@ class Compiler O2K_LCLVAR_COPY, O2K_CONST_INT, O2K_CONST_DOUBLE, + + O2K_NEVER_NEGATIVE, // Something that is known to be never negative, e.g., array or span length O2K_CHECKED_BOUND_ADD_CNS, // "checkedBndVN + cns" where op2.vn holds the "checkedBndVN" // and op2.iconVal holds the "cns". + // "Checked bound" alone doesn't mean anything (nor it implies that it's never + // negative) O2K_ZEROOBJ, O2K_SUBRANGE }; @@ -7826,7 +7830,7 @@ class Compiler ValueNum GetVN() const { - assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ)); + assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ, O2K_NEVER_NEGATIVE)); assert(m_vn != ValueNumStore::NoVN); return m_vn; } @@ -7972,10 +7976,7 @@ class Compiler bool IsBoundsCheckNoThrow() const { // O1K_VN (idx) u< O2K_VN (len) - return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && - (GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS) && - // We really only want to cover "idx < len" form here, which is represented as "idx < (len + 0)". - GetOp2().GetCheckedBoundConstant() == 0); + return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && (GetOp2().KindIs(O2K_NEVER_NEGATIVE)); } // Convert VNFunc to optAssertionKind @@ -8148,6 +8149,9 @@ class Compiler case O2K_ZEROOBJ: return true; + case O2K_NEVER_NEGATIVE: + return GetOp2().GetVN() == that.GetOp2().GetVN(); + case O2K_CHECKED_BOUND_ADD_CNS: return GetOp2().GetCheckedBound() == that.GetOp2().GetCheckedBound() && GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant(); @@ -8336,8 +8340,9 @@ class Compiler return dsc; } - // Create a no-throw bounds check assertion: idxVN u< lenVN - static AssertionDsc CreateNoThrowArrBnd(const Compiler* comp, ValueNum idxVN, ValueNum lenVN) + // Create a no-throw bounds check assertion: idxVN u< lenVN where lenVN is never negative + // Effectively, this means "idxVN is in range [0, lenVN)". + static AssertionDsc CreateNoThrowArrBnd(ValueNum idxVN, ValueNum lenVN) { assert(idxVN != ValueNumStore::NoVN); assert(lenVN != ValueNumStore::NoVN); @@ -8346,7 +8351,7 @@ class Compiler dsc.m_assertionKind = OAK_LT_UN; dsc.m_op1.m_kind = O1K_VN; dsc.m_op1.m_vn = idxVN; - dsc.m_op2.m_kind = O2K_CHECKED_BOUND_ADD_CNS; + dsc.m_op2.m_kind = O2K_NEVER_NEGATIVE; dsc.m_op2.m_vn = lenVN; dsc.m_op2.m_icon.m_iconVal = 0; @@ -8354,11 +8359,14 @@ class Compiler } // Create "i (bnd + cns)" assertion - static AssertionDsc CreateCompareCheckedBound(VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns) + static AssertionDsc CreateCompareCheckedBound( + const Compiler* comp, VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns) { assert(op1VN != ValueNumStore::NoVN); assert(checkedBndVN != ValueNumStore::NoVN); + // if cns is 0 and checkedBndVN is never negative, we can convert this into CreateNoThrowArrBnd + AssertionDsc dsc = {}; dsc.m_assertionKind = FromVNFunc(relop); dsc.m_op1.m_kind = O1K_VN; diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 8e64b9ad9a903b..a95912802ccf67 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -939,7 +939,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned); limit = Limit(Limit::keConstant, static_cast(curAssertion.GetOp2().GetIntConstant())); - // Make sure we don't deal with negative constants for unsigned comparisons + // Make sure we don't deal with non-positive constants for unsigned comparisons assert((curAssertion.GetOp2().GetIntConstant() > 0) || !isUnsigned); } // Current assertion is of the form "i == cns" or "i != cns" @@ -997,7 +997,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, { // IsBoundsCheckNoThrow is "op1VN (Idx) LT_UN op2VN (Len)" ValueNum indexVN = curAssertion.GetOp1().GetVN(); - ValueNum lenVN = curAssertion.GetOp2().GetCheckedBound(); + ValueNum lenVN = curAssertion.GetOp2().GetVN(); if (normalLclVN == indexVN) { if (canUseCheckedBounds) diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 228e0352076db5..489c3632287407 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -1118,6 +1118,7 @@ class ValueNumStore bool IsVNUnsignedCompareCheckedBound(ValueNum vn, UnsignedCompareCheckedBoundInfo* info); // If "vn" is of the form "len + cns" return true. + // NOTE: it accepts "cns + len" and "len - cns" as well ("len - cns" is treated as "len + (-cns)"). bool IsVNCheckedBoundAddConst(ValueNum vn, ValueNum* checkedBndVN, int* addCns); // Returns the flags on the current handle. GTF_ICON_SCOPE_HDL for example. From dfdd1582f613398bebb0f8347ca0fd346ce17fc3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 21:26:59 +0100 Subject: [PATCH 09/14] cleanup --- src/coreclr/jit/compiler.h | 14 ++++---------- src/coreclr/jit/rangecheck.cpp | 4 +++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index ae70fe317acb85..2e261789727a72 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7713,8 +7713,8 @@ class Compiler O2K_NEVER_NEGATIVE, // Something that is known to be never negative, e.g., array or span length O2K_CHECKED_BOUND_ADD_CNS, // "checkedBndVN + cns" where op2.vn holds the "checkedBndVN" // and op2.iconVal holds the "cns". - // "Checked bound" alone doesn't mean anything (nor it implies that it's never - // negative) + // "Checked bound" alone doesn't mean anything, + // nor it implies that it's never negative. O2K_ZEROOBJ, O2K_SUBRANGE }; @@ -7926,13 +7926,6 @@ class Compiler return m_op2; } - // Is it "X relop (len + cns)" form? - // NOTE: the cns can be zero, so this implies "X relop len" form. - bool IsCheckedBoundWithConst() const - { - return KindIs(OAK_GE, OAK_GT, OAK_LE, OAK_LT) && GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS); - } - bool IsCopyAssertion() const { return (KindIs(OAK_EQUAL) && GetOp1().KindIs(O1K_LCLVAR) && GetOp2().KindIs(O2K_LCLVAR_COPY)); @@ -7975,7 +7968,8 @@ class Compiler bool IsBoundsCheckNoThrow() const { - // O1K_VN (idx) u< O2K_VN (len) + // O1K_VN (idx) u< O2K_VN (len) where len is never negative. + // Effectively, it's "idx >= 0 && idx < len" return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && (GetOp2().KindIs(O2K_NEVER_NEGATIVE)); } diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index a95912802ccf67..26e97627e8e30b 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -910,7 +910,9 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, bool isUnsigned = false; // Current assertion is of the form "i (checkedBndVN + cns)" - if (canUseCheckedBounds && curAssertion.IsCheckedBoundWithConst()) + if (canUseCheckedBounds && + curAssertion.KindIs(Compiler::OAK_GE, Compiler::OAK_GT, Compiler::OAK_LE, Compiler::OAK_LT) && + curAssertion.GetOp2().KindIs(Compiler::O2K_CHECKED_BOUND_ADD_CNS)) { if (normalLclVN == curAssertion.GetOp1().GetVN()) { From 534628706c7a81eb01f714ab3597448a51e0fed0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 21:40:32 +0100 Subject: [PATCH 10/14] cleanup --- src/coreclr/jit/assertionprop.cpp | 8 ++++---- src/coreclr/jit/compiler.h | 4 +--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index afc14d6c92f574..d242bd831462b7 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1645,7 +1645,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) { // Move the checked bound to the right side for simplicity relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op2VN, op1VN, 0); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, op1VN, 0); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; @@ -1654,7 +1654,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // "X CheckedBnd" if (!isUnsignedRelop && vnStore->IsVNCheckedBound(op2VN)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op1VN, op2VN, 0); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, op2VN, 0); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; @@ -1667,7 +1667,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) { // Move the (CheckedBnd + CNS) part to the right side for simplicity relopFunc = ValueNumStore::SwapRelop(relopFunc); - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op2VN, checkedBnd, checkedBndCns); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op2VN, checkedBnd, checkedBndCns); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; @@ -1676,7 +1676,7 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree) // "X (CheckedBnd + CNS)" if (!isUnsignedRelop && vnStore->IsVNCheckedBoundAddConst(op2VN, &checkedBnd, &checkedBndCns)) { - AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op1VN, checkedBnd, checkedBndCns); + AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(relopFunc, op1VN, checkedBnd, checkedBndCns); AssertionIndex idx = optAddAssertion(dsc); optCreateComplementaryAssertion(idx); return idx; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 2e261789727a72..f24db547128851 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8354,13 +8354,11 @@ class Compiler // Create "i (bnd + cns)" assertion static AssertionDsc CreateCompareCheckedBound( - const Compiler* comp, VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns) + VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns) { assert(op1VN != ValueNumStore::NoVN); assert(checkedBndVN != ValueNumStore::NoVN); - // if cns is 0 and checkedBndVN is never negative, we can convert this into CreateNoThrowArrBnd - AssertionDsc dsc = {}; dsc.m_assertionKind = FromVNFunc(relop); dsc.m_op1.m_kind = O1K_VN; From 87b40b9a079a869400727aecbae4cb093eff475d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 21:57:11 +0100 Subject: [PATCH 11/14] cleanup --- src/coreclr/jit/assertionprop.cpp | 14 +++++++------- src/coreclr/jit/compiler.h | 27 ++++++++++++++++----------- src/coreclr/jit/rangecheck.cpp | 6 +++++- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index d242bd831462b7..e9cba2f6075b21 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -902,10 +902,6 @@ void Compiler::optPrintAssertion(const AssertionDsc& curAssertion, AssertionInde curAssertion.GetOp2().GetCheckedBoundConstant()); break; - case O2K_NEVER_NEGATIVE: - printf("(Never_Negative " FMT_VN ")", curAssertion.GetOp2().GetVN()); - break; - default: unreached(); break; @@ -1548,7 +1544,7 @@ void Compiler::optCreateComplementaryAssertion(AssertionIndex assertionIndex) optMapComplementary(optAddAssertion(reversed), assertionIndex); } else if (candidateAssertion.KindIs(OAK_LT, OAK_LT_UN, OAK_LE, OAK_LE_UN) && - candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS, O2K_NEVER_NEGATIVE)) + candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) { // Assertions such as "X > checkedBndVN" aren't very useful. return; @@ -3593,7 +3589,7 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, // array[idx] = 42; // array.Length is known to be non-negative and non-zero here // - if (curAssertion.IsBoundsCheckNoThrow() && (curAssertion.GetOp2().GetVN() == treeVN)) + if (curAssertion.IsBoundsCheckNoThrow() && (curAssertion.GetOp2().GetCheckedBound() == treeVN)) { *isKnownNonNegative = true; *isKnownNonZero = true; @@ -4978,8 +4974,12 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree continue; } + assert(curAssertion.GetOp2().GetCheckedBoundConstant() == 0); + assert(curAssertion.GetOp2().IsCheckedBoundNeverNegative()); + // Do we have a previous range check involving the same 'vnLen' upper bound? - if (curAssertion.GetOp2().GetVN() == vnStore->VNConservativeNormalValue(arrBndsChk->GetArrayLength()->gtVNPair)) + if (curAssertion.GetOp2().GetCheckedBound() == + vnStore->VNConservativeNormalValue(arrBndsChk->GetArrayLength()->gtVNPair)) { // Do we have the exact same lower bound 'vnIdx'? // a[i] followed by a[i] diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f24db547128851..eb120fa37c7885 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7710,7 +7710,6 @@ class Compiler O2K_CONST_INT, O2K_CONST_DOUBLE, - O2K_NEVER_NEGATIVE, // Something that is known to be never negative, e.g., array or span length O2K_CHECKED_BOUND_ADD_CNS, // "checkedBndVN + cns" where op2.vn holds the "checkedBndVN" // and op2.iconVal holds the "cns". // "Checked bound" alone doesn't mean anything, @@ -7771,7 +7770,8 @@ class Compiler private: optOp2Kind m_kind; - uint16_t m_encodedIconFlags; // encoded icon gtFlags + bool m_checkedBoundIsNeverNegative; // only meaningful for O2K_CHECKED_BOUND_ADD_CNS kind + uint16_t m_encodedIconFlags; // encoded icon gtFlags ValueNum m_vn; union { @@ -7830,7 +7830,7 @@ class Compiler ValueNum GetVN() const { - assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ, O2K_NEVER_NEGATIVE)); + assert(KindIs(O2K_CONST_INT, O2K_CONST_DOUBLE, O2K_ZEROOBJ)); assert(m_vn != ValueNumStore::NoVN); return m_vn; } @@ -7852,6 +7852,12 @@ class Compiler return m_vn; } + bool IsCheckedBoundNeverNegative() const + { + assert(KindIs(O2K_CHECKED_BOUND_ADD_CNS)); + return m_checkedBoundIsNeverNegative; + } + optOp2Kind GetKind() const { return m_kind; @@ -7970,7 +7976,8 @@ class Compiler { // O1K_VN (idx) u< O2K_VN (len) where len is never negative. // Effectively, it's "idx >= 0 && idx < len" - return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && (GetOp2().KindIs(O2K_NEVER_NEGATIVE)); + return GetOp1().KindIs(O1K_VN) && KindIs(OAK_LT_UN) && GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS) && + (GetOp2().GetCheckedBoundConstant() == 0) && GetOp2().IsCheckedBoundNeverNegative(); } // Convert VNFunc to optAssertionKind @@ -8143,9 +8150,6 @@ class Compiler case O2K_ZEROOBJ: return true; - case O2K_NEVER_NEGATIVE: - return GetOp2().GetVN() == that.GetOp2().GetVN(); - case O2K_CHECKED_BOUND_ADD_CNS: return GetOp2().GetCheckedBound() == that.GetOp2().GetCheckedBound() && GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant(); @@ -8345,16 +8349,17 @@ class Compiler dsc.m_assertionKind = OAK_LT_UN; dsc.m_op1.m_kind = O1K_VN; dsc.m_op1.m_vn = idxVN; - dsc.m_op2.m_kind = O2K_NEVER_NEGATIVE; + dsc.m_op2.m_kind = O2K_CHECKED_BOUND_ADD_CNS; dsc.m_op2.m_vn = lenVN; - dsc.m_op2.m_icon.m_iconVal = 0; + // Normally, "Checked bound" doesn't mean it's never negative, but in this particular case we know it is. + dsc.m_op2.m_checkedBoundIsNeverNegative = true; + dsc.m_op2.m_icon.m_iconVal = 0; return dsc; } // Create "i (bnd + cns)" assertion - static AssertionDsc CreateCompareCheckedBound( - VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns) + static AssertionDsc CreateCompareCheckedBound(VNFunc relop, ValueNum op1VN, ValueNum checkedBndVN, int cns) { assert(op1VN != ValueNumStore::NoVN); assert(checkedBndVN != ValueNumStore::NoVN); diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 26e97627e8e30b..e3ae60fc3c63ad 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -999,7 +999,11 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, { // IsBoundsCheckNoThrow is "op1VN (Idx) LT_UN op2VN (Len)" ValueNum indexVN = curAssertion.GetOp1().GetVN(); - ValueNum lenVN = curAssertion.GetOp2().GetVN(); + ValueNum lenVN = curAssertion.GetOp2().GetCheckedBound(); + + assert(curAssertion.GetOp2().GetCheckedBoundConstant() == 0); + assert(curAssertion.GetOp2().IsCheckedBoundNeverNegative()); + if (normalLclVN == indexVN) { if (canUseCheckedBounds) From 4e7417ccf179f086028927d36bf8eb50aae83317 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 6 Feb 2026 21:58:29 +0100 Subject: [PATCH 12/14] fix HasSameOp2 --- src/coreclr/jit/compiler.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index eb120fa37c7885..0ffca3fd771cd4 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8152,7 +8152,8 @@ class Compiler case O2K_CHECKED_BOUND_ADD_CNS: return GetOp2().GetCheckedBound() == that.GetOp2().GetCheckedBound() && - GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant(); + GetOp2().GetCheckedBoundConstant() == that.GetOp2().GetCheckedBoundConstant() && + GetOp2().IsCheckedBoundNeverNegative() == that.GetOp2().IsCheckedBoundNeverNegative(); case O2K_LCLVAR_COPY: return GetOp2().GetLclNum() == that.GetOp2().GetLclNum(); From 0508f871a9ccd15e35504217a47cd9eb689bbdac Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 7 Feb 2026 00:16:01 +0100 Subject: [PATCH 13/14] fix test --- src/coreclr/jit/assertionprop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e9cba2f6075b21..ef619bd418b729 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -1543,7 +1543,7 @@ void Compiler::optCreateComplementaryAssertion(AssertionIndex assertionIndex) AssertionDsc reversed = candidateAssertion.Reverse(); optMapComplementary(optAddAssertion(reversed), assertionIndex); } - else if (candidateAssertion.KindIs(OAK_LT, OAK_LT_UN, OAK_LE, OAK_LE_UN) && + else if (candidateAssertion.KindIs(OAK_LT_UN, OAK_LE_UN) && candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS)) { // Assertions such as "X > checkedBndVN" aren't very useful. From 9834839449e9dfb4a632129c5a017a85e11da288 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 7 Feb 2026 03:14:11 +0100 Subject: [PATCH 14/14] Feedback --- src/coreclr/jit/valuenum.cpp | 45 ++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 7270f95c823871..7ea598add66a78 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -2807,6 +2807,13 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN, V { std::swap(arg0VN, arg1VN); } + + // ... unless one of them is a constant, in which case we want the constant to be arg1 + // NOTE: we don't want to re-order "cns[H] binop cns" + if (IsVNConstantNonHandle(arg0VN)) + { + std::swap(arg0VN, arg1VN); + } } // Have we already assigned a ValueNum for 'func'('arg0VN','arg1VN') ? @@ -7124,36 +7131,24 @@ bool ValueNumStore::IsVNCheckedBoundAddConst(ValueNum vn, ValueNum* checkedBndVN { int cns; VNFuncApp funcApp; - if (GetVNFunc(vn, &funcApp) && ((funcApp.m_func == VNF_ADD) || (funcApp.m_func == VNF_SUB))) + if (GetVNFunc(vn, &funcApp) && ((funcApp.m_func == VNF_ADD) || (funcApp.m_func == VNF_SUB)) && + IsVNCheckedBound(funcApp.m_args[0]) && IsVNIntegralConstant(funcApp.m_args[1], &cns)) { - // ADD is commutative, so check an unusual "cns + checkedBndVN" shape first - if ((funcApp.m_func == VNF_ADD) && IsVNCheckedBound(funcApp.m_args[1]) && - IsVNIntegralConstant(funcApp.m_args[0], &cns)) - { - *checkedBndVN = funcApp.m_args[1]; - *addCns = cns; - return true; - } - - // Otherwise, check if we have "checkedBndVN -/+ cns" - if (IsVNCheckedBound(funcApp.m_args[0]) && IsVNIntegralConstant(funcApp.m_args[1], &cns)) + // Normalize "checkedBndVN - cns" into "checkedBndVN + (-cns)" to make it easier for the caller to handle + // both cases. + if (funcApp.m_func == VNF_SUB) { - // Normalize "checkedBndVN - cns" into "checkedBndVN + (-cns)" to make it easier for the caller to handle - // both cases. - if (funcApp.m_func == VNF_SUB) + if (cns == INT32_MIN) { - if (cns == INT32_MIN) - { - // Avoid possible overflow from negating cns. - return false; - } - cns = -cns; + // Avoid possible overflow from negating cns. + return false; } - - *checkedBndVN = funcApp.m_args[0]; - *addCns = cns; - return true; + cns = -cns; } + + *checkedBndVN = funcApp.m_args[0]; + *addCns = cns; + return true; } return false; }