diff mbox series

aarch64: Fix +nopredres, +nols64 and +nomops

Message ID fd8d2cd0-35b7-f32d-2c9a-f9ddfb924530@e124511.cambridge.arm.com
State Not Applicable
Headers
Series aarch64: Fix +nopredres, +nols64 and +nomops |

Checks

Context Check Description
snail/gcc-patch-check fail Git am fail log

Commit Message

Andrew Carlotti Dec. 5, 2023, 7:34 p.m. UTC 
  For native cpu feature detection, certain features have no entry in
/proc/cpuinfo, so have to be assumed to be present whenever the detected
cpu is supposed to support that feature.

However, the logic for this was mistakenly implemented by excluding
these features from part of aarch64_get_extension_string_for_isa_flags.
This function is also used elsewhere when canonicalising explicit
feature sets, which may require removing features that are normally
implied by the specified architecture version.

This change reenables generation of +nopredres, +nols64 and +nomops
during canonicalisation, by relocating the misplaced native cpu
detection logic.

gcc/ChangeLog:

	* common/config/aarch64/aarch64-common.cc
	(aarch64_get_extension_string_for_isa_flags): Remove filtering
	of features without native detection.
	* config/aarch64/driver-aarch64.cc (host_detect_local_cpu):
	Explicitly add expected features that lack cpuinfo detection.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/options_set_29.c: New test.

Comments

