Skip to content

Conversation

radekdoulik
Copy link
Member

Fix #120092

@radekdoulik radekdoulik added this to the Future milestone Sep 26, 2025
@radekdoulik radekdoulik added the arch-wasm WebAssembly architecture label Sep 26, 2025
@Copilot Copilot AI review requested due to automatic review settings September 26, 2025 17:18
@radekdoulik radekdoulik requested review from jkotas and removed request for Copilot September 26, 2025 17:18
Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

@radekdoulik
Copy link
Member Author

This allows following code to run

TestCallingConvention0(1, 3, 5);

static void TestCallingConvention0(int a, int c, int e)
{
    Console.WriteLine("TestCallingConvention0: a = {0}, c = {1}, e = {2}", a, c, e);
}

Co-authored-by: Jan Kotas <[email protected]>
Co-authored-by: Aaron Robinson <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings September 26, 2025 19:13
Copy link
Contributor

@Copilot 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 fixes unboxing stubs on WebAssembly by restructuring the IL stub generation to work properly with portable entrypoints. The fix addresses issue #120092 by modifying how unboxing stubs are created and handled in the CoreCLR runtime.

  • Renamed and generalized the unboxing IL stub creation function to work with all value type methods, not just shared generic ones
  • Added proper handling for portable entrypoints in the unboxing stub code path
  • Added WebAssembly-specific assertion for unsupported shuffle array generation

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/coreclr/vm/prestub.cpp Core changes to unboxing stub generation, function renaming, and portable entrypoint handling
src/coreclr/vm/comdelegate.cpp Added WebAssembly assertion for unimplemented shuffle array generation
src/coreclr/clrdefinitions.cmake Updated feature flag condition to exclude portable shuffle thunks when portable entrypoints are enabled

Remove GenerateShuffleArray and s_pShuffleThunkCache for FEATURE_PORTABLE_ENTRYPOINTS
Handle all pStubs for FEATURE_PORTABLE_ENTRYPOINTS
@radekdoulik
Copy link
Member Author

radekdoulik commented Sep 26, 2025

I also added new thunk for the IL stub. I wonder if we can avoid that later. By replacing calli with call inside the stub?

The compiled stub

Compiled method: .(dynamicClass):IL_STUB_UnboxingStub(System.Span`1[char],byref,System.ReadOnlySpan`1[char],System.IFormatProvider)
Locals size 112
0004: IR_0000: initlocals     [nil <- nil], 48,0
0010: IR_0003: safepoint      [nil <- nil],
0014: IR_0004: mov.4          [48 <- 0],
0020: IR_0007: ldflda         [64 <- 48], 4
0030: IR_000b: mov.vt         [72 <- 8], 8
0040: IR_000f: mov.4          [80 <- 16],
004c: IR_0012: mov.vt         [88 <- 24], 8
005c: IR_0016: mov.4          [96 <- 32],
0068: IR_0019: ldc.i4         [48 <- nil], 77462664
0074: IR_001c: calli          [48 <- 64 48],0xbfa 
008c: IR_0022: ret            [nil <- 48],
End of method: 0094: IR_0024

@jkotas
Copy link
Member

jkotas commented Sep 28, 2025

/ba-g missing logs, opened #120174 and #120176 on failures with logs

@radekdoulik radekdoulik merged commit 6bbf352 into dotnet:main Sep 28, 2025
110 of 113 checks passed
// need to free the Stub allocation now.
pStub->DecRef();
}
#endif // !FEATURE_PORTABLE_ENTRYPOINTS
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#endif // !FEATURE_PORTABLE_ENTRYPOINTS
#endif // FEATURE_PORTABLE_ENTRYPOINTS

_ASSERTE(ilStubInterpData != NULL);
SetInterpreterCode((InterpByteCodeStart*)ilStubInterpData);
SetCodeEntryPoint(pCode);
#else
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#else
#else // FEATURE_PORTABLE_ENTRYPOINTS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-VM-coreclr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[wasm coreclr] unboxing stubs

3 participants