Skip to content

Conversation

radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Sep 30, 2025

Handle IL unbox helpers

Split unbox.any into 2 opcodes, by adding unbox.end

Add calli to make interpolated strings work

Handle IL unbox helpers
@radekdoulik radekdoulik added this to the Future milestone Sep 30, 2025
@Copilot Copilot AI review requested due to automatic review settings September 30, 2025 15:07
@radekdoulik radekdoulik added arch-wasm WebAssembly architecture area-VM-coreclr labels Sep 30, 2025
Copy link
Contributor

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

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 the INTOP_UNBOX_ANY instruction in the WebAssembly interpreter by adding support for IL unbox helpers. The key changes enable the interpreter to handle cases where unbox operations need to call IL methods rather than native helpers.

  • Adds support for executing IL target methods during unbox operations
  • Introduces a callback mechanism to handle post-execution cleanup
  • Ensures proper initialization of new interpreter frame objects

Reviewed Changes

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

File Description
src/coreclr/vm/interpexec.h Adds std::function include and delegateBeforeExit member to InterpMethodContextFrame
src/coreclr/vm/interpexec.cpp Implements IL unbox helper support with proper frame initialization and cleanup

Add unbox.end opcode to finish unboxing after helper call
@radekdoulik
Copy link
Member Author

If this looks good, I can modify the unbox.any.generic opcode too. I suppose the non generic pMT will always return false for IsNullable(), is that right?

@jkotas
Copy link
Member

jkotas commented Oct 1, 2025

If this looks good

@kg @janvorli @BrzVlad Does this look good to you?

@radekdoulik
Copy link
Member Author

With #120199 in place + changes in this PR we are able to run

TestCallingConvention0(1, 22.0f, 3, 44.0, 5, 66.0);

static void TestCallingConvention0(int a, float b, int c, double d, int e, double f)
{
    Console.WriteLine("TestCallingConvention0: a = {0}, b = {1}, c = {2}, d = {3}, e = {4}, f = {5}", a, b, c, d, e, f);
}

@radekdoulik
Copy link
Member Author

And with the added calli even the interpolated strings work now. Like

int answer = 42;
string answerString = "The ultimate answer";
Console.WriteLine($"{answerString} is {answer}. Some double: {answer + 0.123456789}");

@radekdoulik
Copy link
Member Author

Would it make more sense to name the opcodes unbox.any.start and unbox.any.end? (and similar for the generic version)

@radekdoulik
Copy link
Member Author

@kg @janvorli @BrzVlad does it looks good or do you want some changes?

Copy link
Member

@kg kg left a comment

Choose a reason for hiding this comment

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

I don't see anything that absolutely needs to change, so I'm OK with this. Thanks for doing all the work to fix it!

@radekdoulik
Copy link
Member Author

Thanks for all the feedback, I have opened above issue to address the remaining feedback.

@radekdoulik radekdoulik enabled auto-merge (squash) October 6, 2025 06:09
@radekdoulik radekdoulik merged commit 3c9b03e into dotnet:main Oct 6, 2025
113 checks passed
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.

5 participants