[interp] Some cleanups and fixes for CLR interpreter #117610
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
needs-area-label
kg
changed the title
|
cc @jkotas thank you for pointing out the issue with the use of a tag bit for helpers. this should remove it |
There was a problem hiding this comment.
Pull Request Overview
This PR implements cleanups and fixes for the CLR interpreter focused on improving portability and removing unused functionality. The changes eliminate the use of tag bits for helper function indirection (which is not portable) and remove the GC.Collect intrinsic.
Key changes include:
- Replacing tag-bit based helper function resolution with a structured approach using
InterpHelperData - Removing the
INTOP_GC_COLLECTinstruction and related intrinsic handling - Updating instruction lengths and data layouts to accommodate the new helper data structure
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/interpexec.cpp | Replaces GetPossiblyIndirectHelper implementation and removes GC.Collect instruction handling |
| src/coreclr/interpreter/intops.def | Updates instruction lengths for helper-related opcodes and removes GC_COLLECT opcode |
| src/coreclr/interpreter/interpretershared.h | Defines new InterpHelperData structure and removes tag bit constant |
| src/coreclr/interpreter/compiler.h | Updates method signature for helper function data population |
| src/coreclr/interpreter/compiler.cpp | Implements new helper data population logic and updates debug printing |
jkotas
reviewed
Jul 14, 2025
BrzVlad
reviewed
Jul 14, 2025
Open
3 tasks
jkotas
reviewed
Jul 15, 2025
|
Significantly revised based on vlad and jkotas's feedback. |
jkotas
approved these changes
Jul 15, 2025
kotlarmilos
reviewed
Jul 15, 2025
Checkpoint: Partial removal of tagged helper functions Update ip[] in interpexec side for helper data change Update compiler side for helper indirect tag bit removal Update opcode lengths Cleanup Address copilot review comment Speculative removal of fixme Remove INTOP_FAILFAST Add message to default opcode assert Fix Object.ctor intrinsic eating newobj opcodes Test debugging Checkpoint: Partial revert Repair revert Remove EmitCallIntrinsics Comment out broken check Repair merge damage Test debugging Remove incorrect zeroing of dreg in NEWOBJ_VT Revert diagnostic test changes Prevent newobj dvar from overlapping svars
This was referenced Jul 16, 2025
|
/ba-g #117669 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.