[wasm] Model class-init helper as value-returning in the JIT#130813
[wasm] Model class-init helper as value-returning in the JIT#130813lewing wants to merge 5 commits into
Conversation
|
Azure Pipelines: Successfully started running 5 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 'arch-wasm': @lewing, @pavelsavara |
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR JIT IR modeling on TARGET_WASM so the class-init helpers (CORINFO_HELP_INITCLASS / CORINFO_HELP_INITINSTCLASS) are treated as value-returning (TYP_I_IMPL) rather than TYP_VOID, to align wasm indirect-call signature expectations with the helper’s return arity.
Changes:
- Model
CORINFO_HELP_INITCLASSasTYP_I_IMPLon wasm infgGetStaticsCCtorHelper. - Model
CORINFO_HELP_INITCLASS/CORINFO_HELP_INITINSTCLASSasTYP_I_IMPLon wasm at helper creation sites inimpInitClassandfgInitThisClass. - Add explanatory comments describing the wasm-specific return-arity rationale.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/coreclr/jit/flowgraph.cpp | Changes wasm helper return type modeling for CORINFO_HELP_INITCLASS in fgGetStaticsCCtorHelper, with added rationale comments. |
| src/coreclr/jit/importer.cpp | Models the runtime-lookup InitClass helper call as value-returning on wasm in impInitClass. |
| src/coreclr/jit/morph.cpp | Models the three generic-lookup fgInitThisClass helper call cases as value-returning on wasm. |
…the JIT Completes gate #2 (91d9e36). That change made corelib InitClass/ InitInstantiatedClass (InitHelpers.cs) return void* so the compiled call_indirect signature matches the interpreter's value-returning portable entry point convention, but the JIT caller side still typed CORINFO_HELP_INITCLASS/INITINSTCLASS as TYP_VOID. On wasm the R2R caller therefore emitted a (...)->() call_indirect while the runtime dispatches the cold class-init through the i32-returning thunk, trapping with "function signature mismatch" on the first R2R class-init. Model the class-init helpers as value-returning (TYP_I_IMPL) on wasm at the five sites that create them (fgGetStaticsCCtorHelper, impInitClass runtime lookup, and the three fgInitThisClass generic-lookup cases). The helper result is pushed and discarded via the existing unused-value drop in wasm codegen. Fixes the residual overflow bucket of dotnet#129622 (conv_ovf, 127931, indexMinusOne now pass under R2R in both interpreted- and R2R-corelib configs) and removes the class-init signature-mismatch trap in dotnet#130634. No regression in the previously passing type-load tests. Prototype-only; for the future WASI R2R PR. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2902374a-f352-4c46-bcec-8a35c97733ae
6acd714 to
80ed48b
Compare
|
Thanks — both points addressed in the pushed update (also refactored the per-site
Locally validated with both halves present: the residual overflow bucket of #129622 ( Note This comment was generated by GitHub Copilot (AI). |
|
My speculative "get SPC working" change set fixed this on the helper side, by retyping src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/InitHelpers.cs Doing that seems a bit simpler to me? |
This pr is meant to stack on top of your branch. The diagnosis is that your callee side change requires this caller side change to avoid the call_indirect type mismatch. Will double check. |
|
Thanks Andy — your I ran a clean A/B on
The reason the helper-side change alone isn't enough: the JIT models helper return types explicitly — Note This comment was generated by GitHub Copilot (AI). |
|
With this on top of your speculative branch I get past the call_indirect problems and land on some exception handling issues we're looking into. |
the eh issue is looking like a non-composite cross module r2r eh bug or an incomplete reloc pass |
Per review feedback, condense the HelperInitClassRetType comment to the essential point (on wasm these helpers return void*, so model them as value-returning to match the callee) plus the CoreLib-dependency note. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2902374a-f352-4c46-bcec-8a35c97733ae
figured out the eh issue, r2r functional on the branch. issue/pr incoming |
|
depends on #131119 landing first |
|
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. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
On wasm, the class-init JIT helpers (
CORINFO_HELP_INITCLASS/CORINFO_HELP_INITINSTCLASS) are dispatched through portable entry points to the managedInitClass/InitInstantiatedClasshelpers (InitHelpers.cs). Those helpers were changed to returnvoid*(a discarded dummy value) so the compiledcall_indirectsignature matches the interpreter's value-returning portable-entry-point convention — but the JIT caller side was never updated and still typed these helpers asTYP_VOID.As a result, R2R-compiled wasm code emits a
(...)->()call_indirectfor the first class-init of a type, while the runtime dispatches the cold class-init through the i32-returning thunk. The wasm engine traps withfunction signature mismatch.This change models the class-init helpers as value-returning (
TYP_I_IMPL) on wasm at the five sites that create them:fgGetStaticsCCtorHelper(the shared-cctor path used by non-generic methods)impInitClass(runtime-lookup path)fgInitThisClassgeneric-lookup cases (THISOBJ,CLASSPARAM,METHODPARAM)The helper result is pushed and then discarded via the existing unused-value
dropin wasm codegen. The change is guarded by#ifdef TARGET_WASM; other targets are unaffected.This is the JIT caller-side half of the class-init-helper return-arity change. It depends on the corelib half —
InitClass/InitInstantiatedClassreturningvoid*inInitHelpers.cs— which is not yet onmain(it is part of the in-progress wasm R2R / SPC bring-up work, e.g.AndyAyersMS:next-blocker-spc). This PR is not correct in isolation without that corelib change: the JIT would model the helper as i32-returning while the helper still returned void. The two halves must land together (or this should be folded into the SPC bring-up set).Opening as draft for human review of the JIT modeling change. There is currently no wasm-R2R CI leg, so validation below is from a local sweep.
Validation (local, browser wasm R2R)
Built the fixed cross-JIT, re-crossgen'd, and ran under
corerun.jswithDOTNET_ReadyToRun=1:conv_ovf,127931,indexMinusOne(interpreted corelib)conv_ovf(R2R corelib)b491215,GitHub_13561(type-load)This resolves the residual overflow bucket of #129622 and removes the class-init signature-mismatch trap in #130634 (that repro now progresses into managed execution and surfaces a separate, unrelated
NullReferenceException).Contributes to #129622 / #130634.
Note
This PR was created with assistance from GitHub Copilot (AI).