Skip to content

Conversation

@janvorli
Copy link
Member

@janvorli janvorli commented Nov 4, 2025

In funclets of generic methods, access to the generic argument may be needed. It doesn't work correctly, because the filter funclet has its own locals and various IR instructions that use that argument cannot access it.

This change adds copying of that argument from the parent frame to the funclet frame at the same offset so that the access to it just works.

In funclets of generic methods, access to the generic argument may be
needed. It doesn't work correctly, because the filter funclet has
its own locals and various IR instructions that use that argument
cannot access it.

This change adds copying of that argument from the parent frame to the
funclet frame at the same offset so that the access to it just works.
@janvorli janvorli added this to the 11.0.0 milestone Nov 4, 2025
@janvorli janvorli self-assigned this Nov 4, 2025
@janvorli janvorli requested review from BrzVlad and kg as code owners November 4, 2025 01:24
Copilot AI review requested due to automatic review settings November 4, 2025 01:24
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

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 an issue where filter clauses in generic methods could not access the generic type parameter argument. The fix ensures that the generic parameter argument is copied from the parent frame to the filter's frame when entering a filter clause.

Key Changes

  • Initialized m_paramArgIndex to -1 to indicate when no generic parameter argument exists
  • Added logic to copy the generic parameter argument from parent frame to filter frame when entering filter clauses

Reviewed Changes

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

File Description
src/coreclr/interpreter/compiler.h Initialized m_paramArgIndex to -1 as a sentinel value to indicate absence of a parameter argument
src/coreclr/interpreter/compiler.cpp Added code to copy the generic parameter argument from parent frame to filter frame when entering filter clauses in generic methods

@janvorli
Copy link
Member Author

janvorli commented Nov 4, 2025

There is something wrong now, maybe some cleanup / refactoring I did after the local testing has caused this to not to work. I am investigating.

@janvorli
Copy link
Member Author

janvorli commented Nov 4, 2025

Ah, this fixed two of the related tests, but one is still not passing. The problem is that the generic argument is not the only variable that needs to be copied to the finally local space. E.g for the INTOP_CALL_HELPER_P_GS, the arg2 needs to be copied there too.
But that's a problem for this selective approach because that can be essentially any local.
The original assumption was that only EmitLoadVar and EmitStoreVar need to handle the filter variable accesses. It seems that these assumptions got broken as the implementation of the interpreter progressed.

There was a bug in the SetDVar being passed a variable offset instead of index.

@janvorli
Copy link
Member Author

janvorli commented Nov 5, 2025

/ba-g the failures are infra issues unrelated to the change

@janvorli janvorli merged commit 8c78b43 into dotnet:main Nov 5, 2025
81 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants