Skip to content

Conversation

@davidwrighton
Copy link
Member

Ryujit does implicit promotion using zero-extending promotions for some opcode and sign extending for others. I don't see a particularly good rhyme or reason for the current behavior, but it does not match what the interpreter does.

This PR adds a test for all the opcodes that experience promotion, and attempts to test all the behaviors correctly. In building it, several issues were found/fixed.

This is known to fix the GitHub13501 test. in addition to the various issues found during test development.

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 implicit promotion of I4 (int32) to I (native int) in the CoreCLR interpreter to match RyuJIT's behavior. The fix addresses inconsistencies where some opcodes use zero-extension and others use sign-extension when promoting 32-bit integers to native integers on 64-bit platforms.

  • Adds a new helper function to determine the correct widening operation (sign or zero extension) based on the opcode
  • Implements comprehensive tests for all opcodes that perform implicit promotion, covering arithmetic, comparison, and branch operations
  • Tests verify correct behavior for unsigned overflow operations (add.ovf.un, sub.ovf.un, mul.ovf.un) and unsigned branch/comparison operations (bne.un, blt.un, etc.)

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/coreclr/interpreter/compiler.cpp Adds InterpOpForWideningArgForImplicitUpcast() to select appropriate conversion (sign vs zero extension) and updates EmitTwoArgBranch() and EmitBinaryArithmeticOp() to use it
src/tests/JIT/Methodical/int64/unsigned/implicit_promotion_il.il New IL assembly defining 52 operator methods testing all promotion scenarios with mixed I4/I8 operands
src/tests/JIT/Methodical/int64/unsigned/implicit_promotion_il.ilproj Project file for the IL test
src/tests/JIT/Methodical/int64/unsigned/implicit_promotion.cs New C# test invoking IL operators with negative values to validate sign/zero extension behavior on 32-bit vs 64-bit platforms
src/tests/JIT/Methodical/Methodical_*.csproj Updates to include the new test files in various build configurations

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

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 modulo the copilot nits that seem to make sense.

@davidwrighton davidwrighton enabled auto-merge (squash) December 10, 2025 00:15
@davidwrighton davidwrighton merged commit faff920 into dotnet:main Dec 10, 2025
105 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants