From 98e809e9c10de812a75903801af078a494f3279c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 20 Feb 2025 23:52:44 +0100 Subject: [PATCH 1/5] Use rangecheck in assertprop --- src/coreclr/jit/assertionprop.cpp | 73 ++++++++++++++++++++++++++++--- src/coreclr/jit/rangecheck.cpp | 66 +++++++++++++++++----------- src/coreclr/jit/rangecheck.h | 7 ++- 3 files changed, 113 insertions(+), 33 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index f35612a8943150..4c1f890c775500 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -11,6 +11,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX */ #include "jitpch.h" +#include "rangecheck.h" #ifdef _MSC_VER #pragma hdrstop #endif @@ -4029,11 +4030,36 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, return; } - const ValueNum treeVN = vnStore->VNConservativeNormalValue(tree->gtVNPair); + const ValueNum treeVN = vnStore->VNConservativeNormalValue(tree->gtVNPair); + + if (tree->TypeIs(TYP_INT)) + { + Range rng = Range(Limit(Limit::keDependent)); + RangeCheck::MergeEdgeAssertions(this, treeVN, ValueNumStore::NoVN, assertions, &rng, false); + if (rng.LowerLimit().IsConstant()) + { + int lower = rng.LowerLimit().GetConstant(); + if (lower >= 0) + { + *isKnownNonNegative = true; + } + if (lower > 0) + { + *isKnownNonZero = true; + } + } + } + BitVecOps::Iter iter(apTraits, assertions); unsigned index = 0; while (iter.NextElem(&index)) { + if (*isKnownNonZero && *isKnownNonNegative) + { + // TP: We already have both properties, no need to check assertions. + return; + } + AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); // if treeVN has a bound-check assertion where it's an index, then @@ -4125,6 +4151,39 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, } } } + + if (*isKnownNonZero && *isKnownNonNegative) + { + return; + } + + // Re-use rangecheck for ADD(op, -CNS) to see if op's lower bound can tell us + // whether op is non-negative or non-zero. + // + if (tree->TypeIs(TYP_INT) && tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsIntCnsFitsInI32()) + { + int cns = static_cast(tree->gtGetOp2()->AsIntCon()->IconValue()); + if ((cns < 0) && (cns > INT32_MIN)) + { + ValueNum op1VN = vnStore->VNConservativeNormalValue(tree->gtGetOp1()->gtVNPair); + Range rng = Range(Limit(Limit::keDependent)); + RangeCheck::MergeEdgeAssertions(this, op1VN, ValueNumStore::NoVN, assertions, &rng, false); + + if (rng.LowerLimit().IsConstant()) + { + // Say we have ADD(X, -8) and X is known to be [8..MAX_VALUE] + int lower = rng.LowerLimit().GetConstant(); + if (lower >= -cns) + { + *isKnownNonNegative = true; + } + if (lower > -cns) + { + *isKnownNonZero = true; + } + } + } + } } //------------------------------------------------------------------------ @@ -4851,12 +4910,6 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCas // Skip over a GT_COMMA node(s), if necessary to get to the lcl. GenTree* lcl = op1->gtEffectiveVal(); - // If we don't have a cast of a LCL_VAR then bail. - if (!lcl->OperIs(GT_LCL_VAR)) - { - return nullptr; - } - // Try and see if we can make this cast into a cheaper zero-extending version // if the input is known to be non-negative. if (!cast->IsUnsigned() && genActualTypeIsInt(lcl) && cast->TypeIs(TYP_LONG) && (TARGET_POINTER_SIZE == 8)) @@ -4870,6 +4923,12 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCas } } + // If we don't have a cast of a LCL_VAR then bail. + if (!lcl->OperIs(GT_LCL_VAR)) + { + return nullptr; + } + IntegralRange range = IntegralRange::ForCastInput(cast); AssertionIndex index = optAssertionIsSubrange(lcl, range, assertions); if (index != NO_ASSERTION_INDEX) diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 65ec3a88c77890..203f14ddf36e47 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -251,7 +251,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree* { JITDUMP("Looking for array size assertions for: " FMT_VN "\n", arrLenVn); Range arrLength = Range(Limit(Limit::keDependent)); - MergeEdgeAssertions(arrLenVn, block->bbAssertionIn, &arrLength); + MergeEdgeAssertions(m_pCompiler, arrLenVn, arrLenVn, block->bbAssertionIn, &arrLength); if (arrLength.lLimit.IsConstant()) { arrSize = arrLength.lLimit.GetConstant(); @@ -640,7 +640,8 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP LclSsaVarDsc* ssaData = m_pCompiler->lvaGetDesc(lcl)->GetPerSsaData(lcl->GetSsaNum()); ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair); - MergeEdgeAssertions(normalLclVN, assertions, pRange); + ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->GetArrayLength()->gtVNPair); + MergeEdgeAssertions(m_pCompiler, normalLclVN, arrLenVN, assertions, pRange); } //------------------------------------------------------------------------ @@ -651,9 +652,14 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP // assertions - the assertions to use // pRange - the range to tighten with assertions // -void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP assertions, Range* pRange) +void RangeCheck::MergeEdgeAssertions(Compiler* comp, + ValueNum normalLclVN, + ValueNum preferredBoundVN, + ASSERT_VALARG_TP assertions, + Range* pRange, + bool log) { - if (BitVecOps::IsEmpty(m_pCompiler->apTraits, assertions)) + if (BitVecOps::IsEmpty(comp->apTraits, assertions)) { return; } @@ -664,13 +670,13 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse } // Walk through the "assertions" to check if the apply. - BitVecOps::Iter iter(m_pCompiler->apTraits, assertions); + BitVecOps::Iter iter(comp->apTraits, assertions); unsigned index = 0; while (iter.NextElem(&index)) { AssertionIndex assertionIndex = GetAssertionIndex(index); - Compiler::AssertionDsc* curAssertion = m_pCompiler->optGetAssertion(assertionIndex); + Compiler::AssertionDsc* curAssertion = comp->optGetAssertion(assertionIndex); Limit limit(Limit::keUndef); genTreeOps cmpOper = GT_NONE; @@ -683,7 +689,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse ValueNumStore::CompareCheckedBoundArithInfo info; // Get i, len, cns and < as "info." - m_pCompiler->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info); + comp->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info); // If we don't have the same variable we are comparing against, bail. if (normalLclVN != info.cmpOp) @@ -697,12 +703,12 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse } // If the operand that operates on the bound is not constant, then done. - if (!m_pCompiler->vnStore->IsVNInt32Constant(info.arrOp)) + if (!comp->vnStore->IsVNInt32Constant(info.arrOp)) { continue; } - int cons = m_pCompiler->vnStore->ConstantValue(info.arrOp); + int cons = comp->vnStore->ConstantValue(info.arrOp); limit = Limit(Limit::keBinOpArray, info.vnBound, info.arrOper == GT_SUB ? -cons : cons); cmpOper = (genTreeOps)info.cmpOper; } @@ -712,7 +718,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse ValueNumStore::CompareCheckedBoundArithInfo info; // Get the info as "i", "<" and "len" - m_pCompiler->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info); + comp->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info); // If we don't have the same variable we are comparing against, bail. if (normalLclVN == info.cmpOp) @@ -736,7 +742,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse ValueNumStore::ConstantBoundInfo info; // Get the info as "i", "<" and "100" - m_pCompiler->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info); + comp->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info); // If we don't have the same variable we are comparing against, bail. if (normalLclVN != info.cmpOpVN) @@ -756,10 +762,10 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse continue; } - int cnstLimit = m_pCompiler->vnStore->CoercedConstantValue(curAssertion->op2.vn); + int cnstLimit = comp->vnStore->CoercedConstantValue(curAssertion->op2.vn); if ((cnstLimit == 0) && (curAssertion->assertionKind == Compiler::OAK_NOT_EQUAL) && - m_pCompiler->vnStore->IsVNCheckedBound(curAssertion->op1.vn)) + comp->vnStore->IsVNCheckedBound(curAssertion->op1.vn)) { // we have arr.Len != 0, so the length must be atleast one limit = Limit(Limit::keConstant, 1); @@ -804,31 +810,31 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse // Make sure the assertion is of the form != 0 or == 0 if it isn't a constant assertion. if (!isConstantAssertion && (curAssertion->assertionKind != Compiler::OAK_NO_THROW) && - (curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT))) + (curAssertion->op2.vn != comp->vnStore->VNZeroForType(TYP_INT))) { continue; } #ifdef DEBUG - if (m_pCompiler->verbose) + if (comp->verbose) { - m_pCompiler->optPrintAssertion(curAssertion, assertionIndex); + comp->optPrintAssertion(curAssertion, assertionIndex); } #endif // Limits are sometimes made with the form vn + constant, where vn is a known constant // see if we can simplify this to just a constant - if (limit.IsBinOpArray() && m_pCompiler->vnStore->IsVNInt32Constant(limit.vn)) + if (limit.IsBinOpArray() && comp->vnStore->IsVNInt32Constant(limit.vn)) { - Limit tempLimit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue(limit.vn)); + Limit tempLimit = Limit(Limit::keConstant, comp->vnStore->ConstantValue(limit.vn)); if (tempLimit.AddConstant(limit.cns)) { limit = tempLimit; } } - ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->GetArrayLength()->gtVNPair); + ValueNum arrLenVN = preferredBoundVN; - if (m_pCompiler->vnStore->IsVNConstant(arrLenVN)) + if (comp->vnStore->IsVNConstant(arrLenVN)) { // Set arrLenVN to NoVN; this will make it match the "vn" recorded on // constant limits (where we explicitly track the constant and don't @@ -917,7 +923,10 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse if (limit.vn != arrLenVN) { - JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn); + if (log) + { + JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn); + } continue; } @@ -927,7 +936,10 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse // Incoming limit doesn't tighten the existing upper limit. if (limCns >= curCns) { - JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns); + if (log) + { + JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns); + } continue; } } @@ -967,9 +979,13 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse // All other 'cmpOper' kinds leave lLimit/uLimit unchanged break; } - JITDUMP("The range after edge merging:"); - JITDUMP(pRange->ToString(m_pCompiler)); - JITDUMP("\n"); + + if (log) + { + JITDUMP("The range after edge merging:"); + JITDUMP(pRange->ToString(comp)); + JITDUMP("\n"); + } } } diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index de44b88d3ae52d..847c96d319dd1a 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/src/coreclr/jit/rangecheck.h @@ -686,7 +686,12 @@ class RangeCheck void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange); // Inspect the assertions about the current ValueNum to refine pRange - void MergeEdgeAssertions(ValueNum num, ASSERT_VALARG_TP assertions, Range* pRange); + static void MergeEdgeAssertions(Compiler* comp, + ValueNum num, + ValueNum preferredBoundVN, + ASSERT_VALARG_TP assertions, + Range* pRange, + bool log = true); // The maximum possible value of the given "limit". If such a value could not be determined // return "false". For example: CORINFO_Array_MaxLength for array length. From 597e2ad428afcf04d1a692ca90c6efe3688442f3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 21 Feb 2025 20:17:14 +0100 Subject: [PATCH 2/5] Fix CI --- src/coreclr/jit/assertionprop.cpp | 68 ++++++++++++++++--------------- src/coreclr/jit/rangecheck.cpp | 8 +++- 2 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 4c1f890c775500..936d73a84cd2be 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4032,24 +4032,6 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, const ValueNum treeVN = vnStore->VNConservativeNormalValue(tree->gtVNPair); - if (tree->TypeIs(TYP_INT)) - { - Range rng = Range(Limit(Limit::keDependent)); - RangeCheck::MergeEdgeAssertions(this, treeVN, ValueNumStore::NoVN, assertions, &rng, false); - if (rng.LowerLimit().IsConstant()) - { - int lower = rng.LowerLimit().GetConstant(); - if (lower >= 0) - { - *isKnownNonNegative = true; - } - if (lower > 0) - { - *isKnownNonZero = true; - } - } - } - BitVecOps::Iter iter(apTraits, assertions); unsigned index = 0; while (iter.NextElem(&index)) @@ -4157,27 +4139,47 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, return; } - // Re-use rangecheck for ADD(op, -CNS) to see if op's lower bound can tell us - // whether op is non-negative or non-zero. - // - if (tree->TypeIs(TYP_INT) && tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsIntCnsFitsInI32()) + // Let's see if MergeEdgeAssertions can help us: + if (tree->TypeIs(TYP_INT)) { - int cns = static_cast(tree->gtGetOp2()->AsIntCon()->IconValue()); - if ((cns < 0) && (cns > INT32_MIN)) - { - ValueNum op1VN = vnStore->VNConservativeNormalValue(tree->gtGetOp1()->gtVNPair); - Range rng = Range(Limit(Limit::keDependent)); - RangeCheck::MergeEdgeAssertions(this, op1VN, ValueNumStore::NoVN, assertions, &rng, false); + auto getLowerBound = [&](ValueNum vn) -> Limit { + Range rng = Range(Limit(Limit::keDependent)); + RangeCheck::MergeEdgeAssertions(this, vn, ValueNumStore::NoVN, assertions, &rng, false); + return rng.LowerLimit(); + }; - if (rng.LowerLimit().IsConstant()) + // See if (X + CNS) is known to be non-negative + if (tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsIntCnsFitsInI32()) + { + int cns = static_cast(tree->gtGetOp2()->AsIntCon()->IconValue()); + if ((cns < 0) && (cns > INT32_MIN)) + { + Limit lowerBound = getLowerBound(vnStore->VNConservativeNormalValue(tree->gtGetOp1()->gtVNPair)); + if (lowerBound.IsConstant()) + { + // Say we have ADD(X, -8) and X is known to be [8..MAX_VALUE] + int lower = lowerBound.GetConstant(); + if (lower >= -cns) + { + *isKnownNonNegative = true; + } + if (lower > -cns) + { + *isKnownNonZero = true; + } + } + } + } + else + { + Limit lowerBound = getLowerBound(treeVN); + if (lowerBound.IsConstant()) { - // Say we have ADD(X, -8) and X is known to be [8..MAX_VALUE] - int lower = rng.LowerLimit().GetConstant(); - if (lower >= -cns) + if (lowerBound.GetConstant() >= 0) { *isKnownNonNegative = true; } - if (lower > -cns) + if (lowerBound.GetConstant() > 0) { *isKnownNonZero = true; } diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 203f14ddf36e47..230f35f27dc323 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -966,8 +966,12 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, case GT_GT: case GT_GE: - pRange->lLimit = limit; - // it doesn't matter if it's isUnsigned or not here - it's not negative anyway. + // GT/GE being unsigned creates a non-contiguous range which we can't represent + // using single Range object. + if (!isUnsigned) + { + pRange->lLimit = limit; + } break; case GT_EQ: From 30104168270cfcf4b530c15a850f5f0881f872ef Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 21 Feb 2025 20:30:36 +0100 Subject: [PATCH 3/5] clean up --- src/coreclr/jit/assertionprop.cpp | 9 +-------- src/coreclr/jit/rangecheck.cpp | 8 +++++--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 936d73a84cd2be..c334dd5be457ff 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4030,18 +4030,11 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, return; } - const ValueNum treeVN = vnStore->VNConservativeNormalValue(tree->gtVNPair); - + const ValueNum treeVN = vnStore->VNConservativeNormalValue(tree->gtVNPair); BitVecOps::Iter iter(apTraits, assertions); unsigned index = 0; while (iter.NextElem(&index)) { - if (*isKnownNonZero && *isKnownNonNegative) - { - // TP: We already have both properties, no need to check assertions. - return; - } - AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); // if treeVN has a bound-check assertion where it's an index, then diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 230f35f27dc323..56003a3f543077 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -648,9 +648,11 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP // MergeEdgeAssertions: Merge assertions on the edge flowing into the block about a variable // // Arguments: -// normalLclVN - the value number to look for assertions for -// assertions - the assertions to use -// pRange - the range to tighten with assertions +// comp - the compiler instance +// normalLclVN - the value number to look for assertions for +// preferredBoundVN - when this VN is set, it will be given preference over constant limits +// assertions - the assertions to use +// pRange - the range to tighten with assertions // void RangeCheck::MergeEdgeAssertions(Compiler* comp, ValueNum normalLclVN, From c239189714775cea7263e823988d4a5075508cc6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 21 Feb 2025 21:02:55 +0100 Subject: [PATCH 4/5] Clean up --- src/coreclr/jit/assertionprop.cpp | 44 +++++++++++++++++-------------- src/coreclr/jit/rangecheck.cpp | 2 +- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index c334dd5be457ff..1536404680cb29 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4135,37 +4135,41 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, // Let's see if MergeEdgeAssertions can help us: if (tree->TypeIs(TYP_INT)) { - auto getLowerBound = [&](ValueNum vn) -> Limit { - Range rng = Range(Limit(Limit::keDependent)); - RangeCheck::MergeEdgeAssertions(this, vn, ValueNumStore::NoVN, assertions, &rng, false); - return rng.LowerLimit(); - }; - // See if (X + CNS) is known to be non-negative if (tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsIntCnsFitsInI32()) { + Range rng = Range(Limit(Limit::keDependent)); + ValueNum vn = vnStore->VNConservativeNormalValue(tree->gtGetOp1()->gtVNPair); + RangeCheck::MergeEdgeAssertions(this, vn, ValueNumStore::NoVN, assertions, &rng, false); + int cns = static_cast(tree->gtGetOp2()->AsIntCon()->IconValue()); - if ((cns < 0) && (cns > INT32_MIN)) + rng.LowerLimit().AddConstant(cns); + rng.UpperLimit().AddConstant(cns); + + if (rng.LowerLimit().IsConstant()) { - Limit lowerBound = getLowerBound(vnStore->VNConservativeNormalValue(tree->gtGetOp1()->gtVNPair)); - if (lowerBound.IsConstant()) + // E.g. "X + -8" when X's range is [8..unknown] + // it's safe to say "X + -8" is non-negative + if ((rng.LowerLimit().GetConstant() == 0)) { - // Say we have ADD(X, -8) and X is known to be [8..MAX_VALUE] - int lower = lowerBound.GetConstant(); - if (lower >= -cns) - { - *isKnownNonNegative = true; - } - if (lower > -cns) - { - *isKnownNonZero = true; - } + *isKnownNonNegative = true; + } + + // E.g. "X + 8" when X's range is [0..CNS] + // Here we have to check the upper bound as well to avoid overflow + if ((rng.LowerLimit().GetConstant() > 0) && rng.UpperLimit().IsConstant() && + rng.UpperLimit().GetConstant() > rng.LowerLimit().GetConstant()) + { + *isKnownNonNegative = true; + *isKnownNonZero = true; } } } else { - Limit lowerBound = getLowerBound(treeVN); + Range rng = Range(Limit(Limit::keDependent)); + RangeCheck::MergeEdgeAssertions(this, treeVN, ValueNumStore::NoVN, assertions, &rng, false); + Limit lowerBound = rng.LowerLimit(); if (lowerBound.IsConstant()) { if (lowerBound.GetConstant() >= 0) diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 56003a3f543077..608aa4a27b03f5 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -671,7 +671,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp, return; } - // Walk through the "assertions" to check if the apply. + // Walk through the "assertions" to check if they apply. BitVecOps::Iter iter(comp->apTraits, assertions); unsigned index = 0; while (iter.NextElem(&index)) From 08338d5b65086ebd88a2a2f5a3615d4325c0b011 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 22 Feb 2025 11:49:13 +0100 Subject: [PATCH 5/5] Check overflow --- src/coreclr/jit/assertionprop.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 1536404680cb29..57c26dd1ff2603 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4144,7 +4144,13 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, int cns = static_cast(tree->gtGetOp2()->AsIntCon()->IconValue()); rng.LowerLimit().AddConstant(cns); - rng.UpperLimit().AddConstant(cns); + + if ((rng.LowerLimit().IsConstant() && !rng.LowerLimit().AddConstant(cns)) || + (rng.UpperLimit().IsConstant() && !rng.UpperLimit().AddConstant(cns))) + { + // Add cns to both bounds if they are constants. Make sure the addition doesn't overflow. + return; + } if (rng.LowerLimit().IsConstant()) {