Remove redundant bounds checks around arr[arr.Length - cns] pattern #116105
Conversation
github-actions
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
EgorBo
force-pushed
the
drop-more-bounds-checks
branch
from
7c46126 to
b417da0
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR removes redundant bounds checks around the arr[arr.Length - cns] pattern by tightening the comparison operator and directly eliminating unnecessary checks when certain patterns are detected.
- In rangecheck.cpp the check is converted from a >= to a > comparison to correctly reflect that arr.Length must be strictly greater than the constant.
- In assertionprop.cpp a new lambda (dropBoundsCheck) is introduced to centralize and simplify the removal of redundant checks based on different index patterns.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/jit/rangecheck.cpp | Adjusts the bounds check operator and inline comment to ensure strict inequality. |
| src/coreclr/jit/assertionprop.cpp | Refactors redundant bounds check removal using an inline lambda to enable early returns. |
| if (arrBndsChk != stmt->GetRootNode()) | ||
| { | ||
| // Defer the removal. | ||
| arrBndsChk->gtFlags |= GTF_CHK_INDEX_INBND; |
There was a problem hiding this comment.
this is the pre-existing logic, I didn't change it, just moved up.
We can drop the bounds checks node unconditionally in fact, I might look into it later
|
PTAL @jakobbotsch @dotnet/jit-contrib |
| int indexCns = comp->vnStore->GetConstantInt32(indexVN); | ||
| if (indexCns >= 0) | ||
| { | ||
| cmpOper = GT_GE; | ||
| cmpOper = GT_GT; |
There was a problem hiding this comment.
A small pre-existing issue. E.g. arr[1] means that arr is at least 2 elements long, not 1.
|
/ba-g "NativeAOT timeout" |
Closes #116088
Diffs