Remove x86 per-source-register write barrier helpers#128797
Closed
EgorBo wants to merge 2 commits into
Closed
Conversation
Match what x64 and other targets already do: always emit a call to the unified CORINFO_HELP_ASSIGN_REF / CORINFO_HELP_CHECKED_ASSIGN_REF helper instead of selecting between per-source-register specialized helpers (CORINFO_HELP_ASSIGN_REF_EAX, etc.). The runtime universal helper already has the narrow ABI: ecx = dst, edx = src, only eax/edx trashed, all callee-saved registers and ecx preserved. The JIT continues to use RBM_CALLEE_TRASH_WRITEBARRIER (RBM_EAX | RBM_EDX) as the kill set, so it knows what's preserved across the call. This follows the same JIT-only first-step pattern as dotnet#128542 (Remove CORINFO_HELP_ASSIGN_BYREF). The enum values, jithelpers.h, readytorunhelpers.h, R2R format, runtime asm helpers (jithelp.asm/S, WriteBarriers.asm/S), jitinterfacex86.cpp, excep.cpp, and AOT tools are intentionally left alone for a follow-up cleanup similar to dotnet#128687. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the per-source-register x86 write barrier infrastructure (CORINFO_HELP_ASSIGN_REF_<reg> / CORINFO_HELP_CHECKED_ASSIGN_REF_<reg> and the matching Rhp*Reg, JIT_WriteBarrier<reg>, READYTORUN_HELPER_WriteBarrier_<reg> helpers) so x86 codegen always uses the unified CORINFO_HELP_ASSIGN_REF / CORINFO_HELP_CHECKED_ASSIGN_REF with a fixed ECX = dst, EDX = src ABI, matching x64. JIT-EE GUID and R2R major version are bumped to reflect the breaking change.
Changes:
- Delete per-register helpers from CorInfo, jithelpers, ReadyToRun, AOT tools, CoreCLR i386 asm/cpp, NativeAOT i386 asm and EH tables.
- Collapse JIT write-barrier lowering on x86 to the unified path (
genUseOptimizedWriteBarriers,NOGC_WRITE_BARRIERS, per-arch defines, andgenEmitOptimizedGCWriteBarrierall removed). - Bump
JITEEVersionIdentifier,READYTORUN_MAJOR_VERSIONandMINIMUM_READYTORUN_MAJOR_VERSIONfrom 19 to 20; mark the old R2R helper IDs as unused.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/inc/corinfo.h, tools/Common/JitInterface/CorInfoHelpFunc.cs | Remove CORINFO_HELP_(CHECKED_)ASSIGN_REF_<reg> enum entries |
| src/coreclr/inc/jithelpers.h | Drop x86 per-register Rhp(Checked)AssignRef<reg> mappings |
| src/coreclr/inc/jiteeversionguid.h | Bump JIT/EE GUID |
| src/coreclr/inc/readytorun.h, readytorunhelpers.h, tools/Common/Internal/Runtime/ReadyToRunConstants.cs, ModuleHeaders.cs, nativeaot/Runtime/inc/ModuleHeaders.h | Bump R2R version to 20 and annotate retired x86 helper IDs |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs, ILCompiler.ReadyToRun/.../CorInfoImpl.ReadyToRun.cs, ILCompiler.RyuJit/.../CorInfoImpl.RyuJit.cs, ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs | Remove AOT/R2R mappings + display names for the deleted helpers |
| src/coreclr/jit/codegencommon.cpp, codegeninterface.h, codegen.h, codegenxarch.cpp, lsrabuild.cpp, utils.cpp | Drop genUseOptimizedWriteBarriers / genEmitOptimizedGCWriteBarrier / RBM_OPTIMIZED_WRITE_BARRIER_* and per-register kill-sets; route x86 through unified helper |
| src/coreclr/jit/targetx86.h, targetamd64.h, targetarm.h, targetarm64.h, targetloongarch64.h, targetriscv64.h, targetwasm.h | Remove NOGC_WRITE_BARRIERS and RBM_OPTIMIZED_WRITE_BARRIER_* defines; update x86 write-barrier ABI comments |
| src/coreclr/vm/jitinterface.h, threads.cpp, i386/jithelp.asm, i386/jithelp.S, i386/jitinterfacex86.cpp | Keep only the EAX template; simplify init/patch/stomp logic; remove per-register init loop |
| src/coreclr/vm/excep.cpp, nativeaot/Runtime/EHHelpers.cpp | Drop x86 per-register Rhp(Checked)AssignRef<reg>AVLocation AV tables |
| src/coreclr/runtime/i386/WriteBarriers.asm, WriteBarriers.S | Remove the per-source-register DEFINE_(CHECKED_)WRITE_BARRIER EDX, <reg> instantiations |
| docs/design/coreclr/botr/readytorun-format.md | Annotate retired x86 R2R write-barrier helper IDs |
This was referenced May 30, 2026
Member
Member
Author
The motiviation is simplification if the diffs imply it's not worth having multiple versions of WB and +700 LOC. My assumption was "if we don't do this on any other arch - maybe it's not worth it?" I'm fine keeping this open till other PRs you listed land. |
EgorBo
force-pushed
the
egor/x86-wb-cleanup
branch
from
75d9d11 to
6fcdf3b
Compare
EgorBo
force-pushed
the
egor/x86-wb-cleanup
branch
from
6fcdf3b to
69d63c6
Compare
… code Follow-up to the prior JIT-only cleanup. Removes the now-dead infrastructure for the per-source-register x86 write barriers across the JIT-EE interface, R2R format, AOT tools (crossgen2 / NativeAOT compiler), and the CoreCLR + NativeAOT runtimes. Changes: - JIT-EE interface: remove the 12 CORINFO_HELP_(CHECKED_)ASSIGN_REF_E[reg] enum values from corinfo.h, jithelpers.h, and the managed mirror; bump JITEEVersionIdentifier; remove the X86 case-label block in utils.cpp's HelperCallProperties::init. - R2R format: bump major version to 20; mark slots 0x100-0x10B (READYTORUN_HELPER_(Checked)WriteBarrier_E[reg]) as unused (kept for image-compat parsing); update botr/readytorun-format.md, ModuleHeaders.h/cs, ReadyToRunConstants.cs; remove HELPER() lines in readytorunhelpers.h. - AOT tools: remove switch cases in CorInfoImpl.RyuJit.cs, CorInfoImpl.ReadyToRun.cs, ReadyToRunSignature.cs, and JitHelper.cs. - NativeAOT runtime: remove the per-register DEFINE_(CHECKED_)WRITE_BARRIER invocations and the corresponding RhpAssignRefE[reg]AVLocation references in EHHelpers.cpp. - CoreCLR x86 VM: remove all per-register variants except EAX from vm/i386/jithelp.asm/.S (EAX is kept as the patched target of the universal @JIT_WriteBarrier@8 trampoline). Drop the never-called @JIT_CheckedWriteBarrier@8 trampoline entirely (x86's CORINFO_HELP_CHECKED_ASSIGN_REF maps to RhpCheckedAssignRef, not JIT_CheckedWriteBarrier). Simplify the c_rgWriteBarriers/c_rgDebugWriteBarriers tables and patching loops in vm/i386/jitinterfacex86.cpp to operate on the single EAX entry. Drop the dead per-reg AV location declarations from vm/excep.cpp and the X86_WRITE_BARRIER_REGISTER macro in vm/threads.cpp and vm/jitinterface.h. Validation: clr+libs builds cleanly for x86 and x64; clr.tools and clr.nativeaotruntime build cleanly; x64 GC write-barrier exercising test passes with the rebuilt JIT (exit 100). The same x86 e2e test cannot be run on this machine due to a pre-existing testhost startup issue that affects baseline main as well. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
EgorBo
force-pushed
the
egor/x86-wb-cleanup
branch
from
69d63c6 to
d9ac263
Compare
Member
Author
|
Seems like it's quite a big size regression (obviously, I did expect it, but not that big): cc @jkotas @dotnet/jit-contrib I am not sure what that is telling us for x64/arm64 whether we want to do that for those |
Member
Author
Example codegen regression
; Assembly listing for method Newtonsoft.Json.Converters.XDocumentTypeWrapper:.ctor(System.Xml.Linq.XDocumentType):this (FullOpts)
; Emitting BLENDED_CODE for x86 + VEX + EVEX on Windows
; FullOpts code
; optimized code
; esp based frame
; partially interruptible
; No PGO data
; 0 inlinees with PGO data; 2 single block inlinees; 0 inlinees without PGO data
; Final local variable assignments
;
-; V00 this [V00,T00] ( 4, 4 ) ref -> ecx this class-hnd single-def <Newtonsoft.Json.Converters.XDocumentTypeWrapper>
-; V01 arg1 [V01,T01] ( 4, 4 ) ref -> eax class-hnd single-def <System.Xml.Linq.XDocumentType>
+; V00 this [V00,T00] ( 4, 4 ) ref -> [esp+0x00] this class-hnd single-def <Newtonsoft.Json.Converters.XDocumentTypeWrapper>
+; V01 arg1 [V01,T01] ( 4, 4 ) ref -> esi class-hnd single-def <System.Xml.Linq.XDocumentType>
;
-; Lcl frame size = 0
+; Lcl frame size = 4
G_M23744_IG01:
- mov eax, edx
- ;; size=2 bbWeight=1 PerfScore 0.25
+ push esi
+ push eax
+ mov esi, edx
+ ;; size=4 bbWeight=1 PerfScore 2.25
G_M23744_IG02:
- lea edx, bword ptr [ecx+0x04]
- call CORINFO_HELP_ASSIGN_REF_EAX
- lea edx, bword ptr [ecx+0x08]
- call CORINFO_HELP_ASSIGN_REF_EAX
- ;; size=16 bbWeight=1 PerfScore 3.00
+ mov gword ptr [esp], ecx
+ lea ecx, bword ptr [ecx+0x04]
+ mov edx, esi
+ call CORINFO_HELP_ASSIGN_REF
+ mov ecx, gword ptr [esp]
+ lea ecx, bword ptr [ecx+0x08]
+ mov edx, esi
+ call CORINFO_HELP_ASSIGN_REF
+ ;; size=26 bbWeight=1 PerfScore 5.50
G_M23744_IG03:
+ pop ecx
+ pop esi
ret
- ;; size=1 bbWeight=1 PerfScore 1.00
+ ;; size=3 bbWeight=1 PerfScore 2.00
-; Total bytes of code 19, prolog size 0, PerfScore 4.25, instruction count 6, allocated bytes for code 19 (MethodHash=0332a33f) for method Newtonsoft.Json.Converters.XDocumentTypeWrapper:.ctor(System.Xml.Linq.XDocumentType):this (FullOpts)
+; Total bytes of code 33, prolog size 2, PerfScore 9.75, instruction count 14, allocated bytes for code 35 (MethodHash=0332a33f) for method Newtonsoft.Json.Converters.XDocumentTypeWrapper:.ctor(System.Xml.Linq.XDocumentType):this (FullOpts)Summary
Why it regressed
|
Member
Author
|
I wonder if it's mostly just the typical |
Member
Author
|
I've modified the JIT to assume that whatever CC |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the per-source-register x86 write barrier helpers (
CORINFO_HELP_ASSIGN_REF_EAXetc.) and the related dead infrastructure across the JIT-EE interface, R2R format, AOT tools, and the CoreCLR + NativeAOT runtimes. The JIT now always emits the unifiedCORINFO_HELP_ASSIGN_REF/CORINFO_HELP_CHECKED_ASSIGN_REFon x86, matching x64.Bumps JITEEVersionIdentifier and R2R major version (19 → 20).
Note
This pull request was created with help from GitHub Copilot.