Skip to content

Conversation

@davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Oct 22, 2025

  • Compare operations between I4 and I need to work
  • Storing a floating point value in an array may need conversions
  • Storing an I4 into an I with stelem is a supported operation

- Compare operations between I4 and I need to work
- Storing a floating point value in an array may need conversions
- Storing an I4 into an I with stelem is a supported operation
- LDLOCA is specified to return a byref, but the existing compilers treat it as a simple pointer, and we can trivially do it in the interpreter as well
@Copilot Copilot AI review requested due to automatic review settings October 22, 2025 22:24
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 22, 2025
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 fixes several IL conformance test issues in the CoreCLR interpreter by adding implicit type conversions and fixing type handling in various operations.

Key Changes:

  • Adds implicit conversions between I4 (int32) and I8 (nint) for comparison operations and array element storage on 64-bit platforms
  • Adds implicit floating-point conversions (R4 ↔ R8) for array storage and return statements
  • Changes LDLOCA to return InterpTypeI instead of InterpTypeByRef to match existing compiler behavior

@jakobbotsch
Copy link
Member

jakobbotsch commented Oct 22, 2025

  • LDLOCA is specified to return a byref, but the existing compilers treat it as a simple pointer, and we can trivially do it in the interpreter as well. This fixes a bunch of tests.

RyuJIT does not treat LDLOCA as native pointers internally. They are byrefs until they get used in a context that requires a native pointer type (see impBashVarAddrsToI).

I think treating them as native pointers unconditionally will run into other problems... For example, subtracting the address of a local from a byref should result in a native int, but if you treat the address of the local as a native int, then it will result in a byref. Something like:

ref int GetY(ref S s) => ref s.Y;
void Problem()
{
  S s = default;
  GetY(ref s) - ref s; // should be a native int on the stack
}

@davidwrighton
Copy link
Member Author

@jakobbotsch :( Now I have to do a bunch more work. Bummer.

@davidwrighton
Copy link
Member Author

@jakobbotsch I'll tackle the I vs Byref thing in another PR.

@davidwrighton
Copy link
Member Author

/azp run runtime-interpreter

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidwrighton
Copy link
Member Author

/azp run runtime-interpreter

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidwrighton
Copy link
Member Author

/azp run runtime-interpreter

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants