`ldelem` and `ldelema` check for negative array length

[MichalPetryka]

Description

ldelem and ldelema without an explicit length check assume that array length can be negative and emit jbe instead of je like in case of a manual one (see linked SharpLab Ldelem* methods).

In case of array pinning, this leads to array length being checked twice which results in an always false check and a dead throw branch being emitted (see Array method).

Configuration

SharpLab Core CLR 7.0.122.56804 on x64

Regression?

I don't know

Data

SharpLab

Analysis

Citing @EgorBo from discord:

EgorBo: that's not about "never negativeness" it's about range check hating ==/!= - I tried to fix that one once
EgorBo: I tried to give "len != 0"  the same VN as "len > 0"  and it did fix the issue
EgorBo: but came with a lot of size regressions I didn't want to investigate

[EgorBo]

From what I see there are two issues here: 1) https://github.com/dotnet/runtime/issues/67270 (that I tried to fix via https://github.com/dotnet/runtime/pull/67281) 2) https://github.com/dotnet/runtime/issues/35748 (range check elimination gives up on non-tracked pinned locals)

so presumably we can close this as dup

[ghost]

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

Issue Details

---

### Description `ldelem` and `ldelema` without an explicit length check assume that array length can be negative and emit `jbe` instead of `je` like in case of a manual one (see linked SharpLab Ldelem* methods). In case of array pinning, this leads to array length being checked twice which results in an always false check and a dead throw branch being emitted (see Array method). ### Configuration SharpLab Core CLR 7.0.122.56804 on x64 ### Regression? I don't know ### Data [SharpLab](https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABABgAJiBGAOgCUBXAOwwEt8YaBJFmKCAA4BlPgDdWYGLgDcAWABQZSrUYt2nAMIR8A1gBs+IqOMkyFxAExUF5gMzKk5ZrmwAzGJQvkAKlIwKAbwVyEMp7akdiFHIAGQATGAN8AApWFgBtAF1ybABKcgBeAD5yAFVcGGTsdNJM3Ll5ULCHSmj4xJh8bFSM7LzCkvLK2FccmrqGpuJwqki2hKSAUQQBPQlWDA0ACxgwAGsejCyc/OLgptChqpoYmCYAcwwtwoLyCgB+N/IQMdr685C0xaUViC062GWq3Wmx2+0Ox36Z0aFxCV2wNzuj2eBVeH3IIzKTBc7hoADkGHo9HQYK4ADxpDBFZL5H4E6p/SahAHNCKtcgAQSgUGwAE94X1ckFkSjXKwEDA4uRDgAqcgCDBQQonbko1EVZLqqD/aXkAC+3KBvJBguFIos4pOUt15Fl8sVKrVGq1ADIALKdaAi33YKC4LbYPQ0ADiMAwNtFABFsBhsNT3LAmJIqrlcjrdVdDcamuaTZbZnyhAJsExktTsHEAPJMPQiyvV+ksEp5J2610KpUM1WGrV5PMogsaouhEtTGZzchtpj2uuN5utqtMDuMx1jl1y/se4evP0BqBBkNhiPR2Npvh3LN5XMm50To2ckIzrkm9L+p4QOJcDoejJL+Wz/oBqwNuqrAQESZIQDwaxMGk9y5Jk3ICFArCiMmHhWtEVyDp6RrkAEn4hNyP6xmBAFASB1HgUBUFsLBuDwYhaQoWhGFYThGB4eWIJXASDLEfkZHcpRoGMas9F/rRkHQax7HNpxDzcSamHYbhwIEfqomFqRJamkAA===) ### Analysis Citing @EgorBo from discord: ``` EgorBo: that's not about "never negativeness" it's about range check hating ==/!= - I tried to fix that one once EgorBo: I tried to give "len != 0" the same VN as "len > 0" and it did fix the issue EgorBo: but came with a lot of size regressions I didn't want to investigate ```

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

[MichalPetryka]

From what I see there are two issues here:

1. Redundant bound check for span.Length == 0 pattern #67270 (that I tried to fix via JIT: Normalize "X==/!=0" in VN #67281)
2. Extra bound checks are not eliminated for pinning #35748 (range check elimination gives up on non-tracked pinned locals)

so presumably we can close this as dup

I don't think either talks about just ldelem and ldelema?

[JulieLeeMSFT]

Setting to future to improve this code quality issue.