From 1ce416be736322ea33c9be46954f7792dd8f8fe1 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sun, 7 Jun 2026 23:15:11 +0200 Subject: [PATCH] Misc improvements for rangecheck - Fix unsound bounds in RangeOps::Or (could exclude reachable values); use max-based lower/upper bounds - Add UDIV constant-range support to GetRangeFromAssertionsWorker - Populate m_isVNNeverNegative on checked-bound / VN-relop assertions (and fix the operand used) - Restrict the never-negative O2K_VN_ADD_CNS range deduction to LT/LE relops (GT/GE were unsound) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/compiler.h | 17 ++++----- src/coreclr/jit/rangecheck.cpp | 12 ++++++- src/coreclr/jit/rangecheck.h | 63 +++++++++++++++++++++++++++++----- 3 files changed, 74 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b25ce125243fa1..070fb14f53d50d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8976,13 +8976,14 @@ class Compiler assert(op1VN != ValueNumStore::NoVN); assert(checkedBndVN != ValueNumStore::NoVN); - AssertionDsc dsc = CreateEmptyAssertion(comp); - dsc.m_assertionKind = FromVNFunc(relop); - dsc.m_op1.m_kind = O1K_VN; - dsc.m_op1.m_vn = op1VN; - dsc.m_op2.m_kind = O2K_VN_ADD_CNS; - dsc.m_op2.m_vn = checkedBndVN; - dsc.m_op2.m_icon.m_iconVal = cns; + AssertionDsc dsc = CreateEmptyAssertion(comp); + dsc.m_assertionKind = FromVNFunc(relop); + dsc.m_op1.m_kind = O1K_VN; + dsc.m_op1.m_vn = op1VN; + dsc.m_op2.m_kind = O2K_VN_ADD_CNS; + dsc.m_op2.m_vn = checkedBndVN; + dsc.m_op2.m_icon.m_iconVal = cns; + dsc.m_op2.m_isVNNeverNegative = comp->vnStore->IsVNNeverNegative(checkedBndVN); return dsc; } @@ -9021,7 +9022,7 @@ class Compiler dsc.m_op2.m_kind = O2K_VN_ADD_CNS; dsc.m_op2.m_vn = op2VN; dsc.m_op2.m_icon.m_iconVal = 0; // TODO-CQ: consider decomposing VN to VN* + CNS if beneficial. - dsc.m_op2.m_isVNNeverNegative = false; + dsc.m_op2.m_isVNNeverNegative = comp->vnStore->IsVNNeverNegative(op2VN); return dsc; } }; diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 868d0ffd1aea52..eb6ee627f0f2fc 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -770,6 +770,7 @@ Range RangeCheck::GetRangeFromAssertionsWorker( case VNF_RSH: case VNF_RSZ: case VNF_UMOD: + case VNF_UDIV: { // Get ranges of both operands and perform the same operation on the ranges. Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.GetArg(0), assertions, --budget, visited); @@ -804,6 +805,9 @@ Range RangeCheck::GetRangeFromAssertionsWorker( case VNF_UMOD: binOpResult = RangeOps::UnsignedMod(r1, r2); break; + case VNF_UDIV: + binOpResult = RangeOps::UnsignedDivide(r1, r2); + break; default: unreached(); } @@ -1272,7 +1276,13 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler* comp // Example: "(uint)normalLclVN < span.Length" means normalLclVN's range is [0..INT32_MAX-1] // Example: "(uint)normalLclVN <= array.Length" means normalLclVN's range is [0..Array.MaxLength] // - else if (!canUseCheckedBounds && curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == normalLclVN) && + // This is restricted to upper-bound (LT/LE) relops: we substitute op2 with its upper bound + // (maxValue), which is only valid when op2 bounds normalLclVN from above. GT/GE relops (where + // op2 is a lower bound) would need op2's lower bound instead, so they fall through to the + // general "X Y" branch below. + else if (!canUseCheckedBounds && + curAssertion.KindIs(Compiler::OAK_LT, Compiler::OAK_LE, Compiler::OAK_LT_UN, Compiler::OAK_LE_UN) && + (curAssertion.GetOp1().GetVN() == normalLclVN) && (curAssertion.GetOp2().KindIs(Compiler::O2K_VN_ADD_CNS)) && (curAssertion.GetOp2().IsVNNeverNegative()) && (curAssertion.GetOp2().GetCns() == 0)) { diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h index c9f189deeb73de..1eb87b68ec9156 100644 --- a/src/coreclr/jit/rangecheck.h +++ b/src/coreclr/jit/rangecheck.h @@ -443,19 +443,39 @@ struct RangeOps static Range Or(const Range& r1, const Range& r2) { - // For OR we require both operands to be constant to produce a constant result. - // No useful information can be derived if only one operand is constant. + // For OR we require both operands to be non-negative constant ranges. // // Example: [0..3] | [1..255] = [1..255] // [X..Y] | [1..255] = [unknown..unknown] // - return ApplyRangeOp(r1, r2, [](const Limit& a, const Limit& b) { - if (a.IsConstant() && b.IsConstant() && (a.GetConstant() >= 0) && (b.GetConstant() >= 0)) - { - return Limit(Limit::keConstant, a.GetConstant() | b.GetConstant()); - } - return Limit(Limit::keUnknown); - }); + if (!r1.IsConstantRange() || !r2.IsConstantRange()) + { + return Range(Limit(Limit::keUnknown)); + } + + const int r1lo = r1.LowerLimit().GetConstant(); + const int r1hi = r1.UpperLimit().GetConstant(); + const int r2lo = r2.LowerLimit().GetConstant(); + const int r2hi = r2.UpperLimit().GetConstant(); + + if ((r1lo < 0) || (r2lo < 0)) + { + return Range(Limit(Limit::keUnknown)); + } + + // a | b >= max(a, b), so max(r1lo, r2lo) is a sound lower bound. + const int lo = max(r1lo, r2lo); + + // a | b cannot set any bit above the most-significant set bit of max(r1hi, r2hi), so it is + // bounded above by that value with all lower bits filled in (smallest 2^k - 1 >= the max). + int hi = max(r1hi, r2hi); + hi |= hi >> 1; + hi |= hi >> 2; + hi |= hi >> 4; + hi |= hi >> 8; + hi |= hi >> 16; + + return Range(Limit(Limit::keConstant, lo), Limit(Limit::keConstant, hi)); } static Range And(const Range& r1, const Range& r2) @@ -501,6 +521,31 @@ struct RangeOps return Range(Limit(Limit::keUnknown)); } + static Range UnsignedDivide(const Range& r1, const Range& r2) + { + // We only handle constant ranges for both operands. + if (!r1.IsConstantRange() || !r2.IsConstantRange()) + { + return Range(Limit(Limit::keUnknown)); + } + + const int numLo = r1.LowerLimit().GetConstant(); + const int numHi = r1.UpperLimit().GetConstant(); + const int divLo = r2.LowerLimit().GetConstant(); + const int divHi = r2.UpperLimit().GetConstant(); + + // Require a non-negative dividend and a strictly-positive divisor so that the signed + // [lo..hi] bounds coincide with their unsigned interpretation (and no division by zero). + if ((numLo < 0) || (divLo <= 0)) + { + return Range(Limit(Limit::keUnknown)); + } + + // (uint)x / (uint)y is maximized by the largest dividend over the smallest divisor and + // minimized by the smallest dividend over the largest divisor. + return Range(Limit(Limit::keConstant, numLo / divHi), Limit(Limit::keConstant, numHi / divLo)); + } + // Given two ranges "r1" and "r2", do a Phi merge. If "monIncreasing" is true, // then ignore the dependent variables for the lower bound but not for the upper bound. static Range Merge(const Range& r1, const Range& r2, bool monIncreasing)