Repository navigation
JIT: Normalize small unmanaged args on wasm - #133511
Conversation
Fixes dotnet#133506 Co-authored-by: Copilot <[email protected]>
github-actions
Bot
added
the
area-CodeGen-coreclr
|
fyi @dotnet/wasm-contrib |
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 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 |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes wasm unmanaged ABI-related normalization in JIT import logic, and correctness depends on platform ABI/codegen semantics best validated by a human reviewer and targeted wasm/R2R testing results.
Pull request overview
This PR updates CoreCLR JIT argument-import logic to apply the existing “small unmanaged integer argument normalization” (zero/sign extension to 32-bit) when targeting WebAssembly, aligning wasm with the same handling already used for ARM32 and Apple ARM64 unmanaged calls.
Changes:
- Extend the unmanaged-call small-integer normalization condition to include
TargetArchitecture::IsWasm. - Add
TargetArchitecture::IsWasmto the shared target architecture feature flags to enable that check in the JIT codebase.
File summaries
| File | Description |
|---|---|
| src/coreclr/jit/importercalls.cpp | Applies small-integer widening/normalization for wasm unmanaged-call arguments (same normalization pattern as ARM32/Apple ARM64). |
| src/coreclr/inc/targetosarch.h | Introduces TargetArchitecture::IsWasm compile-time flag keyed off TARGET_WASM. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes wasm unmanaged-call ABI-related argument normalization in the JIT, which has broad behavioral impact and should get final confirmation from a human reviewer with wasm ABI/codegen context.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
/ba-g build insights seems stuck |
Fixes #133506
Wasm follows the Basic C ABI requirement that small integer arguments are extended to 32 bits. Include wasm in the existing unmanaged-call normalization used for ARM32 and Apple ARM64.
The existing
Runtime_101046test now generatesi32.load16_sinstead of passing the result ofi32.load16_udirectly to the native call.Note
This PR description was generated with GitHub Copilot.