Skip to content

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Apr 3, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings April 3, 2025 09:15
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 3, 2025
@BrzVlad BrzVlad added area-CodeGen-Interpreter-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Apr 3, 2025
@BrzVlad BrzVlad requested a review from janvorli April 3, 2025 09:15
Copy link
Contributor

@Copilot 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 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)
Copy link

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copilot uses AI. Check for mistakes.

@BrzVlad BrzVlad force-pushed the feature-clr-interp4 branch from a9427f3 to fa39117 Compare April 3, 2025 09:27
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants