Richard Earnshaw <richard.earns...@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)}}" }, \