Richard Sandiford Dec. 9, 2023, 7:22 p.m. UTC | #1 
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> For native cpu feature detection, certain features have no entry in
> /proc/cpuinfo, so have to be assumed to be present whenever the detected
> cpu is supposed to support that feature.
>
> However, the logic for this was mistakenly implemented by excluding
> these features from part of aarch64_get_extension_string_for_isa_flags.
> This function is also used elsewhere when canonicalising explicit
> feature sets, which may require removing features that are normally
> implied by the specified architecture version.
>
> This change reenables generation of +nopredres, +nols64 and +nomops
> during canonicalisation, by relocating the misplaced native cpu
> detection logic.
>
> gcc/ChangeLog:
>
> 	* common/config/aarch64/aarch64-common.cc
> 	(aarch64_get_extension_string_for_isa_flags): Remove filtering
> 	of features without native detection.
> 	* config/aarch64/driver-aarch64.cc (host_detect_local_cpu):
> 	Explicitly add expected features that lack cpuinfo detection.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/aarch64/options_set_29.c: New test.
>
>
> diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
> index ee2ea7eae105d19ec906ef8d25d3a237fbeac4b4..37e60d6083e290b18b1f4c6274123b0a58de5476 100644
> --- a/gcc/common/config/aarch64/aarch64-common.cc
> +++ b/gcc/common/config/aarch64/aarch64-common.cc
> @@ -357,8 +357,7 @@ aarch64_get_extension_string_for_isa_flags
>        }
>  
>    for (auto &opt : all_extensions)
> -    if (opt.native_detect_p
> -	&& (opt.flag_canonical != AARCH64_FL_CRYPTO)
> +    if ((opt.flag_canonical != AARCH64_FL_CRYPTO)
>  	&& (opt.flag_canonical & current_flags & ~isa_flags))
>        {
>  	current_flags &= ~opt.flags_off;

This is the only use of native_detect_p, so it'd be good to remove
the field itself.

> diff --git a/gcc/config/aarch64/driver-aarch64.cc b/gcc/config/aarch64/driver-aarch64.cc
> index 8e318892b10aa2288421fad418844744a2f5a3b4..470c19b650f1ae953918eaeddbf0f768c12a99d9 100644
> --- a/gcc/config/aarch64/driver-aarch64.cc
> +++ b/gcc/config/aarch64/driver-aarch64.cc
> @@ -262,6 +262,7 @@ host_detect_local_cpu (int argc, const char **argv)
>    unsigned int n_variants = 0;
>    bool processed_exts = false;
>    aarch64_feature_flags extension_flags = 0;
> +  aarch64_feature_flags unchecked_extension_flags = 0;
>    aarch64_feature_flags default_flags = 0;
>    std::string buf;
>    size_t sep_pos = -1;
> @@ -348,7 +349,10 @@ host_detect_local_cpu (int argc, const char **argv)
>  	      /* If the feature contains no HWCAPS string then ignore it for the
>  		 auto detection.  */
>  	      if (val.empty ())
> -		continue;
> +		{
> +		  unchecked_extension_flags |= aarch64_extensions[i].flag;
> +		  continue;
> +		}
>  
>  	      bool enabled = true;
>  
> @@ -447,6 +451,13 @@ host_detect_local_cpu (int argc, const char **argv)
>    if (tune)
>      return res;
>  
> +  if (!processed_exts)
> +    goto not_found;

Could you explain this part?  It seems like more of a parsing change
(i.e. being more strict about what we accept).

If that's the intention, it probably belongs in:

  if (n_cores == 0
      || n_cores > 2
      || (n_cores == 1 && n_variants != 1)
      || imp == INVALID_IMP)
    goto not_found;

But maybe it should be a separate patch.

Looks good otherwise, thanks.

Richard

> +
> +  /* Add any features that should be be present, but can't be verified using
> +     the /proc/cpuinfo "Features" list.  */
> +  extension_flags |= unchecked_extension_flags & default_flags;
> +
>    {
>      std::string extension
>        = aarch64_get_extension_string_for_isa_flags (extension_flags,
> diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_29.c b/gcc/testsuite/gcc.target/aarch64/options_set_29.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..01bb73c02e232bdfeca5f16dad3fa2a6484843d5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/options_set_29.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-march=armv9.3-a+nopredres+nols64+nomops" } */
> +
> +int main ()
> +{
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times {\.arch armv9\.3\-a\+crc\+nopredres\+nols64\+nomops\n} 1 } } */
> +
> +/* Checking if enabling default features drops the superfluous bits.   */
diff mbox series

Patch

diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
index ee2ea7eae105d19ec906ef8d25d3a237fbeac4b4..37e60d6083e290b18b1f4c6274123b0a58de5476 100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -357,8 +357,7 @@  aarch64_get_extension_string_for_isa_flags
       }
 
   for (auto &opt : all_extensions)
-    if (opt.native_detect_p
-	&& (opt.flag_canonical != AARCH64_FL_CRYPTO)
+    if ((opt.flag_canonical != AARCH64_FL_CRYPTO)
 	&& (opt.flag_canonical & current_flags & ~isa_flags))
       {
 	current_flags &= ~opt.flags_off;
diff --git a/gcc/config/aarch64/driver-aarch64.cc b/gcc/config/aarch64/driver-aarch64.cc
index 8e318892b10aa2288421fad418844744a2f5a3b4..470c19b650f1ae953918eaeddbf0f768c12a99d9 100644
--- a/gcc/config/aarch64/driver-aarch64.cc
+++ b/gcc/config/aarch64/driver-aarch64.cc
@@ -262,6 +262,7 @@  host_detect_local_cpu (int argc, const char **argv)
   unsigned int n_variants = 0;
   bool processed_exts = false;
   aarch64_feature_flags extension_flags = 0;
+  aarch64_feature_flags unchecked_extension_flags = 0;
   aarch64_feature_flags default_flags = 0;
   std::string buf;
   size_t sep_pos = -1;
@@ -348,7 +349,10 @@  host_detect_local_cpu (int argc, const char **argv)
 	      /* If the feature contains no HWCAPS string then ignore it for the
 		 auto detection.  */
 	      if (val.empty ())
-		continue;
+		{
+		  unchecked_extension_flags |= aarch64_extensions[i].flag;
+		  continue;
+		}
 
 	      bool enabled = true;
 
@@ -447,6 +451,13 @@  host_detect_local_cpu (int argc, const char **argv)
   if (tune)
     return res;
 
+  if (!processed_exts)
+    goto not_found;
+
+  /* Add any features that should be be present, but can't be verified using
+     the /proc/cpuinfo "Features" list.  */
+  extension_flags |= unchecked_extension_flags & default_flags;
+
   {
     std::string extension
       = aarch64_get_extension_string_for_isa_flags (extension_flags,
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_29.c b/gcc/testsuite/gcc.target/aarch64/options_set_29.c
new file mode 100644
index 0000000000000000000000000000000000000000..01bb73c02e232bdfeca5f16dad3fa2a6484843d5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_29.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-march=armv9.3-a+nopredres+nols64+nomops" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times {\.arch armv9\.3\-a\+crc\+nopredres\+nols64\+nomops\n} 1 } } */
+
+/* Checking if enabling default features drops the superfluous bits.   */