Interpreter GC info stage 3: Report locals to the GC #114469
Conversation
ghost
added
the
|
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.
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.
I'll create a 'jitshared' and move JitHashTable into it and use that, then.
There was a problem hiding this comment.
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
GcInfoEncoderto allocate slot IDs for interp vars and then report their liveness ranges (unless they're globals).I template-specialized the implementation of
TGcInfoDecoder<InterpreterGcInfoEncoding>::GetStackSlotso 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.