Eliminate redundant bounds checks for arr[^N-1] after arr[^N]#124571
Eliminate redundant bounds checks for arr[^N-1] after arr[^N]#124571
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Add optimization in optAssertionProp_BndsChk: when a IsBoundsCheckNoThrow assertion exists for VNF_ADD(vnLen, -K1) and the current index is VNF_ADD(vnLen, -K2) with 0 < K2 <= K1, the current bounds check is redundant because the earlier check proved vnLen >= K1 >= K2. This eliminates 3 out of 4 bounds checks in patterns like: source[^4] | source[^3] | source[^2] | source[^1] Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
|
@copilot actually, I think the fix must be in MergeEdgeAssertions. Basically, if we have IsBoundsCheckNoThrow assertion |
Revert the previous approach in optAssertionProp_BndsChk and instead fix range extraction in MergeEdgeAssertions. When a IsBoundsCheckNoThrow assertion has indexVN = VNF_ADD(lenVN, -CNS), decompose it to deduce that lenVN >= CNS. This allows the existing range-based bounds check elimination in optAssertionProp_BndsChk to work correctly. Also remove the test as requested - the existing infrastructure validates correctness. Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Done in 991c696. Moved the fix to |
There was a problem hiding this comment.
Pull request overview
This PR improves CoreCLR JIT bounds-check elimination for index-from-end (^N) access patterns by enhancing range extraction from IsBoundsCheckNoThrow assertions so later assertion propagation can prove subsequent checks redundant.
Changes:
- Extend
RangeCheck::MergeEdgeAssertionsto recognizeVNF_ADD(lenVN, -CNS)index patterns fromIsBoundsCheckNoThrowassertions. - Derive a tighter lower bound for the array/span length VN from those assertions (enabling elimination of later
arr[^N-1]checks afterarr[^N]).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
adamperlin
left a comment
There was a problem hiding this comment.
Not super familiar with this part of the code, but this looks sound logically!
Description
The JIT generates redundant bounds checks for index-from-end patterns like
source[^4] | source[^3] | source[^2] | source[^1]. After provingarr[arr.Length - 4]is in bounds (implyingarr.Length >= 4), the checks forarr[arr.Length - 3],arr[arr.Length - 2], andarr[arr.Length - 1]are provably redundant but were not eliminated.Fixes range extraction in
MergeEdgeAssertionsso thatIsBoundsCheckNoThrowassertions withVNF_ADD(lenVN, -CNS)index patterns properly tighten the lower bound of the array length. This allows the existing range-based bounds check elimination inoptAssertionProp_BndsChkto work correctly for index-from-end patterns.src/coreclr/jit/rangecheck.cpp: InMergeEdgeAssertions, when aIsBoundsCheckNoThrowassertion hasnormalLclVN == lenVNand the index isVNF_ADD(lenVN, -CNS), decompose theVNF_ADDto deduce thatlenVN >= CNS(expressed asGT_GTwith limit-CNS - 1). Previously, this case fell through to the generic "array has at least one element" deduction (lenVN > 0), which was too weak to eliminate subsequent bounds checks.Before (4 bounds checks, 3 redundant):
After (1 bounds check):
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.