JIT: fold relop on ADD(base, 1) using base-vs-other assertion

[EgorBo]

Fixes #112953

Allows the JIT to remove the redundant Slice bounds check in patterns like:

ReadOnlySpan<char> Test(ReadOnlySpan<char> span, int offset)
{
    if ((uint)offset < (uint)span.Length)
    {
        return span.Slice(offset + 1);
    }
    return span;
}

 G_M56038_IG02:
        mov      rsi, bword ptr [rdx]
        mov      edi, dword ptr [rdx+0x08]
        cmp      r8d, edi
        jae      SHORT G_M56038_IG08
 G_M56038_IG03:
        inc      r8d
-       cmp      r8d, edi
-       ja       SHORT G_M56038_IG09
        mov      ecx, r8d
        lea      rsi, bword ptr [rsi+2*rcx]
        sub      edi, r8d
        jns      SHORT G_M56038_IG05
-G_M56038_IG09:
-       call     [System.ThrowHelper:ThrowArgumentOutOfRangeException()]
-       int3

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR enhances CoreCLR JIT range-check elimination by using existing relational assertions to fold comparisons involving ADD(base, 1) (e.g., enabling removal of redundant ReadOnlySpan<T>.Slice(offset + 1) bounds checks when guarded by (uint)offset < (uint)len).

Changes:

• Add a new JIT regression test covering the Slice(offset + 1) pattern and asserting no remaining range-check-fail helper in the generated code.
• Extend RangeCheck::GetRangeFromAssertionsWorker to fold certain ADD(base, 1) relops to constant true/false when a matching strict < assertion exists (with matching signedness).
• Refine unsigned assertion/range handling: allow some unsigned VN-vs-VN assertion tightening in MergeEdgeAssertionsWorker, and reduce assertion-table pressure by skipping complementary assertions for some unsigned cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs	Adds a regression test for eliding the redundant Slice(offset + 1) bounds check under a (uint)offset < (uint)Length guard.
src/coreclr/jit/rangecheck.cpp	Adds assertion-driven folding for ADD(base, 1) relops and improves unsigned VN-assertion range tightening rules.
src/coreclr/jit/assertionprop.cpp	Expands VN-relop assertion creation to include unsigned int32 relops (with existing pressure gating) and avoids generating low-value unsigned complementary assertions.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[github-actions]

Caution

Security scanning requires review for Code Review

Details

The threat detection results could not be parsed. The workflow output should be reviewed before merging.

Review the workflow run logs for details.

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #128101

Holistic Assessment

Motivation: Justified. The linked issue (#112953) demonstrates a real missed optimization in a common Span.Slice(offset + 1) pattern after a manual bounds check. This affects real-world IndexOf+Slice loops.

Approach: Sound. The PR extends the existing assertion propagation and range-check infrastructure in two complementary ways: (1) creating unsigned relop VN assertions so the JIT records (uint)offset < (uint)len, and (2) adding a targeted fold in RangeCheck that proves ADD(base, 1) <= other from an asserted base < other. The mathematical proof is correct and well-documented.

Summary: ✅ LGTM. The changes are well-scoped, the correctness argument is rigorous, edge cases are properly guarded, and the test validates the motivating pattern. Minor observations below.

---

Detailed Findings

✅ Correctness of TryFoldRelopOfAddByOneFromAssertions

The core proof is correct for both signed and unsigned domains:

• Unsigned: base u< other ⟹ base ≤ UINT32_MAX − 1 ⟹ base + 1 cannot wrap ⟹ base + 1 u≤ other.
• Signed: base < other ⟹ base ≤ INT32_MAX − 1 (since other ≤ INT32_MAX) ⟹ no signed overflow.

The signedness-matching guard (aIsUnsigned != isUnsigned) is essential and correctly prevents cross-domain reasoning. The base == op2VN guard correctly rejects the degenerate ADD(x, 1) vs x case. The restriction to addCns == 1 is conservative but sufficient for the motivating pattern.

✅ Unsigned assertions in MergeEdgeAssertionsWorker

The new unsigned LT/LE handling in MergeEdgeAssertionsWorker is well-guarded:

• Only GT_LT/GT_LE directions are accepted (GT/GE would produce non-contiguous ranges in the signed domain).
• The otherRange.LowerLimit().GetConstant() < 0 check ensures the derived range stays in the non-negative signed domain, which the Range representation requires.

✅ Complementary assertion suppression

Extending the complementary-skip to O2K_VN alongside O2K_CHECKED_BOUND_ADD_CNS for OAK_LT_UN/OAK_LE_UN is correct. Unsigned > / >= complementaries can't tighten signed-int ranges and would waste assertion table slots.

✅ Test coverage

The SliceOffsetPlusOne test covers the core optimization (offset in-range) and the fallthrough (offset out-of-range). The disassembly check (X64-NOT: ThrowArgumentOutOfRangeException) validates that the bounds check is actually elided.

💡 Boundary edge case test

The test doesn't exercise offset == span.Length - 1 (e.g., SliceOffsetPlusOne("hello".AsSpan(), 4) → expected length 0), which is the tightest boundary where offset + 1 == span.Length. This was noted in a prior review comment (now resolved). Not blocking since the proof covers this case, but it would strengthen confidence.

💡 Generalization beyond +1

The fold is hardcoded to addCns == 1. A future generalization to arbitrary positive constants (base + K <= other from base < other when K >= 1 and other >= K) could help patterns like Slice(offset + 2). Not in scope for this PR.

Generated by Code Review for issue #128101 · ● 2.3M · ◷

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.