Skip to content

Remove JIT-workaround for eliminating bound check #67044

Open
Open
Remove JIT-workaround for eliminating bound check#67044
Assignees
gfoidl
Labels
area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIenhancementProduct code improvement that does NOT require public API changes/additionshelp wanted[up-for-grabs] Good issue for external contributors
Milestone
Future

Description

@gfoidl
gfoidl
opened on Mar 23, 2022

#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

Activity

  1. gfoidl commented on Mar 23, 2022

    @gfoidl
    gfoidl
    on Mar 23, 2022
    MemberAuthor

    I can look into this issue next week.

  2. danmoseley
    added
    area-Meta
    on Mar 23, 2022

  3. EgorBo commented on Mar 23, 2022

    @EgorBo
    EgorBo
    on Mar 23, 2022
    Member

    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 🤷‍♂️

  4. gfoidl commented on Mar 23, 2022

    @gfoidl
    gfoidl
    on Mar 23, 2022
    MemberAuthor

    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!

  5. EgorBo commented on Mar 23, 2022

    @EgorBo
    EgorBo
    on Mar 23, 2022
    Member

    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

  6. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Mar 23, 2022

  8. gfoidl commented on Mar 23, 2022

    @gfoidl
    gfoidl
    on Mar 23, 2022
    MemberAuthor

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

  9. ericstj commented on Mar 29, 2022

    @ericstj
    ericstj
    on Mar 29, 2022
    Member

    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.

  10. ericstj
    added
    help wanted[up-for-grabs] Good issue for external contributors
    and removed
    untriagedNew issue has not been triaged by the area owner
    on Mar 29, 2022
  11. ericstj
    added this to the Future milestone on Mar 29, 2022
  12. ericstj
    added
    enhancementProduct code improvement that does NOT require public API changes/additions
    on Mar 29, 2022

  13. gfoidl commented on Mar 30, 2022

    @gfoidl
    gfoidl
    on Mar 30, 2022
    MemberAuthor

    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.

  14. gfoidl commented on Mar 31, 2022

    @gfoidl
    gfoidl
    on Mar 31, 2022
    MemberAuthor

    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.

  15. EgorBo commented on Apr 1, 2022

    @EgorBo
    EgorBo
    on Apr 1, 2022
    Member

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

  16. jeffhandley
    added
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    and removed
    area-Meta
    on Apr 9, 2022
  18. JulieLeeMSFT
    assigned
    gfoidl
    on Jun 10, 2022

  19. xtqqczze commented on Sep 22, 2025

    @xtqqczze
    xtqqczze
    on Sep 22, 2025
    Contributor

    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)

    Elided in .NET 9: https://csharp.godbolt.org/z/xn6Yjnqbc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIenhancementProduct code improvement that does NOT require public API changes/additionshelp wanted[up-for-grabs] Good issue for external contributors

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions