Skip to content

Commit 1ce416b

Browse files
EgorBoCopilot
andcommitted
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>
1 parent ecaa1c5 commit 1ce416b

3 files changed

Lines changed: 74 additions & 18 deletions

File tree

src/coreclr/jit/compiler.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8976,13 +8976,14 @@ class Compiler
89768976
assert(op1VN != ValueNumStore::NoVN);
89778977
assert(checkedBndVN != ValueNumStore::NoVN);
89788978

8979-
AssertionDsc dsc = CreateEmptyAssertion(comp);
8980-
dsc.m_assertionKind = FromVNFunc(relop);
8981-
dsc.m_op1.m_kind = O1K_VN;
8982-
dsc.m_op1.m_vn = op1VN;
8983-
dsc.m_op2.m_kind = O2K_VN_ADD_CNS;
8984-
dsc.m_op2.m_vn = checkedBndVN;
8985-
dsc.m_op2.m_icon.m_iconVal = cns;
8979+
AssertionDsc dsc = CreateEmptyAssertion(comp);
8980+
dsc.m_assertionKind = FromVNFunc(relop);
8981+
dsc.m_op1.m_kind = O1K_VN;
8982+
dsc.m_op1.m_vn = op1VN;
8983+
dsc.m_op2.m_kind = O2K_VN_ADD_CNS;
8984+
dsc.m_op2.m_vn = checkedBndVN;
8985+
dsc.m_op2.m_icon.m_iconVal = cns;
8986+
dsc.m_op2.m_isVNNeverNegative = comp->vnStore->IsVNNeverNegative(checkedBndVN);
89868987
return dsc;
89878988
}
89888989

@@ -9021,7 +9022,7 @@ class Compiler
90219022
dsc.m_op2.m_kind = O2K_VN_ADD_CNS;
90229023
dsc.m_op2.m_vn = op2VN;
90239024
dsc.m_op2.m_icon.m_iconVal = 0; // TODO-CQ: consider decomposing VN to VN* + CNS if beneficial.
9024-
dsc.m_op2.m_isVNNeverNegative = false;
9025+
dsc.m_op2.m_isVNNeverNegative = comp->vnStore->IsVNNeverNegative(op2VN);
90259026
return dsc;
90269027
}
90279028
};

