[clrinterp] Add support for ldc.r4, ldc.r8, ldc.i8 and ldstr#114201
Merged
janvorli merged 3 commits intodotnet:mainfrom Apr 3, 2025
Merged
[clrinterp] Add support for ldc.r4, ldc.r8, ldc.i8 and ldstr#114201janvorli merged 3 commits intodotnet:mainfrom
janvorli merged 3 commits intodotnet:mainfrom
Conversation
ghost
added
the
area-CodeGen-coreclr
BrzVlad
added
area-CodeGen-Interpreter-coreclr
and removed
area-CodeGen-coreclr
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for new IL instructions (ldc.r4, ldc.r8, ldc.i8, and ldstr) by updating the interpreter and compiler as well as extending test coverage.
- Implements new opcode cases in the interpreter and compiler for handling floating point and 64‐bit integers.
- Adds enum values for new instruction types.
- Introduces a new test (TestFloat) to verify the correct handling of floating point operations.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/tests/JIT/interpreter/Interpreter.cs | Adds a new TestFloat to check floating point operation accuracy |
| src/coreclr/vm/interpexec.cpp | Implements new IL opcode cases for ldc.i8, ldc.r4, and ldc.r8 |
| src/coreclr/interpreter/intops.h | Updates opcode enum with new values for long int, float, and double |
| src/coreclr/interpreter/compiler.cpp | Handles code generation for new ldc instructions and updates PrintInsData |
Files not reviewed (1)
- src/coreclr/interpreter/intops.def: Language not supported
Comments suppressed due to low confidence (1)
src/coreclr/interpreter/compiler.cpp:3112
- Replace the pointer cast with memcpy to copy the bits from i64 into a double variable for safe type punning when printing.
printf(" %g", *(double*)&i64);
|
|
||
| float sum = f1 + f2; | ||
|
|
||
| if ((sum - 27098.3) > 0.001 || (sum - 27098.3) < -0.001) |
There was a problem hiding this comment.
Consider using an absolute difference check (e.g., if (fabs(sum - 27098.3) > 0.001)) for clearer and more robust floating point comparisons.
Suggested change
| if ((sum - 27098.3) > 0.001 || (sum - 27098.3) < -0.001) | |
| if (Math.Abs(sum - 27098.3) > 0.001) |
BrzVlad
force-pushed
the
feature-clr-interp4
branch
from
a9427f3 to
fa39117
Compare
This was referenced Apr 3, 2025
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
No description provided.