Delia Burduv <delia.bur...@arm.com> writes:
> [...]
> diff --git a/gcc/config/aarch64/arm_bf16.h b/gcc/config/aarch64/arm_bf16.h
> index 
> 3759c0d1cb449a7f0125cc2a1433127564d66622..fb2150e1d60a590046e2c034422021aafc721e23
>  100644
> --- a/gcc/config/aarch64/arm_bf16.h
> +++ b/gcc/config/aarch64/arm_bf16.h
> @@ -28,5 +28,13 @@
>  #define _AARCH64_BF16_H_
>  
>  typedef __bf16 bfloat16_t;
> +typedef float float32_t;
> +
> +__extension__ extern __inline bfloat16_t
> +__attribute__ ((__always_inline__, __gnu_inline__, __artificial__))
> +vcvth_bf16_f32 (float32_t __a)
> +{
> +  return __builtin_aarch64_bfcvtbf (__a);
> +}

Sorry for not noticing last time, but this should be wrapped in:

#pragma GCC push_options
#pragma GCC target ("+nothing+bf16")

...

#pragma GCC pop_options

otherwise I think calling this function without +bf16 would trigger
an internal compiler error.  It would be good to have a test that
that doesn't happen (something along the lines of bfcvt-nobf16.c,
but for the scalar case).

> [...]
> diff --git 
> a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/bfcvt-compile.c 
> b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/bfcvt-compile.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..ffb5305e2e5ea1aadae07e82fd8ed6f9f247c1a9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/bfcvt-compile.c
> @@ -0,0 +1,48 @@
> +/* { dg-do assemble { target { aarch64*-*-* } } } */

The { target ... } isn't necessary here.  (Missed that in the other
review, sorry.)

> +/* { dg-require-effective-target arm_v8_2a_bf16_neon_ok } */
> +/* { dg-add-options arm_v8_2a_bf16_neon } */
> +/* { dg-additional-options "-save-temps" } */
> +/* { dg-final { check-function-bodies "**" "" {-O[^0]} } } */
> +/* { dg-skip-if "" { *-*-* } { "-fno-fat-lto-objects" } } */
> +
> +#include <arm_neon.h>
> +
> +/*
> +**test_bfcvtn:
> +**     bfcvtn\tv0.4h, v0.4s

Like with the other review, I think the literal tab you had in the
original patch looks better than \t.

> [...]
> diff --git 
> a/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/bfcvt-nosimd.c 
> b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/bfcvt-nosimd.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..8d7dffe16275de60e884c449afa0fea0b1af6081
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/bfcvt-nosimd.c
> @@ -0,0 +1,15 @@
> +/* { dg-do assemble { target { aarch64*-*-* } } } */

This needs:

/* { dg-require-effective-target aarch64_asm_bf16_ok } */

(Doesn't exist yet, but I hope to post a patch soon.)

Looks good otherwise, thanks.

Richard

Reply via email to