public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <richard.sandiford@arm.com>
To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [RFA configure parts] aarch64: Make cc1 &co handle --with options
Date: Fri, 05 Aug 2022 14:53:07 +0100	[thread overview]
Message-ID: <mptczdeiypo.fsf@arm.com> (raw)
In-Reply-To: <65bfeff9-ae1d-9643-e17e-0b08b56ba78e@foss.arm.com> (Richard Earnshaw's message of "Tue, 2 Aug 2022 15:01:28 +0100")

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

  reply	other threads:[~2022-08-05 13:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-13 14:33 Richard Sandiford
2022-06-20  8:03 ` Ping: " Richard Sandiford
2022-07-12 12:25 ` Ping^2: " Richard Sandiford
2022-08-02  7:59 ` Ping^3: " Richard Sandiford
2022-08-02 14:01 ` Richard Earnshaw
2022-08-05 13:53   ` Richard Sandiford [this message]
2022-08-05 15:01     ` Richard Earnshaw
2022-08-16  7:51       ` Richard Sandiford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mptczdeiypo.fsf@arm.com \
    --to=richard.sandiford@arm.com \
    --cc=Richard.Earnshaw@foss.arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).