Skip to content

Conversation

@AndyAyersMS
Copy link
Member

Here I've tried to fix things that were not NYI's in a replay of the x86 crossgen MCH.

python superpmi.py replay -filter crossgen -target_arch wasm -mch_arch x86

Copilot AI review requested due to automatic review settings December 18, 2025 00:39
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 18, 2025
@AndyAyersMS
Copy link
Member Author

@dotnet/jit-contrib PTAL

Binning of failures (with this PR) shows mostly NYIs are left:

Assertion #1/21 (count: 117635): NYI_WASM: gtMarkAddrMode (C:\repos\runtime3\src\coreclr\jit\gentree.cpp:4765)
Assertion #2/21 (count: 43914): abiInfo.NumSegments == 1 (C:\repos\runtime3\src\coreclr\jit\lower.cpp:1624)
Assertion #3/21 (count: 931): NYI_WASM: LowerBlockStore (C:\repos\runtime3\src\coreclr\jit\lowerwasm.cpp:174)
Assertion #4/21 (count: 2649): NYI_WASM: ins_Load (C:\repos\runtime3\src\coreclr\jit\instr.cpp:2113)
Assertion #5/21 (count: 11999): NYI_WASM: Cast costing (C:\repos\runtime3\src\coreclr\jit\gentree.cpp:5449)
Assertion #6/21 (count: 267): (gtRegTag == GT_REGTAG_NONE) || (reg >= REG_FIRST && reg <= REG_COUNT) (C:\repos\runtime3\src\coreclr\jit\gentree.h:953)
Assertion #7/21 (count: 458): IND (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:331)
Assertion #8/21 (count: 53): varTypeIsI(genActualType(indir->Addr())) (C:\repos\runtime3\src\coreclr\jit\gentree.cpp:8759)
Assertion #9/21 (count: 105): INTRINSIC (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:331)
Assertion #10/21 (count: 199): STOREIND (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:331)
Assertion #11/21 (count: 4): NYI_WASM: Overflow checks (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:461)
Assertion #12/21 (count: 3655): NYI_WASM: WasmClassifier::Classify - structs (C:\repos\runtime3\src\coreclr\jit\targetwasm.cpp:51)
Assertion #13/21 (count: 37): NYI_WASM: isContainableMemoryOp (C:\repos\runtime3\src\coreclr\jit\regallocwasm.cpp:51)
Assertion #14/21 (count: 6): NYI_WASM: Overflow checks (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:478)
Assertion #15/21 (count: 6): !"Unhandled must expand intrinsic, throwing PlatformNotSupportedException" (C:\repos\runtime3\src\coreclr\jit\importercalls.cpp:5084)
Assertion #16/21 (count: 6): NYI_WASM: Overflow checks (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:456)
Assertion #17/21 (count: 5): NYI_WASM: Overflow checks (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:490)
Assertion #18/21 (count: 2): NYI_WASM: Overflow checks (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:495)
Assertion #19/21 (count: 10): NYI_WASM: Overflow checks (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:473)
Assertion #20/21 (count: 2): CAST (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:331)
Assertion #21/21 (count: 3): MEMORYBARRIER (C:\repos\runtime3\src\coreclr\jit\codegenwasm.cpp:331)

Not focused on success rates just yet as we know things are still incomplete. While fixing some of these will just result in failures popping up elsewhere, we should focus on fixing the biggest bins first when possible.

The abiInfo.NumSegments == 1 assert reflects the fact that we don't have a way to describe args that are passed on the Wasm stack (we just have "registers" and "stack").

varTypeIsI(genActualType(indir->Addr())) is probably something similar to much what I tried to fix here, confusion about a 32 bit target that supports 64 bit ints.

Some of the patterns in the fixes below may indicate things we should reconsider, eg perhaps we ought to do the work to remove GT_CNS_LNG. IsCnsIntOrI() is going to be one thing to review carefully, as this fails for Wasm long constants.

The handful of bailouts are also food for thought. For example, optimizing a divide via magic shifts and adds may not be worth the trouble, if it increases code size and the underlying engine can do something similar.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses various issues encountered when running WASM JIT compilation on x86 crossgen MCH, focusing on fixing cases where the code didn't handle WASM's unique characteristics (32-bit pointers with 64-bit integer operations).

Key Changes

  • Updated constant value accessors to use IntegralValue() instead of type-specific methods for broader type support
  • Added TARGET_WASM guards alongside TARGET_64BIT checks to prevent WASM from using 32-bit-only helper call paths for 64-bit arithmetic operations
  • Added LIR-aware code insertion in WASM flow graph transformation to support both statement-based and LIR phases

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/coreclr/jit/valuenum.cpp Changed from LngValue() to IntegralValue() for accessing long constants, enabling consistent handling across different integer constant types
src/coreclr/jit/morph.cpp Added TARGET_WASM guards to conditional blocks that previously only checked TARGET_64BIT, preventing WASM from incorrectly using helper calls for long arithmetic; added early exit in fgOptimizeMultiply for TYP_LONG on WASM; updated fgMorphReduceAddOps guard
src/coreclr/jit/lower.cpp Changed from GenTreeIntCon to GenTreeIntConCommon and from IconValue() to IntegralValue() for broader type support; added TARGET_WASM guards for argument lowering; added early return in constant division/modulo optimization for WASM; fixed register type selection for WASM to use TYP_LONG instead of TYP_I_IMPL
src/coreclr/jit/gentree.h Added TARGET_WASM to the conditional that determines when to use indirection cells for R2R calls
src/coreclr/jit/fgwasm.cpp Added LIR-aware code insertion that checks if a block is in LIR form and uses appropriate insertion methods (LIR::Range vs Statement-based) for both control variable assignments and switch dispatch creation

@SingleAccretion SingleAccretion self-requested a review December 18, 2025 16:24
@am11 am11 added the arch-wasm WebAssembly architecture label Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants