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 C8FF739A183A for ; Fri, 6 Aug 2021 14:25:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C8FF739A183A 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 74BF031B; Fri, 6 Aug 2021 07:25:16 -0700 (PDT) Received: from [10.57.6.202] (unknown [10.57.6.202]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B8EE53F719; Fri, 6 Aug 2021 07:25:15 -0700 (PDT) Subject: Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] To: Christophe Lyon , Richard Earnshaw Cc: GCC Patches References: <20210805115750.3824520-1-rearnsha@arm.com> <20210805115750.3824520-4-rearnsha@arm.com> From: Richard Earnshaw Message-ID: <7b9e5270-040c-376a-6546-1cd1082a7aa0@foss.arm.com> Date: Fri, 6 Aug 2021 15:25:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3498.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, NICE_REPLY_A, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 06 Aug 2021 14:25:18 -0000 On 06/08/2021 15:08, Christophe Lyon via Gcc-patches wrote: > On Thu, Aug 5, 2021 at 1:58 PM Richard Earnshaw wrote: > >> >> A change to the way gas interprets the .fpu directive in binutils-2.34 >> means that issuing .fpu will clear any features set by .arch_extension >> that apply to the floating point or simd units. This unfortunately >> causes problems for more recent versions of the architecture because >> we currently emit .arch, .arch_extension and .fpu directives at >> different times and try to suppress redundant changes. >> >> This change addresses this by firstly unifying all the places where we >> emit these directives to a single block of code and secondly >> (re)emitting all the directives if any changes have been made to the >> target options. Whilst this is slightly more than the strict minimum >> it should be enough to catch all cases where a change could have >> happened. The new code also emits the directives in the order: .arch, >> .fpu, .arch_extension. This ensures that the additional architectural >> extensions are not removed by a later .fpu directive. >> >> Whilst writing this patch I also noticed that in the corner case where >> the last function to be compiled had a non-standard set of >> architecture flags, the assembler would add an incorrect set of >> derived attributes for the file as a whole. Instead of reflecting the >> command-line options it would reflect the flags from the last file in >> the function. To address this I've also added a call to re-emit the >> flags from the asm_file_end callback so the assembler will be in the >> correct state when it finishes processing the intput. >> >> There's some slight churn to the testsuite as a consequence of this, >> because previously we had a hack to suppress emitting a .fpu directive >> for one specific case, but with the new order this is no-longer >> necessary. >> >> gcc/ChangeLog: >> >> PR target/101723 >> * config/arm/arm-cpus.in (generic-armv7-a): Add quirk to suppress >> writing .cpu directive in asm output. >> * config/arm/arm.c (arm_identify_fpu_from_isa): New variable. >> (arm_last_printed_arch_string): Delete. >> (arm_last-printed_fpu_string): Delete. >> (arm_configure_build_target): If use of floating-point/SIMD is >> disabled, remove all fp/simd related features from the target ISA. >> (last_arm_targ_options): New variable. >> (arm_print_asm_arch_directives): Add new parameters. Change order >> of emitted directives and handle all cases here. >> (arm_file_start): Always call arm_print_asm_arch_directives, move >> all generation of .arch/.arch_extension here. >> (arm_file_end): Call arm_print_asm_arch. >> (arm_declare_function_name): Call arm_print_asm_arch_directives >> instead of printing .arch/.fpu directives directly. >> >> gcc/testsuite/ChangeLog: >> >> PR target/101723 >> * gcc.target/arm/cortex-m55-nofp-flag-hard.c: Update expected >> output. >> * gcc.target/arm/cortex-m55-nofp-flag-softfp.c: Likewise. >> * gcc.target/arm/cortex-m55-nofp-nomve-flag-softfp.c: Likewise. >> * gcc.target/arm/mve/intrinsics/mve_fpu1.c: Convert to dg-do >> assemble. >> Add a non-no-op function body. >> * gcc.target/arm/mve/intrinsics/mve_fpu2.c: Likewise. >> * gcc.target/arm/pr98636.c (dg-options): Add -mfloat-abi=softfp. >> * gcc.target/arm/attr-neon.c: Tighten scan-assembler tests. >> * gcc.target/arm/attr-neon2.c: Use -Ofast, convert test to use >> check-function-bodies. >> * gcc.target/arm/attr-neon3.c: Likewise. >> * gcc.target/arm/pr69245.c: Tighten scan-assembler match, but allow >> multiple instances. >> * gcc.target/arm/pragma_fpu_attribute.c: Likewise. >> * gcc.target/arm/pragma_fpu_attribute_2.c: Likewise. >> > > > Hi Richard, > > There were a couple of typos in the tests updates, fixed as follows: > > Author: Christophe Lyon > Date: Fri Aug 6 14:06:44 2021 +0000 > > arm: Fix typos for reorder assembler architecture directives [PR101723] > > Two tests had typos preventing them from passing, committed as obvious. > > 2021-08-06 Christophe Lyon > > gcc/testsuite/ > PR target/101723 > * gcc.target/arm/attr-neon3.c: Fix typo. > * gcc.target/arm/pragma_fpu_attribute_2.c: Fix typo. > > diff --git a/gcc/testsuite/gcc.target/arm/attr-neon3.c > b/gcc/testsuite/gcc.target/arm/attr-neon3.c > index 0fbce6e4cd4..b6171e72d89 100644 > --- a/gcc/testsuite/gcc.target/arm/attr-neon3.c > +++ b/gcc/testsuite/gcc.target/arm/attr-neon3.c > @@ -33,7 +33,7 @@ my (float32x2_t __a, float32x2_t __b) > ** | > ** vld1.64 {d[0-9]+}, \[r[0-9]+:64\]! > ** vld1.64 {d[0-9]+}, \[r[0-9]+:64\] > -** } > +** ) > ** ... > ** bx lr > */ > diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c > b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c > index 3d33b04b787..398d8fff35c 100644 > --- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c > +++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c > @@ -28,5 +28,5 @@ uint32_t restored () > /* We can't tell exactly how many times the following tests will match > because > command-line options may cause additional instances to be generated, but > each must be present at least once. */ > -/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4\n} } } */ > -/* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16\n} } } */ > +/* { dg-final { scan-assembler {\.fpu\s+vfpv4\n} } } */ > +/* { dg-final { scan-assembler {\.fpu\s+vfpv3-d16\n} } } * > > Christophe > > > > >> --- >> gcc/config/arm/arm-cpus.in | 1 + >> gcc/config/arm/arm.c | 186 ++++++++---------- >> gcc/testsuite/gcc.target/arm/attr-neon.c | 9 +- >> gcc/testsuite/gcc.target/arm/attr-neon2.c | 35 ++-- >> gcc/testsuite/gcc.target/arm/attr-neon3.c | 48 +++-- >> .../arm/cortex-m55-nofp-flag-hard.c | 2 +- >> .../arm/cortex-m55-nofp-flag-softfp.c | 2 +- >> .../arm/cortex-m55-nofp-nomve-flag-softfp.c | 2 +- >> .../gcc.target/arm/mve/intrinsics/mve_fpu1.c | 5 +- >> .../gcc.target/arm/mve/intrinsics/mve_fpu2.c | 5 +- >> gcc/testsuite/gcc.target/arm/pr69245.c | 6 +- >> gcc/testsuite/gcc.target/arm/pr98636.c | 3 +- >> .../gcc.target/arm/pragma_fpu_attribute.c | 7 +- >> .../gcc.target/arm/pragma_fpu_attribute_2.c | 7 +- >> 14 files changed, 169 insertions(+), 149 deletions(-) >> >> OK. R.