[Wasm RyuJit] Add binary operation implementations to codegenwasm.cpp

[Copilot]

• Add overflow checks for integer arithmetic operations (GT_ADD, GT_SUB, GT_MUL only)
• Remove overflow checks from GT_DIV and GT_MOD
• Separate shift/rotate operations into genCodeForShiftOrRotate
• Add TODO comment for shift/rotate operand extension
• Verify changes compile successfully

Changes Made

Latest Update

Added TODO-WASM comment in genCodeForShiftOrRotate documenting the need to zero-extend the 2nd operand for shifts and rotates when the 1st and 2nd operands have different types. This addresses SingleAccretion's feedback about the shift operand width requirements.

Previous Updates

• Created CodeGen::genCodeForShiftOrRotate with same structure as genCodeForBinary
• Moved GT_LSH, GT_RSH, GT_RSZ, GT_ROL, GT_ROR cases from genCodeForBinary
• Added #ifdef TARGET_WASM declaration in codegen.h
• Updated genCodeForTreeNode to route shifts to the new function
• Added treeNode->gtOverflow() checks for GT_ADD, GT_SUB, and GT_MUL
• Removed overflow checks from GT_DIV and GT_MOD (they have divide-by-zero semantics instead)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Wasm RyuJit] Add new binary ops to codegenwasm.cpp</issue_title>
<issue_description>The Wasm RyuJit backend needs to be updated with a basic implementation of the new binary operations added to instrswasm.h in #122081. This will be done by adding new code to codegenwasm.cpp.

