Add more checking to GenTreeArrAddr::ParseArrayAddress()

[BruceForstall]

If the ARR_ADDR node doesn't make sense, namely if the array offset does not appear to be in the array, then bail.

[BruceForstall]

Extracted from #94250

[BruceForstall]

There are a few diffs in ILGEN tests that construct array index expressions with negative indices.

[BruceForstall]

With JitOptRepeat we end up with the following situation.

We hit the assert:

Assertion failed '(constIndexOffset % elemSize) == 0'

(e.g.,

System.Runtime.CompilerServices.PoolingAsyncValueTaskMethodBuilder`1+StateMachineBox`1[int,System.IO.Stream+<ReadAtLeastAsyncCore>d__46]:System.Threading.Tasks.Sources.IValueTaskSource.GetResult(short):this'

We start with:

[000895] -----O-----                            \--*  ARR_ADDR  byref Internal.PaddedReference[]
[000894] -----------                               \--*  ADD       byref
[000885] -----------                                  +--*  LCL_VAR   ref    V56 tmp54
[000893] -----------                                  \--*  ADD       long
[000891] -----------                                     +--*  MUL       long
[000889] ---------U-                                     |  +--*  CAST      long <- uint
[000886] -----------                                     |  |  \--*  LCL_VAR   int    V14 tmp12
[000890] -------N---                                     |  \--*  CNS_INT   long   128
[000892] -----------                                     \--*  CNS_INT   long   16

After optimization, this becomes:

N018 (  2,  2) [000895] -A---O-----                                  \--*  ARR_ADDR  byref Internal.PaddedReference[] $641
N017 (  2,  2) [000894] -A-----N---                                     \--*  ADD       byref  $199
N007 (  1,  1) [000885] -----------                                        +--*  LCL_VAR   ref    V56 tmp54        u:1 (last use) $84
N016 (  7, 10) [000995] -A---------                                        \--*  COMMA     long   $687
N014 (  6,  9) [000993] DA---------                                           +--*  STORE_LCL_VAR long   V62 cse3         d:1 $VN.Void
N013 (  6,  9) [000893] -----------                                           |  \--*  ADD       long   $687
N011 (  4,  6) [000891] -----------                                           |     +--*  LSH       long   $686
N009 (  2,  3) [000889] ---------U-                                           |     |  +--*  CAST      long <- uint $685
N008 (  1,  1) [000886] -----------                                           |     |  |  \--*  LCL_VAR   int    V14 tmp12        u:1 $1f5
N010 (  1,  2) [000890] -------N---                                           |     |  \--*  CNS_INT   long   7 $14e
N012 (  1,  2) [000892] -----------                                           |     \--*  CNS_INT   long   16 $146
N015 (  1,  1) [000994] -----------                                           \--*  LCL_VAR   long   V62 cse3         u:1 $687

On the next iteration of JitOptRepeat, the CSE injected COMMA is not parseable by GenTreeArrAddr::ParseArrayAddress. We could add tunnel through for this CSE def.

However, it might be harder for the CSE use. In that case, we start with something similar:

[000941] -----O-----                            \--*  ARR_ADDR  byref Internal.PaddedReference[]
[000940] -----------                               \--*  ADD       byref
[000931] -----------                                  +--*  LCL_VAR   ref    V57 tmp55
[000939] -----------                                  \--*  ADD       long
[000937] -----------                                     +--*  MUL       long
[000935] ---------U-                                     |  +--*  CAST      long <- uint
[000932] -----------                                     |  |  \--*  LCL_VAR   int    V14 tmp12         (last use)
[000936] -------N---                                     |  \--*  CNS_INT   long   128
[000938] -----------                                     \--*  CNS_INT   long   16

and, after CSE, have:

N006 (  2,  2) [000941] -----O-N---                            \--*  ARR_ADDR  byref Internal.PaddedReference[]
N005 (  2,  2) [000940] -------N---                               \--*  ADD       byref
N003 (  1,  1) [000931] -----------                                  +--*  LCL_VAR   ref    V57 tmp55        u:1 (last use)
N004 (  1,  1) [000996] -----------                                  \--*  LCL_VAR   long   V62 cse3         u:1 (last use)

This would require parsing the CSE value number for [000996], not the tree.

Since this is VN iteration #2, I'm not sure we lose anything by assigning the ARR_ADDR a new, unique value number when we fail to parse the tree: we've already optimized it once.

[BruceForstall]

@jakobbotsch PTAL
cc @dotnet/jit-contrib

[BruceForstall]

It's interesting that there are diffs on x86, apparently because the offset of data of an array from the base of an array is 8 (it is 16 on 64-bit). This happens to be the same size as a pointer and a double. I see cases where we start with (loc5 - 1) * 8 + 8 which morph transforms to loc5 * 8 - 8 + 8 => loc5 * 8, thus we no longer have an "offset" into the array data (and at runtime loc5 better be > 0, which is checked by the bounds check).

The result is that we give the ARR_ADDR a new, unique VN and we lose some CSEs.

[BruceForstall]

GitHub is confused; all the tests passed.

Diffs as described