[Wasm / RyuJit]: implement CKFINITE#129061
Open
AndyAyersMS wants to merge 3 commits into
Open
Conversation
Adds wasm codegen for the CKFINITE IL opcode, which throws
ArithmeticException if the float/double operand is NaN or +/-Inf and
otherwise passes the value through.
The check is implemented by computing "!(|x| < +Inf)". This is true for
NaN and +/-Inf and false for every finite value: for finite values
|x| < +Inf is 1; for +/-Inf it is 0; for NaN any wasm float comparison
other than "ne" returns 0. If the resulting bool is non-zero we jump to
the SCK_ARITH_EXCPN throw helper; otherwise the original value (left on
the wasm operand stack by genConsumeOperands) flows through as the
result.
To make the operand available twice (once for the abs+lt check and once
as the produced value), GT_CKFINITE is wired into the wasm multi-use
mechanism:
- Lowering::LowerCkfinite calls SetMultiplyUsed on op1, which triggers
the regalloc layer to allocate a temporary local and emit a
local.tee.
- WasmRegAlloc::CollectReferencesForNode releases the temporary local
once the GT_CKFINITE consumer is processed.
- CodeGen::genCkfinite emits the local.get + f{32,64}.abs +
f{32,64}.const +Inf + f{32,64}.lt + i32.eqz + br_if sequence and then
calls WasmProduceReg on the GT_CKFINITE node itself.
Fixes the NYI assert that was blocking R2R compilation of the
b25459 (JIT/Regression/CLR-x86-JIT/V1-M09.5-PDC) regression test on
wasm:
codegenwasm.cpp:878 Assertion failed 'CKFINITE' ... during
'Generate code'
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
area-CodeGen-coreclr
Member
Author
|
@dotnet/wasm-contrib PTAL |
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds WebAssembly (RyuJIT) support for GT_CKFINITE by ensuring the operand can be re-read during codegen (via MultiplyUsed/temp locals) and emitting the Wasm finiteness check that throws on NaN / ±Inf.
Changes:
- Lowering: mark
GT_CKFINITEoperand asMultiplyUsedto enable re-reading during codegen. - Regalloc (Wasm): consume/release the temporary local allocated for the multiply-used operand at the
GT_CKFINITEuse site. - Codegen (Wasm): generate the finiteness check and branch to the arithmetic-exception throw helper when not finite.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/regallocwasm.cpp | Adds GT_CKFINITE handling to consume the temp local allocated for multiply-used operands. |
| src/coreclr/jit/lowerwasm.cpp | Introduces LowerCkfinite to mark the operand as multiply-used for Wasm codegen. |
| src/coreclr/jit/lower.h | Declares the new Wasm-only lowering helper LowerCkfinite. |
| src/coreclr/jit/lower.cpp | Wires GT_CKFINITE into the Wasm lowering switch. |
| src/coreclr/jit/codegenwasm.cpp | Adds GT_CKFINITE dispatch and implements genCkfinite Wasm emission logic. |
Document that values may remain on the Wasm stack underneath the predicate; only the predicate is consumed on fall-through, and anything underneath is unwound or unreachable on the throw path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
@adamperlin can you review? |
adamperlin
reviewed
Jun 8, 2026
Comment on lines
+1870
to
+1872
| // Push the operand value on the wasm stack. Because op1 was flagged as | ||
| // MultiplyUsed during lowering, "WasmProduceReg" will tee it into a | ||
| // temporary local so we can re-read it for the exponent check. |
Contributor
There was a problem hiding this comment.
Can you move this comment below the genConsumeOperands?
adamperlin
reviewed
Jun 8, 2026
| // | ||
| genJumpToThrowHlpBlk(SCK_ARITH_EXCPN); | ||
|
|
||
| // The operand value is still on the wasm stack from genConsumeOperands; |
Contributor
There was a problem hiding this comment.
This comment is a little confusing. I don't think genConsumeOperands behavior would have caused the value to be on the top of the stack here, right? The value should have been produced by a previous node (and also tee'd into a temporary local which was used above), and we just haven't generated actual Wasm to consume it yet?
Member
Author
There was a problem hiding this comment.
yeah, let me fix this.
The comments suggested that genConsumeOperands pushes op1's value onto the wasm operand stack. On wasm, genConsumeOperands only updates liveness bookkeeping; op1's value is on the stack because op1's producer emitted local.tee via WasmProduceReg (driven by the MultiplyUsed flag set during lowering). Rework the surrounding comments to reflect that, and move the explanation below the genConsumeOperands call per review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds wasm codegen for the CKFINITE.
Sequence here was shorter than masking off and testing the exponent, especially for doubles.