diff mbox series

[Arm] Define __ARM_FEATURE_BF16 when +bf16 feature is enabled

Message ID 6fa4d701-845e-48a5-a952-f8ee97d2aa4b@arm.com
State Under Review
Delegated to: Richard Earnshaw
Headers
Series [Arm] Define __ARM_FEATURE_BF16 when +bf16 feature is enabled |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Matthieu Longo Jan. 8, 2024, 5:21 p.m. UTC 
  Hi,

Arm GCC backend does not define __ARM_FEATURE_BF16 when +bf16 is 
specified (via -march option, or target pragma) whereas it is supposed 
to be tested before including arm_bf16.h (as specified in ACLE document: 
https://arm-software.github.io/acle/main/acle.html#arm_bf16h).

gcc/ChangeLog:

         * config/arm/arm-c.cc (arm_cpu_builtins): define __ARM_FEATURE_BF16
         * config/arm/arm.h: define TARGET_BF16

Ok for master ?

Matthieu

Comments

Richard Earnshaw Jan. 10, 2024, 3:25 p.m. UTC | #1 
On 08/01/2024 17:21, Matthieu Longo wrote:
> Hi,
> 
> Arm GCC backend does not define __ARM_FEATURE_BF16 when +bf16 is 
> specified (via -march option, or target pragma) whereas it is supposed 
> to be tested before including arm_bf16.h (as specified in ACLE document: 
> https://arm-software.github.io/acle/main/acle.html#arm_bf16h).
> 
> gcc/ChangeLog:
> 
>          * config/arm/arm-c.cc (arm_cpu_builtins): define 
> __ARM_FEATURE_BF16
>          * config/arm/arm.h: define TARGET_BF16
> 
> Ok for master ?
> 
> Matthieu
index 
2e181bf7f36bab1209d5358e65d9513541683632..21ca22ac71119eda4ff01709aa95002ca13b1813 
100644
--- a/gcc/config/arm/arm-c.cc
+++ b/gcc/config/arm/arm-c.cc
@@ -425,12 +425,14 @@ arm_cpu_builtins (struct cpp_reader* pfile)
  				   arm_arch_cde_coproc);

    def_or_undef_macro (pfile, "__ARM_FEATURE_MATMUL_INT8", TARGET_I8MM);
+
+  def_or_undef_macro (pfile, "__ARM_FEATURE_BF16", TARGET_BF16);
+  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
+		      TARGET_BF16_FP);
    def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_SCALAR_ARITHMETIC",
  		      TARGET_BF16_FP);
    def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_VECTOR_ARITHMETIC",
  		      TARGET_BF16_SIMD);
-  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
-		      TARGET_BF16_FP || TARGET_BF16_SIMD);

Why is the definition of __ARM_BF16_FORMAT_ALTERNATIVE changed?  And why 
is there explanation of that change?  It doesn't seem directly related 
to $subject.

R.

  }

  void
Matthieu Longo Jan. 22, 2024, 11:02 a.m. UTC | #2 
Hi Richard,

The conditions for __ARM_BF16_FORMAT_ALTERNATIVE are redundant, so I wanted to simplify it.
TARGET_BF16_SIMD implies TARGET_BF16_FP to be true as well.

#define TARGET_BF16_FP (TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_VFP5 \
			&& arm_arch8_2 && arm_arch_bf16)
#define TARGET_BF16_SIMD (TARGET_NEON && TARGET_VFP5 \
			  && arm_arch8_2 && arm_arch_bf16)
#define TARGET_NEON							\
  (TARGET_32BIT && TARGET_HARD_FLOAT					\
   && bitmap_bit_p (arm_active_target.isa, isa_bit_neon))

Please let me know if you agree on the simplification.

Regards,
Matthieu

-----Original Message-----
From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> 
Sent: Wednesday, January 10, 2024 3:26 PM
To: Matthieu Longo <Matthieu.Longo@arm.com>; gcc-patches@gcc.gnu.org
Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
Subject: Re: [PATCH][GCC][Arm] Define __ARM_FEATURE_BF16 when +bf16 feature is enabled



