Remove JIT-workaround for eliminating bound check

[gfoidl]

#62864 taught the JIT to recognize more pattern to eliminate bound checks for array and span indexing. Thus the workarounds like the uint-casts aren't needed anymore, and could be undone.

Also cf. #64899 (comment)

category:cq
theme:bounds-checks
skill-level:beginner
cost:small
impact:small

[gfoidl]

I can look into this issue next week.

[EgorBo]

Quick summary:
uint cast is not needed for:

1. array.Length cmp ConstantValue (not sure about cases where cmp is == or !=)
2. array.Length cmp X where X is unsigned
3. X <= array.Length && X >= 0 - where we explicitly state that X is never negative via two expressions
4. (uint)index < (uint)array.Length - uint cast for array.Length is not necessary (but it is necessary in .NET 6.0)

NOTE: array is either Array or [ReadOnly]Span

for the rest cases where we don't tell jit that X is never negative we have to use uint cast for it - and we can't do anything with it 🤷‍♂️

[gfoidl]

When working on this issue, the found patterns should be validated manually that excpected codegen happens. If not an issue should be filed, so it can be addressed.

Thanks for the summary!

[EgorBo]

When working on this issue, the found patterns should be validated manually that excpected codegen happens. If not an issue should be filed, so it can be addressed.

Thanks for the summary!

It's probably simpler to blindly change all the patterns which satisfy the conditions above ^ and run jit-diffs, let me know if you need a step-by-step instruction how to do it

[ghost]

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

---

#62864 taught the JIT to recognize more pattern to eliminate bound checks for array and span indexing. Thus the workarounds like the uint-casts aren't needed anymore, and could be undone.

Also cf. #64899 (comment)

Author:	gfoidl
Assignees:	-
Labels:	area-Meta, untriaged
Milestone:	-

[gfoidl]

Ah, good to know. Will reach out to you when I need some help on the jit-diffs.

[ericstj]

Since it looks like this work was made possible by a change to the JIT in net7.0 be careful that your changes won't impact assemblies used on older frameworks.

[gfoidl]

I'll do it in CoreLib first, then in a "second iteration" on other libs that are .NET 7 only.
Hoping to start tommorrow with this issue.

[gfoidl]

It looks like (uint)idx < span.Length-pattern doesn't elide the bound-check.
Tested with SDK download from today (7.0.100-preview.4.22181.7) which is based on cbf3f9c, so the work should be in according #67044 (comment) (point 4)

C# repro

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkRunner.Run<Bench>();

[DisassemblyDiagnoser]
[ShortRunJob]
public class Bench
{
    private int[] _arr = new int[10];
    private int _idx = 3;
    private int _value = 42;

    [Benchmark]
    public int Length_Cmp_Uint_Read()
    {
        Span<int> span = _arr;
        int idx = _idx;

        if ((uint)idx < span.Length)
        {
            return span[idx];
        }

        return 0;
    }

    [Benchmark]
    public int Length_Uint_Cmp_Uint_Read()
    {
        Span<int> span = _arr;
        int idx = _idx;

        if ((uint)idx < (uint)span.Length)
        {
            return span[idx];
        }

        return 0;
    }

    [Benchmark]
    public void Length_cmp_Uint_Write()
    {
        Span<int> span = _arr;
        int idx = _idx;

        if ((uint)idx < span.Length)
        {
            span[idx] = _value;
        }
    }

    [Benchmark]
    public void Length_Uint_cmp_Uint_Write()
    {
        Span<int> span = _arr;
        int idx = _idx;

        if ((uint)idx < (uint)span.Length)
        {
            span[idx] = _value;
        }
    }
}

machine code (x64)

```assembly
; Bench.Length_Cmp_Uint_Read()
       sub       rsp,28
       mov       rax,[rcx+8]
       test      rax,rax
       je        short M00_L02
       lea       rdx,[rax+10]
       mov       r8d,[rax+8]
M00_L00:
       mov       eax,[rcx+10]
       mov       ecx,eax
       movsxd    r9,r8d
       cmp       rcx,r9
       jge       short M00_L01
       cmp       eax,r8d
       jae       short M00_L03
       mov       eax,eax
       mov       eax,[rdx+rax*4]
       add       rsp,28
       ret
M00_L01:
       xor       eax,eax
       add       rsp,28
       ret
M00_L02:
       xor       edx,edx
       xor       r8d,r8d
       jmp       short M00_L00
M00_L03:
       call      CORINFO_HELP_RNGCHKFAIL
       int       3
; Total bytes of code 69

; Bench.Length_Uint_Cmp_Uint_Read()
       mov       rax,[rcx+8]
       test      rax,rax
       je        short M00_L02
       lea       rdx,[rax+10]
       mov       r8d,[rax+8]
M00_L00:
       mov       eax,[rcx+10]
       cmp       eax,r8d
       jae       short M00_L01
       mov       eax,eax
       mov       eax,[rdx+rax*4]
       ret
M00_L01:
       xor       eax,eax
       ret
M00_L02:
       xor       edx,edx
       xor       r8d,r8d
       jmp       short M00_L00
; Total bytes of code 41
;------------------------------------------------------------------------------
; Bench.Length_cmp_Uint_Write()
       sub       rsp,28
       mov       rax,[rcx+8]
       test      rax,rax
       je        short M00_L02
       lea       rdx,[rax+10]
       mov       eax,[rax+8]
M00_L00:
       mov       r8d,[rcx+10]
       mov       r9d,r8d
       movsxd    r10,eax
       cmp       r9,r10
       jge       short M00_L01
       cmp       r8d,eax
       jae       short M00_L03
       mov       eax,r8d
       mov       ecx,[rcx+14]
       mov       [rdx+rax*4],ecx
M00_L01:
       add       rsp,28
       ret
M00_L02:
       xor       edx,edx
       xor       eax,eax
       jmp       short M00_L00
M00_L03:
       call      CORINFO_HELP_RNGCHKFAIL
       int       3
; Total bytes of code 66

; Bench.Length_Uint_cmp_Uint_Write()
       mov       rax,[rcx+8]
       test      rax,rax
       je        short M00_L02
       lea       rdx,[rax+10]
       mov       eax,[rax+8]
M00_L00:
       mov       r8d,[rcx+10]
       cmp       r8d,eax
       jae       short M00_L01
       mov       ecx,[rcx+14]
       mov       eax,r8d
       mov       [rdx+rax*4],ecx
M00_L01:
       ret
M00_L02:
       xor       edx,edx
       xor       eax,eax
       jmp       short M00_L00
; Total bytes of code 41

With an array instead of span, the bound check gets elided as expected.

[EgorBo]

@gfoidl yes, I forgot to warn you about it, but the fix shouldn't be difficult - I'll file a PR

[ghost]

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

Issue Details

---

#62864 taught the JIT to recognize more pattern to eliminate bound checks for array and span indexing. Thus the workarounds like the uint-casts aren't needed anymore, and could be undone.

Also cf. #64899 (comment)

Author:	gfoidl
Assignees:	-
Labels:	enhancement, up-for-grabs, area-CodeGen-coreclr
Milestone:	Future

[xtqqczze]

Related: #105593