> -----Original Message-----
> From: Richard Sandiford <richard.sandif...@arm.com>
> Sent: Monday, January 13, 2025 8:55 PM
> 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:
> >>> -----Original Message-----
> >>> From: Richard Sandiford <richard.sandif...@arm.com>
> >>> Sent: Monday, January 13, 2025 6:35 PM
> >>> 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]
> >>>
> >>> Tamar Christina <tamar.christ...@arm.com> writes:
> >>> > Hi All,
> >>> >
> >>> > in g:e91a17fe39c39e98cebe6e1cbc8064ee6846a3a7 we added the ability
> for
> >>> > -mcpu=native on unknown CPUs to still enable architecture extensions.
> >>> >
> >>> > This has worked great but was only added for homogenous systems.
> >>> >
> >>> > However the same thing works for big.LITTLE as in such system the cores
> must
> >>> > have the same extensions otherwise it doesn't fundamentally work.
> >>> >
> >>> > i.e. task migration from one core to the other wouldn't work.
> >>> >
> >>> > This extends the same handling to non-homogenous systems.
> >>> >
> >>> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >>> >
> >>> > Ok for master?
> >>> >
> >>> > Thanks,
> >>> > Tamar
> >>> >
> >>> > gcc/ChangeLog:
> >>> >
> >>> > PR target/113257
> >>> > * config/aarch64/driver-aarch64.cc (host_detect_local_cpu):
> >>> >
> >>> > gcc/testsuite/ChangeLog:
> >>> >
> >>> > PR target/113257
> >>> > * gcc.target/aarch64/cpunative/info_34: New test.
> >>> > * gcc.target/aarch64/cpunative/native_cpu_34.c: New test.
> >>> >
> >>> > ---
> >>> >
> >>> > diff --git a/gcc/config/aarch64/driver-aarch64.cc
> b/gcc/config/aarch64/driver-
> >>> aarch64.cc
> >>> > index
> >>>
> 45fce67a646351b848b7cd7d0fd35d343731c0d1..2a454daf031aa3ac81a9a2c0
> >>> 3b15c09731e4f56e 100644
> >>> > --- a/gcc/config/aarch64/driver-aarch64.cc
> >>> > +++ b/gcc/config/aarch64/driver-aarch64.cc
> >>> > @@ -449,6 +449,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 (aarch64_cpu_data[i].name == NULL)
> >>> > + {
> >>> > + 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;
> >>> > + }
> >>> > +
> >>>
> >>> Currently, if gcc recognises the host cpu, and if one-thing is more
> >>> restrictive than that cpu, gcc will warn on:
> >>>
> >>> gcc -march=one-thing -mcpu=native
> >>>
> >>> and choose one-thing. It looks like one consequence of this patch
> >>> is that, for unrecognised big.LITTLE, the command line would get
> >>> converted to:
> >>>
> >>> gcc -march=one-thing -march=above-replacement
> >>>
> >>> and so -mcpu=native would silently "win" over one-thing. Is that right?
> >>>
> >>> Perhaps we should adjust:
> >>>
> >>> " %{mcpu=native:%<mcpu=native %:local_cpu_detect(cpu)}" \
> >>>
> >>> to pass something like "cpu/arch" rather than "cpu" when -march
> >>> is not specified, so that the routine knows that it has the choice
> >>> of using either -mcpu or -march. We wouldn't get the warning, but we
> >>> would get predictable preemption of -march over -mcpu.
> >>>
> >>> Admittedly, it looks like we already have this problem with:
> >>>
> >>> if (aarch64_cpu_data[i].name == NULL)
> >>> {
> >>> 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;
> >>> }
> >>>
> >>> so I guess this is pre-existing.
> >>
> >> Yes, it looks like this was a deliberate choice that mcpu=native
> >> overrides any march, this is the case for all previous GCCs as well.
> >>
> >> i.e. GCC 12-15 (ones I had at hand) convert
> >>
> >> -march=armv8.8-a+sve -mcpu=native on an Neoverse-V1 system to
> >>
> >> '-march=armv8.8-a+sve' '-c' '-mlittle-endian' '-mabi=lp64'
> >> '-mcpu=neoverse-
> v1+sm4+crc+aes+sha3+nossbs'
> >>
> >> In both the calls to CC1 and to AS
> >>
> >> "-march=armv8.8-a+sve" "-march=armv8.4-
> a+rng+sm4+crc+aes+sha3+i8mm+bf16+sve+profile"
> >>
> >> Your request would change this long standing behavior but I don't know If
> that's correct or not.
> >>
> >> I'll admit it's inconsistent with the warning given by CC1 for the flags.
> >>
> >> The warnings are coming from CC1, and the question is if it's up to GCC to
> enforce the CC1 constraint onto binutils or not.
> >
> > I was talking about what DRIVER_SELF_SPECS does, and therefore what cc1
> > sees. My point was that, like you say, if you use:
> >
> > -march=armv8.8-a+sve -mcpu=native
> >
> > on a host that GCC recognises, cc1 will see:
> >
> > -march=armv8.8-a+sve -mcpu=...
> >
> > cc1 will then warn and compile for the -march.
> >
> > But with this patch, if you run gcc on a big.LITTLE host that GCC
> > doesn't recognise, cc1 will see:
> >
> > -march=armv8.8-a+sve -march=...
> >
> > The second -march will then silently override the first -march,
> > so cc1 won't warn, and cc1 will compile for the second -march.
> >
> > Like I say, that also seems to be the existing behaviour for
> > homogeneous systems that GCC doesn't recognise.
> >
> > That is, there are two stages:
> >
> > (1) DRIVER_SELF_SPECS converts -mcpu=native to -mcpu=something-specific,
> > via local_cpu_detect. This happens on the driver command line before
> > the driver runs any subcommands. And this is the part that is being
> > changed by the patch.
> >
> > (2) ASM_SPEC converts -mcpu to -march, regardless of where the -mcpu
> > came from. This part isn't changed by the patch (but is by the
> > other patch that you sent).
> >
> > I'm not questioning (2), which like you say is a deliberate choice.
> > But (1) means that the driver replaces -mcpu=native with -mcpu=...
> > if the driver recognises the CPU, but replaces -mcpu=native with
> > -march=... if it doesn't recognise the CPU. This means that cc1
> > sees -mcpu=native as an -march=... rather than an -mcpu=... on
> > CPUs that GCC doesn't recognise.
> >
> > So if GCC recognises the CPU, cc1 will consistently honour an
> > explicit -march over whatever replaces -mcpu=native. But if GCC doesn't
> > recognise the CPU, cc1 won't consistently honour an explicit -march over
> > whatever replaces -mcpu=native.
> >
> > So the idea was that "cpu/arch" (or whatever) would tell
> > local_cpu_detect that it can replace -mcpu with -march (because
> > there's no existing -march that it would conflict with).
> > On the other hand, "cpu" would tell local_cpu_detect that there is
> > already an -march option, and so local_cpu_detect should either
> > replace -mcpu=native with another -mcpu or drop the option.
>
> Or local_cpu_detect might be able to use -mcpu=generic-armv8-a+... instead
> of -march=armv8-a+... If so, that would be even better :)
>
> > (Sorry for over-laboured explanation -- just trying to make it
> > clearer than my first try.)
> >
I see.. Sorry for being dense... And thanks for the detailed explanation!
Here's an updated patch:
Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
Ok for master? and how do you feel about a backport for the two patches to help
distros?
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.
-- inline patch --
diff --git a/gcc/config/aarch64/driver-aarch64.cc
b/gcc/config/aarch64/driver-aarch64.cc
index
45fce67a646351b848b7cd7d0fd35d343731c0d1..731b5e486879d3670ad0feb02906c6d6bdbb1f01
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];
+ }
+
+ 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. */
@@ -405,12 +421,12 @@ host_detect_local_cpu (int argc, const char **argv)
if (aarch64_cpu_data[i].name == NULL)
{
- auto arch_info = get_arch_from_id (DEFAULT_ARCH);
+ auto cpu_info = get_cpu_from_id (DEFAULT_CPU);
- gcc_assert (arch_info);
+ gcc_assert (cpu_info);
- res = concat ("-march=", arch_info->name, NULL);
- default_flags = arch_info->flags;
+ res = concat ("-mcpu=", cpu_info->name, NULL);
+ default_flags = cpu_info->flags;
}
else if (arch)
{
@@ -449,6 +465,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 (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. */
rb19149.patch
Description: rb19149.patch
