Message ID | mpt5yl4eipx.fsf@arm.com |
---|---|
State | New |
Headers | show |
Series | [RFA,configure,parts] aarch64: Make cc1 &co handle --with options | expand |
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)}}" }, \