* Ping: [RFA configure parts] aarch64: Make cc1 &co handle --with options
2022-06-13 14:33 [RFA configure parts] aarch64: Make cc1 &co handle --with options Richard Sandiford
@ 2022-06-20 8:03 ` Richard Sandiford
2022-07-12 12:25 ` Ping^2: " Richard Sandiford
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2022-06-20 8:03 UTC (permalink / raw)
To: gcc-patches
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)}}" }, \
^ permalink raw reply [flat|nested] 8+ messages in thread* Ping^2: [RFA configure parts] aarch64: Make cc1 &co handle --with options
2022-06-13 14:33 [RFA configure parts] aarch64: Make cc1 &co handle --with options Richard Sandiford
2022-06-20 8:03 ` Ping: " Richard Sandiford
@ 2022-07-12 12:25 ` Richard Sandiford
2022-08-02 7:59 ` Ping^3: " Richard Sandiford
2022-08-02 14:01 ` Richard Earnshaw
3 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2022-07-12 12:25 UTC (permalink / raw)
To: gcc-patches
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)}}" }, \
^ permalink raw reply [flat|nested] 8+ messages in thread* Ping^3: [RFA configure parts] aarch64: Make cc1 &co handle --with options
2022-06-13 14:33 [RFA configure parts] aarch64: Make cc1 &co handle --with options Richard Sandiford
2022-06-20 8:03 ` Ping: " Richard Sandiford
2022-07-12 12:25 ` Ping^2: " Richard Sandiford
@ 2022-08-02 7:59 ` Richard Sandiford
2022-08-02 14:01 ` Richard Earnshaw
3 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2022-08-02 7:59 UTC (permalink / raw)
To: gcc-patches
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)}}" }, \
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFA configure parts] aarch64: Make cc1 &co handle --with options
2022-06-13 14:33 [RFA configure parts] aarch64: Make cc1 &co handle --with options Richard Sandiford
` (2 preceding siblings ...)
2022-08-02 7:59 ` Ping^3: " Richard Sandiford
@ 2022-08-02 14:01 ` Richard Earnshaw
2022-08-05 13:53 ` Richard Sandiford
3 siblings, 1 reply; 8+ messages in thread
From: Richard Earnshaw @ 2022-08-02 14:01 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
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)}}" }, \
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFA configure parts] aarch64: Make cc1 &co handle --with options
2022-08-02 14:01 ` Richard Earnshaw
@ 2022-08-05 13:53 ` Richard Sandiford
2022-08-05 15:01 ` Richard Earnshaw
0 siblings, 1 reply; 8+ messages in thread
From: Richard Sandiford @ 2022-08-05 13:53 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: gcc-patches
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)}}" }, \
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFA configure parts] aarch64: Make cc1 &co handle --with options
2022-08-05 13:53 ` Richard Sandiford
@ 2022-08-05 15:01 ` Richard Earnshaw
2022-08-16 7:51 ` Richard Sandiford
0 siblings, 1 reply; 8+ messages in thread
From: Richard Earnshaw @ 2022-08-05 15:01 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
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)}}" }, \
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [RFA configure parts] aarch64: Make cc1 &co handle --with options
2022-08-05 15:01 ` Richard Earnshaw
@ 2022-08-16 7:51 ` Richard Sandiford
0 siblings, 0 replies; 8+ messages in thread
From: Richard Sandiford @ 2022-08-16 7:51 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread