Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable f16 in assembly on aarch64 platforms that support it #127043

Closed
wants to merge 1 commit into from
Closed

Enable f16 in assembly on aarch64 platforms that support it #127043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compiler/rustc_target/src/asm/aarch64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ impl AArch64InlineAsmRegClass {
_arch: InlineAsmArch,
) -> &'static [(InlineAsmType, Option<Symbol>)] {
match self {
Self::reg => types! { _: I8, I16, I32, I64, F32, F64; },
Self::reg => types! { _: I8, I16, I32, I64, F16, F32, F64; },
Self::vreg | Self::vreg_low16 => types! {
neon: I8, I16, I32, I64, F32, F64,
neon: I8, I16, I32, I64, F16, F32, F64,
VecI8(8), VecI16(4), VecI32(2), VecI64(1), VecF32(2), VecF64(1),
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF32(4), VecF64(2);
VecI8(16), VecI16(8), VecI32(4), VecI64(2), VecF16(4),VecF16(8), VecF32(4), VecF64(2);
},
Self::preg => &[],
}
Expand Down
29 changes: 28 additions & 1 deletion tests/assembly/asm/aarch64-types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//@ [arm64ec] compile-flags: --target arm64ec-pc-windows-msvc
//@ [arm64ec] needs-llvm-components: aarch64

#![feature(no_core, lang_items, rustc_attrs, repr_simd, asm_experimental_arch)]
#![feature(no_core, lang_items, rustc_attrs, repr_simd, asm_experimental_arch, f16)]
#![crate_type = "rlib"]
#![no_core]
#![allow(asm_sub_register, non_camel_case_types)]
Expand Down Expand Up @@ -39,6 +39,10 @@ pub struct i32x2(i32, i32);
#[repr(simd)]
pub struct i64x1(i64);
#[repr(simd)]
pub struct f16x4(f16, f16, f16, f16);
#[repr(simd)]
pub struct f16x8(f16, f16, f16, f16, f16, f16, f16, f16);
#[repr(simd)]
pub struct f32x2(f32, f32);
#[repr(simd)]
pub struct f64x1(f64);
Expand All @@ -57,6 +61,7 @@ pub struct f64x2(f64, f64);

impl Copy for i8 {}
impl Copy for i16 {}
impl Copy for f16 {}
impl Copy for i32 {}
impl Copy for f32 {}
impl Copy for i64 {}
Expand All @@ -67,11 +72,13 @@ impl Copy for i16x4 {}
impl Copy for i32x2 {}
impl Copy for i64x1 {}
impl Copy for f32x2 {}
impl Copy for f16x4 {}
impl Copy for f64x1 {}
impl Copy for i8x16 {}
impl Copy for i16x8 {}
impl Copy for i32x4 {}
impl Copy for i64x2 {}
impl Copy for f16x8 {}
impl Copy for f32x4 {}
impl Copy for f64x2 {}

Expand Down Expand Up @@ -165,6 +172,12 @@ check!(reg_i16 i16 reg "mov" "");
// CHECK: //NO_APP
check!(reg_i32 i32 reg "mov" "");

// CHECK-LABEL: reg_f16:
// CHECK: @APP
// CHECK: mov {{[a-z0-9]+}}, {{[a-z0-9]+}}
// CHECK: @NO_APP
check!(reg_f16 f16 reg "mov");

// CHECK-LABEL: {{("#)?}}reg_f32{{"?}}
// CHECK: //APP
// CHECK: mov x{{[0-9]+}}, x{{[0-9]+}}
Expand Down Expand Up @@ -255,6 +268,20 @@ check!(vreg_i32x2 i32x2 vreg "fmov" "s");
// CHECK: //NO_APP
check!(vreg_i64x1 i64x1 vreg "fmov" "s");

// neon-LABEL: vreg_f16x4:
// neon: @APP
// neon: vmov.f64 d{{[0-9]+}}, d{{[0-9]+}}
// neon: @NO_APP
#[cfg(neon)]
check!(vreg_f16x4 f16x4 vreg "vmov.f64");

