Tamar Christina <tamar.christ...@arm.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford <richard.sandif...@arm.com>
>> Sent: Thursday, January 16, 2025 7:11 AM
>> To: Tamar Christina <tamar.christ...@arm.com>
>> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; Richard Earnshaw
>> <richard.earns...@arm.com>; ktkac...@gcc.gnu.org
>> Subject: Re: [PATCH]AArch64: have -mcpu=native detect architecture extensions
>> for unknown non-homogenous systems [PR113257]
>>
>> Richard Sandiford <richard.sandif...@arm.com> writes:
>> > Tamar Christina <tamar.christ...@arm.com> writes:
>> >> Ok for master? and how do you feel about a backport for the two patches to
>> help
>> >> distros?
>> >
>> > Backporting to GCC 14 & GCC 13 sounds good. Not so sure about GCC 12,
>> > since I think we should be extra cautious with the "most stable" branch,
>> > but let's see what others think.
>> >
>> > OK for trunk, and for GCC 14 & 13 after a grace period, with one
>> > trivial nit below:
>>
>> Sorry, was concentrating too much on the -mcpu vs. -march preemption
>> thing and forgot to think about other aspects of the patch. The routine
>> is used for all three of -march=native, -mcpu=native, and -mtune=native,
>> so I think we want something like the following on top of your patch
>> (untested so far).
>>
>
> Cool, how's this one?
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master? and for backport to GCC 13 and 14?
>
> Thanks,
> Tamar
>
> gcc/ChangeLog:
>
> PR target/113257
> * config/aarch64/driver-aarch64.cc (get_cpu_from_id, DEFAULT_CPU): New.
> (host_detect_local_cpu): Use it.
>
> gcc/testsuite/ChangeLog:
>
> PR target/113257
> * gcc.target/aarch64/cpunative/info_34: New test.
> * gcc.target/aarch64/cpunative/native_cpu_34.c: New test.
> * gcc.target/aarch64/cpunative/info_35: New test.
> * gcc.target/aarch64/cpunative/native_cpu_35.c: New test.
>
> Co-authored-by: Richard Sandiford <richard.sandif...@arm.com>
>
> -- inline copy of patch --
>
> diff --git a/gcc/config/aarch64/driver-aarch64.cc
> b/gcc/config/aarch64/driver-aarch64.cc
> index
> 45fce67a646351b848b7cd7d0fd35d343731c0d1..26ba2cd6f8883300951268aab7d0a22ec2588a0d
> 100644
> --- a/gcc/config/aarch64/driver-aarch64.cc
> +++ b/gcc/config/aarch64/driver-aarch64.cc
> @@ -60,6 +60,7 @@ struct aarch64_core_data
> #define ALL_VARIANTS ((unsigned)-1)
> /* Default architecture to use if -mcpu=native did not detect a known CPU.
> */
> #define DEFAULT_ARCH "8A"
> +#define DEFAULT_CPU "generic-armv8-a"
>
> #define AARCH64_CORE(CORE_NAME, CORE_IDENT, SCHED, ARCH, FLAGS, COSTS, IMP,
> PART, VARIANT) \
> { CORE_NAME, #ARCH, IMP, PART, VARIANT, feature_deps::cpu_##CORE_IDENT },
> @@ -106,6 +107,21 @@ get_arch_from_id (const char* id)
> return NULL;
> }
>
> +/* Return an aarch64_core_data for the cpu described
> + by ID, or NULL if ID describes something we don't know about. */
> +
> +static const aarch64_core_data *
> +get_cpu_from_id (const char* name)
> +{
> + for (unsigned i = 0; aarch64_cpu_data[i].name != NULL; i++)
> + {
> + if (strcmp (name, aarch64_cpu_data[i].name) == 0)
> + return &aarch64_cpu_data[i];
> + }
Redundant braces, the convention says:
for (unsigned i = 0; aarch64_cpu_data[i].name != NULL; i++)
if (strcmp (name, aarch64_cpu_data[i].name) == 0)
return &aarch64_cpu_data[i];
OK with that change, thanks, and sorry for back-tracking on the
original ack.
Richard
> +
> + return NULL;
> +}
> +
> /* Check wether the CORE array is the same as the big.LITTLE BL_CORE.
> For an example CORE={0xd08, 0xd03} and
> BL_CORE=AARCH64_BIG_LITTLE (0xd08, 0xd03) will return true. */
> @@ -403,18 +419,11 @@ host_detect_local_cpu (int argc, const char **argv)
> || variants[0] == aarch64_cpu_data[i].variant))
> break;
>
> - if (aarch64_cpu_data[i].name == NULL)
> + if (arch)
> {
> - auto arch_info = get_arch_from_id (DEFAULT_ARCH);
> -
> - gcc_assert (arch_info);
> -
> - res = concat ("-march=", arch_info->name, NULL);
> - default_flags = arch_info->flags;
> - }
> - else if (arch)
> - {
> - const char *arch_id = aarch64_cpu_data[i].arch;
> + const char *arch_id = (aarch64_cpu_data[i].name
> + ? aarch64_cpu_data[i].arch
> + : DEFAULT_ARCH);
> auto arch_info = get_arch_from_id (arch_id);
>
> /* We got some arch indentifier that's not in aarch64-arches.def? */
> @@ -424,12 +433,15 @@ host_detect_local_cpu (int argc, const char **argv)
> res = concat ("-march=", arch_info->name, NULL);
> default_flags = arch_info->flags;
> }
> - else
> + else if (cpu || aarch64_cpu_data[i].name)
> {
> - default_flags = aarch64_cpu_data[i].flags;
> + auto cpu_info = (aarch64_cpu_data[i].name
> + ? &aarch64_cpu_data[i]
> + : get_cpu_from_id (DEFAULT_CPU));
> + default_flags = cpu_info->flags;
> res = concat ("-m",
> cpu ? "cpu" : "tune", "=",
> - aarch64_cpu_data[i].name,
> + cpu_info->name,
> NULL);
> }
> }
> @@ -449,6 +461,20 @@ host_detect_local_cpu (int argc, const char **argv)
> break;
> }
> }
> +
> + /* On big.LITTLE if we find any unknown CPUs we can still pick arch
> + features as the cores should have the same features. So just pick
> + the feature flags from any of the cpus. */
> + if (cpu && aarch64_cpu_data[i].name == NULL)
> + {
> + auto cpu_info = get_cpu_from_id (DEFAULT_CPU);
> +
> + gcc_assert (cpu_info);
> +
> + res = concat ("-mcpu=", cpu_info->name, NULL);
> + default_flags = cpu_info->flags;
> + }
> +
> if (!res)
> goto not_found;
> }
> diff --git a/gcc/testsuite/gcc.target/aarch64/cpunative/info_34
> b/gcc/testsuite/gcc.target/aarch64/cpunative/info_34
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..61cb254785a4b9ec19ebe388402223c9a82af7ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/cpunative/info_34
> @@ -0,0 +1,18 @@
> +processor : 0
> +BogoMIPS : 100.00
> +Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 asimddp sve sve2
> fphp asimdhp fcma
> +CPU implementer : 0xfe
> +CPU architecture: 8
> +CPU variant : 0x0
> +CPU part : 0xd08
> +CPU revision : 2
> +
> +processor : 0
> +BogoMIPS : 100.00
> +Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 asimddp sve sve2
> fphp asimdhp fcma
> +CPU implementer : 0xfe
> +CPU architecture: 8
> +CPU variant : 0x0
> +CPU part : 0xd09
> +CPU revision : 2
> +
> diff --git a/gcc/testsuite/gcc.target/aarch64/cpunative/info_35
> b/gcc/testsuite/gcc.target/aarch64/cpunative/info_35
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..61cb254785a4b9ec19ebe388402223c9a82af7ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/cpunative/info_35
> @@ -0,0 +1,18 @@
> +processor : 0
> +BogoMIPS : 100.00
> +Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 asimddp sve sve2
> fphp asimdhp fcma
> +CPU implementer : 0xfe
> +CPU architecture: 8
> +CPU variant : 0x0
> +CPU part : 0xd08
> +CPU revision : 2
> +
> +processor : 0
> +BogoMIPS : 100.00
> +Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 asimddp sve sve2
> fphp asimdhp fcma
> +CPU implementer : 0xfe
> +CPU architecture: 8
> +CPU variant : 0x0
> +CPU part : 0xd09
> +CPU revision : 2
> +
> diff --git a/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_34.c
> b/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_34.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..168140002a0f0205c0f552de0cce9b2d356e09e2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_34.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile { target { { aarch64*-*-linux*} && native } } } */
> +/* { dg-set-compiler-env-var GCC_CPUINFO
> "$srcdir/gcc.target/aarch64/cpunative/info_34" } */
> +/* { dg-additional-options "-mcpu=native" } */
> +
> +int main()
> +{
> + return 0;
> +}
> +
> +/* { dg-final { scan-assembler {\.arch
> armv8-a\+dotprod\+crc\+crypto\+sve2\n} } } */
> +
> +/* Test a normal looking procinfo. */
> diff --git a/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_35.c
> b/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_35.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..8c3bd6111c77c63541c49e8085ee79deea7a8a54
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_35.c
> @@ -0,0 +1,13 @@
> +/* { dg-do compile { target { { aarch64*-*-linux*} && native } } } */
> +/* { dg-set-compiler-env-var GCC_CPUINFO
> "$srcdir/gcc.target/aarch64/cpunative/info_34" } */
> +/* { dg-additional-options "-march=armv8.8-a+sve -mcpu=native" } */
> +
> +int main()
> +{
> + return 0;
> +}
> +
> +/* { dg-final { scan-assembler {\.arch armv8.8-a\+crc\+sve\n} } } */
> +/* { dg-excess-errors "" } */
> +
> +/* Test a normal looking procinfo. */
