Add `f16` inline ASM support for RISC-V #126530

beetrees · 2024-06-15T21:47:13Z

This PR adds f16 inline ASM support for RISC-V. A FIXME is left for f128 support as LLVM does not support the required Q (Quad-Precision Floating-Point) extension yet.

Relevant issue: #125398
Tracking issue: #116909

@rustbot label +F-f16_and_f128

rustbot · 2024-06-15T21:47:21Z

r? @cjgillot

rustbot has assigned @cjgillot.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

cjgillot · 2024-06-17T10:13:10Z

r? compiler

lcnr · 2024-06-17T10:15:01Z

r? compiler

nnethercote · 2024-06-17T10:28:28Z

Looks ok to me, though I don't know anything about RISC-V. @tgross35, does it look ok to you?

compiler/rustc_target/src/asm/riscv.rs

tgross35 · 2024-06-17T17:19:19Z

I also don't know RISC-V :) (@MabezDev and @jessebraham might, based on the embedded wg teams).

It looks consistent with #126417 so I think it is probably fine. Amanieu did the review on that one if you prefer to pass it off.

nnethercote · 2024-06-17T22:44:41Z

Ok, let's do r? @Amanieu

Amanieu · 2024-06-17T23:34:13Z

compiler/rustc_codegen_llvm/src/asm.rs

+            let value = bx.bitcast(value, bx.type_i16());
+            let value = bx.zext(value, bx.type_i32());
+            let value = bx.or(value, bx.const_u32(0xFFFF_0000));
+            bx.bitcast(value, bx.type_f32())


Does LLVM not support using f16 directly here? I expect this would result in much better codegen.

Unfortunately not. Regardless, LLVM's codegen isn't very good around when NaN-boxing needs to occur at the moment anyway; for example, an identity function will needlessly re-NaN-box an argument that is already guaranteed to be NaN-boxed.

My reading of the LLVM code is that it will accept f16 directly if the zfhmin feature is enabled for the current function. Can you use f16 directly in this case and only do the manual conversion to/from f32 if zfhmin is not available?

Amanieu · 2024-06-17T23:34:26Z

compiler/rustc_codegen_llvm/src/asm.rs

+        {
+            let value = bx.bitcast(value, bx.type_i32());
+            let value = bx.trunc(value, bx.type_i16());
+            bx.bitcast(value, bx.type_f16())


compiler/rustc_target/src/asm/riscv.rs

Amanieu · 2024-06-20T00:07:40Z

compiler/rustc_codegen_llvm/src/asm.rs

@@ -913,6 +915,46 @@ fn llvm_asm_scalar_type<'ll>(cx: &CodegenCx<'ll, '_>, scalar: Scalar) -> &'ll Ty
    }
 }

+fn function_target_features<'ll>(builder: &Builder<'_, 'll, '_>) -> impl Iterator<Item = &'ll str> {


You can use self.tcx.asm_target_features(instance.def_id()).contains(&feature) instead and avoid all this logic. It's used earlier in this file to convert unsupported registers to clobbers.

Amanieu · 2024-06-21T14:41:55Z

compiler/rustc_codegen_llvm/src/asm.rs

@@ -1029,6 +1037,16 @@ fn llvm_fixup_input<'ll, 'tcx>(
                _ => value,
            }
        }
+        (InlineAsmRegClass::RiscV(RiscVInlineAsmRegClass::freg), Abi::Scalar(s))
+            if s.primitive() == Primitive::Float(Float::F16)
+                && !bx.tcx.asm_target_features(instance.def_id()).contains(&sym::zfhmin) =>


This needs to check for zfh as well. We currently won't automatically enable zfhmin if you only enable zfh for a function with #[target_feature] because rustc isn't aware of feature dependencies.

Added. I've also improved the test to check that manual NaN-boxing is being avoided when zfhmin is enabled.

Amanieu · 2024-06-21T18:00:32Z

@bors r+

bors · 2024-06-21T18:00:34Z

📌 Commit 771e44e has been approved by Amanieu

It is now in the queue for this repository.

