JIT: Elide BOUNDS_CHECK(0, length) when length != 0 is proven by assertion

[Copilot]

BOUNDS_CHECK(0, length) checks (uint)0 < (uint)length, which is semantically equivalent to length != 0. However, the JIT failed to eliminate this check even when a dominating length != 0 assertion was live — a common pattern after null/empty guards on spans and arrays.

Root cause

optAssertionProp_BndsChk computes constant ranges via GetRangeFromAssertions. That function's OAK_NOT_EQUAL handler can only tighten a range when the excluded constant matches the current lower or upper bound. Starting from [INT32_MIN, INT32_MAX], a length != 0 assertion never narrows the range, so lenLo stays at INT32_MIN and the idxHi < lenLo check always fails.

Fix

• src/coreclr/jit/assertionprop.cpp — After GetRangeFromAssertions, when the index is known to be exactly 0 and lenLo <= 0, scan live assertions for an OAK_NOT_EQUAL on the length VN against constant 0. If found, tighten lenLo to 1. GT_BOUNDS_CHECK guarantees the length operand is non-negative, so length != 0 ⟹ length >= 1, making idxHi=0 < lenLo=1 hold and the check drops.

• src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs — Add TryStripFirstChar disasm check test case (repro from Bounds check around Span.IsEmpty is not elided #126857) verifying the bounds check is eliminated for the canonical pattern:

static bool TryStripFirstChar(ref ReadOnlySpan<char> span, char value)
{
    if (!span.IsEmpty && span[0] == value)
    {
        return true;
    }
    return false;
}

Generated asm confirms no CORINFO_HELP_RNGCHKFAIL call — just a zero-check on the length followed by a direct element access.

Original prompt

[Chronological Review] 1. User reported redundant `tbnz` instructions on ARM64 in profiler codegen, asked for emitter-level peephole fix 2. Explored emitter infrastructure: `emitLastIns`, `emitCanPeepholeLastIns()`, existing peepholes like `IsRedundantMov`, `IsRedundantLdStr`, `OptimizeLdrStr` 3. Found emission functions: `emitIns_J_R` (cbz/cbnz), `emitIns_J_R_I` (tbz/tbnz), `emitIns_J` (b.cond) 4. Implemented emitter peephole `IsRedundantBranch` in emitarm64.cpp/h - build clean, SPMI clean (14707 contexts) 5. User asked to revert emitter changes and investigate SPMI diff file to understand WHY the redundant instruction appears 6. Analyzed diff_summary.6.md - found clear pattern: `cbz wN, IG_normalPath` followed by redundant `cbz wN, IG_throwHelper` 7. Investigated `optRedundantBranch` in redundantbranchopts.cpp - only handles GT_JTRUE, not GT_JCMP/GT_JTEST 8. Discovered root cause: GT_BOUNDS_CHECK stays as a node through all optimizer phases, only expanded to cbz during codegen 9. Analyzed assertionprop.cpp and rangecheck.cpp to understand why bounds check isn't eliminated 10. User asked to bring back emitter changes minimally - started implementing 11. User changed mind: asked to implement assertion-level fix instead (options 1 and 2) 12. Implemented fix in assertionprop.cpp (optAssertionProp_BndsChk) and rangecheck.cpp (MergeEdgeAssertions with IsVNCheckedBound) 13. User correctly pointed out: IsVNCheckedBound alone doesn't prove non-negativity - the rangecheck.cpp change is invalid 14. Reverted both changes, reconsidered the problem 15. Key insight: BOUNDS_CHECK(0, length) checks `(uint)0 < (uint)length` ≡ `length != 0`. So `length != 0` assertion directly proves bounds check won't throw, regardless of sign 16. Implemented correct fix: single change in assertionprop.cpp scanning for OAK_NOT_EQUAL assertion on length VN with constant 0 17. Build clean, SPMI x64 clean (14707 contexts)

[Intent Mapping]

• Initial: "remove redundant tbnz in JIT emitter using emitLastIns infrastructure, arm64 only"
• Pivot 1: "revert emitter changes, investigate SPMI diffs to understand root cause"
• Pivot 2: "bring back emitter changes, keep minimal"
• Pivot 3: "implement assertion-level fix in optAssertionProp_BndsChk and MergeEdgeAssertions"
• Pivot 4: "IsVNCheckedBound is invalid reasoning, fix it correctly"
• Final: correct single-file fix in assertionprop.cpp

[Technical Inventory]

• dotnet/runtime JIT (RyuJIT) at C:\prj\runtime-main2
• ARM64 emitter: emitarm64.cpp/h, emit.h, emit.cpp
• Optimizer: assertionprop.cpp, rangecheck.cpp, redundantbranchopts.cpp, optimizebools.cpp
• Phase ordering: optRedundantBranches → optAssertionPropMain → rangeCheckPhase → ... → Lowering (GT_JTRUE→GT_JCMP) → Codegen (GT_BOUNDS_CHECK→cbz)
• Key types: instrDesc, instrDescJmp, AssertionDsc, Range, Limit
• Key functions: emitCanPeepholeLastIns(), optAssertionProp_BndsChk(), MergeEdgeAssertions(), GetRangeFromAssertions()
• SPMI collections: 5fad64ab for linux.arm64 and windows.x64

[Code Archaeology]

• emitarm64.cpp: emitIns_J_R (cbz/cbnz @9163), emitIns_J_R_I (tbz/tbnz @9201), emitIns_J (b.cond @9242)
• assertionprop.cpp: optAssertionProp_BndsChk (@5177-5345) - THE file being modified
• rangecheck.cpp: MergeEdgeAssertions (@973-1230), GetRangeFromAssertions (@657-876)
• redundantbranchopts.cpp: optRedundantBranch (@1044) - only handles GT_JTRUE
• codegenarmarch.cpp: genRangeCheck (@1196) - where GT_BOUNDS_CHECK becomes cbz at codegen time
• lower.cpp: LowerJTrue (@4747) - where GT_JTRUE becomes GT_JCMP/GT_JTEST

[Progress Assessment]

• COMPLETED: Root cause analysis
• COMPLETED: Correct fix implementation in assertionprop.cpp
• COMPLETED: Build verification (0 warnings, 0 errors)
• COMPLETED: SPMI x64 replay (14707 contexts clean)
• NOT YET DONE: User hasn't confirmed the final fix is acceptable

[Context Validation]
All critical context captured: the diff, the reasoning, the incorrect attempt, and the correct fix.

[Recent Commands Analysis]
Most recent operations:

1. git checkout -- src/coreclr/jit/rangecheck.cpp src/coreclr/jit/assertionprop.cpp - reverted invalid changes
2. Viewed assertionprop.cpp around optAssertionProp_BndsChk to plan correct fix
3. edit on assertionprop.cpp - added new bounds check elimination for index==0 with length!=0 assertion
4. build.cmd Clr.AllJits -c Checked - built successfully (0 warnings, 0 errors)
5. SPMI x64 replay - 14707 contexts clean
6. git diff - showed final single-file change

1. Conversation Overview: - Primary Objectives: User initially asked to remove redundant conditional branch instructions (tbnz/tbz/cbz/cbnz) on ARM64 using emitter peephole. After investigation, pivoted to fixing the root cause in assertion propagation to eliminate redundant bounds checks that produce the duplicate branches. - Session Context: Started with emitter-level fix → investigated SPMI diffs to find root cause → discovered bounds checks survive through optimizer a...

Created from Copilot CLI via the copilot delegate command.

[EgorBo]

@copilot please change the title & description. This PR is not arm64 specific. It closes #126857 issue. Please add 1 FILECHECK test to ElidedBoundsChecks.cs with the repro from #126857

[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 improves CoreCLR JIT range-check elimination by recognizing that a dominating length != 0 assertion is sufficient to prove BOUNDS_CHECK(0, length) cannot throw, enabling removal of redundant bounds checks in common span/array guard patterns.

Changes:

• Update optAssertionProp_BndsChk to treat a live lenVN != 0 assertion as implying lenLo >= 1 (given non-negative length), allowing index==0 bounds checks to be elided.
• Add a JIT regression test covering the canonical if (span.IsEmpty) return ...; return span[0]; pattern and assert no CORINFO_HELP_RNGCHKFAIL appears in x64/arm64 asm.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/jit/assertionprop.cpp	Tightens inferred length lower bound to enable dropping BOUNDS_CHECK(0, len) when len != 0 is asserted.
src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs	Adds a regression case validating bounds-check elision after an IsEmpty guard.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

@copilot please change the title & description. This PR is not arm64 specific. It closes #126857 issue. Please add 1 FILECHECK test to ElidedBou...

Updated the PR description to remove ARM64-specific language and added Fixes #126857. Replaced the test with the exact repro from the issue (TryStripFirstChar with !span.IsEmpty && span[0] == value pattern). JIT disasm confirms no CORINFO_HELP_RNGCHKFAIL in the output. See commit 6685332d.

[EgorBo]

PTAL @dotnet/jit-contrib trivial change with diffs