JIT: Range Checks for "(uint)i > cns" pattern

[EgorBo]

Closes#13464
Closes#1343

Example:

staticReadOnlySpan<byte>MySpan=>newbyte[]{1,2,3,4,5};intGetElement(intindex){if((uint)index<(uint)MySpan.Length)// and all variationsreturnMySpan[index];return0;}

Codegen diff:

; Method Program:GetElement(int):int:this
G_M56827_IG01:
- sub rsp, 40
G_M56827_IG02:
- mov eax, edx- cmp rax, 5- jge SHORT G_M56827_IG05+ cmp edx, 5+ jae SHORT G_M56827_IG05
G_M56827_IG03:
- cmp edx, 5- jae SHORT G_M56827_IG07
mov eax, edx
mov rdx, 0xD1FFAB1E
movzx rax, byte ptr [rax+rdx]
G_M56827_IG04:
- add rsp, 40
ret G_M56827_IG05:
xor eax, eax
G_M56827_IG06:
- add rsp, 40
ret -G_M56827_IG07:- call CORINFO_HELP_RNGCHKFAIL- int3 -; Total bytes of code: 51+; Total bytes of code: 25

(uint) cast is still needed for index because it can be negative. Alternative pattern index >= 0 && index < MySpan.Length also works here.

For all of these variations bound checks are now eliminated (none of them are eliminated in .NET 6.0):

publicclassTests{staticReadOnlySpan<byte>MySpan=>newbyte[]{1,2,3,4,5};publicstaticintTest1(intindex){if((uint)index<MySpan.Length)returnMySpan[index];return0;}publicstaticintTest2(intindex){if(MySpan.Length>(uint)index)returnMySpan[index];return0;}publicstaticintTest3(intindex){if(MySpan.Length<=(uint)index)return0;returnMySpan[index];}publicstaticintTest4(intindex){if((uint)index>=MySpan.Length)return0;returnMySpan[index];}publicstaticintTest5(intindex){if(index<0||index>=MySpan.Length)return0;returnMySpan[index];}publicstaticintTest6(intindex){if(index>=0&&index<MySpan.Length)returnMySpan[index];return0;}publicstaticintTest7(intindex){if(index<0)thrownewException();if(index>=MySpan.Length)thrownewArgumentException();returnMySpan[index];}publicstaticintTest8(intindex){if((uint)index<2)// 2 is less than MySpan.LengthreturnMySpan[index];return0;}publicstaticintTest9(intindex){if(index<2||index>4)// e.g. we restrict index to be in [2..3] range despite the fact MySpan is fine with [0..4]thrownewArgumentException();returnMySpan[index];}}

Unrelated pattern that got fixed too:

intfoo(uinti,int[]array){if(i<array.Length)returnarray[i];return0;}

if index is defined as unsigned - we don't need any casts to uint.

[ghost]

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

Issue Details

---

Closes #13464
Closes #1343

Example:

staticReadOnlySpan<byte>MySpan=>newbyte[]{1,2,3,4,5};intGetElement(intindex){if((uint)index<MySpan.Length)returnMySpan[index];return0;}

Codegen diff:

; Method Program:GetElement(int):int:this
G_M56827_IG01:
- sub rsp, 40
G_M56827_IG02:
- mov eax, edx- cmp rax, 5- jge SHORT G_M56827_IG05+ cmp edx, 5+ jae SHORT G_M56827_IG05
G_M56827_IG03:
- cmp edx, 5- jae SHORT G_M56827_IG07
mov eax, edx
mov rdx, 0xD1FFAB1E
movzx rax, byte ptr [rax+rdx]
G_M56827_IG04:
- add rsp, 40
ret G_M56827_IG05:
xor eax, eax
G_M56827_IG06:
- add rsp, 40
ret -G_M56827_IG07:- call CORINFO_HELP_RNGCHKFAIL- int3 -; Total bytes of code: 51+; Total bytes of code: 25

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[EgorBo]

As a side bonus it removes bound checks for this:

staticintFoo(int[]array,inti){if((uint)i<array.Length)returnarray[i];return0;}

Previously it needed(uint) cast for array.Length too. But it won't work if array is Span yet (but I am planning to handle it at some point - e.g. #62421 adds [Intrinsic] for Span.get_Length())

[EgorBo]

For all of these variations bound checks are now eliminated (none of them are eliminated in .NET 6.0):

publicclassTests{staticReadOnlySpan<byte>MySpan=>newbyte[]{1,2,3,4,5};publicstaticintTest1(intindex){if((uint)index<MySpan.Length)returnMySpan[index];return0;}publicstaticintTest2(intindex){if(MySpan.Length>(uint)index)returnMySpan[index];return0;}publicstaticintTest3(intindex){if(MySpan.Length<=(uint)index)return0;returnMySpan[index];}publicstaticintTest4(intindex){if((uint)index>=MySpan.Length)return0;returnMySpan[index];}publicstaticintTest5(intindex){if(index<0||index>=MySpan.Length)return0;returnMySpan[index];}publicstaticintTest6(intindex){if(index>=0&&index<MySpan.Length)returnMySpan[index];return0;}publicstaticintTest7(intindex){if(index<0)thrownewException();if(index>=MySpan.Length)thrownewArgumentException();returnMySpan[index];}publicstaticintTest8(intindex){if((uint)index<2)// 2 is less than MySpan.LengthreturnMySpan[index];return0;}publicstaticintTest9(intindex){if(index<2||index>4)// e.g. we restrict index to be in [2..3] range despite the fact MySpan is fine with [0..4]thrownewArgumentException();returnMySpan[index];}}

Unrelated pattern that got fixed too:

intfoo(uinti,int[]array){if(i<array.Length)returnarray[i];return0;}

if index is defined as unsigned - we don't need any casts to uint.

[EgorBo]

/azp run Fuzzlyn, Antigen

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[EgorBo]

@dotnet/jit-contrib @SingleAccretion PTAL (Antigen failures are not related)

I generated a lot of test cases locally (~100k lines of code) e.g. https://gist.github.com/EgorBo/1ba16f0d9b1b7d87044c7ed4d14db117 to validate there are no surprises. But I don't think I should add them to the repo as they are too big/slow to run and don't add much value.

[EgorBo]

Diffs: https://dev.azure.com/dnceng/public/_build/results?buildId=1521711&view=ms.vss-build-web.run-extensions-tab

[EgorBo]

@dotnet/jit-contrib PTAL

[EgorBo]

@jakobbotsch can you take a look again? New diffs: https://dev.azure.com/dnceng/public/_build/results?buildId=1621068&view=ms.vss-build-web.run-extensions-tab

I inspected a couple of minor regressions and they seem to be caused by CSE/RA

[jakobbotsch]

LGTM, just had one question.
cc @SingleAccretion in case you want to give this a lookover as well.

[EgorBo]

@SingleAccretion thanks for the feedback!! addressed almost all of it except BashToNop - I'm leaving it to whoever will refactor it to optNarrowTree.

Failures are not related: #65818