…kingjubilee Rollup of 7 pull requests Successful merges: - rust-lang#126530 (Add `f16` inline ASM support for RISC-V) - rust-lang#126712 (Migrate `relocation-model`, `error-writing-dependencies` and `crate-name-priority` `run-make` tests to rmake) - rust-lang#126722 (Add method to get `FnAbi` of function pointer) - rust-lang#126787 (Add direct accessors for memory addresses in `Machine` (for Miri)) - rust-lang#126798 ([fuchsia-test-runner] Remove usage of kw_only) - rust-lang#126809 (Remove stray `.` from error message) - rust-lang#126811 (Add a tidy rule to check that fluent messages and attrs don't end in `.`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#126530 - beetrees:f16-inline-asm-riscv, r=Amanieu Add `f16` inline ASM support for RISC-V This PR adds `f16` inline ASM support for RISC-V. A `FIXME` is left for `f128` support as LLVM does not support the required `Q` (Quad-Precision Floating-Point) extension yet. Relevant issue: rust-lang#125398 Tracking issue: rust-lang#116909 `@rustbot` label +F-f16_and_f128

Update Rust toolchain from nightly-2024-06-22 to nightly-2024-06-23 without any other source changes. This is an automatically generated pull request. If any of the CI checks fail, manual intervention is required. In such a case, review the changes at https://github.com/rust-lang/rust from rust-lang@c1b336c up to rust-lang@3cb521a. The log for this commit range is: rust-lang@3cb521a434 Auto merge of rust-lang#126761 - GuillaumeGomez:unsafe_extern_blocks, r=spastorino rust-lang@a0f01c3c10 Auto merge of rust-lang#126838 - matthiaskrgr:rollup-qkop22o, r=matthiaskrgr rust-lang@dc9a08f535 Rollup merge of rust-lang#126552 - fee1-dead-contrib:rmfx, r=compiler-errors rust-lang@162120b4fa Rollup merge of rust-lang#126318 - Kobzol:bootstrap-perf, r=onur-ozkan rust-lang@f3ced9d540 Rollup merge of rust-lang#126140 - eduardosm:stabilize-fs_try_exists, r=Amanieu rust-lang@f944afe380 Auto merge of rust-lang#116113 - kpreid:arcmut, r=dtolnay rust-lang@88c3db57e4 Generalize `{Rc,Arc}::make_mut()` to unsized types. rust-lang@a9a4830d25 Replace `WriteCloneIntoRaw` with `CloneToUninit`. rust-lang@ec201b8650 Add `core::clone::CloneToUninit`. rust-lang@81da6a6d40 Make `effects` an incomplete feature rust-lang@ac47dbad50 Auto merge of rust-lang#126824 - GuillaumeGomez:rollup-sybv8o7, r=GuillaumeGomez rust-lang@d265538016 Rollup merge of rust-lang#126823 - GuillaumeGomez:migrate-run-make-inline-always-many-cgu, r=Kobzol rust-lang@25bcc7d130 Rollup merge of rust-lang#126731 - Kobzol:bootstrap-cmd-refactor, r=onur-ozkan rust-lang@399c5cabdd Rollup merge of rust-lang#126723 - estebank:dot-dot-dot, r=Nadrieril rust-lang@3ed2cd74b5 Rollup merge of rust-lang#126686 - fmease:dump-preds-n-item-bounds, r=compiler-errors rust-lang@07e8b3ac01 Rollup merge of rust-lang#126555 - beetrees:f16-inline-asm-arm, r=Amanieu rust-lang@d03d6c0fea Auto merge of rust-lang#126750 - scottmcm:less-unlikely, r=jhpratt rust-lang@e7dfd4a913 Migrate `run-make/inline-always-many-cgu` to `rmake.rs` rust-lang@d9962bb4d8 Make `read_dir` method take a mutable callback rust-lang@f1b0d54ca9 Auto merge of rust-lang#126816 - weihanglo:update-cargo, r=weihanglo rust-lang@0bd58d8122 Apply review comments. rust-lang@250586cb2e Wrap std `Output` in `CommandOutput` rust-lang@f0aceed540 Auto merge of rust-lang#126817 - workingjubilee:rollup-0rg0k55, r=workingjubilee rust-lang@38bd7a0fcb Add `#[rustc_dump_{predicates,item_bounds}]` rust-lang@1916b3d57f Rollup merge of rust-lang#126811 - compiler-errors:tidy-ftl, r=estebank rust-lang@539090e5cd Rollup merge of rust-lang#126809 - estebank:wording-tweak, r=oli-obk rust-lang@b9ab6c3501 Rollup merge of rust-lang#126798 - miguelfrde:master, r=tmandry rust-lang@9498d5cf2f Rollup merge of rust-lang#126787 - Strophox:get-bytes, r=RalfJung rust-lang@1f9793f1aa Rollup merge of rust-lang#126722 - adwinwhite:ptr_fn_abi, r=celinval rust-lang@84b0922565 Rollup merge of rust-lang#126712 - Oneirical:bootest-constestllation, r=jieyouxu rust-lang@e7956cd994 Rollup merge of rust-lang#126530 - beetrees:f16-inline-asm-riscv, r=Amanieu rust-lang@10e1f5d212 Auto merge of rust-lang#124101 - the8472:pidfd-methods, r=cuviper rust-lang@2c65a24b8c Update cargo rust-lang@fcae62649e Auto merge of rust-lang#126758 - spastorino:avoid-safe-outside-unsafe-blocks, r=compiler-errors rust-lang@ffd72b1700 Fix remaining cases rust-lang@ea681ef281 Add a tidy rule to make sure that diagnostics don't end in periods rust-lang@8abf149bde to extract a pidfd we must consume the child rust-lang@0787c7308c Add PidFd::{kill, wait, try_wait} rust-lang@5d5892e966 Remove stray `.` from error message rust-lang@d94a40516e [fuchsia-test-runner] Remove usage of kw_only rust-lang@771e44ebd3 Add `f16` inline ASM support for RISC-V rust-lang@753fb070bb Add `f16` inline ASM support for 32-bit ARM rust-lang@22831ed117 Do not allow safe usafe on static and fn items rust-lang@a6a83d3d4e bless tests rust-lang@b512bf6f77 add as_ptr to trait AllocBytes, fix 2 impls; add pub fn get_bytes_unchecked_raw in allocation.rs; add pub fn get_alloc_bytes_unchecked_raw[_mut] in memory.rs rust-lang@02aaea1803 update intrinsic const param counting rust-lang@3b14b756d8 Remove `feature(effects)` from the standard library rust-lang@a314f7363a Stop using `unlikely` in `strict_*` methods rust-lang@225796a2df Add method to get `FnAbi` of function pointer rust-lang@630c3adb14 Add regression test for `unsafe_extern_blocks` rust-lang@bb9a3ef90c Implement `unsafe_extern_blocks` feature in rustdoc rust-lang@3c0a4bc915 rewrite crate-name-priority to rmake rust-lang@bc12972bcd Slightly refactor the dumping of HIR analysis data rust-lang@3fe4d134dd Appease `clippy` rust-lang@c15293407f Remove unused import rust-lang@5c4318d02c Implement `run_cmd` in terms of `run_tracked` rust-lang@0de7b92cc6 Remove `run_delaying_failure` rust-lang@e933cfb13c Remove `run_quiet_delaying_failure` rust-lang@949e667d3f Remove `run_quiet` rust-lang@a12f541a18 Implement new command execution logic rust-lang@9fd7784b97 Fix `...` in multline code-skips in suggestions rust-lang@f22b5afa6a rewrite error-writing-dependencies to rmake rust-lang@75ee1d74a9 rewrite relocation-model to rmake rust-lang@87d2e61428 Add `x perf` command for profiling the compiler using `rustc-perf` rust-lang@fd44aca2aa Copy `rustc-fake` binary when building the `rustc-perf` tool rust-lang@9e0b76201b Add `RustcPerf` bootstrap tool rust-lang@9ec178df0b Add `cargo_args` to `ToolBuild` rust-lang@6a04dfe78c Rename `std::fs::try_exists` to `std::fs::exists` and stabilize fs_try_exists Co-authored-by: celinval <35149715+celinval@users.noreply.github.com>

