| Message ID | 1581650284-3332-1-git-send-email-apinski@marvell.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Allow -mcpu=generic -march=armv8.5-a | expand |
On 14/02/2020 03:18, apinski@marvell.com wrote: > From: Andrew Pinski <apinski@marvell.com> > > Right if someone supplies a -mcpu= option and then overrides > that option with -march=*, we get a warning when they conflict. > What we need is a generic cpu for each arch level but that is not > that useful because the only difference would be the arch level. > The best option is to allow -mcpu=generic -march=armv8.5-a not to > warn and that is now a generic armv8.5-a arch. > Then they should use -mtune=generic, rather than -mcpu. R. > OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. > > Thanks, > Andrew Pinski > > ChangeLog: > * config/aarch64/aarch64.c (aarch64_override_options): Don't > warn when the selected cpu was generic. > --- > gcc/config/aarch64/aarch64.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 4a34dce..9173afe 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -14075,10 +14075,12 @@ aarch64_override_options (void) > explicit_tune_core = selected_tune->ident; > } > /* If both -mcpu and -march are specified check that they are architecturally > - compatible, warn if they're not and prefer the -march ISA flags. */ > + compatible, warn if they're not and prefer the -march ISA flags. > + Only warn if not using the generic cpu. */ > else if (selected_arch) > { > - if (selected_arch->arch != selected_cpu->arch) > + if (selected_cpu->ident != generic > + && selected_arch->arch != selected_cpu->arch) > { > warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch", > all_architectures[selected_cpu->arch].name, >
On Fri, Feb 14, 2020 at 2:12 AM Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote: > > On 14/02/2020 03:18, apinski@marvell.com wrote: > > From: Andrew Pinski <apinski@marvell.com> > > > > Right if someone supplies a -mcpu= option and then overrides > > that option with -march=*, we get a warning when they conflict. > > What we need is a generic cpu for each arch level but that is not > > that useful because the only difference would be the arch level. > > The best option is to allow -mcpu=generic -march=armv8.5-a not to > > warn and that is now a generic armv8.5-a arch. > > > > Then they should use -mtune=generic, rather than -mcpu. Does it make sense to run: "make check RUNTESTFLAGS="--target_board=unix/{,-mcpu=octeontx2}" to make sure there are no latent bugs floating around with slightly different tuning/scheduling? The majority of the aarch64.exp failures are due to that warning. If not how would suggest to test a -mcpu= option? There is another use case: A specific object file is to be run only on armv8.5-a processors but someone sets CFLAGS to include -mcpu=octeontx2. How would you suggest going about handling this case? These are the two major cases where having a -mcpu=generic which overrides a previous -mcpu= option and still able to select a higher -march= option. Thanks, Andrew Pinski > > R. > > OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. > > > > Thanks, > > Andrew Pinski > > > > ChangeLog: > > * config/aarch64/aarch64.c (aarch64_override_options): Don't > > warn when the selected cpu was generic. > > --- > > gcc/config/aarch64/aarch64.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > > index 4a34dce..9173afe 100644 > > --- a/gcc/config/aarch64/aarch64.c > > +++ b/gcc/config/aarch64/aarch64.c > > @@ -14075,10 +14075,12 @@ aarch64_override_options (void) > > explicit_tune_core = selected_tune->ident; > > } > > /* If both -mcpu and -march are specified check that they are architecturally > > - compatible, warn if they're not and prefer the -march ISA flags. */ > > + compatible, warn if they're not and prefer the -march ISA flags. > > + Only warn if not using the generic cpu. */ > > else if (selected_arch) > > { > > - if (selected_arch->arch != selected_cpu->arch) > > + if (selected_cpu->ident != generic > > + && selected_arch->arch != selected_cpu->arch) > > { > > warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch", > > all_architectures[selected_cpu->arch].name, > > >
On 14/02/2020 10:41, Andrew Pinski wrote: > On Fri, Feb 14, 2020 at 2:12 AM Richard Earnshaw (lists) > <Richard.Earnshaw@arm.com> wrote: >> >> On 14/02/2020 03:18, apinski@marvell.com wrote: >>> From: Andrew Pinski <apinski@marvell.com> >>> >>> Right if someone supplies a -mcpu= option and then overrides >>> that option with -march=*, we get a warning when they conflict. >>> What we need is a generic cpu for each arch level but that is not >>> that useful because the only difference would be the arch level. >>> The best option is to allow -mcpu=generic -march=armv8.5-a not to >>> warn and that is now a generic armv8.5-a arch. >>> >> >> Then they should use -mtune=generic, rather than -mcpu. > > Does it make sense to run: > "make check RUNTESTFLAGS="--target_board=unix/{,-mcpu=octeontx2}" > to make sure there are no latent bugs floating around with slightly > different tuning/scheduling? > The majority of the aarch64.exp failures are due to that warning. > If not how would suggest to test a -mcpu= option? > > There is another use case: > A specific object file is to be run only on armv8.5-a processors but > someone sets CFLAGS to include -mcpu=octeontx2. > How would you suggest going about handling this case? > > These are the two major cases where having a -mcpu=generic which > overrides a previous -mcpu= option and still able to select a higher > -march= option. > -mcpu=generic should behave *exactly* the same way as specifying the CPU you have, so to take an example, if your cpu is a Cortex-A72, then -mcpu=generic and -mcpu=cortex-a72 should result in exactly the same compiler output and diagnostics should be generated as if you'd specified the latter. Changing the behaviour just for generic therefore does not make sense. R. > Thanks, > Andrew Pinski > > >> >> R. >>> OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. >>> >>> Thanks, >>> Andrew Pinski >>> >>> ChangeLog: >>> * config/aarch64/aarch64.c (aarch64_override_options): Don't >>> warn when the selected cpu was generic. >>> --- >>> gcc/config/aarch64/aarch64.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >>> index 4a34dce..9173afe 100644 >>> --- a/gcc/config/aarch64/aarch64.c >>> +++ b/gcc/config/aarch64/aarch64.c >>> @@ -14075,10 +14075,12 @@ aarch64_override_options (void) >>> explicit_tune_core = selected_tune->ident; >>> } >>> /* If both -mcpu and -march are specified check that they are architecturally >>> - compatible, warn if they're not and prefer the -march ISA flags. */ >>> + compatible, warn if they're not and prefer the -march ISA flags. >>> + Only warn if not using the generic cpu. */ >>> else if (selected_arch) >>> { >>> - if (selected_arch->arch != selected_cpu->arch) >>> + if (selected_cpu->ident != generic >>> + && selected_arch->arch != selected_cpu->arch) >>> { >>> warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch", >>> all_architectures[selected_cpu->arch].name, >>> >>
"Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: > On 14/02/2020 10:41, Andrew Pinski wrote: >> On Fri, Feb 14, 2020 at 2:12 AM Richard Earnshaw (lists) >> <Richard.Earnshaw@arm.com> wrote: >>> >>> On 14/02/2020 03:18, apinski@marvell.com wrote: >>>> From: Andrew Pinski <apinski@marvell.com> >>>> >>>> Right if someone supplies a -mcpu= option and then overrides >>>> that option with -march=*, we get a warning when they conflict. >>>> What we need is a generic cpu for each arch level but that is not >>>> that useful because the only difference would be the arch level. >>>> The best option is to allow -mcpu=generic -march=armv8.5-a not to >>>> warn and that is now a generic armv8.5-a arch. >>>> >>> >>> Then they should use -mtune=generic, rather than -mcpu. >> >> Does it make sense to run: >> "make check RUNTESTFLAGS="--target_board=unix/{,-mcpu=octeontx2}" >> to make sure there are no latent bugs floating around with slightly >> different tuning/scheduling? >> The majority of the aarch64.exp failures are due to that warning. >> If not how would suggest to test a -mcpu= option? >> >> There is another use case: >> A specific object file is to be run only on armv8.5-a processors but >> someone sets CFLAGS to include -mcpu=octeontx2. >> How would you suggest going about handling this case? >> >> These are the two major cases where having a -mcpu=generic which >> overrides a previous -mcpu= option and still able to select a higher >> -march= option. >> > > -mcpu=generic should behave *exactly* the same way as specifying the CPU > you have, so to take an example, if your cpu is a Cortex-A72, then > -mcpu=generic and -mcpu=cortex-a72 should result in exactly the same > compiler output and diagnostics should be generated as if you'd > specified the latter. Changing the behaviour just for generic therefore > does not make sense. Are you sure? That sounds more like -mcpu=native than -mcpu=generic. AFAICT, -mcpu=generic always selects base Armv8 with generic tuning (i.e. the default choice for default configure-time arguments). It sounds like the use case here is to nullify the effect of a previous -mcpu, a bit like "-mno-cpu" would if it was allowed. If so, I guess that would need to be a new option. But could you test using: "make check RUNTESTFLAGS="--target_board=unix/{,-march=armv8.2-a+.../-mtune=octeontx2}" instead? It's more awkward to specify, but probably easier than making sure that the "-mno-cpu" equivalent is used in every test that needs it. Thanks, Richard
On 17/02/2020 15:42, Richard Sandiford wrote: > "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: >> On 14/02/2020 10:41, Andrew Pinski wrote: >>> On Fri, Feb 14, 2020 at 2:12 AM Richard Earnshaw (lists) >>> <Richard.Earnshaw@arm.com> wrote: >>>> >>>> On 14/02/2020 03:18, apinski@marvell.com wrote: >>>>> From: Andrew Pinski <apinski@marvell.com> >>>>> >>>>> Right if someone supplies a -mcpu= option and then overrides >>>>> that option with -march=*, we get a warning when they conflict. >>>>> What we need is a generic cpu for each arch level but that is not >>>>> that useful because the only difference would be the arch level. >>>>> The best option is to allow -mcpu=generic -march=armv8.5-a not to >>>>> warn and that is now a generic armv8.5-a arch. >>>>> >>>> >>>> Then they should use -mtune=generic, rather than -mcpu. >>> >>> Does it make sense to run: >>> "make check RUNTESTFLAGS="--target_board=unix/{,-mcpu=octeontx2}" >>> to make sure there are no latent bugs floating around with slightly >>> different tuning/scheduling? >>> The majority of the aarch64.exp failures are due to that warning. >>> If not how would suggest to test a -mcpu= option? >>> >>> There is another use case: >>> A specific object file is to be run only on armv8.5-a processors but >>> someone sets CFLAGS to include -mcpu=octeontx2. >>> How would you suggest going about handling this case? >>> >>> These are the two major cases where having a -mcpu=generic which >>> overrides a previous -mcpu= option and still able to select a higher >>> -march= option. >>> >> >> -mcpu=generic should behave *exactly* the same way as specifying the CPU >> you have, so to take an example, if your cpu is a Cortex-A72, then >> -mcpu=generic and -mcpu=cortex-a72 should result in exactly the same >> compiler output and diagnostics should be generated as if you'd >> specified the latter. Changing the behaviour just for generic therefore >> does not make sense. > > Are you sure? That sounds more like -mcpu=native than -mcpu=generic. > > AFAICT, -mcpu=generic always selects base Armv8 with generic tuning (i.e. the > default choice for default configure-time arguments). > > It sounds like the use case here is to nullify the effect of a previous -mcpu, > a bit like "-mno-cpu" would if it was allowed. If so, I guess that would need > to be a new option. But could you test using: > > "make check RUNTESTFLAGS="--target_board=unix/{,-march=armv8.2-a+.../-mtune=octeontx2}" > > instead? It's more awkward to specify, but probably easier than making sure > that the "-mno-cpu" equivalent is used in every test that needs it. > > Thanks, > Richard > My apologies, yes, you're right. I was thinking about 'native' rather than generic. However, the real problem is that -mcpu=generic is (and always has been) misnamed. Generic what? What it should have is an architecture version in the name directly, so generic-armv8-a, generic-armv8.1-a, etc. R.
On Mon, Feb 17, 2020 at 7:56 AM Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote: > > On 17/02/2020 15:42, Richard Sandiford wrote: > > "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: > >> On 14/02/2020 10:41, Andrew Pinski wrote: > >>> On Fri, Feb 14, 2020 at 2:12 AM Richard Earnshaw (lists) > >>> <Richard.Earnshaw@arm.com> wrote: > >>>> > >>>> On 14/02/2020 03:18, apinski@marvell.com wrote: > >>>>> From: Andrew Pinski <apinski@marvell.com> > >>>>> > >>>>> Right if someone supplies a -mcpu= option and then overrides > >>>>> that option with -march=*, we get a warning when they conflict. > >>>>> What we need is a generic cpu for each arch level but that is not > >>>>> that useful because the only difference would be the arch level. > >>>>> The best option is to allow -mcpu=generic -march=armv8.5-a not to > >>>>> warn and that is now a generic armv8.5-a arch. > >>>>> > >>>> > >>>> Then they should use -mtune=generic, rather than -mcpu. > >>> > >>> Does it make sense to run: > >>> "make check RUNTESTFLAGS="--target_board=unix/{,-mcpu=octeontx2}" > >>> to make sure there are no latent bugs floating around with slightly > >>> different tuning/scheduling? > >>> The majority of the aarch64.exp failures are due to that warning. > >>> If not how would suggest to test a -mcpu= option? > >>> > >>> There is another use case: > >>> A specific object file is to be run only on armv8.5-a processors but > >>> someone sets CFLAGS to include -mcpu=octeontx2. > >>> How would you suggest going about handling this case? > >>> > >>> These are the two major cases where having a -mcpu=generic which > >>> overrides a previous -mcpu= option and still able to select a higher > >>> -march= option. > >>> > >> > >> -mcpu=generic should behave *exactly* the same way as specifying the CPU > >> you have, so to take an example, if your cpu is a Cortex-A72, then > >> -mcpu=generic and -mcpu=cortex-a72 should result in exactly the same > >> compiler output and diagnostics should be generated as if you'd > >> specified the latter. Changing the behaviour just for generic therefore > >> does not make sense. > > > > Are you sure? That sounds more like -mcpu=native than -mcpu=generic. > > > > AFAICT, -mcpu=generic always selects base Armv8 with generic tuning (i.e. the > > default choice for default configure-time arguments). > > > > It sounds like the use case here is to nullify the effect of a previous -mcpu, > > a bit like "-mno-cpu" would if it was allowed. If so, I guess that would need > > to be a new option. But could you test using: > > > > "make check RUNTESTFLAGS="--target_board=unix/{,-march=armv8.2-a+.../-mtune=octeontx2}" > > > > instead? It's more awkward to specify, but probably easier than making sure > > that the "-mno-cpu" equivalent is used in every test that needs it. > > > > Thanks, > > Richard > > > > My apologies, yes, you're right. I was thinking about 'native' rather > than generic. > > However, the real problem is that -mcpu=generic is (and always has been) > misnamed. Generic what? What it should have is an architecture version > in the name directly, so generic-armv8-a, generic-armv8.1-a, etc. Then how about we add those (keeping generic as ARMv8-a though). Is adding those a good idea? Something like: diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index b58aca652b2..d9e42c790f8 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1278,6 +1278,10 @@ static const struct processor all_cores[] = #include "aarch64-cores.def" {"generic", generic, cortexa53, AARCH64_ARCH_8A, 8, AARCH64_FL_FOR_ARCH8, &generic_tunings}, +#define AARCH64_ARCH(NAME, CORE, ARCH_IDENT, ARCH_REV, FLAGS) \ + {"generic-" NAME, CORE, cortexa53, AARCH64_ARCH_##ARCH_IDENT, \ + ARCH_REV, FLAGS, &generic_tunings }, +#include "aarch64-arches.def" {NULL, aarch64_none, aarch64_none, aarch64_no_arch, 0, 0, NULL} }; Will add "generic-armv8.1-a", etc. And then we can use that in the testsuite. Thanks, Andrew Pinski Thanks, Andrew Pinski > > R.
On 17/02/2020 17:54, Andrew Pinski wrote: > On Mon, Feb 17, 2020 at 7:56 AM Richard Earnshaw (lists) > <Richard.Earnshaw@arm.com> wrote: >> >> On 17/02/2020 15:42, Richard Sandiford wrote: >>> "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes: >>>> On 14/02/2020 10:41, Andrew Pinski wrote: >>>>> On Fri, Feb 14, 2020 at 2:12 AM Richard Earnshaw (lists) >>>>> <Richard.Earnshaw@arm.com> wrote: >>>>>> >>>>>> On 14/02/2020 03:18, apinski@marvell.com wrote: >>>>>>> From: Andrew Pinski <apinski@marvell.com> >>>>>>> >>>>>>> Right if someone supplies a -mcpu= option and then overrides >>>>>>> that option with -march=*, we get a warning when they conflict. >>>>>>> What we need is a generic cpu for each arch level but that is not >>>>>>> that useful because the only difference would be the arch level. >>>>>>> The best option is to allow -mcpu=generic -march=armv8.5-a not to >>>>>>> warn and that is now a generic armv8.5-a arch. >>>>>>> >>>>>> >>>>>> Then they should use -mtune=generic, rather than -mcpu. >>>>> >>>>> Does it make sense to run: >>>>> "make check RUNTESTFLAGS="--target_board=unix/{,-mcpu=octeontx2}" >>>>> to make sure there are no latent bugs floating around with slightly >>>>> different tuning/scheduling? >>>>> The majority of the aarch64.exp failures are due to that warning. >>>>> If not how would suggest to test a -mcpu= option? >>>>> >>>>> There is another use case: >>>>> A specific object file is to be run only on armv8.5-a processors but >>>>> someone sets CFLAGS to include -mcpu=octeontx2. >>>>> How would you suggest going about handling this case? >>>>> >>>>> These are the two major cases where having a -mcpu=generic which >>>>> overrides a previous -mcpu= option and still able to select a higher >>>>> -march= option. >>>>> >>>> >>>> -mcpu=generic should behave *exactly* the same way as specifying the CPU >>>> you have, so to take an example, if your cpu is a Cortex-A72, then >>>> -mcpu=generic and -mcpu=cortex-a72 should result in exactly the same >>>> compiler output and diagnostics should be generated as if you'd >>>> specified the latter. Changing the behaviour just for generic therefore >>>> does not make sense. >>> >>> Are you sure? That sounds more like -mcpu=native than -mcpu=generic. >>> >>> AFAICT, -mcpu=generic always selects base Armv8 with generic tuning (i.e. the >>> default choice for default configure-time arguments). >>> >>> It sounds like the use case here is to nullify the effect of a previous -mcpu, >>> a bit like "-mno-cpu" would if it was allowed. If so, I guess that would need >>> to be a new option. But could you test using: >>> >>> "make check RUNTESTFLAGS="--target_board=unix/{,-march=armv8.2-a+.../-mtune=octeontx2}" >>> >>> instead? It's more awkward to specify, but probably easier than making sure >>> that the "-mno-cpu" equivalent is used in every test that needs it. >>> >>> Thanks, >>> Richard >>> >> >> My apologies, yes, you're right. I was thinking about 'native' rather >> than generic. >> >> However, the real problem is that -mcpu=generic is (and always has been) >> misnamed. Generic what? What it should have is an architecture version >> in the name directly, so generic-armv8-a, generic-armv8.1-a, etc. > > Then how about we add those (keeping generic as ARMv8-a though). Is > adding those a good idea? > Something like: > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index b58aca652b2..d9e42c790f8 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -1278,6 +1278,10 @@ static const struct processor all_cores[] = > #include "aarch64-cores.def" > {"generic", generic, cortexa53, AARCH64_ARCH_8A, 8, > AARCH64_FL_FOR_ARCH8, &generic_tunings}, > +#define AARCH64_ARCH(NAME, CORE, ARCH_IDENT, ARCH_REV, FLAGS) \ > + {"generic-" NAME, CORE, cortexa53, AARCH64_ARCH_##ARCH_IDENT, \ > + ARCH_REV, FLAGS, &generic_tunings }, > +#include "aarch64-arches.def" > {NULL, aarch64_none, aarch64_none, aarch64_no_arch, 0, 0, NULL} > }; The concept is OK, but the macro expansion doesn't look right. We want to pick up the cost tuning/scheduling specified with each architecture (currently they all fall back to generic, but in future we might have a separate set of costs and scheduling for a later generic architecture. So why hard-code the tunings (and why cortexa53 in there at all)? R. > > Will add "generic-armv8.1-a", etc. And then we can use that in the testsuite. > > Thanks, > Andrew Pinski > > > Thanks, > Andrew Pinski > >> >> R.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 4a34dce..9173afe 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -14075,10 +14075,12 @@ aarch64_override_options (void) explicit_tune_core = selected_tune->ident; } /* If both -mcpu and -march are specified check that they are architecturally - compatible, warn if they're not and prefer the -march ISA flags. */ + compatible, warn if they're not and prefer the -march ISA flags. + Only warn if not using the generic cpu. */ else if (selected_arch) { - if (selected_arch->arch != selected_cpu->arch) + if (selected_cpu->ident != generic + && selected_arch->arch != selected_cpu->arch) { warning (0, "switch %<-mcpu=%s%> conflicts with %<-march=%s%> switch", all_architectures[selected_cpu->arch].name,
From: Andrew Pinski <apinski@marvell.com> Right if someone supplies a -mcpu= option and then overrides that option with -march=*, we get a warning when they conflict. What we need is a generic cpu for each arch level but that is not that useful because the only difference would be the arch level. The best option is to allow -mcpu=generic -march=armv8.5-a not to warn and that is now a generic armv8.5-a arch. OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. Thanks, Andrew Pinski ChangeLog: * config/aarch64/aarch64.c (aarch64_override_options): Don't warn when the selected cpu was generic. --- gcc/config/aarch64/aarch64.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)