src/coreclr/jit/rangecheck.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,7 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
770770
case VNF_RSH:
771771
case VNF_RSZ:
772772
case VNF_UMOD:
773+
case VNF_UDIV:
773774
{
774775
// Get ranges of both operands and perform the same operation on the ranges.
775776
Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.GetArg(0), assertions, --budget, visited);
@@ -804,6 +805,9 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
804805
case VNF_UMOD:
805806
binOpResult = RangeOps::UnsignedMod(r1, r2);
806807
break;
808+
case VNF_UDIV:
809+
binOpResult = RangeOps::UnsignedDivide(r1, r2);
810+
break;
807811
default:
808812
unreached();
809813
}
@@ -1272,7 +1276,13 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler* comp
12721276
// Example: "(uint)normalLclVN < span.Length" means normalLclVN's range is [0..INT32_MAX-1]
12731277
// Example: "(uint)normalLclVN <= array.Length" means normalLclVN's range is [0..Array.MaxLength]
12741278
//
1275-
else if (!canUseCheckedBounds && curAssertion.IsRelop() && (curAssertion.GetOp1().GetVN() == normalLclVN) &&
1279+
// This is restricted to upper-bound (LT/LE) relops: we substitute op2 with its upper bound
1280+
// (maxValue), which is only valid when op2 bounds normalLclVN from above. GT/GE relops (where
1281+
// op2 is a lower bound) would need op2's lower bound instead, so they fall through to the
1282+
// general "X <relop> Y" branch below.
1283+
else if (!canUseCheckedBounds &&
1284+
curAssertion.KindIs(Compiler::OAK_LT, Compiler::OAK_LE, Compiler::OAK_LT_UN, Compiler::OAK_LE_UN) &&
1285+
(curAssertion.GetOp1().GetVN() == normalLclVN) &&
12761286
(curAssertion.GetOp2().KindIs(Compiler::O2K_VN_ADD_CNS)) &&
12771287
(curAssertion.GetOp2().IsVNNeverNegative()) && (curAssertion.GetOp2().GetCns() == 0))
12781288
{

src/coreclr/jit/rangecheck.h

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -443,19 +443,39 @@ struct RangeOps
443443

444444
static Range Or(const Range& r1, const Range& r2)
445445
{
446-
// For OR we require both operands to be constant to produce a constant result.
447-
// No useful information can be derived if only one operand is constant.
446+
// For OR we require both operands to be non-negative constant ranges.
448447
//
449448
// Example: [0..3] | [1..255] = [1..255]
450449
// [X..Y] | [1..255] = [unknown..unknown]
451450
//
452-
return ApplyRangeOp(r1, r2, [](const Limit& a, const Limit& b) {
453-
if (a.IsConstant() && b.IsConstant() && (a.GetConstant() >= 0) && (b.GetConstant() >= 0))
454-
{
455-
return Limit(Limit::keConstant, a.GetConstant() | b.GetConstant());
456-
}
457-
return Limit(Limit::keUnknown);
458-
});
451+
if (!r1.IsConstantRange() || !r2.IsConstantRange())
452+
{
453+
return Range(Limit(Limit::keUnknown));
454+
}
455+
456+
const int r1lo = r1.LowerLimit().GetConstant();
457+
const int r1hi = r1.UpperLimit().GetConstant();
458+
const int r2lo = r2.LowerLimit().GetConstant();
459+
const int r2hi = r2.UpperLimit().GetConstant();
460+
461+
if ((r1lo < 0) || (r2lo < 0))
462+
{
463+
return Range(Limit(Limit::keUnknown));
464+
}
465+
466+
// a | b >= max(a, b), so max(r1lo, r2lo) is a sound lower bound.
467+
const int lo = max(r1lo, r2lo);
468+
469+
// a | b cannot set any bit above the most-significant set bit of max(r1hi, r2hi), so it is
470+
// bounded above by that value with all lower bits filled in (smallest 2^k - 1 >= the max).
471+
int hi = max(r1hi, r2hi);
472+
hi |= hi >> 1;
473+
hi |= hi >> 2;
474+
hi |= hi >> 4;
475+
hi |= hi >> 8;
476+
hi |= hi >> 16;
477+
478+
return Range(Limit(Limit::keConstant, lo), Limit(Limit::keConstant, hi));
459479
}
460480

461481
static Range And(const Range& r1, const Range& r2)
@@ -501,6 +521,31 @@ struct RangeOps
501521
return Range(Limit(Limit::keUnknown));
502522
}
503523

524+
static Range UnsignedDivide(const Range& r1, const Range& r2)
525+
{
526+
// We only handle constant ranges for both operands.
527+
if (!r1.IsConstantRange() || !r2.IsConstantRange())
528+
{
529+
return Range(Limit(Limit::keUnknown));
530+
}
531+
532+
const int numLo = r1.LowerLimit().GetConstant();
533+
const int numHi = r1.UpperLimit().GetConstant();
534+
const int divLo = r2.LowerLimit().GetConstant();
535+
const int divHi = r2.UpperLimit().GetConstant();
536+
537+
// Require a non-negative dividend and a strictly-positive divisor so that the signed
538+
// [lo..hi] bounds coincide with their unsigned interpretation (and no division by zero).
539+
if ((numLo < 0) || (divLo <= 0))
540+
{
541+
return Range(Limit(Limit::keUnknown));
542+
}
543+
544+
// (uint)x / (uint)y is maximized by the largest dividend over the smallest divisor and
545+
// minimized by the smallest dividend over the largest divisor.
546+
return Range(Limit(Limit::keConstant, numLo / divHi), Limit(Limit::keConstant, numHi / divLo));
547+
}
548+
504549
// Given two ranges "r1" and "r2", do a Phi merge. If "monIncreasing" is true,
505550
// then ignore the dependent variables for the lower bound but not for the upper bound.
506551
static Range Merge(const Range& r1, const Range& r2, bool monIncreasing)

0 commit comments

Comments
 (0)