Assertion failed 'putArgRegNode->gtOper == GT_PUTARG_REG'

[BruceForstall]

osx-arm64 random JitStress

Test: JIT/HardwareIntrinsics/General/Vector128_1/Vector128_1_ro/Vector128_1_ro.sh

https://dev.azure.com/dnceng-public/public/_build/results?buildId=47392&view=ms.vss-test-web.build-test-results-tab&runId=953920&resultId=103768&paneView=debug

export COMPlus_TieredCompilation=0
export COMPlus_JitStress=35a

    JIT/HardwareIntrinsics/General/Vector128_1/Vector128_1_ro/Vector128_1_ro.sh [FAIL]
      
      Assert failure(PID 12800 [0x00003200], Thread: 8914595 [0x8806a3]): Assertion failed 'putArgRegNode->gtOper == GT_PUTARG_REG' in 'JIT.HardwareIntrinsics.General.VectorExtend__ToVector256UInt64:RunReflectionScenario():this' during 'Generate code' (IL size 233; hash 0x76b37d97; FullOpts)
      
          File: /Users/runner/work/1/s/src/coreclr/jit/codegenarmarch.cpp Line: 3085
          Image: /private/tmp/helix/working/B5B60A2F/p/corerun
      
      [createdump] Invalid process id: task_for_pid(12800) FAILED (os/kern) failure (5)
      [createdump] This failure may be because createdump or the application is not properly signed and entitled.
      [createdump] Failure took 0ms
      [createdump] waitpid() returned successfully (wstatus 0000ff00)
      /private/tmp/helix/working/B5B60A2F/w/A9B00977/e/JIT/HardwareIntrinsics/General/Vector128_1/Vector128_1_ro/Vector128_1_ro.sh: line 396: 12800 Abort trap: 6           (core dumped) $LAUNCHER $ExePath "${CLRTestExecutionArguments[@]}"
      
      Return code:      1

@dotnet/jit-contrib

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

Issue Details

---

osx-arm64 random JitStress

Test: JIT/HardwareIntrinsics/General/Vector128_1/Vector128_1_ro/Vector128_1_ro.sh

https://dev.azure.com/dnceng-public/public/_build/results?buildId=47392&view=ms.vss-test-web.build-test-results-tab&runId=953920&resultId=103768&paneView=debug

export COMPlus_TieredCompilation=0
export COMPlus_JitStress=35a

    JIT/HardwareIntrinsics/General/Vector128_1/Vector128_1_ro/Vector128_1_ro.sh [FAIL]
      
      Assert failure(PID 12800 [0x00003200], Thread: 8914595 [0x8806a3]): Assertion failed 'putArgRegNode->gtOper == GT_PUTARG_REG' in 'JIT.HardwareIntrinsics.General.VectorExtend__ToVector256UInt64:RunReflectionScenario():this' during 'Generate code' (IL size 233; hash 0x76b37d97; FullOpts)
      
          File: /Users/runner/work/1/s/src/coreclr/jit/codegenarmarch.cpp Line: 3085
          Image: /private/tmp/helix/working/B5B60A2F/p/corerun
      
      [createdump] Invalid process id: task_for_pid(12800) FAILED (os/kern) failure (5)
      [createdump] This failure may be because createdump or the application is not properly signed and entitled.
      [createdump] Failure took 0ms
      [createdump] waitpid() returned successfully (wstatus 0000ff00)
      /private/tmp/helix/working/B5B60A2F/w/A9B00977/e/JIT/HardwareIntrinsics/General/Vector128_1/Vector128_1_ro/Vector128_1_ro.sh: line 396: 12800 Abort trap: 6           (core dumped) $LAUNCHER $ExePath "${CLRTestExecutionArguments[@]}"
      
      Return code:      1

@dotnet/jit-contrib

Author:	BruceForstall
Assignees:	-
Labels:	JitStress, area-CodeGen-coreclr
Milestone:	8.0.0

[kunalspathak]

Related to #76671? @jakobbotsch ?

[jakobbotsch]

Unlikely since that PR shouldn't affect the IR (well, I suppose it could via resolutions/spills/reloads). I can take a look though.

[jakobbotsch]

I suppose it could via resolutions/spills/reloads

...and that's exactly what happens, looks like LSRA is inserting a reload between a PUTARG_REG and its parent FIELD_LIST:

N585 (  1,  1) [000182] -----------                  t182 =    LCL_VAR   ref    V23 tmp17        u:2 x25 REG x25 $245
                                                            ┌──▌  t182   ref    
