> -----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.

So up to you, but I'm a bit hesitant to change this at this point without being 
able to tell what the impact is on downstream tools.

Thanks,
Tamar

> 
> Thanks,
> Richard
> 
> >        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..61cb254785a4b9ec19ebe3
> 88402223c9a82af7ed
> > --- /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/native_cpu_34.c
> b/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_34.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..168140002a0f0205c0f552
> de0cce9b2d356e09e2
> > --- /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.  */

Reply via email to