For floating point types (f32 and f64) the binary operations begin with the add opcode and end with the copysign opcode. For integer types (i32 and i64) these begin with the add opcode and end with the rotr opcode. The Wasm instructions are defined in instrswasm.h using INST( statements, where i.e. INST(f32_eq, "f32.eq", 0, IF_OPCODE, 0x5B) defines the binary operation INS_f32_eq.

We want to add implementations for each of these assuming there is a corresponding gentree GT_xxx enum, similar to how add has GT_ADD as its counterpart. For any Wasm binary opcode where there is no obviously corresponding GT_xxx gentree enum, we should skip it. For example, there is no GT_xxx enum for the copysign binary operator, so we can skip that one specifically.

The GT_xxx enum is defined in gtlist.h using GTNODE( statements, where i.e. GTNODE(ADD, GenTreeOp, 1, 0, GTK_BINOP defines the binary operation GT_ADD.

In some cases the mapping between operations may be non-obvious. For example, GT_UDIV with a TYP_INT maps to INS_i32_div_u. For example, GT_RSZ maps to INS_i32_shr_u because RSZ is a zero-extended right shift.

The process for adding a binary operation from instrswasm.h to codegenwasm.cpp is as follows:

1. Add its corresponding gentree enum GT_xxx to the switch in CodeGen::genCodeForTreeNode in codegenwasm.cpp to make sure that it calls genCodeForBinary. GT_ADD for example is already there, so the additional items can be added as cases below GT_ADD. For example, INS_i32_sub corresponds to i32.sub which corresponds to GT_SUB.
2. Add unique cases to genCodeForBinary in codegenwasm.cpp for each GT_xxx and TYP_xxx pairing that map to the appropriate wasm INS_type_xxx enumeration. For example, for GT_SUB and TYP_INT it would be case PackOperAndType(GT_SUB, TYP_INT): ins = INS_i32_sub; break; with appropriate newlines.</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes [Wasm RyuJit] Add new binary ops to codegenwasm.cpp #122118

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[dotnet-policy-service]

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

[kg]

cc @dotnet/jit-contrib

[Copilot]

Pull request overview

This PR implements basic binary operations for the WebAssembly RyuJit backend, building upon the instruction definitions added in PR #122081. The implementation adds code generation support for 15 GenTree operations by routing them through the genCodeForBinary function.

Key Changes

• Added 14 new GT operation cases to the switch statement in genCodeForTreeNode (GT_SUB through GT_ROR)
• Implemented 38 PackOperAndType mappings in genCodeForBinary covering integer, floating-point, bitwise, shift, and rotation operations for TYP_INT, TYP_LONG, TYP_FLOAT, and TYP_DOUBLE types

[kg]

@copilot please update the cases in genCodeForBinary as follows:
For the arithmetic operations add, sub, div, mul, and rem: check whether treeNode->gtOverflow() is true, and if so, fail with NYI_WASM. i.e.

        case PackOperAndType(GT_ADD, TYP_INT):
            if (treeNode->gtOverflow()) NYI_WASM("Overflow checks");
            ins = INS_i32_add;
            break;

[SingleAccretion]

add, sub, div, mul, and rem

(It needs to be only add, sub, mul - see e. g. OperMayOverflow)

[kg]

add, sub, div, mul, and rem

(It needs to be only add, sub, mul - see e. g. OperMayOverflow)

You're right, I was thinking of the other checks for div and rem (divide by zero) - though i guess maybe we'd get a trap?

[SingleAccretion]

Yeah, that's also needed. But there is no way to "assert" it, it just need to be added like a TODO-WASM.

Edit: I think in the end this means that what we should leave in this method is basically only add/sub/mul/and/or/xor. The rest should get their own methods.

[kg]

Yeah, that's also needed. But there is no way to "assert" it, it just need to be added like a TODO-WASM.

Edit: I think in the end this means that what we should leave in this method is basically only add/sub/mul/and/or/xor. The rest should get their own methods.

I think the next step (probably for the sake of not confusing copilot, a separate PR?) is to split the relevant ops out into new methods. But if you'd like I can try and make it happen in this one, I can always clean it up manually.

[SingleAccretion]

I think the next step (probably for the sake of not confusing copilot, a separate PR?) is to split the relevant ops out into new methods. But if you'd like I can try and make it happen in this one, I can always clean it up manually.

Whatever works better for you :).

[Copilot]

@copilot please update the cases in genCodeForBinary as follows:
For the arithmetic operations add, sub, div, mul, and rem: check whether treeNode->gtOverflow() is true, and if so, fail with NYI_WASM...

Added overflow checks as requested in commit 7b223fc. The checks are now in place for GT_ADD, GT_SUB, GT_MUL, GT_DIV, and GT_MOD for both TYP_INT and TYP_LONG types.

[kg]

@copilot Please remove the overflow checks for GT_MOD and GT_DIV in genCodeForBinary in codegenwasm.cpp, I had you add them in error. The others can stay.

[Copilot]

@copilot Please remove the overflow checks for GT_MOD and GT_DIV in genCodeForBinary in codegenwasm.cpp, I had you add them in error. The others can stay.

Removed overflow checks from GT_DIV and GT_MOD in commit fa3c96e. Only GT_ADD, GT_SUB, and GT_MUL now have overflow checks.

[kg]

@copilot Please move the cases for the shifts - GT_LSH, GT_RSH, GT_RSZ, GT_ROL, GT_ROR - out of genCodeForBinary into a new member function CodeGen::genCodeForShiftOrRotate. This function will need the same header and footer as genCodeForBinary. You will need to update the class CodeGen in codegen.h to add an ifdef'd declaration for this new member function, something like:

#ifdef TARGET_WASM
    void genCodeForShiftOrRotate(GenTreeOp* treeNode);
#endif

And then please update genCodeForTreeNode so that the shifts - GT_LSH, GT_RSH, GT_RSZ, GT_ROL, GT_ROR call the new genCodeForShiftOrRotate member function instead of calling genCodeForBinary.

[Copilot]

@copilot Please move the cases for the shifts - GT_LSH, GT_RSH, GT_RSZ, GT_ROL, GT_ROR - out of genCodeForBinary into a new member function CodeGen::genCodeForShiftOrRotate. This function will n...

Completed in commit 8c63dfa. Created genCodeForShiftOrRotate with all shift/rotate operations (GT_LSH, GT_RSH, GT_RSZ, GT_ROL, GT_ROR), added the ifdef'd declaration to codegen.h, and updated genCodeForTreeNode to call the new function.

[kg]

@copilot please update genCodeForShiftOrRotate to add a TODO comment documenting SingleAccretion's comment #122119 (comment) which said:

The shift operand width in IR is always TYP_INT; the WASM operations have the same widths for both the shift and shiftee. So the shift may need to be extended (zero-extended) for TYP_LONG. It is probably best done in a separate genCodeForShift.
I don't remember the exact rules for rotates. You'll need to check them manually.

The comment should say something like 'TODO: zero-extend the 2nd operand for shifts and rotates as needed when the 1st and 2nd operand are different types'.

[Copilot]

@copilot please update genCodeForShiftOrRotate to add a TODO comment documenting SingleAccretion's comment #122119 (comment) which said:

The shi...

Added TODO-WASM comment in commit b608464. The comment documents the need to zero-extend the 2nd operand for shifts and rotates when operands have different types, as the shift operand width in IR is TYP_INT but WASM operations require matching widths.

[kg]

/ba-g OSX lanes are still stuck due to ongoing issue and this PR only impacts WASM.