Fix GC hole with rorx instruction

[BruceForstall]

The rorx instruction uses format IF_RWR_ARD_CNS. This format was not handled when killing GC refs in emitOutputAM.

I added other cases of register write "ARD" address mode formats to the same case as "defense in depth" -- most or all of them are probably SIMD instructions with SIMD destination registers, and won't go down this code path.

Diffs include cases in the HardwareIntrinsics tests, but also in the libraries code, for both x64 and x86.

Fixes #114445

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/coreclr/jit/emitxarch.cpp:15334

• Ensure that the newly added case labels (IF_RWR_ARD_CNS, IF_RRW_ARD_CNS, etc.) correctly trigger the GC reference update as intended and that there are no side effects in the emitter's GC handling logic.

case IF_RWR_ARD_CNS:

[dotnet-policy-service]

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

[BruceForstall]

The bug was introduced with #111778, cc @saucecontrol

[BruceForstall]

@kunalspathak @tannergooding PTAL
cc @dotnet/jit-contrib @dotnet/avx512-contrib

[tannergooding]

This and other places likely shouldn't be switching on the format at all. The emitfmtsxarch.h entries (https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/emitfmtsxarch.h) all have relevant flags indicating whether given registers a read, write, or read-write (tracked via IS_INFO).

We should probably have this be, instead:

if (id->idIsReg1Write())
{
    emitGCregDeadUpd(id->idReg1(), dst);
}

if (id->idIsReg2Write())
{
    emitGCregDeadUpd(id->idReg2(), dst);
}

Technically we also define idIsReg3Write() and idIsReg4Write() for parity, but these are completely unused today and could be commented out.

Realistically we could be centralizing these checks for all instructions in some emitGCregDeadUpd(id, dst) helper given we don't need to be checking specific formats at all, which seems much more robust and less error prone.

[tannergooding]

LGTM as it appears to fix the issue, but I think in general this is still error prone and could be made significantly more robust if we centralized the GC updates for all emitted instructions using the IS_INFO available for each instrDesc

[BruceForstall]

I agree that a more robust, generalized checking refactoring would be good. It also seems weird to call emitInsCanOnlyWriteSSE2OrAVXReg instead of just checking the register number to determine if it's a GPR or not (and it seems bad that emitInsCanOnlyWriteSSE2OrAVXReg has a list of instructions that might write GPRs, which it already has a comment about generalizing).