JIT: Stop sinking stores below commas in impStoreStruct

[jakobbotsch]

The following logic in the importer creates unnaturally/incorrectly typed COMMA nodes:

https://github.com/dotnet/runtime/blob/790c4c02d11b63d39fc87bec1e6a71af88293a4b/src/coreclr/jit/importer.cpp#L962-L993

It changes for instance

               [000006] --CXG------                         ▌  COMMA     simd12
               [000005] H-CXG------                         ├──▌  CALL help int    CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
               [000003] ----------- arg0                    │  ├──▌  CNS_INT   int    0x8A2B390
               [000004] ----------- arg1                    │  └──▌  CNS_INT   int    1
               [000001] I---G------                         └──▌  IND       simd12
               [000000] H----------                            └──▌  CNS_INT(h) int    0x8601620 static Fseq[s]

to

               [000006] -ACXG------                         ▌  COMMA     simd12
               [000005] H-CXG------                         ├──▌  CALL help int    CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE
               [000003] ----------- arg0                    │  ├──▌  CNS_INT   int    0x8A2B390
               [000004] ----------- arg1                    │  └──▌  CNS_INT   int    1
               [000087] DA--G------                         └──▌  STORE_LCL_VAR simd12<System.Numerics.Vector3> V02 tmp1         
               [000001] I---G------                            └──▌  IND       simd12
               [000000] H----------                               └──▌  CNS_INT(h) int    0x8601620 static Fseq[s]

We would usually expect [000006] to be TYP_VOID in the latter tree, as evidenced by gtExtractSideEffList. Morph also has code to compensate:

https://github.com/dotnet/runtime/blob/790c4c02d11b63d39fc87bec1e6a71af88293a4b/src/coreclr/jit/morph.cpp#L9665-L9669

We should fix this; we can likely just avoid sinking these stores below the COMMAs -- this used to be necessary because block morphing did not handle the pattern, but it should be handled after #83590.

91443 has an example bug because of this JIT IR oddity.

[ghost]

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

Issue Details

---

The following logic in the importer creates unnaturally/incorrectly typed `COMMA` nodes: https://github.com/dotnet/runtime/blob/790c4c02d11b63d39fc87bec1e6a71af88293a4b/src/coreclr/jit/importer.cpp#L962-L993 It changes for instance ```scala [000006] --CXG------ ▌ COMMA simd12 [000005] H-CXG------ ├──▌ CALL help int CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE [000003] ----------- arg0 │ ├──▌ CNS_INT int 0x8A2B390 [000004] ----------- arg1 │ └──▌ CNS_INT int 1 [000001] I---G------ └──▌ IND simd12 [000000] H---------- └──▌ CNS_INT(h) int 0x8601620 static Fseq[s] ``` to ```scala [000006] -ACXG------ ▌ COMMA simd12 [000005] H-CXG------ ├──▌ CALL help int CORINFO_HELP_GETSHARED_NONGCSTATIC_BASE [000003] ----------- arg0 │ ├──▌ CNS_INT int 0x8A2B390 [000004] ----------- arg1 │ └──▌ CNS_INT int 1 [000087] DA--G------ └──▌ STORE_LCL_VAR simd12 V02 tmp1 [000001] I---G------ └──▌ IND simd12 [000000] H---------- └──▌ CNS_INT(h) int 0x8601620 static Fseq[s] ``` We would usually expect `[000006]` to be `TYP_VOID` in the latter tree, as evidenced by `gtExtractSideEffList`. Morph also has code to compensate: https://github.com/dotnet/runtime/blob/790c4c02d11b63d39fc87bec1e6a71af88293a4b/src/coreclr/jit/morph.cpp#L9665-L9669 We should fix this; we can likely just avoid sinking these stores below the COMMAs anymore -- this used to be necessary because block morphing did not handle the pattern, but it should be handled after #83590.

Author:	jakobbotsch
Assignees:	-
Labels:	`area-CodeGen-coreclr`, `untriaged`
Milestone:	-

[jakobbotsch]

106380 is another example of a bug due to this issue.

We should fix this; we can likely just avoid sinking these stores below the COMMAs -- this used to be necessary because block morphing did not handle the pattern, but it should be handled after https://github.com/dotnet/runtime/pull/83590.

Another thing is that, since LIR does not support TYP_STRUCT edges beyond some very restricted patterns, we should ensure that the handling for these struct operations properly handle interfering side effects.