-
Notifications
You must be signed in to change notification settings - Fork 5k
Interpreter GC info stage 3: Report locals to the GC #114469
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
Conversation
Tagging subscribers to this area: @mangod9 |
Rebased and updated to report byrefs. Tweaked the test to explicitly use a ref local. |
Initial pass at handling structs containing refs. Will need to refactor it to not rely on lambdas. |
eeinterp.cpp) | ||
eeinterp.cpp | ||
stackmap.cpp | ||
../../native/containers/dn-simdhash.c |
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.
As discussed in offline conversation, it would be better to start reusing code from the regular JIT. There appears to be a quite a bit of code that can be shared between regular JIT and the interpreter JIT.
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'll create a 'jitshared' and move JitHashTable into it and use that, then.
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.
Moving JitHashTable is looking very time intensive, could I do it in a follow-up PR?
…ss ranges Use ConvertOffset and live range helpers to record actual byte offsets Fix globals not being reported Specialize GetStackSlot for the interpreter because we stash the actual stack location in the frame register Doing aggressive blocking GC produces earlier, more consistent failures Calculate stack slot addresses correctly, at least on x64 Disable problematic GC in the test for now Better debug spew and adjust test Fix nativeAOT build on CI Implement additional architectures in GetStackSlot based on CONTEXTGetFp Maybe fix CI build by using TARGET instead of HOST Updated guesses at fp register / more architectures Report and explicitly test byrefs Refactoring Maintain separate slot tables for byref and non-byref locals Handle allocation failure in simdhash Checkpoint support for byref and ref fields inside VT interpreter locals Add test coverage for structs containing refs Match getClassGClayout's size calculation Remove use of lambdas Fix release build Fix linux build Fix linux build more Use GetFP to get the frame pointer in interpreter's GcInfoDecoder Formatting cleanup Patch initlocals once we have allocated offsets so that we zero our vars in addition to IL vars (GC safety) Preserve effect of not initializing IL locals even when we expand the initlocals Introduce globalVarsStackTop, the high water mark for global vars that might contain refs or interior pointers Use globalVarsStackTop to reduce how much stack we zero on method entry Formatting cleanups Repair damage to simdhash files
Enabled the disabled sub-tests and made the other requested changes, thanks for the reviews. |
/ba-g Failures are on mono lanes which are unaffected by this PR. android-arm issue is #115130 (comment) and browser-wasm issues are known to build analysis |
This updates the interpreter compiler to use
GcInfoEncoder
to allocate slot IDs for interp vars and then report their liveness ranges (unless they're globals).I template-specialized the implementation of
TGcInfoDecoder<InterpreterGcInfoEncoding>::GetStackSlot
so that it knows how to find the interpreter stack and look up slots in it, since the default implementation isn't compatible with the way we store the interpreter SP.I've verified that when a GC happens inside TestFields, the object reference(s) on the interpreter stack are reported to the GC, and the GC is able to successfully move them and update the stack.
I had to modify the way we do INITLOCALS to ensure that any global vars containing pointers are zeroed at method entry, because we don't have accurate liveness information for them to report, otherwise the GC might see garbage data where it expects a managed pointer.
This PR also adds some basic test coverage for structs containing object references.