Consecutive range checks are not combined

[svick]

Description

Consider the following code (inspired by dotnet/csharplang#4082 (comment)):

static int ArrayAccess(int[] abcd)
{
    return abcd[0] + abcd[1] + abcd[2] + abcd[3];
}

Note that it accesses the array multiple times at increasing indexes, which I think is a relatively common pattern. This results in one range check for every array access:

L0000: mov eax, [ecx+4]
L0003: test eax, eax
L0005: je short L0025
L0007: mov edx, [ecx+8]
L000a: cmp eax, 1
L000d: jbe short L0025
L000f: add edx, [ecx+0xc]
L0012: cmp eax, 2
L0015: jbe short L0025
L0017: add edx, [ecx+0x10]
L001a: cmp eax, 3
L001d: jbe short L0025
L001f: mov eax, edx
L0021: add eax, [ecx+0x14]
L0024: ret
L0025: call 0x71efa060
L002a: int3

This is annoying, because only the check for abcd[3] is necessary, since there are no side-effects between the array accesses. Even though these range checks are likely to be well predicted by the CPU, I think it would be nice if they could be elided by the JIT.

This may very well be a duplicate, but I couldn't find it. Or it may not be worth tracking, in which case, feel free to close.

Configuration

Current SharpLab: Core CLR 7.0.22.51805 on x86

Regression?

Not that I know of.

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

Issue Details

---

Description

Consider the following code (inspired by dotnet/csharplang#4082 (comment)):

static int ArrayAccess(int[] abcd)
{
    return abcd[0] + abcd[1] + abcd[2] + abcd[3];
}

Note that it accesses the array multiple times at increasing indexes, which I think is a relatively common pattern. This results in one range check for every array access:

L0000: mov eax, [ecx+4]
L0003: test eax, eax
L0005: je short L0025
L0007: mov edx, [ecx+8]
L000a: cmp eax, 1
L000d: jbe short L0025
L000f: add edx, [ecx+0xc]
L0012: cmp eax, 2
L0015: jbe short L0025
L0017: add edx, [ecx+0x10]
L001a: cmp eax, 3
L001d: jbe short L0025
L001f: mov eax, edx
L0021: add eax, [ecx+0x14]
L0024: ret
L0025: call 0x71efa060
L002a: int3

This is annoying, because only the check for abcd[3] is necessary, since there are no side-effects between the array accesses. Even though these range checks are likely to be well predicted by the CPU, I think it would be nice if they could be elided by the JIT.

This may very well be a duplicate, but I couldn't find it. Or it may not be worth tracking, in which case, feel free to close.

Configuration

Current SharpLab: Core CLR 7.0.22.51805 on x86

Regression?

Not that I know of.

Author:	svick
Assignees:	-
Labels:	tenet-performance, area-CodeGen-coreclr, untriaged
Milestone:	-

[gfoidl]

A workaround for this is to put the highest index first, then there's only one bound check emitted (for abcd[3]).

Didn't find a dedicated issue for this too.

[jakobbotsch]

This is the same underlying issue as #12549 but this one is easier findable so let's keep both open.

[MihaZupan]

A (possibly similar) variant of this with an extra offset: sharplab

public static int Bar(string s, int i)
{
    if ((uint)i >= (uint)s.Length ||
        (uint)(i + 2) >= (uint)s.Length ||
        // This check is redundant, but currently needed to avoid CORINFO_HELP_RNGCHKFAIL
        (uint)(i + 1) >= (uint)s.Length)
    {
        return 0;
    }

    return s[i] + s[i + 1] + s[i + 2];
}