Fix R2R detected layout issues#128275
Conversation
- There were two places in R2R which hardcoded an Arm check when they should be checking against SupportsAlign8
- GCRefMap/ArgIterator construction was fairly confused - Reflection invocation added a "ret buf parameter to the arg iteration that didn't respect 16 byte alignment correctly - Crossgen2 ArgIterator incorrectly merged in the WasmLowering code for struct args and returns
There was a problem hiding this comment.
Pull request overview
This PR adjusts WebAssembly R2R/interpreter calling-convention handling so layout, alignment, return buffers, and GC ref maps are computed consistently for Wasm-specific interpreter transition blocks.
Changes:
- Uses
SupportsAlign8instead of hardcoded ARM checks for layout/alignment decisions. - Separates Wasm interpreter transition-block layout from Wasm ABI struct lowering in ReadyToRun thunk generation.
- Updates reflection invocation and CallDescr handling so large Wasm struct returns use an explicit return buffer outside the normal argument stream.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/coreclr/vm/wasm/calldescrworkerwasm.cpp |
Uses explicit Wasm return-buffer metadata instead of rewriting argument slots. |
src/coreclr/vm/reflectioninvocation.cpp |
Computes Wasm return-buffer needs independently and passes the buffer separately. |
src/coreclr/vm/callingconvention.h |
Removes Wasm retbuf args from the normal ArgIterator stream. |
src/coreclr/vm/callhelpers.h |
Adds Wasm-specific return-buffer pointer storage. |
src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs |
Generalizes alignment layout logic from ARM to SupportsAlign8. |
src/coreclr/tools/Common/JitInterface/WasmLowering.cs |
Makes Wasm lowering partial for ReadyToRun-specific helpers. |
src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs |
Generalizes class alignment requirement from ARM to SupportsAlign8. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/WasmLowering.ReadyToRun.cs |
Adds helper to identify Wasm ABI byref-lowered struct arguments. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj |
Includes the new ReadyToRun Wasm lowering helper file. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmR2RToInterpreterThunkNode.cs |
Uses Wasm ABI lowering helper for indirect struct arguments. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmInterpreterToR2RThunkNode.cs |
Uses Wasm ABI lowering helper for interpreter-to-R2R argument reconstruction. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmImportThunk.cs |
Uses Wasm ABI lowering helper when saving/restoring import thunk arguments. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TransitionBlock.cs |
Makes Wasm TransitionBlock follow interpreter layout instead of ABI byref/retbuf lowering. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ArgIterator.cs |
Reworks Wasm argument offset computation to use interpreter stack alignment/layout. |
|
/ba-g test failures are #128293 |
AndyAyersMS
left a comment
There was a problem hiding this comment.
Looks reasonable to me, but I don't have all the context needed to do a review. Interpreter calling convention doesn't describe return value handling. Not sure if it's worth adding more there or not, since it's all a within-runtime issue.
|
@AndyAyersMS interpreter return values are ... not part of the calling convention. They are handled as buffers passed to the interpreter execution function and are an entire separate thing. |
Note
This PR description was generated with AI assistance.
This PR fixes two R2R layout issues:
SupportAlign8 on Wasm: Two places in R2R hardcoded an Arm check when they should be checking against SupportsAlign8. This now handles Wasm the same way as Arm.
Incorrectly constructed GCRefMap: GCRefMap/ArgIterator construction was confused — reflection invocation added a ret buf parameter to the arg iteration that didn't respect 16 byte alignment correctly, and Crossgen2 ArgIterator incorrectly merged in the WasmLowering code for struct args and returns.