[11/11] aarch64: Make AARCH64_FL_CRYPTO always unset

Message ID b86c4ae5-d0ac-c60b-2a63-86a115e611cb@e124511.cambridge.arm.com
State New
Headers
Series aarch64: Refactor target parsing |

Checks

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

Commit Message

Andrew Carlotti Jan. 10, 2025, 5:24 p.m. UTC
  This feature flag bit only exists to support the +crypto alias.  Outside
of option processing this bit needs to be set or unset consistently.
This patch goes with the latter option.

gcc/ChangeLog:

	* common/config/aarch64/aarch64-common.cc: Assert that CRYPTO
	bit is not set.
	* config/aarch64/aarch64-feature-deps.h
	(info<FEAT>.explicit_on): Unset CRYPTO bit.
	(cpu_##CORE_IDENT): Ditto.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/crypto-alias-1.c: New test.
  

Comments

Richard Sandiford Jan. 16, 2025, 1:36 p.m. UTC | #1
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> This feature flag bit only exists to support the +crypto alias.  Outside
> of option processing this bit needs to be set or unset consistently.
> This patch goes with the latter option.
>
> gcc/ChangeLog:
>
> 	* common/config/aarch64/aarch64-common.cc: Assert that CRYPTO
> 	bit is not set.
> 	* config/aarch64/aarch64-feature-deps.h
> 	(info<FEAT>.explicit_on): Unset CRYPTO bit.
> 	(cpu_##CORE_IDENT): Ditto.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/aarch64/crypto-alias-1.c: New test.

OK, thanks.  I'd rather get rid of AARCH64_FL_CRYPTO instead (without
changing observable behaviour), but that's a bigger project and not
suitable for this stage.

Richard

> diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
> index 1848d31c2c23e053535458044e0fcfd38b8f659b..8af3aa71be8a8d56ea3654e194dc58e81345178f 100644
> --- a/gcc/common/config/aarch64/aarch64-common.cc
> +++ b/gcc/common/config/aarch64/aarch64-common.cc
> @@ -620,6 +620,10 @@ aarch64_get_extension_string_for_isa_flags
>  {
>    std::string outstr = "";
>  
> +  /* The CRYPTO bit should only be used to support the +crypto alias
> +     during option processing, and should be cleared at all other times.
> +     Verify this property for the supplied flags bitmask.  */
> +  gcc_assert (!(AARCH64_FL_CRYPTO & aarch64_isa_flags));
>    aarch64_feature_flags current_flags = default_arch_flags;
>  
>    /* As a special case, do not assume that the assembler will enable CRC
> diff --git a/gcc/config/aarch64/aarch64-feature-deps.h b/gcc/config/aarch64/aarch64-feature-deps.h
> index 67c3a5da8aa3f59607b9eb20fb329c6fdef2d46f..55a0dbfae6107388d97528b637f9120cc6b933a1 100644
> --- a/gcc/config/aarch64/aarch64-feature-deps.h
> +++ b/gcc/config/aarch64/aarch64-feature-deps.h
> @@ -56,7 +56,8 @@ get_enable (T1 i, Ts... args)
>  
>     - explicit_on: the transitive closure of the features that an
>       explicit +FEATURE enables, including FLAG itself.  This is
> -     always a superset of ENABLE
> +     always a superset of ENABLE, except that the CRYPTO alias bit is
> +     explicitly unset for consistency.
>  
>     Also define a function FEATURE () that returns an info<FEATURE>
>     (which is an empty structure, since all members are static).
> @@ -69,7 +70,8 @@ template<aarch64_feature> struct info;
>    template<> struct info<aarch64_feature::IDENT> {			\
>      static constexpr auto flag = AARCH64_FL_##IDENT;			\
>      static constexpr auto enable = flag | get_enable REQUIRES;		\
> -    static constexpr auto explicit_on = enable | get_enable EXPLICIT_ON; \
> +    static constexpr auto explicit_on                                   \
> +      = (enable | get_enable EXPLICIT_ON) & ~AARCH64_FL_CRYPTO;         \
>    };									\
>    constexpr aarch64_feature_flags info<aarch64_feature::IDENT>::flag;	\
>    constexpr aarch64_feature_flags info<aarch64_feature::IDENT>::enable;	\
> @@ -114,9 +116,11 @@ get_flags_off (aarch64_feature_flags mask)
>  #include "config/aarch64/aarch64-option-extensions.def"
>  
>  /* Define cpu_<NAME> variables for each CPU, giving the transitive
> -   closure of all the features that the CPU supports.  */
> +   closure of all the features that the CPU supports.  The CRYPTO bit is just
> +   an alias, so explicitly unset it for consistency.  */
>  #define AARCH64_CORE(A, CORE_IDENT, C, ARCH_IDENT, FEATURES, F, G, H, I) \
> -  constexpr auto cpu_##CORE_IDENT = ARCH_IDENT ().enable | get_enable FEATURES;
> +  constexpr auto cpu_##CORE_IDENT \
> +    = (ARCH_IDENT ().enable | get_enable FEATURES) & ~AARCH64_FL_CRYPTO;
>  #include "config/aarch64/aarch64-cores.def"
>  
>  /* Define fmv_deps_<NAME> variables for each FMV feature, giving the transitive
> diff --git a/gcc/testsuite/gcc.target/aarch64/crypto-alias-1.c b/gcc/testsuite/gcc.target/aarch64/crypto-alias-1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..e2662b44e2db0b26f30c44e62c4b873a12a37972
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/crypto-alias-1.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mcpu=ampere1" } */
> +
> +__attribute__ ((__always_inline__))
> +__attribute__ ((target ("arch=armv8-a+crypto")))
> +inline int bar()
> +{
> +  return 5;
> +}
> +
> +int foo()
> +{
> +  return bar();
> +}
  

Patch

diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
index 1848d31c2c23e053535458044e0fcfd38b8f659b..8af3aa71be8a8d56ea3654e194dc58e81345178f 100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -620,6 +620,10 @@  aarch64_get_extension_string_for_isa_flags
 {
   std::string outstr = "";
 
+  /* The CRYPTO bit should only be used to support the +crypto alias
+     during option processing, and should be cleared at all other times.
+     Verify this property for the supplied flags bitmask.  */
+  gcc_assert (!(AARCH64_FL_CRYPTO & aarch64_isa_flags));
   aarch64_feature_flags current_flags = default_arch_flags;
 
   /* As a special case, do not assume that the assembler will enable CRC
diff --git a/gcc/config/aarch64/aarch64-feature-deps.h b/gcc/config/aarch64/aarch64-feature-deps.h
index 67c3a5da8aa3f59607b9eb20fb329c6fdef2d46f..55a0dbfae6107388d97528b637f9120cc6b933a1 100644
--- a/gcc/config/aarch64/aarch64-feature-deps.h
+++ b/gcc/config/aarch64/aarch64-feature-deps.h
@@ -56,7 +56,8 @@  get_enable (T1 i, Ts... args)
 
    - explicit_on: the transitive closure of the features that an
      explicit +FEATURE enables, including FLAG itself.  This is
-     always a superset of ENABLE
+     always a superset of ENABLE, except that the CRYPTO alias bit is
+     explicitly unset for consistency.
 
    Also define a function FEATURE () that returns an info<FEATURE>
    (which is an empty structure, since all members are static).
@@ -69,7 +70,8 @@  template<aarch64_feature> struct info;
   template<> struct info<aarch64_feature::IDENT> {			\
     static constexpr auto flag = AARCH64_FL_##IDENT;			\
     static constexpr auto enable = flag | get_enable REQUIRES;		\
-    static constexpr auto explicit_on = enable | get_enable EXPLICIT_ON; \
+    static constexpr auto explicit_on                                   \
+      = (enable | get_enable EXPLICIT_ON) & ~AARCH64_FL_CRYPTO;         \
   };									\
   constexpr aarch64_feature_flags info<aarch64_feature::IDENT>::flag;	\
   constexpr aarch64_feature_flags info<aarch64_feature::IDENT>::enable;	\
@@ -114,9 +116,11 @@  get_flags_off (aarch64_feature_flags mask)
 #include "config/aarch64/aarch64-option-extensions.def"
 
 /* Define cpu_<NAME> variables for each CPU, giving the transitive
-   closure of all the features that the CPU supports.  */
+   closure of all the features that the CPU supports.  The CRYPTO bit is just
+   an alias, so explicitly unset it for consistency.  */
 #define AARCH64_CORE(A, CORE_IDENT, C, ARCH_IDENT, FEATURES, F, G, H, I) \
-  constexpr auto cpu_##CORE_IDENT = ARCH_IDENT ().enable | get_enable FEATURES;
+  constexpr auto cpu_##CORE_IDENT \
+    = (ARCH_IDENT ().enable | get_enable FEATURES) & ~AARCH64_FL_CRYPTO;
 #include "config/aarch64/aarch64-cores.def"
 
 /* Define fmv_deps_<NAME> variables for each FMV feature, giving the transitive
diff --git a/gcc/testsuite/gcc.target/aarch64/crypto-alias-1.c b/gcc/testsuite/gcc.target/aarch64/crypto-alias-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..e2662b44e2db0b26f30c44e62c4b873a12a37972
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/crypto-alias-1.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcpu=ampere1" } */
+
+__attribute__ ((__always_inline__))
+__attribute__ ((target ("arch=armv8-a+crypto")))
+inline int bar()
+{
+  return 5;
+}
+
+int foo()
+{
+  return bar();
+}