Re: [PATCH v3 3/3] aarch64: Add fpm register helper functions.

[Kyrylo Tkachov <ktkachov@nvidia.com>]

Hi Claudio,

> On 26 Jul 2024, at 18:32, Claudio Bantaloukas <claudio.bantaloukas@arm.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> The ACLE declares several helper types and functions to facilitate construction
> of `fpm` arguments. These are available when one of the arm_neon.h, arm_sve.h,
> or arm_sme.h headers is included. These helpers don't map to specific FP8
> instructions and there's no expectation that they will produce a given code
> sequence, they're just an abstraction and an aid to the programmer. Thus they are
> implemented in a new header file arm_private_fp8.h
> Users are not expected to include this file, as it is a mere implementation detail,
> subject to change. A check is included to guard against direct inclusion.
> 
> gcc/ChangeLog:
> 
>        * config.gcc (extra_headers): Install arm_private_fp8.h.
>        * config/aarch64/arm_neon.h: Include arm_private_fp8.h.
>        * config/aarch64/arm_sve.h: Likewise.
>        * config/aarch64/arm_private_fp8.h: New file
>        (fpm_t): New type representing fpmr values.
>        (enum __ARM_FPM_FORMAT): New enum representing valid fp8 formats.
>        (enum __ARM_FPM_OVERFLOW): New enum representing how some fp8
>        calculations work.
>        (__arm_fpm_init): New.
>        (__arm_set_fpm_src1_format): Likewise.
>        (__arm_set_fpm_src2_format): Likewise.
>        (__arm_set_fpm_dst_format): Likewise.
>        (__arm_set_fpm_overflow_cvt): Likewise.
>        (__arm_set_fpm_overflow_mul): Likewise.
>        (__arm_set_fpm_lscale): Likewise.
>        (__arm_set_fpm_lscale2): Likewise.
>        (__arm_set_fpm_nscale): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
>        * gcc.target/aarch64/acle/fp8-helpers-neon.c: New test of fpmr helper
>        functions.
>        * gcc.target/aarch64/acle/fp8-helpers-sve.c: New test of fpmr helper
>        functions presence.
>        * gcc.target/aarch64/acle/fp8-helpers-sme.c: New test of fpmr helper
>        functions presence.
> ---
> gcc/config.gcc                                |  2 +-
> gcc/config/aarch64/arm_neon.h                 |  1 +
> gcc/config/aarch64/arm_private_fp8.h          | 80 +++++++++++++++++++
> gcc/config/aarch64/arm_sve.h                  |  1 +
> .../aarch64/acle/fp8-helpers-neon.c           | 53 ++++++++++++
> .../gcc.target/aarch64/acle/fp8-helpers-sme.c | 12 +++
> .../gcc.target/aarch64/acle/fp8-helpers-sve.c | 12 +++
> 7 files changed, 160 insertions(+), 1 deletion(-)
> create mode 100644 gcc/config/aarch64/arm_private_fp8.h
> create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-neon.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sme.c
> create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sve.c
> 
> diff --git a/gcc/config.gcc b/gcc/config.gcc
> index 7453ade0782..a36dd1bcbc6 100644
> --- a/gcc/config.gcc
> +++ b/gcc/config.gcc
> @@ -347,7 +347,7 @@ m32c*-*-*)
>         ;;
> aarch64*-*-*)
> cpu_type=aarch64
> - extra_headers="arm_fp16.h arm_neon.h arm_bf16.h arm_acle.h arm_sve.h arm_sme.h arm_neon_sve_bridge.h"
> + extra_headers="arm_fp16.h arm_neon.h arm_bf16.h arm_acle.h arm_sve.h arm_sme.h arm_neon_sve_bridge.h arm_private_fp8.h"
> c_target_objs="aarch64-c.o"
> cxx_target_objs="aarch64-c.o"
> d_target_objs="aarch64-d.o"
> diff --git a/gcc/config/aarch64/arm_neon.h b/gcc/config/aarch64/arm_neon.h
> index c4a09528ffd..e376685489d 100644
> --- a/gcc/config/aarch64/arm_neon.h
> +++ b/gcc/config/aarch64/arm_neon.h
> @@ -30,6 +30,7 @@
> #pragma GCC push_options
> #pragma GCC target ("+nothing+simd")
> 
> +#include <arm_private_fp8.h>
> #pragma GCC aarch64 "arm_neon.h"
> 
> #include <stdint.h>
> diff --git a/gcc/config/aarch64/arm_private_fp8.h b/gcc/config/aarch64/arm_private_fp8.h
> new file mode 100644
> index 00000000000..ba93bc526c1
> --- /dev/null
> +++ b/gcc/config/aarch64/arm_private_fp8.h
> @@ -0,0 +1,80 @@
> +/* AArch64 FP8 helper functions.
> +   Do not include this file directly. Use one of arm_neon.h
> +   arm_sme.h arm_sve.h instead.
> +
> +   Copyright (C) 2024 Free Software Foundation, Inc.
> +   Contributed by ARM Ltd.
> +
> +   This file is part of GCC.
> +
> +   GCC is free software; you can redistribute it and/or modify it
> +   under the terms of the GNU General Public License as published
> +   by the Free Software Foundation; either version 3, or (at your
> +   option) any later version.
> +
> +   GCC is distributed in the hope that it will be useful, but WITHOUT
> +   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> +   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> +   License for more details.
> +
> +   Under Section 7 of GPL version 3, you are granted additional
> +   permissions described in the GCC Runtime Library Exception, version
> +   3.1, as published by the Free Software Foundation.
> +
> +   You should have received a copy of the GNU General Public License and
> +   a copy of the GCC Runtime Library Exception along with this program;
> +   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _GCC_ARM_PRIVATE_FP8_H
> +#define _GCC_ARM_PRIVATE_FP8_H
> +
> +#if !defined(_AARCH64_NEON_H_) && !defined(_ARM_SVE_H_)
> +#error "This file should not be used standalone. Please include arm_neon.h or arm_sve.h instead."
> +#endif

The message should also mention arm_sme.h as a potential solution (even though it implicitly includes arm_sve.h)

Ok with that change once the rest is approved.
Thanks,
Kyrill


> +
> +#include <stdint.h>
> +
> +#ifdef __cplusplus
> +extern "C"
> +{
> +#endif
> +
> +  typedef uint64_t fpm_t;
> +
> +  enum __ARM_FPM_FORMAT
> +  {
> +    __ARM_FPM_E5M2,
> +    __ARM_FPM_E4M3,
> +  };
> +
> +  enum __ARM_FPM_OVERFLOW
> +  {
> +    __ARM_FPM_INFNAN,
> +    __ARM_FPM_SATURATE,
> +  };
> +
> +#define __arm_fpm_init() (0)
> +
> +#define __arm_set_fpm_src1_format(__fpm, __format)                             \
> +  ((__fpm & ~(uint64_t)0x7) | (__format & (uint64_t)0x7))
> +#define __arm_set_fpm_src2_format(__fpm, __format)                             \
> +  ((__fpm & ~((uint64_t)0x7 << 3)) | ((__format & (uint64_t)0x7) << 3))
> +#define __arm_set_fpm_dst_format(__fpm, __format)                              \
> +  ((__fpm & ~((uint64_t)0x7 << 6)) | ((__format & (uint64_t)0x7) << 6))
> +#define __arm_set_fpm_overflow_cvt(__fpm, __behaviour)                         \
> +  ((__fpm & ~((uint64_t)0x1 << 15)) | ((__behaviour & (uint64_t)0x1) << 15))
> +#define __arm_set_fpm_overflow_mul(__fpm, __behaviour)                         \
> +  ((__fpm & ~((uint64_t)0x1 << 14)) | ((__behaviour & (uint64_t)0x1) << 14))
> +#define __arm_set_fpm_lscale(__fpm, __scale)                                   \
> +  ((__fpm & ~((uint64_t)0x7f << 16)) | ((__scale & (uint64_t)0x7f) << 16))
> +#define __arm_set_fpm_lscale2(__fpm, __scale)                                  \
> +  ((__fpm & ~((uint64_t)0x3f << 32)) | ((__scale & (uint64_t)0x3f) << 32))
> +#define __arm_set_fpm_nscale(__fpm, __scale)                                   \
> +  ((__fpm & ~((uint64_t)0xff << 24)) | ((__scale & (uint64_t)0xff) << 24))
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/gcc/config/aarch64/arm_sve.h b/gcc/config/aarch64/arm_sve.h
> index c2db63736a1..aa0bd9909f9 100644
> --- a/gcc/config/aarch64/arm_sve.h
> +++ b/gcc/config/aarch64/arm_sve.h
> @@ -26,6 +26,7 @@
> #define _ARM_SVE_H_
> 
> #include <stdint.h>
> +#include <arm_private_fp8.h>
> #include <arm_bf16.h>
> 
> typedef __fp16 float16_t;
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-neon.c b/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-neon.c
> new file mode 100644
> index 00000000000..ade99557a29
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-neon.c
> @@ -0,0 +1,53 @@
> +/* Test the fp8 ACLE helper functions including that they are available.
> +   unconditionally when including arm_neon.h */
> +/* { dg-do compile } */
> +/* { dg-options "-std=c90 -pedantic-errors -O1 -march=armv8-a" } */
> +
> +#include <arm_neon.h>
> +
> +void
> +test_prepare_fpmr_sysreg ()
> +{
> +
> +#define _S_EQ(expr, expected)                                                  \
> +  _Static_assert (expr == expected, #expr " == " #expected)
> +
> +  _S_EQ (__arm_fpm_init (), 0);
> +
> +  /* Bits [2:0] */
> +  _S_EQ (__arm_set_fpm_src1_format (__arm_fpm_init (), __ARM_FPM_E5M2), 0);
> +  _S_EQ (__arm_set_fpm_src1_format (__arm_fpm_init (), __ARM_FPM_E4M3), 0x1);
> +
> +  /* Bits [5:3] */
> +  _S_EQ (__arm_set_fpm_src2_format (__arm_fpm_init (), __ARM_FPM_E5M2), 0);
> +  _S_EQ (__arm_set_fpm_src2_format (__arm_fpm_init (), __ARM_FPM_E4M3), 0x8);
> +
> +  /* Bits [8:6] */
> +  _S_EQ (__arm_set_fpm_dst_format (__arm_fpm_init (), __ARM_FPM_E5M2), 0);
> +  _S_EQ (__arm_set_fpm_dst_format (__arm_fpm_init (), __ARM_FPM_E4M3), 0x40);
> +
> +  /* Bit 14 */
> +  _S_EQ (__arm_set_fpm_overflow_mul (__arm_fpm_init (), __ARM_FPM_INFNAN), 0);
> +  _S_EQ (__arm_set_fpm_overflow_mul (__arm_fpm_init (), __ARM_FPM_SATURATE),
> + 0x4000);
> +
> +  /* Bit 15 */
> +  _S_EQ (__arm_set_fpm_overflow_cvt (__arm_fpm_init (), __ARM_FPM_INFNAN), 0);
> +  _S_EQ (__arm_set_fpm_overflow_cvt (__arm_fpm_init (), __ARM_FPM_SATURATE),
> + 0x8000);
> +
> +  /* Bits [22:16] */
> +  _S_EQ (__arm_set_fpm_lscale (__arm_fpm_init (), 0), 0);
> +  _S_EQ (__arm_set_fpm_lscale (__arm_fpm_init (), 127), 0x7F0000);
> +
> +  /* Bits [37:32] */
> +  _S_EQ (__arm_set_fpm_lscale2 (__arm_fpm_init (), 0), 0);
> +  _S_EQ (__arm_set_fpm_lscale2 (__arm_fpm_init (), 63), 0x3F00000000);
> +
> +  /* Bits [31:24] */
> +  _S_EQ (__arm_set_fpm_nscale (__arm_fpm_init (), 0), 0);
> +  _S_EQ (__arm_set_fpm_nscale (__arm_fpm_init (), 127), 0x7F000000);
> +  _S_EQ (__arm_set_fpm_nscale (__arm_fpm_init (), -128), 0x80000000);
> +
> +#undef _S_EQ
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sme.c b/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sme.c
> new file mode 100644
> index 00000000000..5daab730fbe
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sme.c
> @@ -0,0 +1,12 @@
> +/* Test availability of the fp8 ACLE helper functions when including arm_sme.h.
> + */
> +/* { dg-do compile } */
> +/* { dg-options "-std=c90 -pedantic-errors -O1 -march=armv8-a" } */
> +
> +#include <arm_sme.h>
> +
> +void
> +test_fpmr_helpers_present ()
> +{
> +  (__arm_fpm_init ());
> +}
> diff --git a/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sve.c b/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sve.c
> new file mode 100644
> index 00000000000..99c5aa90cf4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/acle/fp8-helpers-sve.c
> @@ -0,0 +1,12 @@
> +/* Test availability of the fp8 ACLE helper functions when including arm_sve.h.
> + */
> +/* { dg-do compile } */
> +/* { dg-options "-std=c90 -pedantic-errors -O1 -march=armv8-a" } */
> +
> +#include <arm_sve.h>
> +
> +void
> +test_fpmr_helpers_present ()
> +{
> +  (__arm_fpm_init ());
> +}