Skip to content

JIT: don't kill FP/SIMD/mask regs across x64 write barriers#128778

Open
EgorBo wants to merge 3 commits into
dotnet:mainfrom
EgorBo:jit-wb-preserve-fp
Open

JIT: don't kill FP/SIMD/mask regs across x64 write barriers#128778
EgorBo wants to merge 3 commits into
dotnet:mainfrom
EgorBo:jit-wb-preserve-fp

Conversation

@EgorBo
Copy link
Copy Markdown
Member

@EgorBo EgorBo commented May 29, 2026

{7366C871-08EB-48F1-897F-021AE10303FC}

What I deliberately did not do

  • Did not exclude RCX/RDI (dst) from the kill set — the asm helpers shift dst in place (shr rcx, 0Bh), so it must be considered clobbered. Preserving dst would require rewriting all ~20 asm variants.
  • Did not exclude RDX/RSI (src) — the Region variants destroy src.
  • Did not exclude R10/R11 on Windows even though the default patched-slot path doesn't touch them. Kept conservative to cover _DEBUG runtime variant (JIT_WriteBarrier_Debug) and the DOTNET_UseGCWriteBarrierCopy=0 config (RhpAssignRef), both of which touch R10/R11.

PS: We probably can do this for APX cc @dotnet/intel

I think it would be nice to preserve dst register unchanged (we do that for arm64), but that requires some changes in the ASM helpers.

The x64 JIT_WriteBarrier / JIT_CheckedWriteBarrier helpers (and all the
patched-slot variants: PreGrow/PostGrow/SVR/Region + WriteWatch flavors)
never execute any SSE/AVX/AVX-512/EVEX-mask instruction. So XMM/YMM/ZMM
and K mask registers can stay live across a write barrier call.

Narrow RBM_CALLEE_TRASH_WRITEBARRIER (and the matching GCTRASH mask)
from the full RBM_CALLEE_TRASH down to RBM_INT_CALLEE_TRASH_INIT on
amd64. Integer callee-trash regs remain conservatively in the kill set
to keep _DEBUG runtime builds and DOTNET_UseGCWriteBarrierCopy=0 working.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 29, 2026 16:56
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 29, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR narrows the amd64 JIT write-barrier register kill masks so FP/SIMD/mask registers can remain live across CORINFO_HELP_ASSIGN_REF and CORINFO_HELP_CHECKED_ASSIGN_REF, matching the documented helper ABI and reducing unnecessary spills.

Changes:

  • Documents the amd64 write-barrier helper register preservation/clobbering contract.
  • Changes amd64 write-barrier trash and GC-trash masks from full callee-trash to integer-only initial callee-trash registers.
  • Keeps conservative integer clobbers for runtime/debug write-barrier variants.

@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented May 29, 2026

@MihuBot

@VSadov
Copy link
Copy Markdown
Member

VSadov commented May 30, 2026

Will this require write barriers that need to call into C++ code on some paths to spill more around the calls?

For context:
https://github.com/dotnet/runtimelab/blob/97a242e8b9a66df1f9de9ff16fc24c42035bcc98/src/coreclr/runtime/amd64/WriteBarriers.asm#L517-L526

@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented May 30, 2026

Will this require write barriers that need to call into C++ code on some paths to spill more around the calls?

For context: https://github.com/dotnet/runtimelab/blob/97a242e8b9a66df1f9de9ff16fc24c42035bcc98/src/coreclr/runtime/amd64/WriteBarriers.asm#L517-L526

Probably not since we cannot guarantee what is happening in a code compiled by C++. But if that is an issue we need a new JIT-EE API, because we already precisely track registers used by WB on arm64 today (and just removing that will lead to quite big regressions) and we've always been doing that for byref WB that I deleted recently

Copilot AI review requested due to automatic review settings May 30, 2026 01:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/coreclr/jit/targetamd64.h
@@ -195,10 +214,12 @@
#define RBM_CALLEE_TRASH_NOGC RBM_CALLEE_TRASH
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other functions that might be worth checking or specializing in emitGetGCRegsKilledByNoGCCall

Copy link
Copy Markdown
Member Author

@EgorBo EgorBo May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking about CORINFO_HELP_JIT_PINVOKE_BEGIN at very least (might be useful for R2R/NAOT)


// Registers no longer containing GC pointers after CORINFO_HELP_ASSIGN_REF and CORINFO_HELP_CHECKED_ASSIGN_REF.
#define RBM_CALLEE_GCTRASH_WRITEBARRIER RBM_CALLEE_TRASH_NOGC
#define RBM_CALLEE_GCTRASH_WRITEBARRIER RBM_INT_CALLEE_TRASH_INIT
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not as critical, but I'd presume the same guarantee applies to x86?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants