Re: [RFA configure parts] aarch64: Make cc1 &co handle --with options

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

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)}}" },   \