[clr-interp] Fix data loss issue at IL merge points #122247
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
Since the interpreter uses either 32bit or 64 bit floats for values on the stack it is possible to be in a situation where we lose precision in a floating point value at an IL merge point. The architecture of the compiler doesn't have a scheme which would allow the compiler to avoid hitting this problem, so instead, I've built a scheme to record the problematic details, and retry the entire compilation of the method while providing a way to carry the data that certain stack slots need to be more precise than previously expected. This fixes jit\jit64\opt\regress\vswhidbey\481244\foo2_d which tests for this exact scenario In addition, on 64bit platforms we also have some situations where we can silently upgrade to a 64 bit int, this covers that situation too, as well as the case where we merge a native integer and a ByRef so we should treat the result as a ByRef to avoid any cases where we lose track of a GC pointer.
Contributor
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a data loss issue in the CLR interpreter that occurs at IL merge points when different floating-point precisions or integer sizes are involved. The solution implements a retry mechanism that detects type mismatches during compilation and retries with corrected stack type information.
Key changes:
- Adds a new
u64_ptrhash table specialization to track IL merge point stack states across compilation retries - Implements
InterpreterRetryDataclass to manage the retry logic and store override stack type information - Modifies the compilation loop in
compileMethodto support retrying when stack type mismatches are detected - Updates
EmitBBEndVarMovesto detect and handle data-loss conversions (R8→R4, I8→I4, ByRef→I) by triggering retries
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/containers/dn-simdhash-u64-ptr.c | New hash table specialization for uint64_t keys and void* values |
| src/native/containers/dn-simdhash-specializations.h | Adds declarations for the new u64_ptr hash table type |
| src/native/containers/CMakeLists.txt | Registers the new u64_ptr source file in the build |
| src/coreclr/interpreter/simdhash.h | Adds holder class for u64_ptr hash table with RAII semantics, and adds null check/HasValue method to ptr_ptr holder |
| src/coreclr/interpreter/eeinterp.cpp | Implements retry loop in compileMethod to handle compilation failures due to stack type mismatches |
| src/coreclr/interpreter/compiler.h | Adds InterpreterRetryData class and updates InterpBasicBlock constructor to track EH clause indices |
| src/coreclr/interpreter/compiler.cpp | Implements retry logic in AllocBB, updates EmitBBEndVarMoves to detect problematic conversions, and implements retry data storage/retrieval methods |
janvorli
reviewed
Dec 5, 2025
This was referenced Dec 6, 2025
Open
janvorli
reviewed
Dec 9, 2025
…an assert which should trigger in cases where it might be
Member
Author
|
/azp list |
Member
Author
|
/azp run runtime-interpreter |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 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.
Since the interpreter uses either 32bit or 64 bit floats for values on the stack it is possible to be in a situation where we lose precision in a floating point value at an IL merge point. The architecture of the compiler doesn't have a scheme which would allow the compiler to avoid hitting this problem, so instead, I've built a scheme to record the problematic details, and retry the entire compilation of the method while providing a way to carry the data that certain stack slots need to be more precise than previously expected.
This fixes jit\jit64\opt\regress\vswhidbey\481244\foo2_d which tests for this exact scenario
In addition, on 64bit platforms we also have some situations where we can silently upgrade to a 64 bit int, this covers that situation too, as well as the case where we merge a native integer and a ByRef so we should treat the result as a ByRef to avoid any cases where we lose track of a GC pointer.