rustbot assigned cjgillot Jun 15, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` labels Jun 15, 2024

This comment has been minimized.

Sign in to view

beetrees force-pushed the f16-inline-asm-riscv branch from 0d6b5dd to e1571bd Compare June 15, 2024 21:59

rustbot assigned lcnr and unassigned cjgillot Jun 17, 2024

rustbot assigned nnethercote and unassigned lcnr Jun 17, 2024

tgross35 mentioned this pull request Jun 17, 2024

Tracking Issue for f16 and f128 float types #116909

Open

84 tasks

tgross35 reviewed Jun 17, 2024

View reviewed changes

compiler/rustc_target/src/asm/riscv.rs Show resolved Hide resolved

rustbot assigned Amanieu and unassigned nnethercote Jun 17, 2024

Amanieu reviewed Jun 17, 2024

View reviewed changes

beetrees force-pushed the f16-inline-asm-riscv branch from e1571bd to a86b38e Compare June 18, 2024 18:45

beetrees requested a review from Amanieu June 18, 2024 18:46

Amanieu reviewed Jun 20, 2024

View reviewed changes

beetrees force-pushed the f16-inline-asm-riscv branch from a86b38e to 9d45532 Compare June 20, 2024 01:19

Amanieu reviewed Jun 21, 2024

View reviewed changes

Add f16 inline ASM support for RISC-V

771e44e

beetrees force-pushed the f16-inline-asm-riscv branch from 9d45532 to 771e44e Compare June 21, 2024 17:48

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2024

workingjubilee mentioned this pull request Jun 22, 2024

Rollup of 7 pull requests #126817

Merged

bors merged commit e7956cd into rust-lang:master Jun 22, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jun 22, 2024

beetrees deleted the f16-inline-asm-riscv branch June 22, 2024 20:46

tgross35 mentioned this pull request Aug 21, 2024

Enable f16 and f128 in assembly on platforms that support it #125398

Open

tgross35 mentioned this pull request Oct 2, 2024

some tier 2 rustc 1.81 hosts cannot bootstrap stable 1.82 #129268

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `f16` inline ASM support for RISC-V #126530

Add `f16` inline ASM support for RISC-V #126530

beetrees commented Jun 15, 2024

rustbot commented Jun 15, 2024

This comment has been minimized.

cjgillot commented Jun 17, 2024

lcnr commented Jun 17, 2024

nnethercote commented Jun 17, 2024

tgross35 commented Jun 17, 2024 •

edited

Loading

nnethercote commented Jun 17, 2024

Amanieu Jun 17, 2024

beetrees Jun 18, 2024 •

edited

Loading

Amanieu Jun 18, 2024

beetrees Jun 18, 2024

Amanieu Jun 17, 2024

Amanieu Jun 20, 2024

beetrees Jun 20, 2024

Amanieu Jun 21, 2024

beetrees Jun 21, 2024

Amanieu commented Jun 21, 2024

bors commented Jun 21, 2024

Add f16 inline ASM support for RISC-V #126530

Add f16 inline ASM support for RISC-V #126530

Conversation

beetrees commented Jun 15, 2024

rustbot commented Jun 15, 2024

This comment has been minimized.

cjgillot commented Jun 17, 2024

lcnr commented Jun 17, 2024

nnethercote commented Jun 17, 2024

tgross35 commented Jun 17, 2024 • edited Loading

nnethercote commented Jun 17, 2024

Choose a reason for hiding this comment

beetrees Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amanieu commented Jun 21, 2024

bors commented Jun 21, 2024

Add `f16` inline ASM support for RISC-V #126530

Add `f16` inline ASM support for RISC-V #126530

tgross35 commented Jun 17, 2024 •

edited

Loading

beetrees Jun 18, 2024 •

edited

Loading