-
Notifications
You must be signed in to change notification settings - Fork 3
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
Adding macros to push pinning roots #43
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest not using 'relaxed' in the name. When I saw 'relaxed', I first thought about memory order. And it always takes an extra step to associate 'relaxed' with 'not transitively pin'. You can use a more direct naming, e.g. push_tpin
vs push_no_tpin
.
#define JL_GC_ENCODE_PUSH(n) ((((size_t)(n))<<3)|1) | ||
|
||
// these only pin the root object itself | ||
#define JL_GC_ENCODE_PUSHARGS_NO_TPIN(n) (((size_t)(n))<<3|4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if Julia documents this anywhere. It is a good idea to explain what those bits are used for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be three different functions:
#define JL_GC_ENCODE_PUSHARGS(n) (((size_t)(n))<<2)
#define JL_GC_ENCODE_PUSH(n) ((((size_t)(n))<<2)|1)
#define JL_GC_ENCODE_PUSHFRAME(n) ((((size_t)(n))<<2)|2)
I think they currently use 2 bits (<<2
). It seems to indicate what stuff you'd push onto the shadow stack, but I might ask around so we can document this properly.
I am wondering if we should merge this. We need the changes in this PR for the static analyser as well. @udesou |
…at are/are not transitively pinning
Sounds good to me. I've added a comment to explain how the bits work, in particular for transitively pinning roots. If you have no issues with the PR, then we can merge it. |
Supporting processing roots in the shadow stack that are not transitively pinned (only the root object is pinned), and therefore should enable moving more objects. NB: Should be merged with mmtk/julia#43.
Supporting pushing roots to the shadow stack that are not transitively pinned (only the root object is pinned), and therefore should enable moving more objects.
For backwards compatibility the original functions will be used to represent transitively pinned roots, whereas the
*_RELAXED
version will represent roots in which only the root object needs to be pinned. Therefore, to support moving move objects the goal is to replace as much as possible of the original functions with their_RELAXED
counterpart.