// neon-LABEL: vreg_f16x8:
// neon: @APP
// neon: vorr q{{[0-9]+}}, q{{[0-9]+}}, q{{[0-9]+}}
// neon: @NO_APP
#[cfg(neon)]
check!(vreg_f16x8 f16x8 vreg "vmov");
Comment on lines +271 to +283
Copy link
Contributor

@tgross35 tgross35 Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these neon labels are actually getting tested, you need to add neon in the //@ revisions: list at the top and make sure --cfg neon gets passed for at least one of the tests (like

rust/tests/assembly/asm/arm-types.rs

Lines 1 to 7 in 2495953

//@ revisions: base d32 neon
//@ assembly-output: emit-asm
//@ compile-flags: --target armv7-unknown-linux-gnueabihf
//@ compile-flags: -C opt-level=0
//@[d32] compile-flags: -C target-feature=+d32
//@[neon] compile-flags: -C target-feature=+neon --cfg d32
//@[neon] filecheck-flags: --check-prefix d32
)

I'm not sure what combinations it makes sense to test, there's also a fp16 target feature. Cc @workingjubilee since you did some work on neon and flags

Copy link
Contributor

@beetrees beetrees Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The neon target feature is enabled by default on aarch64-unknown-linux-gnu and arm64ec-pc-windows-msvc, so there's no need to have any cfg unless it's been explicitly disabled.

However, once the cfg is removed there might be compilation failures on arm64ec-pc-windows-msvc due to llvm/llvm-project#94434, in which case the options are to either change the check! macro to not pass/return f16 by value or just cfg(aarch64) the f16 test cases and leave a FIXME(f16_f128) comment.


// CHECK-LABEL: {{("#)?}}vreg_f32x2{{"?}}
// CHECK: //APP
// CHECK: fmov s{{[0-9]+}}, s{{[0-9]+}}
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/asm/aarch64/type-check-3.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,23 +95,23 @@ error: type `i128` cannot be used with this register class
LL | asm!("{}", in(reg) 0i128);
| ^^^^^
|
= note: register class `reg` supports these types: i8, i16, i32, i64, f32, f64
= note: register class `reg` supports these types: i8, i16, i32, i64, f16, f32, f64

error: type `float64x2_t` cannot be used with this register class
--> $DIR/type-check-3.rs:75:28
|
LL | asm!("{}", in(reg) f64x2);
| ^^^^^
|
= note: register class `reg` supports these types: i8, i16, i32, i64, f32, f64
= note: register class `reg` supports these types: i8, i16, i32, i64, f16, f32, f64

error: type `Simd256bit` cannot be used with this register class
--> $DIR/type-check-3.rs:77:29
|
LL | asm!("{}", in(vreg) f64x4);
| ^^^^^
|
= note: register class `vreg` supports these types: i8, i16, i32, i64, f32, f64, i8x8, i16x4, i32x2, i64x1, f32x2, f64x1, i8x16, i16x8, i32x4, i64x2, f32x4, f64x2
= note: register class `vreg` supports these types: i8, i16, i32, i64, f16, f32, f64, i8x8, i16x4, i32x2, i64x1, f32x2, f64x1, i8x16, i16x8, i32x4, i64x2, f16x4, f16x8, f32x4, f64x2

error: incompatible types for asm inout argument
--> $DIR/type-check-3.rs:88:33
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/asm/aarch64/type-f16.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//@ only-aarch64
//@ run-pass

#![feature(f16, f128)]
use std::arch::asm;
#[inline(never)]
pub fn f32_to_f16_asm(a: f32) -> f16 {
let ret: f16;
unsafe {
asm!(
"fcvt {ret:h}, {a:s}",
a = in(vreg) a,
ret = lateout(vreg) ret,
options(nomem, nostack),
);
}
ret
}
fn main() {
assert_eq!(f32_to_f16_asm(1.0 as f32), 1.0);
}
Loading