[Wasm] Write barriers#128225
Conversation
…ing convention on Wasm
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
Initial scaffolding to make the WASM portable write barriers (RhpAssignRef, RhpCheckedAssignRef, RhpByRefAssignRef) use a "raw" calling convention that matches what RyuJIT expects for write barriers, rather than the trampolined WASM FCALL convention that threads callersStackPointer / portableEntryPointContext through. The actual barrier implementations still assert and will be filled in later.
Changes:
- Add new
FCDECL2_RAW/FCIMPL2_RAWmacros that bypass the WASM stack-pointer/entrypoint-context plumbing and define the function with the plain(a1, a2)signature; provide non-WASM fallbacks that alias toFCDECL2/FCIMPL2. - Switch the three
Rhp*AssignRefdeclarations injitinterface.hand definitions inWriteBarriers.cppto the new_RAWvariants. - Trivial whitespace cleanup in
jitinterface.h.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/coreclr/vm/fcall.h |
Adds WASM-specific FCDECL2_RAW / FCIMPL2_RAW macros and non-WASM fallbacks aliasing to the standard FCDECL2 / FCIMPL2. |
src/coreclr/vm/jitinterface.h |
Switches Rhp{Checked,ByRef,}AssignRef declarations to FCDECL2_RAW; trims trailing whitespace. |
src/coreclr/runtime/portable/WriteBarriers.cpp |
Updates the three portable write-barrier stubs to use FCDECL2_RAW / FCIMPL2_RAW. |
| #define FCDECL1(rettype, funcname, a1) rettype F_CALL_CONV funcname(uintptr_t callersStackPointer, a1, PCODE portableEntryPointContext); rettype F_CALL_CONV funcname ## _IMPL(uintptr_t callersStackPointer, a1, PCODE portableEntryPointContext) | ||
| #define FCDECL1_V(rettype, funcname, a1) rettype F_CALL_CONV funcname(uintptr_t callersStackPointer, a1, PCODE portableEntryPointContext); rettype F_CALL_CONV funcname ## _IMPL(uintptr_t callersStackPointer, a1, PCODE portableEntryPointContext) | ||
| #define FCDECL2(rettype, funcname, a1, a2) rettype F_CALL_CONV funcname(uintptr_t callersStackPointer, a1, a2, PCODE portableEntryPointContext); rettype F_CALL_CONV funcname ## _IMPL(uintptr_t callersStackPointer, a1, a2, PCODE portableEntryPointContext) | ||
| #define FCDECL2_RAW(rettype, funcname, a1, a2) rettype F_CALL_CONV funcname(a1, a2); rettype F_CALL_CONV funcname(a1, a2) |
| #ifdef TARGET_WASM | ||
| *dst = ref; | ||
| JIT_WriteBarrier(dst, ref); | ||
| #else | ||
| PORTABILITY_ASSERT("RhpAssignRef is not yet implemented"); | ||
| #endif // TARGET_WASM | ||
| } | ||
| FCIMPLEND | ||
|
|
||
| EXTERN_C FCDECL2(VOID, RhpCheckedAssignRef, Object **dst, Object *ref); | ||
| FCIMPL2(VOID, RhpCheckedAssignRef, Object **dst, Object *ref) | ||
| EXTERN_C FCDECL2_RAW(VOID, RhpCheckedAssignRef, Object **dst, Object *ref); | ||
| FCIMPL2_RAW(VOID, RhpCheckedAssignRef, Object **dst, Object *ref) | ||
| { | ||
| #ifdef TARGET_WASM | ||
| *dst = ref; | ||
| JIT_CheckedWriteBarrier(dst, ref); | ||
| #else | ||
| PORTABILITY_ASSERT("RhpCheckedAssignRef is not yet implemented"); | ||
| #endif // TARGET_WASM | ||
| } | ||
| FCIMPLEND | ||
|
|
||
| EXTERN_C FCDECL2(VOID, RhpByRefAssignRef, Object **dst, Object *ref); | ||
| FCIMPL2(VOID, RhpByRefAssignRef, Object **dst, Object *ref) | ||
| EXTERN_C FCDECL2_RAW(VOID, RhpByRefAssignRef, Object **dst, Object **ref); | ||
| FCIMPL2_RAW(VOID, RhpByRefAssignRef, Object **dst, Object **ref) | ||
| { | ||
| #ifdef TARGET_WASM | ||
| Object *tmp = *ref; | ||
| *dst = tmp; | ||
| JIT_CheckedWriteBarrier(dst, tmp); |
| { | ||
| #ifdef TARGET_WASM | ||
| *dst = ref; | ||
| JIT_WriteBarrier(dst, ref); |
There was a problem hiding this comment.
We should just move the implementation to here, and delete it where it is currently. We do not want to pay for the extra call.
| EXTERN_C FCDECL2_RAW(VOID, RhpAssignRef, Object **dst, Object *ref); | ||
| FCIMPL2_RAW(VOID, RhpAssignRef, Object **dst, Object *ref) | ||
| { | ||
| #ifdef TARGET_WASM |
There was a problem hiding this comment.
These TARGET_WASM ifdefs should not be needed.
| #define HCIMPL1_RAW(rettype, funcname, a1) rettype F_CALL_CONV funcname(uintptr_t callersStackPointer, a1, PCODE portableEntryPointContext) { | ||
| #define HCIMPL1_V(rettype, funcname, a1) rettype F_CALL_CONV funcname(uintptr_t callersStackPointer, a1, PCODE portableEntryPointContext) { HCIMPL_PROLOG(funcname) | ||
| #define HCIMPL2(rettype, funcname, a1, a2) rettype F_CALL_CONV funcname(uintptr_t callersStackPointer, a1, a2, PCODE portableEntryPointContext) { HCIMPL_PROLOG(funcname) | ||
| #define HCIMPL2_RAW(rettype, funcname, a1, a2) rettype F_CALL_CONV funcname(uintptr_t callersStackPointer, a1, a2, PCODE portableEntryPointContext) { |
There was a problem hiding this comment.
I do not think you can drop callersStackPointer. It would be assuming that the C/C++ compiler does not spill anything to the stack. It is very fragile assumption.
If we wanted to drop callersStackPointer arg, we would have to implement these in wasm assembly to make sure that nothing is spilled.
There was a problem hiding this comment.
If we wanted to drop callersStackPointer arg, we would have to implement these in wasm assembly to make sure that nothing is spilled.
That is how I would expect these to be implemented. Inserting the SP argument is somewhat complex for the Jit in case of barriers, and it's worse CQ of course.
| Object *tmp = *ref; | ||
| *dst = tmp; | ||
| JIT_CheckedWriteBarrier(dst, tmp); |
There was a problem hiding this comment.
| Object *tmp = *ref; | |
| *dst = tmp; | |
| JIT_CheckedWriteBarrier(dst, tmp); | |
| JIT_CheckedWriteBarrier(dst, *ref); |
JIT_CheckedWriteBarrier does the ref->dst assignment.
There was a problem hiding this comment.
Interesting, thank you. I guess that's a difference between our WB and the nativeaot one.
| FCIMPL2_RAW(VOID, RhpAssignRef, Object **dst, Object *ref) | ||
| { | ||
| #ifdef TARGET_WASM | ||
| *dst = ref; |
| FCIMPL2_RAW(VOID, RhpCheckedAssignRef, Object **dst, Object *ref) | ||
| { | ||
| #ifdef TARGET_WASM | ||
| *dst = ref; |
Sufficient for the R2R'd version of this to run without crashing:
Currently the write barrier in
ConcatTwoStringsfails due to the calling convention being wrong and then once the calling convention is fixed, it fails because we didn't implement the write barriers yet.This PR:
FCDECL2_RAW/FCIMPL2_RAWmacrosRhpByRefAssignRef's signature being incorrect (missing*)TARGET_WASMTARGET_WASM(we were definingFEATURE_USE_ASM_GC_WRITE_BARRIERSbefore)