On 08/01/2024 17:21, Matthieu Longo wrote:
> Hi,
> 
> Arm GCC backend does not define __ARM_FEATURE_BF16 when +bf16 is 
> specified (via -march option, or target pragma) whereas it is supposed 
> to be tested before including arm_bf16.h (as specified in ACLE document:
> https://arm-software.github.io/acle/main/acle.html#arm_bf16h).
> 
> gcc/ChangeLog:
> 
>          * config/arm/arm-c.cc (arm_cpu_builtins): define
> __ARM_FEATURE_BF16
>          * config/arm/arm.h: define TARGET_BF16
> 
> Ok for master ?
> 
> Matthieu
index
2e181bf7f36bab1209d5358e65d9513541683632..21ca22ac71119eda4ff01709aa95002ca13b1813
100644
--- a/gcc/config/arm/arm-c.cc
+++ b/gcc/config/arm/arm-c.cc
@@ -425,12 +425,14 @@ arm_cpu_builtins (struct cpp_reader* pfile)
  				   arm_arch_cde_coproc);

    def_or_undef_macro (pfile, "__ARM_FEATURE_MATMUL_INT8", TARGET_I8MM);
+
+  def_or_undef_macro (pfile, "__ARM_FEATURE_BF16", TARGET_BF16);
+  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
+		      TARGET_BF16_FP);
    def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_SCALAR_ARITHMETIC",
  		      TARGET_BF16_FP);
    def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_VECTOR_ARITHMETIC",
  		      TARGET_BF16_SIMD);
-  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
-		      TARGET_BF16_FP || TARGET_BF16_SIMD);

Why is the definition of __ARM_BF16_FORMAT_ALTERNATIVE changed?  And why is there explanation of that change?  It doesn't seem directly related to $subject.

R.

  }

  void
diff mbox series

Patch

diff --git a/gcc/config/arm/arm-c.cc b/gcc/config/arm/arm-c.cc
index 2e181bf7f36bab1209d5358e65d9513541683632..21ca22ac71119eda4ff01709aa95002ca13b1813 100644
--- a/gcc/config/arm/arm-c.cc
+++ b/gcc/config/arm/arm-c.cc
@@ -425,12 +425,14 @@  arm_cpu_builtins (struct cpp_reader* pfile)
 				   arm_arch_cde_coproc);
 
   def_or_undef_macro (pfile, "__ARM_FEATURE_MATMUL_INT8", TARGET_I8MM);
+
+  def_or_undef_macro (pfile, "__ARM_FEATURE_BF16", TARGET_BF16);
+  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
+		      TARGET_BF16_FP);
   def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_SCALAR_ARITHMETIC",
 		      TARGET_BF16_FP);
   def_or_undef_macro (pfile, "__ARM_FEATURE_BF16_VECTOR_ARITHMETIC",
 		      TARGET_BF16_SIMD);
-  def_or_undef_macro (pfile, "__ARM_BF16_FORMAT_ALTERNATIVE",
-		      TARGET_BF16_FP || TARGET_BF16_SIMD);
 }
 
 void
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 2a2207c0ba1acef1c7082c89bf5f542b1466d033..e7a7fc47e606d2ead5f778dca2e63b2e894d0efe 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -252,10 +252,10 @@  emission of floating point pcs attributes.  */
 #define TARGET_I8MM (TARGET_NEON && arm_arch8_2 && arm_arch_i8mm)
 
 /* FPU supports Brain half-precision floating-point (BFloat16) extension.  */
-#define TARGET_BF16_FP (TARGET_32BIT && TARGET_HARD_FLOAT && TARGET_VFP5 \
-			&& arm_arch8_2 && arm_arch_bf16)
-#define TARGET_BF16_SIMD (TARGET_NEON && TARGET_VFP5 \
-			  && arm_arch8_2 && arm_arch_bf16)
+#define TARGET_BF16 (TARGET_32BIT && TARGET_HARD_FLOAT && arm_arch8_2 \
+			&& TARGET_VFP5 && arm_arch_bf16)
+#define TARGET_BF16_FP (TARGET_BF16)
+#define TARGET_BF16_SIMD (TARGET_BF16 && TARGET_NEON)
 
 /* Q-bit is present.  */
 #define TARGET_ARM_QBIT \