Fix ARM GCStress hole with byref write barrier helper #15822
Conversation
When unrolling a STOREOBJ, we can generate multiple consecutive byref helper calls. This helper has a unique calling convention where the dst and src addresses are modified by adding pointer size to their original value (thus allowing consecutive helper calls without reloading the dst/src addresses). So, for liveness purposes, the helper call kills the dst/src values. However, for GC purposes, it does not, as the registers still contain byref pointers. We were, in the ARM case, reporting the r0/r1 registers dead after the first call, so a GC didn't update them, and a second call updated garbage. In fixing this, I cleaned up the helper call kill handling a bit. I also fixed and improved RyuJIT/x86 write barrier kill modeling.
|
@dotnet-bot test OSX10.12 x64 Checked gcstress0x3 |
|
@dotnet/jit-contrib PTAL All of the triggered GCStress jobs are failing. Looking at the history, it looks like they've haven't passed in a long time. The number of failures in the ARM GCStress=C jobs has dropped to 3 or 4. One can't run in GCStress (#15832), the other is a bad test (#15835). The remaining failures will be investigated later. There are lots of interesting GC info diffs in an x86_arm_altjit diffs run; there are no codegen diffs. No x86 diffs. |
|
@dotnet-bot test Windows_NT x64 Checked gcstress0x3 |
| // | ||
| // This has the same functionality as the version of | ||
| // genUseOptimizedWriteBarriers that takes a WriteBarrierForm, but avoids | ||
| // determining what the required write barrier form is, if possible. |
There was a problem hiding this comment.
Since it's only in DEBUG that it has to check the WriteBarrierForm, it seems like it would be better just to always use this method. Then it's only in DEBUG that it will (in this method) recompute it.
There was a problem hiding this comment.
Seems reasonable; I'd need to change the args to genEmitOptimizedGCWriteBarrier(), which doesn't have the correct tgt value around. Maybe next time?
3 participants
When unrolling a STOREOBJ, we can generate multiple consecutive
byref helper calls. This helper has a unique calling convention
where the dst and src addresses are modified by adding pointer
size to their original value (thus allowing consecutive helper
calls without reloading the dst/src addresses). So, for liveness
purposes, the helper call kills the dst/src values. However, for
GC purposes, it does not, as the registers still contain byref
pointers. We were, in the ARM case, reporting the r0/r1 registers
dead after the first call, so a GC didn't update them, and a
second call updated garbage.
In fixing this, I cleaned up the helper call kill handling a bit.
I also fixed and improved RyuJIT/x86 write barrier kill modeling.