[x86/Windows] Failure in jitstress=2 jitstressregs=3 job

[erozenfeld]

https://ci.dot.net/job/dotnet_coreclr/job/master/view/x86/job/jitstress/job/x86_checked_windows_nt_jitstress2_jitstressregs3/473/consoleText

BEGIN EXECUTION
"D:\j\workspace\x86_checked_w---508bfa85\tests..\bin\tests\Windows_NT.x86.Checked\Tests\Core_Root\corerun.exe" MaskMove_r.exe
Expected: 100
Actual: 73294136
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1

To run the test:

set CORE_ROOT=D:\j\workspace\x86_checked_w---508bfa85\bin\tests\Windows_NT.x86.Checked\Tests\Core_Root
D:\j\workspace\x86_checked_w---508bfa85\bin\tests\Windows_NT.x86.Checked\JIT\HardwareIntrinsics\X86\Sse2\MaskMove_r\MaskMove_r.cmd

    Expected: True
    Actual:   False

[RussKeldorph]

@fiigii

[fiigii]

It may be a GC info or RA issue. I looked that the codgen and encoding is correct.
maskmovdqu uses edi as the implicit dst register that may not be properly recognized by GC or RA

vmaskmovdqu xmm0,xmm1

@CarolEidt @AndyAyersMS How can I investigate this kind of issue?

[CarolEidt]

maskmovdqu uses edi as the implicit dst register that may not be properly recognized by GC or RA

Yes that looks like it might be the case. I am investigating this, and in Main, the return value is in edi and it is getting trashed by the maskmov.

[CarolEidt]

The register allocator implicitly (and incorrectly, really) assumes that a fixed reference to a register will also be included in the kill set. I believe there may be other cases where this is not done - for example, it's not entirely clear to me why we don't have a similar issue with ECX with shifts. The issue arises when we have a def/use conflict - i.e. the def is constrained to be one register, and the use is constrained to be another. In this case, we use the def assignment, and rely on the code generator to do the copy. This is mostly OK, except that FixedRegs aren't actually treated as kills, on the assumption that it will be spilled, as needed, when it is allocated to the associated reference.
I believe that treating it as killed will fix the issue. But that's an imperfect fix, since the address in edi is not actually killed. I'm going to look into this a bit further, but will probably file an issue to better handle these def/use conflict cases.

[CarolEidt]

To answer your question about how to investigate this kind of issue, there's not great documentation on debugging JitStress, and the stress modes can really make the code difficult to navigate, not to mention that not all the COMPlus variables related to stress work properly. I usually set COMPlus_JitStressModeNamesNot=STRESS_EMITTER since that rarely seems to cause issues, and it makes the dumps and disassembly even harder to read. You can also add stress modes to that "not" list (the ones you see in the jitdump) to narrow it down.
In this case, since it also included COMPlus_JitStressRegs, I was able to determine (by enabling the ifdef'd out code here: https://github.com/dotnet/coreclr/blob/master/src/jit/lsra.cpp#L676) which method was failing with COMPlus_JitStressRegs. Then I just debugged my way through the code. I knew that the return value in edi was bad, but I hadn't realized that edi was killed by the maskmovdqu until I stepped through it, and then I saw your post.

[AndyAyersMS]

We should think about investing in some automated "bisect" tools for failures like these that reliably repro. I have bits and pieces of this lying around from a former project that I can perhaps contribute.

For jitstress you can try and debug the failure but sometimes it can be more effective to first isolate the bad method by selectively enabling stress. That is something we should be able to automate. Then we can start disabling individual stress modes for that method until we pin down the one that causes trouble. Etc.