N587 (  3,  4) [000189] -c---------                  t189 = ▌  LEA(b+8)  byref  REG NA
                                                            ┌──▌  t189   byref  
N589 (  4,  3) [000403] n---GO-----                  t403 = ▌  IND       simd16 REG d0 <l:$547, c:$546>
                                                            ┌──▌  t403   simd16 
N591 (???,???) [000612] ----GO----Z                  t612 = ▌  PUTARG_REG simd16 REG d0
                                                            ┌──▌  t612   simd16 
               [000627] ----GO-----                  t627 = ▌  RELOAD    simd16 REG d0
N593 (  1,  1) [000405] -----------                  t405 =    LCL_VAR   ref    V23 tmp17        u:2 x25 (last use) REG x25 $245
                                                            ┌──▌  t405   ref    
N595 (  3,  4) [000404] -c---------                  t404 = ▌  LEA(b+24) byref  REG NA
                                                            ┌──▌  t404   byref  
N597 (  4,  3) [000409] ---XG------                  t409 = ▌  IND       simd16 REG d1 <l:$549, c:$548>
                                                            ┌──▌  t409   simd16 
N599 (???,???) [000613] ---XG-----Z                  t613 = ▌  PUTARG_REG simd16 REG d1
                                                            ┌──▌  t613   simd16 
               [000628] ---XG------                  t628 = ▌  RELOAD    simd16 REG d1         <- illegal reload position?
                                                            ┌──▌  t627   simd16 
                                                            ├──▌  t628   simd16 
N601 (  8,  6) [000402] -c-XGO-----                  t402 = ▌  FIELD_LIST struct REG NA <l:$683, c:$682>
N603 (  1,  1) [000176] -----------                  t176 =    LCL_VAR   ref    V00 this         u:1 x19 (last use) REG x19 $80
N605 (???,???) [000620] -----------                            PROF_HOOK void   REG NA
                                                            ┌──▌  t176   ref    
N607 (???,???) [000614] -----------                  t614 = ▌  PUTARG_REG ref    REG x0
N609 (  1,  1) [000191] -----------                  t191 =    LCL_VAR   ref    V01 loc0         u:2 x22 (last use) REG x22 $240
                                                            ┌──▌  t191   ref    
N611 (???,???) [000615] -----------                  t615 = ▌  PUTARG_REG ref    REG x1
N613 (  1,  2) [000260] -----------                  t260 =    CNS_INT   int    1 REG x2 $45
                                                            ┌──▌  t260   int    
N615 (???,???) [000616] -----------                  t616 = ▌  PUTARG_REG int    REG x2
N617 (  1,  1) [000504] -----------                  t504 =    LCL_VAR   ref    V29 cse3         u:1 x20 (last use) REG x20 $201
                                                            ┌──▌  t504   ref    
N619 (???,???) [000617] -----------                  t617 = ▌  PUTARG_REG ref    REG x3
N621 (  3, 12) [000618] H----------                  t618 =    CNS_INT(h) long   0x7ff7b9317f60 ftn REG x4
                                                            ┌──▌  t618   long   
N623 (  6, 14) [000619] -----------                  t619 = ▌  IND       long   REG x4
                                                            ┌──▌  t402   struct arg1 d0,d1
                                                            ├──▌  t614   ref    this in x0
                                                            ├──▌  t615   ref    arg2 in x1
                                                            ├──▌  t616   int    arg3 in x2
                                                            ├──▌  t617   ref    arg4 in x3
                                                            ├──▌  t619   long   control expr
N625 ( 26, 18) [000194] --CXGO-----                         ▌  CALL      void   JIT.HardwareIntrinsics.General.VectorExtend__ToVector256UInt64.ValidateResult REG NA $VN.Void

[jakobbotsch]

Actually, lowering does allow reloads between the call and PUTARG_REG, so I guess it is the FIELD_LIST handling that is in the wrong.

[jakobbotsch]

The existing handling in lowering seems wrong to me. genCall just skips copies/reloads which means we never reload those values if we see them in that position. I'm not totally sure how this is supposed to work @BruceForstall @kunalspathak? My understanding is that genConsumeReg has to be called on these copies/reloads to be handled properly.

It does not make much sense to me that we can have them on top of the PUTARG node, but I don't have the LSRA knowledge to understand why ends up there.

[jakobbotsch]

This repros with #76671 reverted as well, so it doesn't seem related after all.

[jakobbotsch]

I think the actual problem is in the insertion of PROF_HOOK. It turns into a call, so it is nonsensical to insert it after PUTARG_REG. InsertProfTailCallHook is missing handling for FIELD_LIST.