Uh oh!
There was an error while loading. Please reload this page.
[Wasm RyuJIT] Fix: Pass Call's struct type to Wasm ABI Classifier Instead of Dest Store Type - #133281
Conversation
…consumer's type in LowerStoreSingleRegCallStruct
|
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.
🟡 Changes recommended
The Wasm lowering path still asserts CORINFO_WASM_TYPE_VOID can’t occur (despite the interface contract defining it as “by-ref”), which can still abort crossgen2 for valid shapes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts Wasm-specific lowering for GT_STORE_BLK nodes sourced from single-register struct-returning calls by using the call’s return class handle (call->gtRetClsHnd) for Wasm ABI classification instead of using the store’s destination layout type.
Changes:
- Use
call->gtRetClsHndas the input togetWasmLowering(...)inLowerStoreSingleRegCallStruct. - Retype the store based on the call’s Wasm ABI-lowered primitive type rather than the destination’s layout classification.
File summaries
| File | Description |
|---|---|
| src/coreclr/jit/lower.cpp | Updates Wasm ABI classification source in LowerStoreSingleRegCallStruct to use the call return type. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
| CorInfoWasmType wasmAbiType = m_compiler->info.compCompHnd->getWasmLowering(call->gtRetClsHnd); | ||
| assert(wasmAbiType != CORINFO_WASM_TYPE_VOID); | ||
| regType = WasmClassifier::ToJitType(wasmAbiType); |
lewing
left a comment
There was a problem hiding this comment.
I've verified this fixes the linked issue and my read is that copilot's review is not correct.
I've also verified that the other wasm failures are known and in the set tracked by #133305 |
When we store the return value of a function returning a struct with a single reg return, the ABI classification may not match the destination's. For example,
with
V2 has a pass by ref ABI classification since it has two fields, whereas Vector64, the result of CreateVector(), has a by-value classification since Vector64 has only one field. We were using the struct ABI classification of the destination of the store in these cases instead of the return value of the call.
Resolves#133224