Implement additional CEE_BOX peepholes #121763
Open
+356
−15
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
This change implements CEE_BOX peepholes like JIT does, mostly to fix issues with sequences starting with CEE_BOX applied to ByRefLike types. While it is illegal to box a ByRefLike types, some sequences can be replaced by constants or nop. There is also one extra fix for the destination type of an unbox. It was incorrectly changed to StackTypeO by a large change in the past from the correct StackTypeI. This fixes a relatively large number of failing libraries tests.
Contributor
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
Copilot finished reviewing on behalf of
janvorli
November 18, 2025 22:54
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements additional CEE_BOX peephole optimizations in the CoreCLR interpreter, mirroring the JIT's Compiler::impBoxPatternMatch functionality. The primary goal is to handle sequences involving BOX operations on ByRefLike types by replacing them with constants or no-ops, as boxing ByRefLike types is illegal but certain code patterns can be safely optimized away.
Key changes:
- Changed
ApplyPeepFunc_treturn type fromvoidtointto support flexible skip distances - Added 6 new peephole optimization patterns for BOX-related sequences (BOX+BRTRUE/BRFALSE, BOX+ISINST, BOX+ISINST+BRTRUE/BRFALSE, etc.)
- Fixed UNBOX destination type from StackTypeO to StackTypeI (correcting a previous regression)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/coreclr/interpreter/compiler.h | Updated ApplyPeepFunc_t return type and added declarations for 6 new BOX-related peephole check/apply function pairs |
| src/coreclr/interpreter/compiler.cpp | Implemented new BOX peephole patterns, updated existing Apply functions to return skip distance, fixed UNBOX type, and registered new peepholes in the optimization pipeline |
kg
reviewed
Nov 19, 2025
kg
approved these changes
Nov 19, 2025
BrzVlad
approved these changes
Nov 20, 2025
| if (helpFunc == CORINFO_HELP_UNBOX) | ||
| { | ||
| EmitPushHelperCall_2(helpFunc, embedInfo, m_pStackPointer[0].var, StackTypeO, (CORINFO_CLASS_HANDLE)embedInfo.compileTimeHandle); | ||
| EmitPushHelperCall_2(helpFunc, embedInfo, m_pStackPointer[0].var, StackTypeI, (CORINFO_CLASS_HANDLE)embedInfo.compileTimeHandle); |
Member
There was a problem hiding this comment.
Isn't this a pointer onto the GC heap? Doesn't it need to be StackTypeByRef?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This change implements CEE_BOX peepholes like JIT does in Compiler::impBoxPatternMatch, mostly to fix issues with sequences starting with CEE_BOX applied to ByRefLike types. While it is illegal to box a ByRefLike types, some sequences can be replaced by constants or nop.
There is also one extra fix for the destination type of an unbox. It was incorrectly changed to StackTypeO by a large change in the past from the correct StackTypeI.
This fixes a relatively large number of failing libraries tests.