Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should JIT_UpdateWriteBarrierState be in the patchable section or not? #92554

Closed
filipnavara opened this issue Sep 24, 2023 · 0 comments · Fixed by #92555
Closed

Should JIT_UpdateWriteBarrierState be in the patchable section or not? #92554

filipnavara opened this issue Sep 24, 2023 · 0 comments · Fixed by #92555

Comments

@filipnavara
Copy link
Member

In the .S file (non-Windows) the function is defined outside of JIT_PatchedCodeStart block:

WRITE_BARRIER_ENTRY JIT_UpdateWriteBarrierState

In the .asm file (Windows) the function is defined inside the patchable block:

; ------------------------------------------------------------------
; Start of the writeable code region
LEAF_ENTRY JIT_PatchedCodeStart
ret lr
LEAF_END
;-----------------------------------------------------------------------------
; void JIT_UpdateWriteBarrierState(bool skipEphemeralCheck, size_t writeableOffset)
;
; Update shadow copies of the various state info required for barrier
;
; State info is contained in a literal pool at the end of the function
; Placed in text section so that it is close enough to use ldr literal and still
; be relocatable. Eliminates need for PREPARE_EXTERNAL_VAR in hot code.
;
; Align and group state info together so it fits in a single cache line
; and each entry can be written atomically
;
WRITE_BARRIER_ENTRY JIT_UpdateWriteBarrierState

Which one is correct?

Ref: #92520 (comment)

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 24, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 26, 2023
@mangod9 mangod9 removed the untriaged New issue has not been triaged by the area owner label Sep 28, 2023
@mangod9 mangod9 added this to the Future milestone Sep 28, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 30, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants