Stream: git-wasmtime

Topic: wasmtime / PR #10652 Add inital support for `f16` without...

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:38):

beetrees opened PR #10652 from beetrees:f16-f128-riscv64-mvp to bytecodealliance:main:

On the riscv64 backend, this PR adds initial support for using f16 without the Zfh extension being enabled, as well as initial support for f128. Support is added for the load, store, bitcast, select, f16const, f128const, bnot, band, bor and bxor CLIF instructions, as well as adding the zfhmin and zvfh target features (alongside the pre-existing zfh target feature).

cc @afonso360 who previously added initial f16 support in #9135

f16/f128 issue: #8312

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:38):

beetrees requested cfallin for a review on PR #10652.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:38):

beetrees requested wasmtime-compiler-reviewers for a review on PR #10652.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:38):

beetrees requested wasmtime-default-reviewers for a review on PR #10652.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:45):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:45):

bjorn3 created PR review comment:

        "Zfhmin: Minimal Half-Precision Floating-Point",

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:45):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 13:45):

bjorn3 created PR review comment:

        "Zvfh: Vector Extension for Half-Precision Floating-Point",

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 14:20):

beetrees updated PR #10652.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 14:20):

beetrees submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 14:20):

beetrees created PR review comment:

Done

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 14:20):

beetrees submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 14:20):

beetrees created PR review comment:

Done

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 15:46):

github-actions[bot] commented on PR #10652:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta", "isle"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 21:36):

cfallin submitted PR review:

A few nits but overall this looks plausible to me -- thanks for working through the tedious details!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 21:36):

cfallin created PR review comment:

Could we add a comment here describing why we're OR'ing in all ones in the high bits? I'm not sure if I understand why myself -- does this allow more immediates to fit into better/smaller codegen patterns for imm perhaps?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 21:36):

cfallin created PR review comment:

Likewise here as above -- where does the constant come from?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 21:36):

cfallin created PR review comment:

Is there any particular reason that we remove aarch64 has_fp16 from the list of targets tested by this runtest and those below? Can we keep that while adding riscv64 below it?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 23:08):

beetrees updated PR #10652.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 23:14):

beetrees submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 23:14):

beetrees created PR review comment:

I've added a comment. RISC-V stores smaller floats inside larger floating-point registers using NaN-boxing, meaning that the smaller float will be stored in the lower bits and all the other bits will be set to 1 (see section 21.2 of the RISC-V ISA manual for details). The correctly-sized fmv instructions do this automatically, but when using a 32-bit fmv for a 16-bit float the NaN-boxing has to be done manually.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 23:15):

beetrees updated PR #10652.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 23:23):

beetrees submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 23 2025 at 23:23):

beetrees created PR review comment:

This was a leftover from when bitcast-f16-f128.clif was split into f16-bitcast.clif and f128-bitcast.clif in #9135. The aarch64 directives were just moved to f128-bitcast.clif, with f16-bitcast.clif only being tested on x86_64 and riscv64. This PR adds aarch64 (both with and without has_fp16) to f16-bitcast.clif and removes the has_fp16 case from f128-bitcast.clif as has_fp16 is only relevant to testing f16, not f128.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 09:26):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 09:26):

bjorn3 created PR review comment:

Does this rule only applies when 16bit float instructions are not supported? If so I think we could get away with only NaN-boxing on the ABI boundary and within the function leaving the high bits undefined. I don't know if that will be faster though.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 12:55):

beetrees submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 12:55):

beetrees created PR review comment:

When the Zfhmin extension is not enabled (which is the only case where NaN-boxing is done manually), the only place NaN-boxing is required is at the ABI boundary. However, I doubt it would be beneficial to move NaN-boxing from bitcasts to the ABI boundary as I expect that most uses of f16 are going to be calls to other functions (such as __extendhfsf2 from compiler_builtins). It would be faster to NaN-box the value at most once (the value will already be NaN-boxed if it is an argument to the function or return value from another function) rather than every time it is used.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 12:55):

beetrees edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 16:38):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 16:38):

cfallin created PR review comment:

Ah, right, of course -- saw the seemingly unrelated change but that cleanup makes sense, thanks.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 16:40):

cfallin submitted PR review:

Thanks, LGTM!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 24 2025 at 17:01):

cfallin merged PR #10652.

Last updated: Dec 06 2025 at 07:03 UTC