Re: [PATCH][GCC][AArch64] Have empty HWCAPs string ignored during native feature detection

[Kyrill Tkachov]

Hi Tamar,

On 18/12/18 13:36, Tamar Christina wrote:
Hi All,
This patch makes the feature detection code for AArch64 GCC not add features
automatically when the feature had no hwcaps string to match against.

This means that -mcpu=native no longer adds feature flags such as +profile.
The behavior wasn't noticed before because at the time +profile was added a bug
was preventing any feature bits from being added by native detections.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for trunk?

Thanks,
Tamar

gcc/ChangeLog:

2018-12-18  Tamar Christina  <tamar.christ...@arm.com>

        PR target/88530
        * config/aarch64/aarch64-option-extensions.def: Document it.
        * config/aarch64/driver-aarch64.c (host_detect_local_cpu): Skip feature
        if empty hwcaps.

gcc/testsuite/ChangeLog:

2018-12-18  Tamar Christina  <tamar.christ...@arm.com>

        PR target/88530
        * gcc.target/aarch64/options_set_10.c: New test.

--

diff --git a/gcc/config/aarch64/aarch64-option-extensions.def 
b/gcc/config/aarch64/aarch64-option-extensions.def
index 
cdf04e2d5fcccb8b9a32af8f83501ce23212bbab..323e642af2e87c2d463681c3a3efbaeff2ede018
 100644
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -43,7 +43,8 @@
    the extension (for example, the 'crypto' extension depends on four
    entries: aes, pmull, sha1, sha2 being present).  In that case this field
    should contain a space (" ") separated list of the strings in 'Features'
-   that are required.  Their order is not important.  */
+   that are required.  Their order is not important.  An empty string means
+   do not detect this feature during auto detection.  */
 
 /* Enabling "fp" just enables "fp".
    Disabling "fp" also disables "simd", "crypto", "fp16", "aes", "sha2",
diff --git a/gcc/config/aarch64/driver-aarch64.c 
b/gcc/config/aarch64/driver-aarch64.c
index 
98f9d7959506338bd6a8524500a168cc22ef5396..4f386dbf5fc29cc54ff85e062d0b9cd146fa00e8
 100644
--- a/gcc/config/aarch64/driver-aarch64.c
+++ b/gcc/config/aarch64/driver-aarch64.c
@@ -253,6 +253,12 @@ host_detect_local_cpu (int argc, const char **argv)
              char *p = NULL;
              char *feat_string
                = concat (aarch64_extensions[i].feat_string, NULL);
+
+             /* If the feature contains no HWCAPS string then ignore it for the
+                auto detection.  */
+             if (strlen (feat_string) == 0)
+               continue;

I think this can avoid a strlen call by checking (*feat_string == '\0') though 
I believe most compilers will optimise it that way anyway.
It might be more immediately readable your way.
I wouldn't let it hold off this patch.

Looks good to me, but you'll need a maintainer to approve.

Thanks,
Kyrill


+
              bool enabled = true;
 
 	      /* This may be a multi-token feature string.  We need
diff --git a/gcc/testsuite/gcc.target/aarch64/options_set_10.c 
b/gcc/testsuite/gcc.target/aarch64/options_set_10.c
new file mode 100644
index 
0000000000000000000000000000000000000000..5ffe83c199165dd4129814674297056bdf27cd83
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/options_set_10.c
@@ -0,0 +1,11 @@
+/* { dg-do compile { target "aarch64*-*-linux*" } } */
+/* { dg-additional-options "-mcpu=native" } */
+
+int main ()
+{
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not {\.arch .+\+profile.*} } } */
+
+ /* Check that an empty feature string is not detected during mcpu=native.  */