Re: [AArch64] Add --with-tune configure flag
Hi Richard, > I specifically want to test generic SVE rather than SVE tuned for a > specific core, so --with-arch=armv8.2-a+sve is the thing I want to test. Btw that's not actually what you get if you use cc1 - you always get armv8.0, so --with-arch doesn't work at all. The only case that appears to work in cc1 is --with-cpu as long as it is not --with-cpu=native. Cheers, Wilco
Re: [AArch64] Add --with-tune configure flag
Hi Richard, >>> I share Richard E's concern about the effect of this on people who run >>> ./cc1 directly. (And I'm being selfish here, because I regularly run >>> ./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.) >>> So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff. >> >> And what exactly should it do? It didn't work correctly before, and even now >> it's not >> clear what TARGET_CPU_DEFAULT should be set to for all possible combinations >> of >> --with-cpu=X --with-arch=Y --with-tune=Z. Should it have exactly the same >> behaviour >> with and without the driver? If so, that's difficult to achieve in >> config.gcc, and it would >> require a completely different mechanism to ensure the default ends up the >> same >> without the driver (eg. store the original strings in TARGET_CPU_DEFAULT and >> then >> postprocess in the backend as if they were actual --cpu/--arch/--tune >> commands if >> the command-line didn't already set them, ie. replicating what the driver >> already does). >> And it would need to be done for --with-abi as well. > > The specific case I mentioned works well enough though (given that I also > run ./cc1 without -march, -mcpu or -mtune flags). I agree it might not > work for all combinations of --with-arch, --with-cpu and --with-tune, > but I'm not sure that's a strong reason to remove the cases that do work > (or at least, work well enough). This cc1 behaviour is a feature for > developers rather than users after all. Well how do you suggest we keep the cases that work but block the cases that don't work, but only in cc1 without knowing when someone uses cc1 without the driver? > Perhaps I've missed the point, but why do you need to remove this code > in order to do what you need to do? Basically it's buggy by design. We have 3 related options which are compressed into a single CPU value plus a set of architecture extension flags. That obviously means we can't describe the --with- flags, let alone any combinations, right? For example the architecture to compile for is taken from that single CPU, so the architecture is often wrong (depending on the order of processing in config.gcc). Try bootstrapping trunk using --with-tune=neoverse-n2 to see what I mean. It doesn't just set the tune, it forces an incorrect architecture! My patch works precisely because I ignore the incorrect TARGET_CPU_DEFAULT and allow the driver to do its job properly in prioritizing the options. Then it's a simple matter of reading out the --cpu/arch/tune flags. Like I said, it may be possible to fix this if we pass the --with strings to the backend and process them there. But that means duplicating functionality of the driver for a few users who use cc1. Is that really worth the effort? >> So is changing your preferred config to --with-cpu=neoverse-v1 really a >> problem? > > I specifically want to test generic SVE rather than SVE tuned for a > specific core, so --with-arch=armv8.2-a+sve is the thing I want to test. > I think the question is instead whether it's really a problem to change > my workflow so that I don't run cc1 directly any more, or that I remember > to respecify the --with-arch value as an -march flag when running cc1. > And yeah, maybe the right answer is that I should learn to do that. > It's just that I don't personally understand why this change is necessary. How about creating a macro that adds the --arch for you? Eg. cc1 becomes ./cc1 -march=armv8.2-a+sve Cheers, Wilco
Re: [AArch64] Add --with-tune configure flag
Wilco Dijkstra via Gcc-patches writes: > Hi Richard, > >> I share Richard E's concern about the effect of this on people who run >> ./cc1 directly. (And I'm being selfish here, because I regularly run >> ./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.) >> So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff. > > And what exactly should it do? It didn't work correctly before, and even now > it's not > clear what TARGET_CPU_DEFAULT should be set to for all possible combinations > of > --with-cpu=X --with-arch=Y --with-tune=Z. Should it have exactly the same > behaviour > with and without the driver? If so, that's difficult to achieve in > config.gcc, and it would > require a completely different mechanism to ensure the default ends up the > same > without the driver (eg. store the original strings in TARGET_CPU_DEFAULT and > then > postprocess in the backend as if they were actual --cpu/--arch/--tune > commands if > the command-line didn't already set them, ie. replicating what the driver > already does). > And it would need to be done for --with-abi as well. The specific case I mentioned works well enough though (given that I also run ./cc1 without -march, -mcpu or -mtune flags). I agree it might not work for all combinations of --with-arch, --with-cpu and --with-tune, but I'm not sure that's a strong reason to remove the cases that do work (or at least, work well enough). This cc1 behaviour is a feature for developers rather than users after all. Perhaps I've missed the point, but why do you need to remove this code in order to do what you need to do? > So is changing your preferred config to --with-cpu=neoverse-v1 really a > problem? I specifically want to test generic SVE rather than SVE tuned for a specific core, so --with-arch=armv8.2-a+sve is the thing I want to test. I think the question is instead whether it's really a problem to change my workflow so that I don't run cc1 directly any more, or that I remember to respecify the --with-arch value as an -march flag when running cc1. And yeah, maybe the right answer is that I should learn to do that. It's just that I don't personally understand why this change is necessary. Like I say though, I'm happy to defer to a maintainer who does think the patch is a good thing. Thanks, Richard
Re: [AArch64] Add --with-tune configure flag
Hi Richard, > I share Richard E's concern about the effect of this on people who run > ./cc1 directly. (And I'm being selfish here, because I regularly run > ./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.) > So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff. And what exactly should it do? It didn't work correctly before, and even now it's not clear what TARGET_CPU_DEFAULT should be set to for all possible combinations of --with-cpu=X --with-arch=Y --with-tune=Z. Should it have exactly the same behaviour with and without the driver? If so, that's difficult to achieve in config.gcc, and it would require a completely different mechanism to ensure the default ends up the same without the driver (eg. store the original strings in TARGET_CPU_DEFAULT and then postprocess in the backend as if they were actual --cpu/--arch/--tune commands if the command-line didn't already set them, ie. replicating what the driver already does). And it would need to be done for --with-abi as well. So is changing your preferred config to --with-cpu=neoverse-v1 really a problem? Cheers, Wilco
Re: [AArch64] Add --with-tune configure flag
"Pop, Sebastian" writes:
> On 11/19/20, 10:52 AM, "Richard Earnshaw (lists)"
> wrote:
>> Having the same option have a completely different meaning would be even
>> worse than not having the option at all. So no, that's a non-starter.
>
> The attached patch 0001 removes --with-{cpu,arch,tune}-32.
> Bootstrap and regression testing pass on aarch64-linux.
> Ok to commit to trunk and active branches?
>
> I would like to ping the two patches from Wilco Dijkstra that fix issues in
> configure --with-mtune flag:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
> https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html
>
> Please see patches 0002 and 0003 attached, rebased on trunk as of today and
> tested with bootstrap and regression testing.
> Ok to commit to trunk and all active branches?
>
> Thanks,
> Sebastian
>
> From f1aa5f45dde8df5eee41ae166176775f5c5f1acc Mon Sep 17 00:00:00 2001
> From: Sebastian Pop
> Date: Thu, 3 Dec 2020 17:35:18 +
> Subject: [PATCH 1/3] [AArch64] disable --with-{cpu,arch,tune}-32
>
> gcc/
> * config.gcc (aarch64*-*-*): Remove --with-{cpu,arch,tune}-32 flags.
OK, thanks.
> ChangeLog:
> 2020-09-03 Wilco Dijkstra
>
> * config.gcc (aarch64*-*-*): Simplify --with-cpu and --with-arch
> processing. Add support for architectural extensions.
> * config/aarch64/aarch64.h (TARGET_CPU_DEFAULT): Remove
> AARCH64_CPU_DEFAULT_FLAGS.
> * config/aarch64/aarch64.c (AARCH64_CPU_DEFAULT_FLAGS): Remove define.
> (get_tune_cpu): Assert CPU is always valid.
> (get_arch): Assert architecture is always valid.
> (aarch64_override_options): Cleanup CPU selection code and simplify
> logic.
I share Richard E's concern about the effect of this on people who run
./cc1 directly. (And I'm being selfish here, because I regularly run
./cc1 directly on toolchains configured with --with-arch=armv8.2-a+sve.)
So TBH my preference would be to keep the TARGET_CPU_DEFAULT_FLAGS stuff.
However, if one of the other maintainers strongly thinks this is the
right way to go, I'm happy to defer to them.
> Add support for --with-tune. Like --with-cpu and --with-arch, the argument is
> validated and transformed into a -mtune option to be processed like any other
> command-line option. --with-tune has no effect if a -mcpu or -mtune option
> is used. The validating code didn't allow --with-cpu=native, so explicitly
> allow that.
>
> Co-authored-by: Delia Burduv
>
> Bootstrap OK, regress pass, OK to commit?
>
> ChangeLog
> 2020-09-03 Wilco Dijkstra
>
> * config.gcc
> (aarch64*-*-*): Add --with-tune. Support --with-cpu=native.
> * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Add --with-tune.
>
> 2020-09-03 Wilco Dijkstra
>
> * gcc/testsuite/lib/target-supports.exp:
> (check_effective_target_tune_cortex_a76): New effective target test.
> * gcc.target/aarch64/with-tune-config.c: New test.
> * gcc.target/aarch64/with-tune-march.c: Likewise.
> * gcc.target/aarch64/with-tune-mcpu.c: Likewise.
> * gcc.target/aarch64/with-tune-mtune.c: Likewise.
OK for this one, thanks.
Richard
Re: [AArch64] Add --with-tune configure flag
On 11/19/20, 10:52 AM, "Richard Earnshaw (lists)"
wrote:
> Having the same option have a completely different meaning would be even
> worse than not having the option at all. So no, that's a non-starter.
The attached patch 0001 removes --with-{cpu,arch,tune}-32.
Bootstrap and regression testing pass on aarch64-linux.
Ok to commit to trunk and active branches?
I would like to ping the two patches from Wilco Dijkstra that fix issues in
configure --with-mtune flag:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html
Please see patches 0002 and 0003 attached, rebased on trunk as of today and
tested with bootstrap and regression testing.
Ok to commit to trunk and all active branches?
Thanks,
Sebastian
0001-AArch64-disable-with-cpu-arch-tune-32.patch
Description: 0001-AArch64-disable-with-cpu-arch-tune-32.patch
0002-AArch64-Cleanup-CPU-option-processing-code.patch
Description: 0002-AArch64-Cleanup-CPU-option-processing-code.patch
0003-AArch64-Add-support-for-with-tune.patch
Description: 0003-AArch64-Add-support-for-with-tune.patch
Re: [AArch64] Add --with-tune configure flag
On 19/11/2020 14:40, Wilco Dijkstra via Gcc-patches wrote: > Hi, > As for your second patch, --with-cpu-64 could be a simple alias indeed, but what is the exact definition/expected behaviour of a --with-cpu-32 on a target that only supports 64-bit code? The AArch64 target cannot generate AArch32 code, so we shouldn't silently accept it. >>> >>> IMO allowing users to specify all the flags available on x86 is important. >>> >> >> This isn't about general users though; it's about how you configure the >> compiler and that's not all the same. I don't mind the --with-cpu-64 as >> a strict alias for --with-cpu, but --with-cpu-32 is both redundant and >> misleading as it might give the impression that it does something useful. > > We could make it do something useful, for example emit a warning, an error > or default to -mabi=ilp32 (since that is similar to what other targets do). > Anything is better than being the only target that doesn't support it... > > Cheers, > Wilco > Having the same option have a completely different meaning would be even worse than not having the option at all. So no, that's a non-starter. It's not like these configure options have wide-spread usage at present. R.
Re: [AArch64] Add --with-tune configure flag
Hi, >>> As for your second patch, --with-cpu-64 could be a simple alias indeed, >>> but what is the exact definition/expected behaviour of a --with-cpu-32 >>> on a target that only supports 64-bit code? The AArch64 target cannot >>> generate AArch32 code, so we shouldn't silently accept it. >> >> IMO allowing users to specify all the flags available on x86 is important. >> > > This isn't about general users though; it's about how you configure the > compiler and that's not all the same. I don't mind the --with-cpu-64 as > a strict alias for --with-cpu, but --with-cpu-32 is both redundant and > misleading as it might give the impression that it does something useful. We could make it do something useful, for example emit a warning, an error or default to -mabi=ilp32 (since that is similar to what other targets do). Anything is better than being the only target that doesn't support it... Cheers, Wilco
Re: [AArch64] Add --with-tune configure flag
On 18/11/2020 17:16, Pop, Sebastian via Gcc-patches wrote: > Hi, > > On 11/18/20, 10:17 AM, "Wilco Dijkstra" wrote: >>I presume you're trying to unify the --with- options across most targets? > > Yes, my intention was to provide the same configure options on arm64 > as on x86, such that projects that already use those options can change > cpu name to "neoverse-n1" and that will build a compiler with the right > tuning for Graviton2. > > Allowing arm64 users to specify all the flags available on x86 is important. > >>That would be very useful! However there are significant differences >> between >>targets in how they interpret options like --with-arch=native (or >> -march). So >>those differences also need to be looked at and fixed to avoid unexpected >> results. >> >>As for the first patch, I think support for --witch-tune requires more >> changes. >>Without proper processing of a --with-tune, you get an incorrect >> architecture >>version (if say the CPU you tune for is newer than the --with-cpu/arch >>or default). >> >> I posted patches to add --with-tune and fix various issues a while back: >>https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html >>https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html > > Thanks for pointing me to your patches, I was not aware of these changes. > I see that your patches enable more use cases and fix several bugs. > These changes would definitely be good to have in trunk and branches. > > My patch was the minimal change to enable --with-tune=neoverse-n1 > >>As for your second patch, --with-cpu-64 could be a simple alias indeed, >>but what is the exact definition/expected behaviour of a --with-cpu-32 >>on a target that only supports 64-bit code? The AArch64 target cannot >>generate AArch32 code, so we shouldn't silently accept it. > > IMO allowing users to specify all the flags available on x86 is important. > This isn't about general users though; it's about how you configure the compiler and that's not all the same. I don't mind the --with-cpu-64 as a strict alias for --with-cpu, but --with-cpu-32 is both redundant and misleading as it might give the impression that it does something useful. R. > Thanks, > Sebastian >
Re: [AArch64] Add --with-tune configure flag
Hi, On 11/18/20, 10:17 AM, "Wilco Dijkstra" wrote: >I presume you're trying to unify the --with- options across most targets? Yes, my intention was to provide the same configure options on arm64 as on x86, such that projects that already use those options can change cpu name to "neoverse-n1" and that will build a compiler with the right tuning for Graviton2. Allowing arm64 users to specify all the flags available on x86 is important. >That would be very useful! However there are significant differences > between >targets in how they interpret options like --with-arch=native (or -march). > So >those differences also need to be looked at and fixed to avoid unexpected > results. > >As for the first patch, I think support for --witch-tune requires more > changes. >Without proper processing of a --with-tune, you get an incorrect > architecture >version (if say the CPU you tune for is newer than the --with-cpu/arch >or default). > > I posted patches to add --with-tune and fix various issues a while back: >https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html >https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html Thanks for pointing me to your patches, I was not aware of these changes. I see that your patches enable more use cases and fix several bugs. These changes would definitely be good to have in trunk and branches. My patch was the minimal change to enable --with-tune=neoverse-n1 >As for your second patch, --with-cpu-64 could be a simple alias indeed, >but what is the exact definition/expected behaviour of a --with-cpu-32 >on a target that only supports 64-bit code? The AArch64 target cannot >generate AArch32 code, so we shouldn't silently accept it. IMO allowing users to specify all the flags available on x86 is important. Thanks, Sebastian
Re: [AArch64] Add --with-tune configure flag
Hi Sebastian, I presume you're trying to unify the --with- options across most targets? That would be very useful! However there are significant differences between targets in how they interpret options like --with-arch=native (or -march). So those differences also need to be looked at and fixed to avoid unexpected results. As for the first patch, I think support for --witch-tune requires more changes. Without proper processing of a --with-tune, you get an incorrect architecture version (if say the CPU you tune for is newer than the --with-cpu/arch or default). I posted patches to add --with-tune and fix various issues a while back: https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553865.html https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553866.html So I think these should go in first. They are simple and useful enough to backport (since they fix bugs) but that decision is up to the AArch64 maintainers. As for your second patch, --with-cpu-64 could be a simple alias indeed, but what is the exact definition/expected behaviour of a --with-cpu-32 on a target that only supports 64-bit code? The AArch64 target cannot generate AArch32 code, so we shouldn't silently accept it. Cheers, Wilco
Re: [AArch64] Add --with-tune configure flag
On 11/17/20 2:27 PM, Pop, Sebastian via Gcc-patches wrote:
> Hi,
>
> here is a follow-up patch to add missing Arm64 configure flags as aliases to
> the existing flags.
>
> gcc/
> * config.gcc: add configure flags --with-{cpu,arch,tune}-{32,64}
> as alias flags for --with-{cpu,arch,tune} on AArch64.
> * doc/install.texi: Document new flags for aarch64.
>
> Tested on aarch64-linux with bootstrap and regression test.
> Ok to commit to trunk and back-port to active branches?
OK
jeff
Re: [AArch64] Add --with-tune configure flag
On 11/17/20 10:53 AM, Pop, Sebastian via Gcc-patches wrote: > Hi, > > the attached patch fixes a configure error on Arm64 when passing > --with-tune=... to configure: > ``` > This target does not support --with-tune. > Valid --with options are: abi cpu arch > ``` > The missing flag sets target tuning to a different value than the generic > tuning. > > gcc/ > * config.gcc: Add --with-tune to AArch64 configure flags. > > Tested on aarch64-linux with bootstrap and regression test. > Ok to commit to trunk and back-port to active branches? OK jeff
Re: [AArch64] Add --with-tune configure flag
Hi,
here is a follow-up patch to add missing Arm64 configure flags as aliases to
the existing flags.
gcc/
* config.gcc: add configure flags --with-{cpu,arch,tune}-{32,64}
as alias flags for --with-{cpu,arch,tune} on AArch64.
* doc/install.texi: Document new flags for aarch64.
Tested on aarch64-linux with bootstrap and regression test.
Ok to commit to trunk and back-port to active branches?
Thanks,
Sebastian
0001-AArch64-add-with-cpu-arch-tune-32-64-as-alias-flags-.patch
Description: 0001-AArch64-add-with-cpu-arch-tune-32-64-as-alias-flags-.patch
