-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JIT doesn't eliminate bounds checks sometimes #11623
Comments
The unsigned compare shouldn't be needed in this case because both the constant and the array length are obviously positive. It's only needed when the index is a variable, basically it helps the JIT figure out that the variable is positive. Ideally the JIT should recognize something like
I think I've seen this issue reported in the past, I'll have to check. Anyway, it doesn't hurt to have an additional example. |
So, something like I don't think that at the time I considered the case of constant indices. Or I did but ignored it because it didn't seem common. I'll try to extend the existing logic and see if there are other hits in the framework to make this worthwhile. |
I thought I'd test a few variants of this pattern:
Interestingly, the span version manages to eliminate more range checks. Also, using unsigned helps with some cases. |
I built the the latest master (30e643e) on macOS and tested with
disasm of G_M58786_IG03:
488B442408 mov rax, bword ptr [rsp+08H]
C60001 mov byte ptr [rax], 1
C6400102 mov byte ptr [rax+1], 2
C6400203 mov byte ptr [rax+2], 3
C6400304 mov byte ptr [rax+3], 4
C6400405 mov byte ptr [rax+4], 5
;; bbWeight=0.50 PerfScore 3.00 vs. G_M58786_IG03:
85C0 test eax, eax
7431 je SHORT G_M58786_IG05
488B7DF0 mov rdi, bword ptr [rbp-10H]
C60701 mov byte ptr [rdi], 1
83F801 cmp eax, 1
7625 jbe SHORT G_M58786_IG05
C6470102 mov byte ptr [rdi+1], 2
83F802 cmp eax, 2
761C jbe SHORT G_M58786_IG05
C6470203 mov byte ptr [rdi+2], 3
83F803 cmp eax, 3
7613 jbe SHORT G_M58786_IG05
C6470304 mov byte ptr [rdi+3], 4
83F804 cmp eax, 4
760A jbe SHORT G_M58786_IG05
C6470405 mov byte ptr [rdi+4], 5
;; bbWeight=0.50 PerfScore 6.12 |
@am11 I was mostly focused on the cases like (5 < destination.Length) without the cast, as I figured it would be more common/natural. I can see about this scenario if you disagree. Unsigned comparisons are currently largely ignored by this area of assertion prop, barring the assertion that leads to the removal of the above checks. |
@nathan-moore, this is a great news. I have updated the table with cases without casts. I have marked two cases with leading asterisk ( |
Reopening this issue to address the remaining cases which aren't handled: We should at least address these two common cases: expression bounds check not eliminated
|
During dotnet/coreclr#21336 I noticed that JIT doesn't eliminate bounds checks in some cases despite the
(uint)
pattern, e.g.:Output:
but if I change the condition a bit - it might help:
if ((uint)destination.Length >= 5)
- remainif ((uint)destination.Length > 4)
- eliminated!if ((uint)destination.Length == 5)
- eliminated!if (5 <= (uint)destination.Length)
- remainIs it a known issue/feature-request?
TieredJit is disabled
.NET Core 3.0.0-preview-27122-01 (CoreCLR 4.6.27121.03, CoreFX 4.7.18.57103), 64bit RyuJIT
category:cq
theme:range-check
skill-level:intermediate
cost:small
The text was updated successfully, but these errors were encountered: