Skip to content

Conversation

@davidwrighton
Copy link
Member

  • There isn't a single instruction for this, but the jit recognizes the conv.r.un + conv.r4 pattern as meaning convert directly from unsigned integer to float. Per ECMA it is valid to do processing at a higher intermediate precision, but RyuJit/interpreter follows tighter rules which result in less loss of precision.

Fixes Runtime_106338 test.

- There isn't a single instruction for this, but the jit recognizes the
conv.r.un + conv.r4 pattern as meaning convert directly from unsigned integer to float. Per ECMA it is valid to do processing at a higher intermediate precision, but RyuJit/interpreter follows tighter rules which result in more less loss of precision.
Copilot AI review requested due to automatic review settings December 10, 2025 00:06
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Dec 10, 2025
@davidwrighton davidwrighton enabled auto-merge (squash) December 10, 2025 00:09
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!

Copy link
Contributor

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 fixes the conversion from unsigned integers to float (R4) in the CLR interpreter. The JIT recognizes the conv.r.un + conv.r4 pattern as a direct unsigned-to-float conversion, and the interpreter now properly implements this pattern with distinct opcodes for R4 (float) versus R8 (double) conversions.

  • Renamed existing INTOP_CONV_R_UN_* opcodes to INTOP_CONV_R8_UN_* to clarify they convert to double
  • Added new INTOP_CONV_R4_UN_* opcodes for direct unsigned-to-float conversion
  • Implemented a peephole optimization to recognize and properly handle the conv.r.un + conv.r4 pattern

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/coreclr/interpreter/inc/intops.def Renamed CONV_R_UN opcodes to CONV_R8_UN for clarity and added new CONV_R4_UN opcodes for unsigned-to-float conversions
src/coreclr/vm/interpexec.cpp Updated case labels to use renamed R8 opcodes and added execution logic for new R4 unsigned conversion opcodes
src/coreclr/interpreter/compiler.h Added declarations for the new ConvRUnR4 peephole optimization methods
src/coreclr/interpreter/compiler.cpp Implemented ConvRUnR4 peephole to recognize conv.r.un + conv.r4 pattern, updated Peeps array size, and updated opcode references to use renamed CONV_R8_UN opcodes

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

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants