JIT arm32: Assertion failed 'refPosition->RegOptional()' in 'System.Numerics.Tests.ComplexTests:Abs(double,double)' during 'LSRA build intervals' when using non-standard arg sort

[jakobbotsch]

1. Comment the code in CallArgs::SortArgs that does the sort (everything after the initial loop that assigns sortedArgs)
2. Run superpmi replay over arm32 with COMPlus_JitStressRegs=3

Result:

[22:19:58] ISSUE: <ASSERT> #271154 D:\a\_work\1\s\src\coreclr\jit\lsra.cpp (11934) - Assertion failed 'refPosition->RegOptional()' in 'System.Numerics.Tests.ComplexTests:Abs(double,double)' during 'LSRA build intervals' (IL size 22; hash 0xbf3bc3d1; FullOpts)

category:correctness
theme:register-allocator

[ghost]

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

Issue Details

---

1. Comment the code in CallArgs::SortArgs that does the sort (everything after the initial loop that assigns sortedArgs)
2. Run superpmi replay over arm32

Result:

[22:19:58] ISSUE: <ASSERT> #271154 D:\a\_work\1\s\src\coreclr\jit\lsra.cpp (11934) - Assertion failed 'refPosition->RegOptional()' in 'System.Numerics.Tests.ComplexTests:Abs(double,double)' during 'LSRA build intervals' (IL size 22; hash 0xbf3bc3d1; FullOpts)

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

[JulieLeeMSFT]

cc @BruceForstall @kunalspathak

seems like a bug in either lowering or LSRA

[kunalspathak]

Do we know why this is 7.0 and not 8.0?

[kunalspathak]

@jakobbotsch - I just tried on main and I do not repro it. Can you confirm the exact .mc file that I can try?

[jakobbotsch]

@kunalspathak The superpmi-replay run that failed was this one: https://dev.azure.com/dnceng-public/public/_build/results?buildId=32422&view=results

Looks like it also needs COMPlus_JitStressRegs=3. Sorry about that, didn't realize superpmi-replay runs with stress modes also.

[kunalspathak]

needs COMPlus_JitStressRegs=3

Yes, was able to repro with that.

superpmi-replay runs with stress modes also.

Yes, it runs with all the JitStressRegs switches because replay doesn't complain about missing contexts for them, and the collection is still valid, unlike with JitStress.

Perhaps it would be too early for me to comment on this given the limited knowledge I have around PutArg. I will spend some more time to see why we hit into it. But essentially, we are not able to find a free pair of registers to allocate because they are either occupied or marked as "busy until next kill". In such case, we expect at least the ref position to be marked as "RegOptional()", but that's not the case either, which I am not sure should be the right thing to do or there is other problem somewhere else.

─────────────────────────────────┼────┼────┼────┼────┼────┼────┼────┼────┼────┼────┼────┼────┤
LocRP# Name Type  Action    Reg  │r0  │r1  │r2  │r3  │r4  │r5  │f0  │f1  │f2  │f3  │f16 │f17 │
─────────────────────────────────┼────┼────┼────┼────┼────┼────┼────┼────┼────┼────┼────┼────┤
 0.#1  V1   Parm   Keep     f2   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
 1.#2  BB1 PredBB0               │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
 7.#3  f0   Fixd   Keep     f0   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
 7.#4  V0   Use    Keep     f0   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
 8.#5  f0   Fixd   Keep     f0   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
 8.#6  I2   Def    PtArg    f0   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
11.#7  f2   Fixd   Keep     f2   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
11.#8  V1   Use    Keep     f2   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
12.#9  f2   Fixd   Keep     f2   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
12.#10 I3   Def    PtArg    f2   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
17.#11 V0   Use    Keep     f0   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
17.#12 V0   Use *  Keep     f0   │    │    │    │    │    │    │V0 a│V0 a│V1 a│V1 a│    │    │
18.#13 I4   Def    Alloc    f16  │    │    │    │    │    │    │Busy│Busy│V1 a│V1 a│I4 a│I4 a│
23.#14 V1   Use    Keep     f2   │    │    │    │    │    │    │Busy│Busy│V1 a│V1 a│I4 a│I4 a│
23.#15 V1   Use *  Keep     f2   │    │    │    │    │    │    │Busy│Busy│V1 a│V1 a│I4 a│I4 a│
24.#16 I5   Def    Spill    f16  │    │    │    │    │    │    │Busy│Busy│Busy│Busy│    │    │
                   Alloc    f16  │    │    │    │    │    │    │Busy│Busy│Busy│Busy│I5 a│I5 a│
25.#17 I4   Use *  ReLod    NA   │    │    │    │    │    │    │Busy│Busy│Busy│Busy│I5 a│I5 a│
                   Spill    f16  │    │    │    │    │    │    │Busy│Busy│Busy│Busy│    │    │
                   Alloc    f16  │    │    │    │    │    │    │Busy│Busy│Busy│Busy│I4 a│I4 a│
