From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id E23503858C53 for ; Fri, 5 Aug 2022 15:01:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E23503858C53 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C11E0106F; Fri, 5 Aug 2022 08:01:41 -0700 (PDT) Received: from [10.2.78.27] (unknown [10.2.78.27]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 91EE03F73B; Fri, 5 Aug 2022 08:01:40 -0700 (PDT) Message-ID: <15d79a1c-7255-6223-baac-dca63b38ff7d@foss.arm.com> Date: Fri, 5 Aug 2022 16:01:39 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [RFA configure parts] aarch64: Make cc1 &co handle --with options Content-Language: en-GB To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com References: <65bfeff9-ae1d-9643-e17e-0b08b56ba78e@foss.arm.com> From: Richard Earnshaw In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3496.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Aug 2022 15:01:44 -0000 On 05/08/2022 14:53, Richard Sandiford via Gcc-patches wrote: > Richard Earnshaw 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 "" #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)}}" }, \