JIT: Fix wasm ABI register type mismatch - #133510
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89a8f4e-a8a4-438c-ae2f-264cc6b2be41
github-actions
Bot
added
the
area-CodeGen-coreclr
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@dotnet/wasm-contrib FYI |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are targeted, preserve existing wasm RA invariants, and add a focused regression test covering the reported type-mismatch scenario.
Pull request overview
This PR fixes a WebAssembly ABI/register-allocation mismatch where a struct parameter could be treated as enregistered in its ABI parameter local even when the wasm value type didn’t match the local’s canonical register type, and adds a small regression to cover the scenario.
Changes:
- Update wasm reg allocation to only pin a parameter local to its ABI register when the wasm value types match; otherwise allocate a new local of the canonical type.
- Teach wasm prolog homing to bitcast (reinterpret) the incoming ABI-typed parameter local into the RA-assigned local when their types differ.
- Add a focused JitBlue regression test for a fixed-buffer wrapper containing a single
double.
File summaries
| File | Description |
|---|---|
| src/coreclr/jit/regallocwasm.cpp | Avoid forcing ABI-parameter register assignment when the wasm value type differs from the virtual/canonical register type. |
| src/coreclr/jit/codegenwasm.cpp | Add a centralized wasm bitcast instruction helper and use it in prolog parameter homing to reinterpret between ABI and local register types. |
| src/tests/JIT/Regression/JitBlue/Runtime_133502/Runtime_133502.csproj | New regression test project enabling unsafe (for fixed buffers) and optimized compilation. |
| src/tests/JIT/Regression/JitBlue/Runtime_133502/Runtime_133502.cs | New regression test exercising struct argument passing/return with a single-double fixed buffer. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8a89f1ed-34bb-452a-aefe-95d85e695c3b
|
@lewing can you re-approve (removed the test exclusion) |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes low-level wasm JIT register assignment and prolog code emission in ways that can subtly affect validation/codegen, so a human maintainer should do the final correctness/risk review.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
57 test failures all in mono testing. Unrelated. @lewing any suggestions as to how to handle these? Are we at some point going to disable mono testing? |
|
Failures are tracked in #133702 |
|
/ba-g known failures with mono testing |
Fixes #133502.
The wasm register allocator previously assigned an enregistered struct argument directly to its ABI parameter register even when their wasm value types differed. Keep the ABI parameter in its original register, allocate the local using its canonical register type, and reinterpret the value while homing parameters in the prolog.
Adds a focused regression test for a single-double fixed-buffer wrapper.
Note
This pull request description was generated with GitHub Copilot.