25.#18 I5   Use *  ReLod    NA   │    │    │    │    │    │    │Busy│Busy│Busy│Busy│I4 a│I4 a│

[jakobbotsch]

What does the IR around this call look like? It's possible the IR produced around the PUTARGs is also wrong (in particular, upstream should ensure we don't see calls with overlapping arg placement).

[kunalspathak]

Here is the full IR:

               [000015] ------------                           IL_OFFSET void   INLRT @ 0x000[E-]
N001 (  2,  8) [000013] I-----------                  t13 =    CNS_INT(h) int    0xE9302B90 static box ptr
                                                             ┌──▌  t13    int    
N002 (  5, 10) [000002] #---G-------                   t2 = ▌  IND       ref   
N003 (  1,  2) [000003] -c----------                   t3 =    CNS_INT   int    4 Fseq[hackishFieldName]
                                                             ┌──▌  t2     ref    
                                                             ├──▌  t3     int    
N004 (  7, 13) [000004] ----G-------                   t4 = ▌  ADD       byref 
                                                             ┌──▌  t4     byref  
               [000018] ----G-------                  t18 = ▌  PUTARG_REG byref  REG r0
N005 (  5, 10) [000000] ------------                   t0 =    CNS_DBL   double 1.0000000000000000
                                                             ┌──▌  t0     double 
               [000019] ------------                  t19 = ▌  PUTARG_REG double REG f0
N001 (  2,  8) [000020] H-----------                  t20 =    CNS_INT(h) int    0xEA4F8720 ftn
                                                             ┌──▌  t20    int    
N002 (  5, 10) [000021] ------------                  t21 = ▌  IND       int   
                                                             ┌──▌  t18    byref  retbuf in r0
                                                             ├──▌  t19    double arg1 f0,f1
                                                             ├──▌  t21    int    control expr
N006 ( 28, 35) [000001] S-CXG-------                        ▌  CALL      void   hackishModuleName.hackishMethodName
               [000016] ------------                           IL_OFFSET void   INLRT @ 0x013[E-]
N001 (  2,  8) [000014] I-----------                  t14 =    CNS_INT(h) int    0xE9302B94 static box ptr
                                                             ┌──▌  t14    int    
N002 (  5, 10) [000008] #---G-------                   t8 = ▌  IND       ref   
N003 (  1,  2) [000009] -c----------                   t9 =    CNS_INT   int    4 Fseq[hackishFieldName]
                                                             ┌──▌  t8     ref    
                                                             ├──▌  t9     int    
N004 (  7, 13) [000010] ----G-------                  t10 = ▌  ADD       byref 
                                                             ┌──▌  t10    byref  
               [000022] ----G-------                  t22 = ▌  PUTARG_REG byref  REG r0
N005 (  5, 10) [000006] ------------                   t6 =    CNS_DBL   double 5.0000000000000000
                                                             ┌──▌  t6     double 
               [000023] ------------                  t23 = ▌  PUTARG_REG double REG f0
N001 (  2,  8) [000024] H-----------                  t24 =    CNS_INT(h) int    0xEA4F87C8 ftn
                                                             ┌──▌  t24    int    
N002 (  5, 10) [000025] ------------                  t25 = ▌  IND       int   
                                                             ┌──▌  t22    byref  retbuf in r0
                                                             ├──▌  t23    double arg1 f0,f1
                                                             ├──▌  t25    int    control expr
N006 ( 28, 35) [000007] S-CXG-------                        ▌  CALL      void   hackishModuleName.hackishMethodName
               [000017] ------------                           IL_OFFSET void   INLRT @ 0x026[E-]
N001 (  0,  0) [000012] ------------                           RETURN    void  

The relevant node that constructs those refpositions is [000019].

DefList: { N012.t18. PUTARG_REG; N014.t0. CNS_DBL }
N016 (???,???) [000019] ------------                        ▌  PUTARG_REG double REG f0
<RefPosition #15  @16  RefTypeFixedReg <Reg:f0 > BB01 regmask=[f0] minReg=1 wt=100.00>
<RefPosition #16  @16  RefTypeUse <Ivl:6> BB01 regmask=[f0] minReg=1 last fixed wt=100.00>
Interval  7: double RefPositions {} physReg:NA Preferences=[allDouble]
<RefPosition #17  @17  RefTypeFixedReg <Reg:f0 > BB01 regmask=[f0] minReg=1 wt=100.00>
<RefPosition #18  @17  RefTypeDef <Ivl:7> PUTARG_REG BB01 regmask=[f0] minReg=1 fixed wt=400.00>

Here are the intervals:

-----------------------
Interval  4: double RefPositions {#13@18 #17@25} physReg:NA Preferences=[allDouble]
Interval  5: double RefPositions {#16@24 #18@25} physReg:NA Preferences=[allDouble]
---------------------

Let's talk offline about this.

[jakobbotsch]

Hmm, the IR look valid enough to me.