Hi All,
Since this hasn't been reviewed yet anyway I've updated this patch to also fix
the memory leaks etc.
--
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.
The loop has also been changed as Jakub specified in order to avoid a memory
leak that was present in the existing code and to be slightly more efficient.
Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
Ok for trunk?
Thanks,
Tamar
gcc/ChangeLog:
2019-02-07 Tamar Christina <[email protected]>
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:
2019-02-07 Tamar Christina <[email protected]>
PR target/88530
* gcc.target/aarch64/options_set_10.c: New test.
> -----Original Message-----
> From: [email protected] <[email protected]>
> On Behalf Of Tamar Christina
> Sent: Wednesday, January 30, 2019 14:48
> To: Jakub Jelinek <[email protected]>
> Cc: Kyrill Tkachov <[email protected]>; [email protected];
> nd <[email protected]>; James Greenhalgh <[email protected]>;
> Richard Earnshaw <[email protected]>; Marcus Shawcroft
> <[email protected]>
> Subject: Re: [PATCH][GCC][AArch64] Have empty HWCAPs string ignored
> during native feature detection
>
> Hi Jakub,
>
> On Wed, Jan 30, 2019 at 02:06:01PM +0000, Tamar Christina wrote:
> > > Thanks for the feedback, but I think those are changes for another patch.
> >
> > At least the memory leak is something that should be fixed even in
> > stage4 IMNSHO.
>
> I'll provide a separate patch for this then.
>
> > Anyway, will defer to aarch64 maintainers here.
>
> > Just one question, for the *feat_string == '\0' case, is continue what
> > you want, rather than just enabled = false; and doing the
> > extension_flags &= ~(aarch64_extensions[i].flag);
> > later on?
>
> Yeah, because the feature may be on by default due to another extension, in
> which case you would erroneously turn it off. The absence of an HWCAPS
> shouldn't pro-actively disable an extension.
>
> Regards,
> Tamar
diff --git a/gcc/config/aarch64/aarch64-option-extensions.def b/gcc/config/aarch64/aarch64-option-extensions.def
index 50909debf5455d57b86db91a0a5539fed1d5b91e..a6b3ae2b73f285e62e0f24c92bc5efefc9ffb59c 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 05f1360d48583473820c8008cc09ed139bddc0ce..9515061ec985cc374c7c510c6954a6a3c240814f 100644
--- a/gcc/config/aarch64/driver-aarch64.c
+++ b/gcc/config/aarch64/driver-aarch64.c
@@ -250,27 +250,35 @@ host_detect_local_cpu (int argc, const char **argv)
{
for (i = 0; i < num_exts; i++)
{
- char *p = NULL;
- char *feat_string
- = concat (aarch64_extensions[i].feat_string, NULL);
+ const char *p = aarch64_extensions[i].feat_string;
+
+ /* If the feature contains no HWCAPS string then ignore it for the
+ auto detection. */
+ if (*p == '\0')
+ continue;
+
bool enabled = true;
/* This may be a multi-token feature string. We need
- to match all parts, which could be in any order.
- If this isn't a multi-token feature string, strtok is
- just going to return a pointer to feat_string. */
- p = strtok (feat_string, " ");
- while (p != NULL)
+ to match all parts, which could be in any order. */
+ size_t len = strlen (buf);
+ do
{
- if (strstr (buf, p) == NULL)
+ const char *end = strchr (p, ' ');
+ if (end == NULL)
+ end = strchr (p, '\0');
+ if (memmem (buf, len, p, end - p) == NULL)
{
/* Failed to match this token. Turn off the
features we'd otherwise enable. */
enabled = false;
break;
}
- p = strtok (NULL, " ");
+ if (*end == '\0')
+ break;
+ p = end + 1;
}
+ while (1);
if (enabled)
extension_flags |= aarch64_extensions[i].flag;
@@ -360,12 +368,12 @@ host_detect_local_cpu (int argc, const char **argv)
not_found:
{
/* If detection fails we ignore the option.
- Clean up and return empty string. */
+ Clean up and return NULL. */
if (f)
fclose (f);
- return "";
+ return NULL;
}
}
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. */
