Unnecessary conditional check

[rameel]

There is an unnecessary check generated when slicing a span with s.Slice(0, s.Length - 1).

godbolt

Repro:

public static int Test(ReadOnlySpan<char> s)
{
    while (s.Length != 0)
    {
        if (s[^1] != ' ')
            break;

        s = s.Slice(0, s.Length - 1); // unnecessary check generated
    }
    
    return s.Length;
}

Sample:Test(System.ReadOnlySpan`1[ushort]):int (FullOpts):
       push     rbp
       mov      rbp, rsp
G_M44937_IG02:  ;; offset=0x0004
       test     esi, esi
       je       SHORT G_M44937_IG04
       lea      eax, [rsi-0x01]
       mov      ecx, eax
       cmp      word  ptr [rdi+2*rcx], 32
       jne      SHORT G_M44937_IG04
       cmp      eax, esi             ; unnecessary check
       ja       SHORT G_M44937_IG06  ;
       mov      esi, eax
       jmp      SHORT G_M44937_IG02
G_M44937_IG04:  ;; offset=0x001C
       mov      eax, esi
       pop      rbp
       ret      
G_M44937_IG06:  ;; offset=0x0020
       call     [System.ThrowHelper:ThrowArgumentOutOfRangeException()]
       int3     

[dotnet-policy-service]

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

[huoyaoyuan]

Is this duplicate of #115090? The code snippets are the same.

[rameel]

No, this is not a duplicate.

Yes, the code snippet is the same, but there are actually two separate issues.

#115090 is about a regression where an extra check being generated which does not exist in .NET 9.

And this issue is about an unnecessary bounds check being generated for the line s = s.Slice(0, s.Length - 1).

[gfoidl]

It's not a range check, rather an ArgumentOutOfRangeException which is correct here, as the length of the span could be 0, so slicing to Length - 1 = 0 - 1 = -1 isn't allowd, thus the exception.

[rameel]

The span's length cannot be 0 due to the loop condition s.Length != 0. Additionally, accessing s[^1] in the check s[^1] != ' ' further confirms s.Length is non-zero. Thus, slicing to s.Length - 1 is safe, and the ArgumentOutOfRangeException check is unnecessary.

[huoyaoyuan]

Span is handled differently than array. For array, the range check is introduced by the JIT, and the JIT can decide to not introduce it. For span, the range check is present in IL, and the JIT needs to recognize the asserts and cancel it.

For this issue, the handling of Slice should be a duplicate of #112953.

[rameel]

and the JIT needs to recognize the asserts and cancel it.

Exactly.

Currently, we can assist the JIT with something like:

if ((uint)s.Length - 1 < (uint)s.Length)
    s = s.Slice(0, s.Length - 1);

But this check is redundant, even if we write it manually.

Of course, we could move this condition entirely into the loop condition, which would help in .NET 9. However, in the current build, even this optimization is broken for this case.

godbolt

[JulieLeeMSFT]

CC @dotnet/jit-contrib.