| Message ID | mpt5yl4eipx.fsf@arm.com |
|---|---|
| State | New |
| Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4FA6B3851A8A for <patchwork@sourceware.org>; Mon, 13 Jun 2022 14:34:04 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4FA6B3851A8A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1655130844; bh=YCw/8MF54unXYRh2sv0bwNh/I3v6kw5/k7mrj3JtpBE=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=c9Geo2OmaewMPAa01R6twMssAyNBI3lGjAg4gRvst420KpdsLYJeCiiJRxdNMD59F ztqc5crWPCo+PIYzLz/c7IhSyYtYpLOWsqg5ILWi3J/MNghuWAQ98q8l2hQFZEuXnF RQ+0sHUr7G9npfNQ29Zoliv6LDhI7BfQ0HLCGTus= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 1E64C385803E for <gcc-patches@gcc.gnu.org>; Mon, 13 Jun 2022 14:33:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1E64C385803E Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C4EC923A for <gcc-patches@gcc.gnu.org>; Mon, 13 Jun 2022 07:33:31 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 548283F792 for <gcc-patches@gcc.gnu.org>; Mon, 13 Jun 2022 07:33:31 -0700 (PDT) To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: [RFA configure parts] aarch64: Make cc1 &co handle --with options Date: Mon, 13 Jun 2022 15:33:30 +0100 Message-ID: <mpt5yl4eipx.fsf@arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-58.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Richard Sandiford <richard.sandiford@arm.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
| Series |
[RFA,configure,parts] aarch64: Make cc1 &co handle --with options
|
|
Commit Message
Richard Sandiford
June 13, 2022, 2:33 p.m. UTC
On aarch64, --with-arch, --with-cpu and --with-tune only have an effect on the driver, so “./xgcc -B./ -O3” can give significantly different results from “./cc1 -O3”. --with-arch did have a limited effect on ./cc1 in previous releases, although it didn't work entirely correctly. Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for --with-arch=armv8.2-a+sve without having to supply an explicit -march, so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. It relies on Wilco's earlier clean-ups. The patch makes config.gcc define WITH_FOO_STRING macros for each supported --with-foo option. This could be done only in aarch64- specific code, but I thought it could be useful on other targets too (and can be safely ignored otherwise). There didn't seem to be any existing and potentially clashing uses of macros with this style of name. Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure bits? Richard gcc/ * config.gcc: Define WITH_FOO_STRING macros for each supported --with-foo option. * config/aarch64/aarch64.cc (aarch64_override_options): Emulate OPTION_DEFAULT_SPECS. * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. --- gcc/config.gcc | 14 ++++++++++++++ gcc/config/aarch64/aarch64.cc | 8 ++++++++ gcc/config/aarch64/aarch64.h | 5 ++++- 3 files changed, 26 insertions(+), 1 deletion(-)
Comments
Ping for the configure bits Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On aarch64, --with-arch, --with-cpu and --with-tune only have an > effect on the driver, so “./xgcc -B./ -O3” can give significantly > different results from “./cc1 -O3”. --with-arch did have a limited > effect on ./cc1 in previous releases, although it didn't work > entirely correctly. > > Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for > --with-arch=armv8.2-a+sve without having to supply an explicit -march, > so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. > It relies on Wilco's earlier clean-ups. > > The patch makes config.gcc define WITH_FOO_STRING macros for each > supported --with-foo option. This could be done only in aarch64- > specific code, but I thought it could be useful on other targets > too (and can be safely ignored otherwise). There didn't seem to > be any existing and potentially clashing uses of macros with this > style of name. > > Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure > bits? > > Richard > > > gcc/ > * config.gcc: Define WITH_FOO_STRING macros for each supported > --with-foo option. > * config/aarch64/aarch64.cc (aarch64_override_options): Emulate > OPTION_DEFAULT_SPECS. > * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. > --- > gcc/config.gcc | 14 ++++++++++++++ > gcc/config/aarch64/aarch64.cc | 8 ++++++++ > gcc/config/aarch64/aarch64.h | 5 ++++- > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/gcc/config.gcc b/gcc/config.gcc > index cdbefb5b4f5..e039230431c 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -5865,6 +5865,20 @@ else > configure_default_options="{ ${t} }" > fi > > +for option in $supported_defaults > +do > + lc_option=`echo $option | sed s/-/_/g` > + uc_option=`echo $lc_option | tr a-z A-Z` > + eval "val=\$with_$lc_option" > + if test -n "$val" > + then > + val="\\\"$val\\\"" > + else > + val=nullptr > + fi > + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" > +done > + > if test "$target_cpu_default2" != "" > then > if test "$target_cpu_default" != "" > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index d21e041eccb..0bc700b81ad 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -18109,6 +18109,14 @@ aarch64_override_options (void) > if (aarch64_branch_protection_string) > aarch64_validate_mbranch_protection (aarch64_branch_protection_string); > > + /* Emulate OPTION_DEFAULT_SPECS. */ > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_arch_string = WITH_ARCH_STRING; > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_cpu_string = WITH_CPU_STRING; > + if (!aarch64_cpu_string && !aarch64_tune_string) > + aarch64_tune_string = WITH_TUNE_STRING; > + > /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU. > If either of -march or -mtune is given, they override their > respective component of -mcpu. */ > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h > index 80cfe4b7407..3122dbd7098 100644 > --- a/gcc/config/aarch64/aarch64.h > +++ b/gcc/config/aarch64/aarch64.h > @@ -1267,7 +1267,10 @@ extern enum aarch64_code_model aarch64_cmodel; > /* Support for configure-time --with-arch, --with-cpu and --with-tune. > --with-arch and --with-cpu are ignored if either -mcpu or -march is used. > --with-tune is ignored if either -mtune or -mcpu is used (but is not > - affected by -march). */ > + affected by -march). > + > + There is corresponding code in aarch64_override_options that emulates > + this behavior when cc1 &co are invoked directly. */ > #define OPTION_DEFAULT_SPECS \ > {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ > {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
Ping^2 for the configure bits. Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On aarch64, --with-arch, --with-cpu and --with-tune only have an > effect on the driver, so “./xgcc -B./ -O3” can give significantly > different results from “./cc1 -O3”. --with-arch did have a limited > effect on ./cc1 in previous releases, although it didn't work > entirely correctly. > > Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for > --with-arch=armv8.2-a+sve without having to supply an explicit -march, > so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. > It relies on Wilco's earlier clean-ups. > > The patch makes config.gcc define WITH_FOO_STRING macros for each > supported --with-foo option. This could be done only in aarch64- > specific code, but I thought it could be useful on other targets > too (and can be safely ignored otherwise). There didn't seem to > be any existing and potentially clashing uses of macros with this > style of name. > > Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure > bits? > > Richard > > > gcc/ > * config.gcc: Define WITH_FOO_STRING macros for each supported > --with-foo option. > * config/aarch64/aarch64.cc (aarch64_override_options): Emulate > OPTION_DEFAULT_SPECS. > * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. > --- > gcc/config.gcc | 14 ++++++++++++++ > gcc/config/aarch64/aarch64.cc | 8 ++++++++ > gcc/config/aarch64/aarch64.h | 5 ++++- > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/gcc/config.gcc b/gcc/config.gcc > index cdbefb5b4f5..e039230431c 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -5865,6 +5865,20 @@ else > configure_default_options="{ ${t} }" > fi > > +for option in $supported_defaults > +do > + lc_option=`echo $option | sed s/-/_/g` > + uc_option=`echo $lc_option | tr a-z A-Z` > + eval "val=\$with_$lc_option" > + if test -n "$val" > + then > + val="\\\"$val\\\"" > + else > + val=nullptr > + fi > + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" > +done > + > if test "$target_cpu_default2" != "" > then > if test "$target_cpu_default" != "" > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index d21e041eccb..0bc700b81ad 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -18109,6 +18109,14 @@ aarch64_override_options (void) > if (aarch64_branch_protection_string) > aarch64_validate_mbranch_protection (aarch64_branch_protection_string); > > + /* Emulate OPTION_DEFAULT_SPECS. */ > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_arch_string = WITH_ARCH_STRING; > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_cpu_string = WITH_CPU_STRING; > + if (!aarch64_cpu_string && !aarch64_tune_string) > + aarch64_tune_string = WITH_TUNE_STRING; > + > /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU. > If either of -march or -mtune is given, they override their > respective component of -mcpu. */ > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h > index 80cfe4b7407..3122dbd7098 100644 > --- a/gcc/config/aarch64/aarch64.h > +++ b/gcc/config/aarch64/aarch64.h > @@ -1267,7 +1267,10 @@ extern enum aarch64_code_model aarch64_cmodel; > /* Support for configure-time --with-arch, --with-cpu and --with-tune. > --with-arch and --with-cpu are ignored if either -mcpu or -march is used. > --with-tune is ignored if either -mtune or -mcpu is used (but is not > - affected by -march). */ > + affected by -march). > + > + There is corresponding code in aarch64_override_options that emulates > + this behavior when cc1 &co are invoked directly. */ > #define OPTION_DEFAULT_SPECS \ > {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ > {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
Ping^3 for the configure bits. Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On aarch64, --with-arch, --with-cpu and --with-tune only have an > effect on the driver, so “./xgcc -B./ -O3” can give significantly > different results from “./cc1 -O3”. --with-arch did have a limited > effect on ./cc1 in previous releases, although it didn't work > entirely correctly. > > Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for > --with-arch=armv8.2-a+sve without having to supply an explicit -march, > so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. > It relies on Wilco's earlier clean-ups. > > The patch makes config.gcc define WITH_FOO_STRING macros for each > supported --with-foo option. This could be done only in aarch64- > specific code, but I thought it could be useful on other targets > too (and can be safely ignored otherwise). There didn't seem to > be any existing and potentially clashing uses of macros with this > style of name. > > Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure > bits? > > Richard > > > gcc/ > * config.gcc: Define WITH_FOO_STRING macros for each supported > --with-foo option. > * config/aarch64/aarch64.cc (aarch64_override_options): Emulate > OPTION_DEFAULT_SPECS. > * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. > --- > gcc/config.gcc | 14 ++++++++++++++ > gcc/config/aarch64/aarch64.cc | 8 ++++++++ > gcc/config/aarch64/aarch64.h | 5 ++++- > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/gcc/config.gcc b/gcc/config.gcc > index cdbefb5b4f5..e039230431c 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -5865,6 +5865,20 @@ else > configure_default_options="{ ${t} }" > fi > > +for option in $supported_defaults > +do > + lc_option=`echo $option | sed s/-/_/g` > + uc_option=`echo $lc_option | tr a-z A-Z` > + eval "val=\$with_$lc_option" > + if test -n "$val" > + then > + val="\\\"$val\\\"" > + else > + val=nullptr > + fi > + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" > +done > + > if test "$target_cpu_default2" != "" > then > if test "$target_cpu_default" != "" > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index d21e041eccb..0bc700b81ad 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -18109,6 +18109,14 @@ aarch64_override_options (void) > if (aarch64_branch_protection_string) > aarch64_validate_mbranch_protection (aarch64_branch_protection_string); > > + /* Emulate OPTION_DEFAULT_SPECS. */ > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_arch_string = WITH_ARCH_STRING; > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_cpu_string = WITH_CPU_STRING; > + if (!aarch64_cpu_string && !aarch64_tune_string) > + aarch64_tune_string = WITH_TUNE_STRING; > + > /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU. > If either of -march or -mtune is given, they override their > respective component of -mcpu. */ > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h > index 80cfe4b7407..3122dbd7098 100644 > --- a/gcc/config/aarch64/aarch64.h > +++ b/gcc/config/aarch64/aarch64.h > @@ -1267,7 +1267,10 @@ extern enum aarch64_code_model aarch64_cmodel; > /* Support for configure-time --with-arch, --with-cpu and --with-tune. > --with-arch and --with-cpu are ignored if either -mcpu or -march is used. > --with-tune is ignored if either -mtune or -mcpu is used (but is not > - affected by -march). */ > + affected by -march). > + > + There is corresponding code in aarch64_override_options that emulates > + this behavior when cc1 &co are invoked directly. */ > #define OPTION_DEFAULT_SPECS \ > {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ > {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
On 13/06/2022 15:33, Richard Sandiford via Gcc-patches wrote: > On aarch64, --with-arch, --with-cpu and --with-tune only have an > effect on the driver, so “./xgcc -B./ -O3” can give significantly > different results from “./cc1 -O3”. --with-arch did have a limited > effect on ./cc1 in previous releases, although it didn't work > entirely correctly. > > Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for > --with-arch=armv8.2-a+sve without having to supply an explicit -march, > so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. > It relies on Wilco's earlier clean-ups. > > The patch makes config.gcc define WITH_FOO_STRING macros for each > supported --with-foo option. This could be done only in aarch64- > specific code, but I thought it could be useful on other targets > too (and can be safely ignored otherwise). There didn't seem to > be any existing and potentially clashing uses of macros with this > style of name. > > Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure > bits? > > Richard > > > gcc/ > * config.gcc: Define WITH_FOO_STRING macros for each supported > --with-foo option. > * config/aarch64/aarch64.cc (aarch64_override_options): Emulate > OPTION_DEFAULT_SPECS. > * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. > --- > gcc/config.gcc | 14 ++++++++++++++ > gcc/config/aarch64/aarch64.cc | 8 ++++++++ > gcc/config/aarch64/aarch64.h | 5 ++++- > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/gcc/config.gcc b/gcc/config.gcc > index cdbefb5b4f5..e039230431c 100644 > --- a/gcc/config.gcc > +++ b/gcc/config.gcc > @@ -5865,6 +5865,20 @@ else > configure_default_options="{ ${t} }" > fi > > +for option in $supported_defaults > +do > + lc_option=`echo $option | sed s/-/_/g` > + uc_option=`echo $lc_option | tr a-z A-Z` > + eval "val=\$with_$lc_option" > + if test -n "$val" > + then > + val="\\\"$val\\\"" > + else > + val=nullptr > + fi > + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" > +done This bit would really be best reviewed by a non-arm maintainer. It generally looks OK. My only comment would be why define anything if the corresponding --with-foo was not specified. They you can use #ifdef to test if the user specified a default. R. > + > if test "$target_cpu_default2" != "" > then > if test "$target_cpu_default" != "" > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index d21e041eccb..0bc700b81ad 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -18109,6 +18109,14 @@ aarch64_override_options (void) > if (aarch64_branch_protection_string) > aarch64_validate_mbranch_protection (aarch64_branch_protection_string); > > + /* Emulate OPTION_DEFAULT_SPECS. */ > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_arch_string = WITH_ARCH_STRING; > + if (!aarch64_arch_string && !aarch64_cpu_string) > + aarch64_cpu_string = WITH_CPU_STRING; > + if (!aarch64_cpu_string && !aarch64_tune_string) > + aarch64_tune_string = WITH_TUNE_STRING; > + > /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU. > If either of -march or -mtune is given, they override their > respective component of -mcpu. */ > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h > index 80cfe4b7407..3122dbd7098 100644 > --- a/gcc/config/aarch64/aarch64.h > +++ b/gcc/config/aarch64/aarch64.h > @@ -1267,7 +1267,10 @@ extern enum aarch64_code_model aarch64_cmodel; > /* Support for configure-time --with-arch, --with-cpu and --with-tune. > --with-arch and --with-cpu are ignored if either -mcpu or -march is used. > --with-tune is ignored if either -mtune or -mcpu is used (but is not > - affected by -march). */ > + affected by -march). > + > + There is corresponding code in aarch64_override_options that emulates > + this behavior when cc1 &co are invoked directly. */ > #define OPTION_DEFAULT_SPECS \ > {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ > {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: > On 13/06/2022 15:33, Richard Sandiford via Gcc-patches wrote: >> On aarch64, --with-arch, --with-cpu and --with-tune only have an >> effect on the driver, so “./xgcc -B./ -O3” can give significantly >> different results from “./cc1 -O3”. --with-arch did have a limited >> effect on ./cc1 in previous releases, although it didn't work >> entirely correctly. >> >> Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for >> --with-arch=armv8.2-a+sve without having to supply an explicit -march, >> so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. >> It relies on Wilco's earlier clean-ups. >> >> The patch makes config.gcc define WITH_FOO_STRING macros for each >> supported --with-foo option. This could be done only in aarch64- >> specific code, but I thought it could be useful on other targets >> too (and can be safely ignored otherwise). There didn't seem to >> be any existing and potentially clashing uses of macros with this >> style of name. >> >> Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure >> bits? >> >> Richard >> >> >> gcc/ >> * config.gcc: Define WITH_FOO_STRING macros for each supported >> --with-foo option. >> * config/aarch64/aarch64.cc (aarch64_override_options): Emulate >> OPTION_DEFAULT_SPECS. >> * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. >> --- >> gcc/config.gcc | 14 ++++++++++++++ >> gcc/config/aarch64/aarch64.cc | 8 ++++++++ >> gcc/config/aarch64/aarch64.h | 5 ++++- >> 3 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/gcc/config.gcc b/gcc/config.gcc >> index cdbefb5b4f5..e039230431c 100644 >> --- a/gcc/config.gcc >> +++ b/gcc/config.gcc >> @@ -5865,6 +5865,20 @@ else >> configure_default_options="{ ${t} }" >> fi >> >> +for option in $supported_defaults >> +do >> + lc_option=`echo $option | sed s/-/_/g` >> + uc_option=`echo $lc_option | tr a-z A-Z` >> + eval "val=\$with_$lc_option" >> + if test -n "$val" >> + then >> + val="\\\"$val\\\"" >> + else >> + val=nullptr >> + fi >> + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" >> +done > > This bit would really be best reviewed by a non-arm maintainer. It > generally looks OK. My only comment would be why define anything if the > corresponding --with-foo was not specified. They you can use #ifdef to > test if the user specified a default. Yeah, could do it that way instead, but: >> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc >> index d21e041eccb..0bc700b81ad 100644 >> --- a/gcc/config/aarch64/aarch64.cc >> +++ b/gcc/config/aarch64/aarch64.cc >> @@ -18109,6 +18109,14 @@ aarch64_override_options (void) >> if (aarch64_branch_protection_string) >> aarch64_validate_mbranch_protection (aarch64_branch_protection_string); >> >> + /* Emulate OPTION_DEFAULT_SPECS. */ >> + if (!aarch64_arch_string && !aarch64_cpu_string) >> + aarch64_arch_string = WITH_ARCH_STRING; >> + if (!aarch64_arch_string && !aarch64_cpu_string) >> + aarch64_cpu_string = WITH_CPU_STRING; >> + if (!aarch64_cpu_string && !aarch64_tune_string) >> + aarch64_tune_string = WITH_TUNE_STRING; (without the preprocessor stuff) IMO reads better. If a preprocessor is/isn't present test turns out to be useful, perhaps we should add macros like HAVE_WITH_TUNE/WITH_TUNE_PRESENT/... too? I guess it should only be done when something needs it though. Thanks, Richard >> + >> /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU. >> If either of -march or -mtune is given, they override their >> respective component of -mcpu. */ >> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h >> index 80cfe4b7407..3122dbd7098 100644 >> --- a/gcc/config/aarch64/aarch64.h >> +++ b/gcc/config/aarch64/aarch64.h >> @@ -1267,7 +1267,10 @@ extern enum aarch64_code_model aarch64_cmodel; >> /* Support for configure-time --with-arch, --with-cpu and --with-tune. >> --with-arch and --with-cpu are ignored if either -mcpu or -march is used. >> --with-tune is ignored if either -mtune or -mcpu is used (but is not >> - affected by -march). */ >> + affected by -march). >> + >> + There is corresponding code in aarch64_override_options that emulates >> + this behavior when cc1 &co are invoked directly. */ >> #define OPTION_DEFAULT_SPECS \ >> {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ >> {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
On 05/08/2022 14:53, Richard Sandiford via Gcc-patches wrote: > Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: >> On 13/06/2022 15:33, Richard Sandiford via Gcc-patches wrote: >>> On aarch64, --with-arch, --with-cpu and --with-tune only have an >>> effect on the driver, so “./xgcc -B./ -O3” can give significantly >>> different results from “./cc1 -O3”. --with-arch did have a limited >>> effect on ./cc1 in previous releases, although it didn't work >>> entirely correctly. >>> >>> Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for >>> --with-arch=armv8.2-a+sve without having to supply an explicit -march, >>> so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. >>> It relies on Wilco's earlier clean-ups. >>> >>> The patch makes config.gcc define WITH_FOO_STRING macros for each >>> supported --with-foo option. This could be done only in aarch64- >>> specific code, but I thought it could be useful on other targets >>> too (and can be safely ignored otherwise). There didn't seem to >>> be any existing and potentially clashing uses of macros with this >>> style of name. >>> >>> Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure >>> bits? >>> >>> Richard >>> >>> >>> gcc/ >>> * config.gcc: Define WITH_FOO_STRING macros for each supported >>> --with-foo option. >>> * config/aarch64/aarch64.cc (aarch64_override_options): Emulate >>> OPTION_DEFAULT_SPECS. >>> * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. >>> --- >>> gcc/config.gcc | 14 ++++++++++++++ >>> gcc/config/aarch64/aarch64.cc | 8 ++++++++ >>> gcc/config/aarch64/aarch64.h | 5 ++++- >>> 3 files changed, 26 insertions(+), 1 deletion(-) >>> >>> diff --git a/gcc/config.gcc b/gcc/config.gcc >>> index cdbefb5b4f5..e039230431c 100644 >>> --- a/gcc/config.gcc >>> +++ b/gcc/config.gcc >>> @@ -5865,6 +5865,20 @@ else >>> configure_default_options="{ ${t} }" >>> fi >>> >>> +for option in $supported_defaults >>> +do >>> + lc_option=`echo $option | sed s/-/_/g` >>> + uc_option=`echo $lc_option | tr a-z A-Z` >>> + eval "val=\$with_$lc_option" >>> + if test -n "$val" >>> + then >>> + val="\\\"$val\\\"" >>> + else >>> + val=nullptr >>> + fi >>> + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" >>> +done >> >> This bit would really be best reviewed by a non-arm maintainer. It >> generally looks OK. My only comment would be why define anything if the >> corresponding --with-foo was not specified. They you can use #ifdef to >> test if the user specified a default. > > Yeah, could do it that way instead, but: > >>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc >>> index d21e041eccb..0bc700b81ad 100644 >>> --- a/gcc/config/aarch64/aarch64.cc >>> +++ b/gcc/config/aarch64/aarch64.cc >>> @@ -18109,6 +18109,14 @@ aarch64_override_options (void) >>> if (aarch64_branch_protection_string) >>> aarch64_validate_mbranch_protection (aarch64_branch_protection_string); >>> >>> + /* Emulate OPTION_DEFAULT_SPECS. */ >>> + if (!aarch64_arch_string && !aarch64_cpu_string) >>> + aarch64_arch_string = WITH_ARCH_STRING; >>> + if (!aarch64_arch_string && !aarch64_cpu_string) >>> + aarch64_cpu_string = WITH_CPU_STRING; >>> + if (!aarch64_cpu_string && !aarch64_tune_string) >>> + aarch64_tune_string = WITH_TUNE_STRING; > > (without the preprocessor stuff) IMO reads better. If a preprocessor > is/isn't present test turns out to be useful, perhaps we should add > macros like HAVE_WITH_TUNE/WITH_TUNE_PRESENT/... too? I guess it > should only be done when something needs it though. It's relatively easy to add #ifndef WITH_TUNE_STRING #define WITH_TUNE_STRING (nulptr) #endif in a header, but much harder to go the other way. The case I was thinking of was something like: #if !defined(WITH_ARCH_STRING) && !defined(WITH_CPU_STRING) #define WITH_ARCH_STRING "<some-target-default>" #endif which saves having to have yet another level of fallback if nothing has been specified, but this is next to impossible if the macros are unconditionally defined. R. > > Thanks, > Richard > >>> + >>> /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU. >>> If either of -march or -mtune is given, they override their >>> respective component of -mcpu. */ >>> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h >>> index 80cfe4b7407..3122dbd7098 100644 >>> --- a/gcc/config/aarch64/aarch64.h >>> +++ b/gcc/config/aarch64/aarch64.h >>> @@ -1267,7 +1267,10 @@ extern enum aarch64_code_model aarch64_cmodel; >>> /* Support for configure-time --with-arch, --with-cpu and --with-tune. >>> --with-arch and --with-cpu are ignored if either -mcpu or -march is used. >>> --with-tune is ignored if either -mtune or -mcpu is used (but is not >>> - affected by -march). */ >>> + affected by -march). >>> + >>> + There is corresponding code in aarch64_override_options that emulates >>> + this behavior when cc1 &co are invoked directly. */ >>> #define OPTION_DEFAULT_SPECS \ >>> {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ >>> {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \
Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: > On 05/08/2022 14:53, Richard Sandiford via Gcc-patches wrote: >> Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: >>> On 13/06/2022 15:33, Richard Sandiford via Gcc-patches wrote: >>>> On aarch64, --with-arch, --with-cpu and --with-tune only have an >>>> effect on the driver, so “./xgcc -B./ -O3” can give significantly >>>> different results from “./cc1 -O3”. --with-arch did have a limited >>>> effect on ./cc1 in previous releases, although it didn't work >>>> entirely correctly. >>>> >>>> Being of a lazy persuasion, I've got used to ./cc1 selecting SVE for >>>> --with-arch=armv8.2-a+sve without having to supply an explicit -march, >>>> so this patch makes ./cc1 emulate the relevant OPTION_DEFAULT_SPECS. >>>> It relies on Wilco's earlier clean-ups. >>>> >>>> The patch makes config.gcc define WITH_FOO_STRING macros for each >>>> supported --with-foo option. This could be done only in aarch64- >>>> specific code, but I thought it could be useful on other targets >>>> too (and can be safely ignored otherwise). There didn't seem to >>>> be any existing and potentially clashing uses of macros with this >>>> style of name. >>>> >>>> Tested on aarch64-linux-gnu & x86_64-linux-gnu. OK for the configure >>>> bits? >>>> >>>> Richard >>>> >>>> >>>> gcc/ >>>> * config.gcc: Define WITH_FOO_STRING macros for each supported >>>> --with-foo option. >>>> * config/aarch64/aarch64.cc (aarch64_override_options): Emulate >>>> OPTION_DEFAULT_SPECS. >>>> * config/aarch64/aarch64.h (OPTION_DEFAULT_SPECS): Reference the above. >>>> --- >>>> gcc/config.gcc | 14 ++++++++++++++ >>>> gcc/config/aarch64/aarch64.cc | 8 ++++++++ >>>> gcc/config/aarch64/aarch64.h | 5 ++++- >>>> 3 files changed, 26 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/gcc/config.gcc b/gcc/config.gcc >>>> index cdbefb5b4f5..e039230431c 100644 >>>> --- a/gcc/config.gcc >>>> +++ b/gcc/config.gcc >>>> @@ -5865,6 +5865,20 @@ else >>>> configure_default_options="{ ${t} }" >>>> fi >>>> >>>> +for option in $supported_defaults >>>> +do >>>> + lc_option=`echo $option | sed s/-/_/g` >>>> + uc_option=`echo $lc_option | tr a-z A-Z` >>>> + eval "val=\$with_$lc_option" >>>> + if test -n "$val" >>>> + then >>>> + val="\\\"$val\\\"" >>>> + else >>>> + val=nullptr >>>> + fi >>>> + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" >>>> +done >>> >>> This bit would really be best reviewed by a non-arm maintainer. It >>> generally looks OK. My only comment would be why define anything if the >>> corresponding --with-foo was not specified. They you can use #ifdef to >>> test if the user specified a default. >> >> Yeah, could do it that way instead, but: >> >>>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc >>>> index d21e041eccb..0bc700b81ad 100644 >>>> --- a/gcc/config/aarch64/aarch64.cc >>>> +++ b/gcc/config/aarch64/aarch64.cc >>>> @@ -18109,6 +18109,14 @@ aarch64_override_options (void) >>>> if (aarch64_branch_protection_string) >>>> aarch64_validate_mbranch_protection (aarch64_branch_protection_string); >>>> >>>> + /* Emulate OPTION_DEFAULT_SPECS. */ >>>> + if (!aarch64_arch_string && !aarch64_cpu_string) >>>> + aarch64_arch_string = WITH_ARCH_STRING; >>>> + if (!aarch64_arch_string && !aarch64_cpu_string) >>>> + aarch64_cpu_string = WITH_CPU_STRING; >>>> + if (!aarch64_cpu_string && !aarch64_tune_string) >>>> + aarch64_tune_string = WITH_TUNE_STRING; >> >> (without the preprocessor stuff) IMO reads better. If a preprocessor >> is/isn't present test turns out to be useful, perhaps we should add >> macros like HAVE_WITH_TUNE/WITH_TUNE_PRESENT/... too? I guess it >> should only be done when something needs it though. > > It's relatively easy to add > > #ifndef WITH_TUNE_STRING > #define WITH_TUNE_STRING (nulptr) > #endif > > in a header, but much harder to go the other way. The case I was > thinking of was something like: > > #if !defined(WITH_ARCH_STRING) && !defined(WITH_CPU_STRING) > #define WITH_ARCH_STRING "<some-target-default>" > #endif > > which saves having to have yet another level of fallback if nothing has > been specified, but this is next to impossible if the macros are > unconditionally defined. Right, but I was suggesting to have both: WITH_TUNE_STRING: always available, as above HAVE_WITH_TUNE: for preprocessor conditions (if something needs it in future) So the C++ code could stay as above (A): /* Emulate OPTION_DEFAULT_SPECS. */ if (!aarch64_arch_string && !aarch64_cpu_string) aarch64_arch_string = WITH_ARCH_STRING; if (!aarch64_arch_string && !aarch64_cpu_string) aarch64_cpu_string = WITH_CPU_STRING; if (!aarch64_cpu_string && !aarch64_tune_string) aarch64_tune_string = WITH_TUNE_STRING; rather than have to become: #ifdef WITH_ARCH_STRING if (!aarch64_arch_string && !aarch64_cpu_string) aarch64_arch_string = WITH_ARCH_STRING; #endif #ifdef WITH_CPU_STRING if (!aarch64_arch_string && !aarch64_cpu_string) aarch64_cpu_string = WITH_CPU_STRING; #endif #ifdef WITH_TUNE_STRING if (!aarch64_cpu_string && !aarch64_tune_string) aarch64_tune_string = WITH_TUNE_STRING; #endif or: #ifndef WITH_ARCH_STRING #define WITH_ARCH_STRING (nulptr) #endif #ifndef WITH_CPU_STRING #define WITH_CPU_STRING (nulptr) #endif #ifndef WITH_TUNE_STRING #define WITH_TUNE_STRING (nulptr) #endif /* Emulate OPTION_DEFAULT_SPECS. */ if (!aarch64_arch_string && !aarch64_cpu_string) aarch64_arch_string = WITH_ARCH_STRING; if (!aarch64_arch_string && !aarch64_cpu_string) aarch64_cpu_string = WITH_CPU_STRING; if (!aarch64_cpu_string && !aarch64_tune_string) aarch64_tune_string = WITH_TUNE_STRING; But your case would still be possible by (B): #if !HAVE_WITH_ARCH && !HAVE_WITH_CPU #define WITH_ARCH_STRING "<some-target-default>" #endif (I think we're in agreement on this, but just in case, since the formulations are similar: the two pieces of code are doing different things. (A) is responding to the command-line options whereas (B) is responding only to configure-time options. IMO, providing fallback --with-arch etc. should be done in config.gcc instead, but I realise the stuff between the #if/#endif was just an example.) Thanks, Richard
diff --git a/gcc/config.gcc b/gcc/config.gcc index cdbefb5b4f5..e039230431c 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -5865,6 +5865,20 @@ else configure_default_options="{ ${t} }" fi +for option in $supported_defaults +do + lc_option=`echo $option | sed s/-/_/g` + uc_option=`echo $lc_option | tr a-z A-Z` + eval "val=\$with_$lc_option" + if test -n "$val" + then + val="\\\"$val\\\"" + else + val=nullptr + fi + tm_defines="$tm_defines WITH_${uc_option}_STRING=$val" +done + if test "$target_cpu_default2" != "" then if test "$target_cpu_default" != "" diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index d21e041eccb..0bc700b81ad 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -18109,6 +18109,14 @@ aarch64_override_options (void) if (aarch64_branch_protection_string) aarch64_validate_mbranch_protection (aarch64_branch_protection_string); + /* Emulate OPTION_DEFAULT_SPECS. */ + if (!aarch64_arch_string && !aarch64_cpu_string) + aarch64_arch_string = WITH_ARCH_STRING; + if (!aarch64_arch_string && !aarch64_cpu_string) + aarch64_cpu_string = WITH_CPU_STRING; + if (!aarch64_cpu_string && !aarch64_tune_string) + aarch64_tune_string = WITH_TUNE_STRING; + /* -mcpu=CPU is shorthand for -march=ARCH_FOR_CPU, -mtune=CPU. If either of -march or -mtune is given, they override their respective component of -mcpu. */ diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 80cfe4b7407..3122dbd7098 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -1267,7 +1267,10 @@ extern enum aarch64_code_model aarch64_cmodel; /* Support for configure-time --with-arch, --with-cpu and --with-tune. --with-arch and --with-cpu are ignored if either -mcpu or -march is used. --with-tune is ignored if either -mtune or -mcpu is used (but is not - affected by -march). */ + affected by -march). + + There is corresponding code in aarch64_override_options that emulates + this behavior when cc1 &co are invoked directly. */ #define OPTION_DEFAULT_SPECS \ {"arch", "%{!march=*:%{!mcpu=*:-march=%(VALUE)}}" }, \ {"cpu", "%{!march=*:%{!mcpu=*:-mcpu=%(VALUE)}}" }, \