HardwareIntrinsics_X86_Avx_ConvertToVector_ro fails deterministically under gcstress

[AndyAyersMS]

In particular gcstress=0xC jitstress=2.

Failure is that ESI is marked as a live GC ref when it is trashed by a block copy.

JIT.HardwareIntrinsics.X86.SimpleUnaryOpTest__DataTable`2[[System.Double, System.Private.CoreLib],[System.Single, System.Private.CoreLib]]..ctor(Single[], Double[], Int32)

;; tail end of method
;; ESI == this ptr in method body
00ee7360 8bce            mov     ecx,esi
00ee7362 ff1574d4c80a    call    dword ptr ds:[0AC8D474h] (JIT.HardwareIntrinsics.X86.SimpleUnaryOpTest__DataTable`2[[System.Double, System.Private.CoreLib],[System.Single, System.Private.CoreLib]].get_inArrayPtr(), mdToken: 060000b6)
00ee7368 837f0400        cmp     dword ptr [edi+4],0
00ee736c 7619            jbe     ConvertToVector_ro!JIT.HardwareIntrinsics.X86.SimpleUnaryOpTest__DataTable`2[[System.Double, System.Private.CoreLib],[System.Single, System.Private.CoreLib]]..ctor(Single[], Double[], Int32)+0x9f (00ee7387)
00ee736e 8d5708          lea     edx,[edi+8]
00ee7371 0fb64e08        movzx   ecx,byte ptr [esi+8]
00ee7375 8bf8            mov     edi,eax
;; ESI needs to be marked dead here
00ee7377 8bf2            mov     esi,edx
00ee7379 f3a4            rep movs byte ptr es:[edi],byte ptr [esi]
;;; fail here at offset 0x93
00ee737b 83c40c          add     esp,0C           <<<< 
00ee737e 5b              pop     ebx (gcstress)
;; offset 0x97, gcinfo has ESI live until here
00ee737f 5e              pop     esi (gcstress)
00ee7380 5f              pop     edi (gcstress)
00ee7381 8be5            mov     esp,ebp (gcstress)
00ee7383 5d              pop     ebp (gcstress)
00ee7384 c20800          ret     8 (gcstress)
00ee7387 e824f9b30e      call    197ce398 (gcstress) (JitHelp: CORINFO_HELP_RNGCHKFAIL)
00ee738c cc              int     3 (gcstress)

GC info is:

GC info 0ac78b34
Method info block:
    method      size   = 00A5
    prolog      size   = 19 
    epilog      size   = 12 
    epilog     count   =  1 
    epilog      end    = no  
    callee-saved regs  = EDI ESI EBX EBP 
    ebp frame          = no  
    fully interruptible= yes  
    double align       = yes  
    arguments size     =  2 DWORDs
    stack frame size   =  3 DWORDs
    untracked count    =  0 
    var ptr tab count  =  0 
    epilog # 0    at   0093
81 25 C2 83 A3 | 
FC 93 F3 C0 41 | 
81 13          | 

Pointer table:
F0 BF 76       | 000E        reg ESI becoming live (iptr)
7A             | 0010        reg EDI becoming live
F1 46          | 0026        reg EAX becoming live
BF 52          | 0028        reg EDX becoming live (iptr)
05             | 002D        reg EAX becoming dead
10             | 002D        reg EDX becoming dead
F1 43          | 0040        reg EAX becoming live
BF 53          | 0043        reg EDX becoming live (iptr)
05             | 0048        reg EAX becoming dead
10             | 0048        reg EDX becoming dead
52             | 004A        reg EDX becoming live
F0 B0          | 0052        push non-ptr (1)
BF 4C          | 0056        reg ECX becoming live (iptr)
15             | 005B        reg EDX becoming dead
08             | 005B        reg ECX becoming dead
C8             | 005B        pop  1 args (0)
F0 4A          | 0065        reg ECX becoming live
F0 0A          | 006F        reg ECX becoming dead
F0 BF 4B       | 007A        reg ECX becoming live (iptr)
0E             | 0080        reg ECX becoming dead
F0 BF 51       | 0089        reg EDX becoming live (iptr)
3E             | 008F        reg EDI becoming dead
BF 78          | 008F        reg EDI becoming live (iptr)
F1 10          | 009F        reg EDX becoming dead
30             | 009F        reg ESI becoming dead
38             | 009F        reg EDI becoming dead
FF             | 

[AndyAyersMS]

Seems at first glance like CodeGen::genCodeForCpBlkRepMovs needs to update gc state of RSI/RDI. But not sure this is the issue.

[fiigii]

Is this a managed code bug (incorrect unsafe code) or runtime bug?

[AndyAyersMS]

I believe it's a jit bug.

@RussKeldorph can you see if anybody familiar with x86 codegen/ gc encoding can pick this one up? I'm happy to work on it if you can't...

[AndyAyersMS]

Suspect most of the other x86 HW intrinsic test gc stress failures may be caused by this too, so perhaps also try and verify these tests pass if/when there is a fix:

JIT_HardwareIntrinsics._X86_Avx2_ConvertToVector256_ro
JIT_HardwareIntrinsics._X86_Avx_InsertExtractVector128_r
JIT_HardwareIntrinsics._X86_Avx_InsertExtractVector128_ro
JIT_HardwareIntrinsics._X86_Avx_Avx_ro

[AndyAyersMS]

Think while going into the rep movs that esi is a valid byref, after the rep movsb it points off the end of the array into free space, and so can't be reported as a gc ref anymore.

Not sure what part of the jit is responsible for fixing this just yet.

[mikedn]

after the rep movsb it points off the end of the array into free space

But it should point right after the end of the array, isn't that valid?

[RussKeldorph]

@BruceForstall Can you take a look?

[AndyAyersMS]

Hmm, good point. Need to debug some more.

[RussKeldorph]

@BruceForstall nevermind. @AndyAyersMS Assigning to you while you're still looking. If you get stuck, perhaps @CarolEidt can help.

[AndyAyersMS]

Ok, now think this is a test issue. Broke in at the rep movsb and have

ESI is 0745E3A0, which is 0th  element of Single[4] array starting at 0745E398
EDI is 0745E600, which is 13th element of  Byte[64] array starting at 0745E5EC

Copy length is 0x20 = 32 bytes

Will leave ESI at 0745E3C0, EDI at 0745E620

So this copies 16 bytes past end of the array, leaving ESI pointing off the end.

Seems like the fix is that LargestVectorSize, which specifies the copy length, needs to be 16 here.

[fiigii]

@tannergooding looks like we need to fix the test template.

[AndyAyersMS]

I cannot determine how (or if) the Convert tests get auto-generated so somebody else will have to figure this part out.

It would be nice for each auto-generated bit of syntax to link back to the bits of input syntax that lead to their generation (say via a comment or two, with repo-relative paths and such).

[fiigii]

@AndyAyersMS let me give it a try to fix the tests.

[AndyAyersMS]

Thanks. There may be other tests with this same kind of pattern, so you might need to fix more than one.