[JIT] ARM32/ARM64 - Fixed zero/sign-extension for GT_STORE_LCL_VAR in codegen

[TIHan]

Resolves #84693

We were not handling zero/sign-extends for NormalizeOnLoad local stores correctly on ARM.

[ghost]

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

Issue Details

---

Resolves #84693

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

[TIHan]

@dotnet/jit-contrib @kunalspathak This is ready pending CI.

[SingleAccretion]

What was the underlying problem here (perhaps the one we discussed a while back)?

If it was NOL, that implies fixing the "store" part of code is a workaround.

[TIHan]

@SingleAccretion From the issue, we were not zero-extending on store for a NOL, here is the before and after diffs in regards to that example in the issue:

[SingleAccretion]

In principle this should not matter, since upper bits of a NOL local have to be assumed to be random. Is the dump available?

[TIHan]

dump.txt

[TIHan]

On x64, we do the zero-extend:

       mov      rax, 0xD1FFAB1E      ; data for test.Program:s_2
       mov      word  ptr [rax], 1
       movsx    rax, word  ptr [rax]
       neg      eax
       movzx    rcx, al      ; <--- zero-extend
       and      ecx, ecx
       call     [System.Console:WriteLine(int)]
       xor      eax, eax

[SingleAccretion]

So this is a variation of the "NOL does not work well with VN" issue. In particular, the problem is how [000012] is numbered here:

***** BB01, STMT00002(before)
N005 ( 20, 10) [000013] --CXG------                         *  CALL      void   System.Console:WriteLine(int)
N004 (  6,  7) [000016] ----------- arg0 in x0              \--*  CAST      int <- ubyte <- int
N003 (  5,  5) [000012] -----------                            \--*  AND       int   
N001 (  2,  2) [000010] -----------                               +--*  LCL_VAR   int    V00 arg0         u:2
N002 (  2,  2) [000011] -----------                               \--*  LCL_VAR   int    V00 arg0         u:2 (last use)

N001 [000010]   LCL_VAR   V00 arg0         u:2 => <l:$46 {IntCns 255}, c:$200 {$1c0, int <- ubyte <- int}>
N002 [000011]   LCL_VAR   V00 arg0         u:2 (last use) => <l:$46 {IntCns 255}, c:$200 {$1c0, int <- ubyte <- int}>
N003 [000012]   AND       => <l:$46 {IntCns 255}, c:$200 {$1c0, int <- ubyte <- int}>
N004 [000016]   CAST      => <l:$46 {IntCns 255}, c:$201 {$200, int <- ubyte <- int}>
  fgCurMemoryVN[GcHeap] assigned for CALL at [000013] to VN: $c1.
N005 [000013]   CALL      => $VN.Void

***** BB01, STMT00002(after)
N005 ( 20, 10) [000013] --CXG------                         *  CALL      void   System.Console:WriteLine(int) $VN.Void
N004 (  6,  7) [000016] ----------- arg0 in x0              \--*  CAST      int <- ubyte <- int <l:$46, c:$201>
N003 (  5,  5) [000012] -----------                            \--*  AND       int    <l:$46, c:$200>
N001 (  2,  2) [000010] -----------                               +--*  LCL_VAR   int    V00 arg0         u:2 <l:$46, c:$200>
N002 (  2,  2) [000011] -----------                               \--*  LCL_VAR   int    V00 arg0         u:2 (last use) <l:$46, c:$200>

In reality, the value of AND's upper bits is undefined (and equal to V00's). A denormalized NOL local without a cast on top of it is not legal (so one can view the original bug to be in the morph's transform which pushed the cast up).

That said, I would not have objections against merging this change anyway, for consistency between platforms if nothing else (if so, we should also fix LA64 and RISC-V).

[TIHan]

Diffs
No regressions for arm64. There are some regressions for arm32, but that is fine since this fixes a correctness issue.

[kunalspathak]

Based on #84693 (comment), does this need to be backported to 7.0?

[kunalspathak]

I believe you will have to fix the formatting issues before merging.

[TIHan]

does this need to be backported to 7.0?

Yes we should backport this since it is a correctness issue.

[TIHan]

CI failures are unrelated. Merging.