Loop condition vs initial guard check impacts bounds checking

[stephentoub]

Consider these two functionally-identical loops:

    public static int M1(int i, ReadOnlySpan<char> src)
    {    
        int sum = 0;
        while ((uint)i < (uint)src.Length)
        {
            sum += src[i++];
        }
        return sum;
    }
    
    public static int M2(int i, ReadOnlySpan<char> src)
    {    
        int sum = 0;
        while (true)
        {
            if ((uint)i >= (uint)src.Length) break;

            sum += src[i++];
        }
        return sum;
    }

The second simply moves the loop condition to be the very first thing in the body. SharpLab even decompiles them to C# identically. However, the former has bounds checks whereas they're appropriately removed for the latter:
SharpLab

C.M1(Int32, System.ReadOnlySpan`1<Char>)
    L0000: sub rsp, 0x28
    L0004: mov rax, [rdx]
    L0007: mov edx, [rdx+8]
    L000a: xor r8d, r8d
    L000d: cmp ecx, edx
    L000f: jae short L0039
    L0011: nop [rax]
    L0018: nop [rax+rax]
    L0020: lea r9d, [rcx+1]
    L0024: cmp ecx, edx
    L0026: jae short L0041
    L0028: mov ecx, ecx
    L002a: movzx ecx, word ptr [rax+rcx*2]
    L002e: add r8d, ecx
    L0031: cmp r9d, edx
    L0034: mov ecx, r9d
    L0037: jb short L0020
    L0039: mov eax, r8d
    L003c: add rsp, 0x28
    L0040: ret
    L0041: call 0x00007fff38258b30
    L0046: int3

C.M2(Int32, System.ReadOnlySpan`1<Char>)
    L0000: mov rax, [rdx]
    L0003: mov edx, [rdx+8]
    L0006: xor r8d, r8d
    L0009: cmp ecx, edx
    L000b: jae short L001f
    L000d: lea r9d, [rcx+1]
    L0011: mov ecx, ecx
    L0013: movzx ecx, word ptr [rax+rcx*2]
    L0017: add r8d, ecx
    L001a: mov ecx, r9d
    L001d: jmp short L0009
    L001f: mov eax, r8d
    L0022: ret

[ghost]

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

Issue Details

---

Consider these two functionally-identical loops:

    public static int M1(int i, ReadOnlySpan<char> src)
    {    
        int sum = 0;
        while ((uint)i < (uint)src.Length)
        {
            sum += src[i++];
        }
        return sum;
    }
    
    public static int M2(int i, ReadOnlySpan<char> src)
    {    
        int sum = 0;
        while (true)
        {
            if ((uint)i >= (uint)src.Length) break;

            sum += src[i++];
        }
        return sum;
    }

The second simply moves the loop condition to be the very first thing in the body. SharpLab even decompiles them to C# identically. However, the former has bounds checks whereas they're appropriately removed for the latter:
SharpLab

C.M1(Int32, System.ReadOnlySpan`1<Char>)
    L0000: sub rsp, 0x28
    L0004: mov rax, [rdx]
    L0007: mov edx, [rdx+8]
    L000a: xor r8d, r8d
    L000d: cmp ecx, edx
    L000f: jae short L0039
    L0011: nop [rax]
    L0018: nop [rax+rax]
    L0020: lea r9d, [rcx+1]
    L0024: cmp ecx, edx
    L0026: jae short L0041
    L0028: mov ecx, ecx
    L002a: movzx ecx, word ptr [rax+rcx*2]
    L002e: add r8d, ecx
    L0031: cmp r9d, edx
    L0034: mov ecx, r9d
    L0037: jb short L0020
    L0039: mov eax, r8d
    L003c: add rsp, 0x28
    L0040: ret
    L0041: call 0x00007fff38258b30
    L0046: int3

C.M2(Int32, System.ReadOnlySpan`1<Char>)
    L0000: mov rax, [rdx]
    L0003: mov edx, [rdx+8]
    L0006: xor r8d, r8d
    L0009: cmp ecx, edx
    L000b: jae short L001f
    L000d: lea r9d, [rcx+1]
    L0011: mov ecx, ecx
    L0013: movzx ecx, word ptr [rax+rcx*2]
    L0017: add r8d, ecx
    L001a: mov ecx, r9d
    L001d: jmp short L0009
    L001f: mov eax, r8d
    L0022: ret

Author:	stephentoub
Assignees:	-
Labels:	tenet-performance, area-CodeGen-coreclr
Milestone:	8.0.0

[EgorBo]

public static int M1(int i, ReadOnlySpan<char> src)
{
    int sum = 0;
    while (i >= 0 && i < src.Length)
    {
        sum += src[i++];
    }
    return sum;
}

this works 🙂

[stephentoub]

this works 🙂

Sort of... it eliminates the comparison as part of the bounds check, but it still has an extra comparison/branch for the i >= 0.

C.M1(Int32, System.ReadOnlySpan`1<Char>)
    L0000: mov rax, [rdx]
    L0003: mov edx, [rdx+8]
    L0006: xor r8d, r8d
    L0009: jmp short L001b
    L000b: lea r9d, [rcx+1]
    L000f: mov ecx, ecx
    L0011: movzx ecx, word ptr [rax+rcx*2]
    L0015: add r8d, ecx
    L0018: mov ecx, r9d
    L001b: test ecx, ecx
    L001d: jl short L0023
    L001f: cmp ecx, edx
    L0021: jl short L000b
    L0023: mov eax, r8d
    L0026: ret

In contrast, this:

public static int M2(int i, ReadOnlySpan<char> src)
{
    int sum = 0;
    while (true)
    {
        if ((uint)i >= (uint)src.Length) break;
        sum += src[i++];
    }
    return sum;
}

is:

C.M2(Int32, System.ReadOnlySpan`1<Char>)
    L0000: mov rax, [rdx]
    L0003: mov edx, [rdx+8]
    L0006: xor r8d, r8d
    L0009: cmp ecx, edx
    L000b: jae short L001f
    L000d: lea r9d, [rcx+1]
    L0011: mov ecx, ecx
    L0013: movzx ecx, word ptr [rax+rcx*2]
    L0017: add r8d, ecx
    L001a: mov ecx, r9d
    L001d: jmp short L0009
    L001f: mov eax, r8d
    L0022: ret

[EgorBo]

The fix doesn't look to be trivial so moving to 9.0 🙁

[jakobbotsch]

Going to grab this one since #100777 should fix it.