-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add a getIndirectSize API to the JIT-EE interface to represent the correct size for ldobj/stobj operations #123740
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
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib |
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.
Pull request overview
This PR adds a new getClassIndirectSize API to the JIT-EE interface to support accurate indirect load/store operations for types with special layout characteristics, particularly Swift structs and enums. These types have different sizes when accessed indirectly (excluding trailing padding) versus their full stride (including padding).
Changes:
- Adds
getClassIndirectSizemethod to the JIT-EE interface (ICorStaticInfo) - Implements the new API in CoreCLR VM and NativeAOT (currently returning the same value as
getClassSizefor zero-diff behavior) - Updates JIT to use
IndirectSize()instead ofSize()for block operations (ldobj/stobj) - Adds full superpmi support for recording and replaying the new API call
- Updates JITEEVERSIONGUID to reflect the interface change
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/inc/corinfo.h | Adds virtual method declaration for getClassIndirectSize |
| src/coreclr/inc/jiteeversionguid.h | Updates GUID to reflect JIT-EE interface change |
| src/coreclr/inc/icorjitinfoimpl_generated.h | Adds override declaration |
| src/coreclr/vm/jitinterface.cpp | Implements getClassIndirectSize returning VMClsHnd.GetSize() (same as getClassSize); minor whitespace cleanup |
| src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp | Adds wrapper implementation |
| src/coreclr/jit/ICorJitInfo_names_generated.h | Adds API name for diagnostics |
| src/coreclr/jit/layout.h | Adds m_indirectSize field and GetIndirectSize() accessor |
| src/coreclr/jit/layout.cpp | Fetches and stores indirect size via new API call |
| src/coreclr/jit/gentree.h | Adds IndirectSize() method to GenTreeBlk |
| src/coreclr/jit/gentree.cpp | Updates GenTreeIndir::Size() to use IndirectSize() for block operations |
| src/coreclr/jit/valuenum.cpp | Uses IndirectSize() for value numbering block loads |
| src/coreclr/jit/lsra*.cpp | Updates BuildBlockStore to use IndirectSize() across all architectures |
| src/coreclr/jit/lower*.cpp | Updates LowerBlockStore to use IndirectSize() across all architectures; updates helper call lowering |
| src/coreclr/jit/codegenlinear.cpp | Uses IndirectSize() for setting block size in register |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | Implements getClassIndirectSize returning GetElementSize() (same as getClassSize) |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs | Adds generated callback and delegate for new API |
| src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt | Adds function signature for thunk generation |
| src/coreclr/tools/aot/jitinterface/jitinterface_generated.h | Adds callback and wrapper implementation |
| src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp | Adds replay support |
| src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp | Adds passthrough interceptor |
| src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp | Adds counting interceptor |
| src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp | Adds recording interceptor |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h | Adds method declarations and packet enum |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp | Implements record/dump/replay methods |
| src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h | Adds LightWeightMap entry for the new API |
|
This concept feels unnatural on the JIT side. Almost everything deals with this size while almost nothing deals with stride as the concept (mostly element address calculations?). Is there a reason why we cannot introduce stride as the concept? (Also, I expect we would not need a new JIT-EE API to compute the stride from the size and alignment.) |
| CORINFO_CLASS_HANDLE cls | ||
| ) = 0; | ||
|
|
||
| // return the number of bytes needed by an instance of the class when accessed indirectly |
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.
This comment should explain what "accessed indirectly" means exactly, an example would be nice.
We will use this to represent accurate indirect loads/stores for structs with ExtendedLayoutKind.SwiftStruct and ExtendedLayoutKind.SwiftEnum. These types have no trailing padding themselves, but their stride includes trailing padding.
I believe this is the easiest way to represent this without breaking requiring special handling for System.Span and the like.
Adding this in a separate PR to be a zero-diff change.
After this PR, I'll put out a PR to update the VMs to correctly handle layout (including recursive layout) for types with Swift layout.
Contributes to #100896