-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[Wasm RyuJit] Wasm abi classifier #123515
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
|
|
||
| TypeHandle th(structHnd); | ||
|
|
||
| bool useNativeLayout = false; |
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 version won't be "live" but is useful for testing. Was not sure about proper handling of native layout here.
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.
For native layout, you'd need to use GetNativeLayoutInfo() when IsBlittable() is false and run the same classification algorithm on the NativeFieldDescriptor objects that represent the fields of the native layout instead of the FieldDesc pointers.
When IsBlittable() is true, just use the managed layout as the native layout.
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.
How does this look in the managed type system?
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.
In the managed type system, we're able to represent the "native" struct layout with a separate derived type of MetadataType and separate derived FieldDesc types as the representations of types and fields are not tied to metadata directly.
For comparison here, I'd recommend looking at the SystemV classification logic in methodtable.cpp (ClassifyEightBytesWithNativeLayout)
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.
Thanks. Is there a preference for making all these classifiers be method table methods?
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.
Honestly I don't like that the SystemV ones are on MethodTable, so I'd be happy to have them live somewhere else
| case CORINFO_TYPE_PTR: | ||
| case CORINFO_TYPE_BYREF: | ||
| case CORINFO_TYPE_CLASS: | ||
| // native int on 32 bit platform in 64 bit struct? |
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.
Similarly for 4 byte pointers passed as single fields of 8 byte structs
|
FYI @dotnet/jit-contrib The Wasm C ABI spec is here: https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md. Clang's implementation: https://github.com/llvm/llvm-project/blob/711e8e56943c0188b037f3a9a42905d071949c90/clang/lib/CodeGen/Targets/WebAssembly.cpp#L99 Not yet, or not properly, handling:
|
Add host (vm, crossgen) logic to classify structs per the Wasm Basic C ABI.
Add calls in the JIT when targeting Wasm; also add extra calls under SPMI when not targeting Wasm, so we can continue to use cross-jitting to help test Wasm bring-up.