[Wasm RyuJit] internal registers, integer binop overflow checks#124731
[Wasm RyuJit] internal registers, integer binop overflow checks#124731AndyAyersMS wants to merge 8 commits intodotnet:mainfrom
Conversation
Add support for internal registers. Use these to implement overflow checks for ADD/SUB/MUL, where the result of the operation is multiply used (though for long MUL, defer to a runtime helper).
github-actions
bot
added
the
area-CodeGen-coreclr
|
@SingleAccretion is this what you envisioned for internal registers? |
|
Also the MUL overflow codegen is not yet finished -- I want to reuse the range checks from the integer casts but either need to fake up a cast Desc or pull the needed cases out into their own methods. It might be better to do this particular expansion in morph. |
There was a problem hiding this comment.
Pull request overview
This PR adds support for internal registers in the WASM RyuJit backend and implements overflow checking for integer binary operations (ADD, SUB, MUL). Internal registers are temporary registers allocated during code generation that are used to store intermediate values needed for complex operations like overflow detection.
Changes:
- Added internal register allocation and management infrastructure to the WASM register allocator
- Implemented overflow checking logic for ADD, SUB, and MUL operations with internal register support
- Configured morph phase to transform 64-bit overflow multiplication into runtime helper calls
- Marked operands as multiply-used in lowering when overflow checks are needed
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/regallocwasm.h | Added declarations for internal register management and binop reference collection |
| src/coreclr/jit/regallocwasm.cpp | Implemented internal register allocation, consumption, and binop overflow handling |
| src/coreclr/jit/morph.cpp | Added logic to transform WASM 64-bit overflow multiplication to helper calls |
| src/coreclr/jit/lowerwasm.cpp | Marked ADD/SUB overflow operands as multiply-used for correct register handling |
| src/coreclr/jit/codegenwasm.cpp | Implemented overflow check code generation for ADD, SUB, and MUL operations |
| src/coreclr/jit/codegen.h | Added declaration for genCodeForBinaryOverflow method |
I think we should return the allocated register from |
Ok, let me revise. |
|
FYI @dotnet/jit-contrib |
| // For long multiply with overflow, call the helper. | ||
| if (tree->TypeIs(TYP_LONG)) | ||
| { | ||
| helper = tree->IsUnsigned() ? CORINFO_HELP_ULMUL_OVF : CORINFO_HELP_LMUL_OVF; |
There was a problem hiding this comment.
I wonder if it'd be simpler and maybe not slower to use helpers for all kinds of overflows? maybe emscripten or whatever tool would be able to optimize it better than us?
There was a problem hiding this comment.
V8 does do inlining in its optimized codegen for wasm, but I don't know how aggressive it is or what the heuristics are like. It's also possible we could ship WABT's wasm-opt and run it on our binaries to optimize them, but I don't know if it would inline our helpers
There was a problem hiding this comment.
We should consider it, but we'd need to add helpers. I'll leave a TODO.
4 participants
Add support for internal registers.
Use these to implement overflow checks for ADD/SUB/MUL, where the result of the operation is multiply used (though for long MUL, defer to a runtime helper).