Richard Earnshaw <[email protected]> writes:
> On 05/08/2022 14:53, Richard Sandiford via Gcc-patches wrote:
>> Richard Earnshaw <[email protected]> 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
