From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x132.google.com (mail-il1-x132.google.com [IPv6:2607:f8b0:4864:20::132]) by sourceware.org (Postfix) with ESMTPS id 1D5643AA983B for ; Fri, 6 Aug 2021 14:44:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1D5643AA983B Received: by mail-il1-x132.google.com with SMTP id r1so9154120iln.6 for ; Fri, 06 Aug 2021 07:44:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9A0735+mffqpq2T0EmSawny8F9QONDxC1kARplbylMU=; b=dF5aYFFdk9HrivBqTtA5KjFWGIFVp9f1hvpLm106d5XVsWpz7IhsOrcQd5Bjf0QgF+ y9LMKMrRvmrTEg5vohtdTGRtiDO/BDf2cQjkNYBhSAG41GuVfX2DOP9FpZVkqC0WrNkS Ovt9FRnxirYAUBaDZgpP0HqAwcyRp/jL7Md6Cm84HyckhXtQJOeLn5dxSXkUvdDG5TpB 9X+12UCdIeyCJrWwrTo90p12hpAEwEzNkjVLlIU/3MuIblIlu7Dw58aMcqSysGisvIwK MX97IA2wMA3msXpqV43UGieXgFopB+fITFmZK7rj3CGCty0pI90qqcvktU7qN8SYEZpd vF9Q== X-Gm-Message-State: AOAM533YhP+EZK6Ssw1dpYQ1MzHtKc5HQCb/E6UzeKyjWy3KZz8ScJ9S GLePT5ZEQA0fKPsX5AdATIUvXnx977c0NEXA/Ns= X-Google-Smtp-Source: ABdhPJzX7lseNex5Y8FS53WU23acJ4w5CPbnf4wxdzoTEfvLWKqcClklYuKVAibhYF1A6+As7+ONXWA0zBpNdfp12sA= X-Received: by 2002:a92:d711:: with SMTP id m17mr150200iln.201.1628261047486; Fri, 06 Aug 2021 07:44:07 -0700 (PDT) MIME-Version: 1.0 References: <20210805115750.3824520-1-rearnsha@arm.com> <20210805115750.3824520-4-rearnsha@arm.com> <48f6bf41-9be8-3fd5-b0ee-624d53f70d49@foss.arm.com> In-Reply-To: <48f6bf41-9be8-3fd5-b0ee-624d53f70d49@foss.arm.com> From: Christophe Lyon Date: Fri, 6 Aug 2021 16:43:56 +0200 Message-ID: Subject: Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] To: Richard Earnshaw Cc: Richard Earnshaw , GCC Patches X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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 Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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:44:10 -0000 On Fri, Aug 6, 2021 at 4:28 PM Richard Earnshaw < Richard.Earnshaw@foss.arm.com> wrote: > > > On 06/08/2021 15:21, Christophe Lyon via Gcc-patches wrote: > > On Fri, Aug 6, 2021 at 4:08 PM Christophe Lyon < > > christophe.lyon.oss@gmail.com> 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 > >> > >> > >> > > > > Hi Richard, > > > > There is an additional fallout: > > > > In gcc.target/arm/pr69245.c, to have a .fpu neon-vfpv4 directive, make > > > > > > > > > > sure code for fn1() is emitted, by removing the static keyword. > > > > > > > > > > > > > > > > > > > > Fix a typo in gcc.target/arm/pr69245.c, where \s should be \\s. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2021-08-06 Christophe Lyon > > > > > > > > > > > > > > > > > > > > gcc/testsuite/ > > > > > > > > > > > > > > > > > > > > PR target/101723 > > > > > > > > > > * gcc.target/arm/pr69245.c: Make sure to emit code for fn1, fix > > typo. > > > > > > > > > > > > > > > > > > diff --git a/gcc/testsuite/gcc.target/arm/pr69245.c > > b/gcc/testsuite/gcc.target/arm/pr69245.c > > > > > > > > index 34b97a22e15..58a6104e8f6 100644 > > > > > > > > > > --- a/gcc/testsuite/gcc.target/arm/pr69245.c > > > > > > > > > > +++ b/gcc/testsuite/gcc.target/arm/pr69245.c > > > > > > > > > > @@ -12,7 +12,7 @@ > > #pragma GCC target "fpu=neon-vfpv4" > > > > > > > > > > int a, c, d; > > > > > > > > > > float b; > > > > > > > > > > -static int fn1 () > > > > > > > > > > + int fn1 () > > > > > > > > > > { > > > > > > > > > > return 0; > > > > > > > > > > } > > > > > > > > > > @@ -26,5 +26,5 @@ void fn2 () > > /* Because we don't know the exact command-line options used to invoke > the > > test > > > > > > > > we cannot expect these tests to match exactly once. But they must > > appear at > > > > > > > > least once. */ > > > > > > > > > > -/* { dg-final { scan-assembler "\.fpu\s+vfp\n" } } */ > > > > > > > > > > -/* { dg-final { scan-assembler "\.fpu\s+neon-vfpv4\n" } } */ > > > > > > > > > > +/* { dg-final { scan-assembler "\.fpu\\s+vfp\n" } } */ > > > > > > > > > > +/* { dg-final { scan-assembler "\.fpu\\s+neon-vfpv4\n" } } */ > > > > > > > > Hmm, there's something gone wrong with the formatting above. Not sure > how. > > I don't know what happened, it looks OK in my "sent" folder... > Argh! the tcl difference between "regexp" and {regexp} strikes again. > > I'll trust you on this. OK! > Thanks, now pushed. > > R. > > > > > > > OK? > > > > 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(-) > >>> > >>> >