Skip to content

Emit rem.un for unsigned modulo expressions#522

Merged
dadhi merged 2 commits intomasterfrom
copilot/fix-modulo-instruction-issue
Apr 21, 2026
Merged

Emit rem.un for unsigned modulo expressions#522
dadhi merged 2 commits intomasterfrom
copilot/fix-modulo-instruction-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026

CompileFast() was emitting signed remainder IL for Expression.Modulo in unsigned scenarios, which breaks results when a uint operand has its high bit set. The regression showed up with block-local uint variables, where the standard compiler/interpreter still produced the correct unsigned remainder.

  • Codegen

    • Update arithmetic opcode selection for ExpressionType.Modulo
    • Emit OpCodes.Rem_Un when the expression result type is an unsigned primitive
    • Preserve existing signed behavior for non-unsigned modulo
  • Regression coverage

    • Add a focused arithmetic test for a block-local uint
    • Use 0x80000000u % 3u to exercise the signed/unsigned distinction
    • Assert both the runtime result and the generated IL opcode
  • Scenario covered

    var b = Expression.Parameter(typeof(uint), "b");
    var a = Expression.Variable(typeof(uint), "a");
    
    var expr = Expression.Lambda<Func<uint, uint>>(
        Expression.Block(new[] { a },
            Expression.Assign(a, Expression.Constant(0x80000000u)),
            Expression.Modulo(a, b)),
        b);

    This path should produce 2u and emit rem.un, not rem.

Copilot AI changed the title [WIP] Fix CompileFast() to emit unsigned modulo for uint variables Emit rem.un for unsigned modulo expressions Apr 20, 2026
Copilot AI requested a review from dadhi April 20, 2026 19:42
@dadhi dadhi marked this pull request as ready for review April 21, 2026 12:09
@dadhi dadhi merged commit 7dbafcf into master Apr 21, 2026
2 checks passed
@dadhi dadhi deleted the copilot/fix-modulo-instruction-issue branch April 21, 2026 12:09
@kevinferrare
Copy link
Copy Markdown

@dadhi Wow thanks for the quick fix!

Do you think it would be possible to do a release with the fix?

@dadhi
Copy link
Copy Markdown
Owner

dadhi commented Apr 21, 2026

Sure. Soon.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: CompileFast() emits rem (signed) instead of rem.un (unsigned) for Expression.Modulo with block-local uint variables

3 participants