Skip to content

Commit 81ffe2a

Browse files
EgorBoCopilot
andcommitted
JIT: restore type-based upper bound for RSZ in range analysis
PR #122263 unified RSZ/RSH range computation, dropping the type-based fallback that derives the upper bound from op1's bit-width when r1's range isn't a useful constant. As a result, patterns like `vals[i >>> 31]` against a length-2 span no longer had their bounds check eliminated. Pass op1's bit-width to `RangeOps::ShiftRight` and, for logical shifts: - when `minShift >= 1`, force the lower bound to 0 (high bit cleared), - derive an upper bound of `UMAX(op1Type) >> minShift` and intersect with any bound already derived from r1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent edcc3af commit 81ffe2a

3 files changed

Lines changed: 90 additions & 9 deletions

File tree

src/coreclr/jit/rangecheck.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,6 +1748,8 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool
17481748
r = RangeOps::ShiftRight(op1Range, op2Range, /*logical*/ false);
17491749
break;
17501750
case GT_RSZ:
1751+
// RSZ here is always 32-bit: TYP_LONG is bailed out by GetRangeWorker.
1752+
assert(genActualType(op1) == TYP_INT);
17511753
r = RangeOps::ShiftRight(op1Range, op2Range, /*logical*/ true);
17521754
break;
17531755
case GT_LSH:

src/coreclr/jit/rangecheck.h

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -402,24 +402,45 @@ struct RangeOps
402402
Range result = Limit(Limit::keUnknown);
403403

404404
// For SHR we require the rhs to be a constant range within [0..31].
405-
// We only perform the shift if the lhs is also a constant range (never-negative for now
406-
// to handle both logical and arithmetic shifts uniformly).
407405
if (!r2.IsConstantRange() || (static_cast<unsigned>(r2.LowerLimit().GetConstant()) > 31) ||
408406
(static_cast<unsigned>(r2.UpperLimit().GetConstant()) > 31))
409407
{
410408
return result;
411409
}
412410

413-
// We shift by r2.UpperLimit() for the lower limit and by r2.LowerLimit() for the upper limit.
414-
// Example: [0..65535] >> [0..3] = [0 >> 3 .. 65535 >> 0] = [0..65535]
415-
// [0..65535] >> [2..2] = [0 >> 2 .. 65535 >> 2] = [0..16383]
416-
if (r1.LowerLimit().IsConstant() && (r1.LowerLimit().GetConstant() >= 0))
411+
const int minShift = r2.LowerLimit().GetConstant();
412+
const int maxShift = r2.UpperLimit().GetConstant();
413+
const bool r1KnownNonNegLow = r1.LowerLimit().IsConstant() && (r1.LowerLimit().GetConstant() >= 0);
414+
const bool r1KnownNonNegHigh = r1.UpperLimit().IsConstant() && (r1.UpperLimit().GetConstant() >= 0);
415+
416+
// Shift by maxShift for the lower limit and by minShift for the upper limit.
417+
// Example: [0..65535] >> [0..3] = [0..65535], [0..65535] >> [2..2] = [0..16383]
418+
if (r1KnownNonNegLow)
417419
{
418-
result.lLimit = Limit(Limit::keConstant, r1.LowerLimit().GetConstant() >> r2.UpperLimit().GetConstant());
420+
result.lLimit = Limit(Limit::keConstant, r1.LowerLimit().GetConstant() >> maxShift);
419421
}
420-
if (r1.UpperLimit().IsConstant() && (r1.UpperLimit().GetConstant() >= 0))
422+
else if (logical && (minShift >= 1))
421423
{
422-
result.uLimit = Limit(Limit::keConstant, r1.UpperLimit().GetConstant() >> r2.LowerLimit().GetConstant());
424+
// RSZ by >= 1 always clears the high bit, so result >= 0 regardless of r1's sign.
425+
result.lLimit = Limit(Limit::keConstant, 0);
426+
}
427+
428+
// For RSZ, r1.UpperLimit() is only a sound upper bound when r1 is fully non-negative
429+
// (negative r1 values reinterpret as large unsigned values). RSH preserves it either way.
430+
if (r1KnownNonNegHigh && (!logical || r1KnownNonNegLow))
431+
{
432+
result.uLimit = Limit(Limit::keConstant, r1.UpperLimit().GetConstant() >> minShift);
433+
}
434+
435+
// RSZ always satisfies result <= UINT_MAX >> minShift (RSZ here is always 32-bit: TYP_LONG
436+
// is bailed out by GetRangeWorker, and CIL shifts have no small-typed operands).
437+
if (logical && (minShift >= 1))
438+
{
439+
const int typeBasedUpper = static_cast<int>(UINT32_MAX >> minShift);
440+
if (!result.uLimit.IsConstant() || (result.uLimit.GetConstant() > typeBasedUpper))
441+
{
442+
result.uLimit = Limit(Limit::keConstant, typeBasedUpper);
443+
}
423444
}
424445
return result;
425446
}

src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,37 @@ static bool TryStripFirstChar(ref ReadOnlySpan<char> span, char value)
9595
return false;
9696
}
9797

98+
[MethodImpl(MethodImplOptions.NoInlining)]
99+
static int UnsignedShiftBySignBit(int i)
100+
{
101+
// X64-NOT: CORINFO_HELP_RNGCHKFAIL
102+
// ARM64-NOT: CORINFO_HELP_RNGCHKFAIL
103+
ReadOnlySpan<int> vals = [0, 1];
104+
return vals[i >>> 31];
105+
}
106+
107+
[MethodImpl(MethodImplOptions.NoInlining)]
108+
static int UnsignedShiftByConst(int i)
109+
{
110+
// X64-NOT: CORINFO_HELP_RNGCHKFAIL
111+
// ARM64-NOT: CORINFO_HELP_RNGCHKFAIL
112+
ReadOnlySpan<int> vals = [0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120, 130, 140, 150];
113+
return vals[i >>> 28];
114+
}
115+
116+
[MethodImpl(MethodImplOptions.NoInlining)]
117+
static int UnsignedShiftStraddlingRange(int i)
118+
{
119+
// i is in [-9..99] (straddles zero). For i=-1, (i >>> 5) is 134217727, so the
120+
// bounds check MUST be preserved (no X64-NOT marker).
121+
ReadOnlySpan<int> vals = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
122+
if (i > -10 && i < 100)
123+
{
124+
return vals[i >>> 5];
125+
}
126+
return -1;
127+
}
128+
98129
[Fact]
99130
public static int TestEntryPoint()
100131
{
@@ -139,6 +170,33 @@ public static int TestEntryPoint()
139170
if (TryStripFirstChar(ref chars, 'h') != false)
140171
return 0;
141172

173+
if (UnsignedShiftBySignBit(0) != 0)
174+
return 0;
175+
176+
if (UnsignedShiftBySignBit(-1) != 1)
177+
return 0;
178+
179+
if (UnsignedShiftBySignBit(int.MaxValue) != 0)
180+
return 0;
181+
182+
if (UnsignedShiftByConst(0) != 0)
183+
return 0;
184+
185+
if (UnsignedShiftByConst(-1) != 150)
186+
return 0;
187+
188+
if (UnsignedShiftStraddlingRange(50) != 1)
189+
return 0;
190+
// For i=-1, (i >>> 5) is 134217727; the bounds check must throw, not be elided.
191+
try
192+
{
193+
UnsignedShiftStraddlingRange(-1);
194+
return 0;
195+
}
196+
catch (IndexOutOfRangeException)
197+
{
198+
}
199+
142200
return 100;
143201
}
144202
}

0 commit comments

Comments
 (0)