Hi Richard,
On Fri, Aug 6, 2021 at 4:08 PM Christophe Lyon <
christophe.lyon....@gmail.com> wrote:
>
>
> On Thu, Aug 5, 2021 at 1:58 PM Richard Earnshaw <rearn...@arm.com> 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 <christophe.l...@foss.st.com>
> 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 <christophe.l...@foss.st.com>
>
> 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
> */
>
After fixing this typo in attr-neon3.c, the test passes on most targets,
except armeb, where the generated code is:
foo:
vldr.64 d17, [r0, #8] @ int
vldr.64 d16, [r0] @ int
vswp d16, d17
vmov r1, r0, d16 @ v2di
vmov r3, r2, d17
bx lr
The little-endian version is:
foo:
vld1.64 {d16-d17}, [r0:64]
vmov r0, r1, d16 @ v2di
vmov r2, r3, d17
bx lr
I haven't checked all configurations, so do we need a third alternative, or
should the second one with :64 be amended to match the big-endian case?
(on which configs did you get the vld1.64 pair?)
Thanks,
Christophe
> 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(-)
>>
>>
