jit x86: possible issue with preserved register set for CORINFO_HELP_INIT_PINVOKE_FRAME

[AndyAyersMS]

The x86 jitstress1 gcstress0xC failure in baseservices_threading._commitstackonlyasneeded_DefaultStackCommit seems to be a problem with CORINFO_HELP_INIT_PINVOKE_FRAME claiming to kill gc refs in EDX when it doesn't.

Problematic method is <>c:<RunTest>b__9_3(ref):this which has disassembly (annotated w/ GC info):

058258f8 55              push    ebp
058258f9 8bec            mov     ebp,esp
058258fb 57              push    edi
058258fc 56              push    esi
058258fd 53              push    ebx
058258fe 83ec1c          sub     esp,1Ch
;; end prolog
05825901 8d7ddc          lea     edi,[ebp-24h]   +EDX
05825904 e85fdd5efb      call    CORINFO_HELP_INIT_PINVOKE_FRAME (00e13668)  +EDI
05825909 8bcd            mov     ecx,ebp   -EDX ?? -EDI
0582590b 894df0          mov     dword ptr [ebp-10h],ecx
0582590e 8bca            mov     ecx,edx
>>> 05825910 ff15b85c280b    call    dword ptr ds:[0B285CB8h] (StackCommitTest.Finalizer.CreateUnreferencedObject(System.Action), mdToken: 06000011)

As you can see EDX is killed for GC after the first call when it is clearly live down to the move before the second call.

The root cause seems to be that CORINFO_HELP_INIT_PINVOKE_FRAME is not special cased in Compiler::compNoGCHelperCallKillSet and so reports the default set of callee killed registers, which includes EDX.

Adding a special case for this for x86 fixes the gc stress regression.

Seems like we could assert that the set of GC kills at a call is a subset of the set of regular kills maybe?

Splitting this one off from #9964 / dotnet/coreclr#17330.

cc @RussKeldorph @dotnet/jit-contrib

[BruceForstall]

It seems to me that the default for Compiler::compNoGCHelperCallKillSet (used only in the emitter) should probably be to call Compiler::compHelperCallKillSet instead of using RBM_CALLEE_TRASH_NOGC but I haven't convinced myself of that yet. I've slowly been trying to eliminate RBM_CALLEE_TRASH_NOGC because I think it's so confusing, and sometimes wrong.

[briansull]

Your analysis sounds correct to me.

[AndyAyersMS]

Pointed fix is something like:

@@ -752,6 +752,11 @@ regMaskTP Compiler::compNoGCHelperCallKillSet(CorInfoHelpFunc helper)
             return RBM_CALLEE_TRASH_NOGC;
 #endif // defined(_TARGET_AMD64_)

+#if defined(_TARGET_X86_)
+        case CORINFO_HELP_INIT_PINVOKE_FRAME:
+            return RBM_INIT_PINVOKE_FRAME_TRASH;
+#endif // defined(_TARGET_X86_)

but you may want to look into something more comprehensive.

[BruceForstall]

We should do pointed fix for now.

[AndyAyersMS]

Ok, I'll put what I have up as a PR.