* [committed v2 0/3] arm: fix problems when targetting extended FPUs [PR101723]
@ 2021-08-05 11:57 Richard Earnshaw
2021-08-05 11:57 ` [committed v2 1/3] arm: ensure the arch_name is always set for the build target Richard Earnshaw
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Richard Earnshaw @ 2021-08-05 11:57 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Earnshaw, christophe.lyon.oss
Thanks, Christophe, I've updated the testsuite to fix all the issues I could
see from your test runs.
This is what I've finally committed, but if there's any more fallout, please
let me know.
R.
Richard Earnshaw (3):
arm: ensure the arch_name is always set for the build target
arm: Don't reconfigure globals in arm_configure_build_target
arm: reorder assembler architecture directives [PR101723]
gcc/config/arm/arm-c.c | 1 +
gcc/config/arm/arm-cpus.in | 1 +
gcc/config/arm/arm.c | 190 ++++++++----------
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 +-
15 files changed, 173 insertions(+), 150 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [committed v2 1/3] arm: ensure the arch_name is always set for the build target 2021-08-05 11:57 [committed v2 0/3] arm: fix problems when targetting extended FPUs [PR101723] Richard Earnshaw @ 2021-08-05 11:57 ` Richard Earnshaw 2021-08-05 11:57 ` [committed v2 2/3] arm: Don't reconfigure globals in arm_configure_build_target Richard Earnshaw 2021-08-05 11:57 ` [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] Richard Earnshaw 2 siblings, 0 replies; 10+ messages in thread From: Richard Earnshaw @ 2021-08-05 11:57 UTC (permalink / raw) To: gcc-patches; +Cc: Richard Earnshaw, christophe.lyon.oss [-- Attachment #1: Type: text/plain, Size: 624 bytes --] This should never happen now if GCC is invoked by the driver, but in the unusual case of calling cc1 (or its ilk) directly from the command line the build target's arch_name string can remain NULL. This can complicate later processing meaning that we need to check for this case explicitly in some circumstances. Nothing should rely on this behaviour, so it's simpler to always set the arch_name when configuring the build target and be done with it. gcc: * config/arm/arm.c (arm_configure_build_target): Ensure the target's arch_name is always set. --- gcc/config/arm/arm.c | 2 ++ 1 file changed, 2 insertions(+) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: v2-0001-arm-ensure-the-arch_name-is-always-set-for-the-bu.patch --] [-- Type: text/x-patch; name="v2-0001-arm-ensure-the-arch_name-is-always-set-for-the-bu.patch", Size: 597 bytes --] diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 6d781e23ee9..b2dd58d8751 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -3432,6 +3432,8 @@ arm_configure_build_target (struct arm_build_target *target, const cpu_tune *tune_data = &all_tunes[arm_selected_tune - all_cores]; /* Finish initializing the target structure. */ + if (!target->arch_name) + target->arch_name = arm_selected_arch->common.name; target->arch_pp_name = arm_selected_arch->arch; target->base_arch = arm_selected_arch->base_arch; target->profile = arm_selected_arch->profile; ^ permalink raw reply [flat|nested] 10+ messages in thread
* [committed v2 2/3] arm: Don't reconfigure globals in arm_configure_build_target 2021-08-05 11:57 [committed v2 0/3] arm: fix problems when targetting extended FPUs [PR101723] Richard Earnshaw 2021-08-05 11:57 ` [committed v2 1/3] arm: ensure the arch_name is always set for the build target Richard Earnshaw @ 2021-08-05 11:57 ` Richard Earnshaw 2021-08-05 11:57 ` [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] Richard Earnshaw 2 siblings, 0 replies; 10+ messages in thread From: Richard Earnshaw @ 2021-08-05 11:57 UTC (permalink / raw) To: gcc-patches; +Cc: Richard Earnshaw, christophe.lyon.oss [-- Attachment #1: Type: text/plain, Size: 1266 bytes --] arm_configure_build_target is usually used to reconfigure the arm_active_target structure, which is then used to reconfigure a number of other global variables describing the current target. Occasionally, however, we need to use arm_configure_build_target to construct a temporary target structure and in that case it is wrong to try to reconfigure the global variables (although probably harmless, since arm_option_reconfigure_globals() only looks at arm_active_target). At the very least, however, this is wasted work, so it is best not to do it unless needed. What's more, several callers of arm_configure_build target call arm_option_reconfigure_globals themselves within a few lines, making the call from within arm_configure_build_target completely redundant. So this patch moves the responsibility of calling of arm_configure_build_target to its callers (only two places needed updating). gcc: * config/arm/arm.c (arm_configure_build_target): Don't call arm_option_reconfigure_globals. (arm_option_restore): Call arm_option_reconfigure_globals after reconfiguring the target. * config/arm/arm-c.c (arm_pragma_target_parse): Likewise. --- gcc/config/arm/arm-c.c | 1 + gcc/config/arm/arm.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: v2-0002-arm-Don-t-reconfigure-globals-in-arm_configure_bu.patch --] [-- Type: text/x-patch; name="v2-0002-arm-Don-t-reconfigure-globals-in-arm_configure_bu.patch", Size: 1263 bytes --] diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c index ae2139c4bfa..cc7901bca8d 100644 --- a/gcc/config/arm/arm-c.c +++ b/gcc/config/arm/arm-c.c @@ -409,6 +409,7 @@ arm_pragma_target_parse (tree args, tree pop_target) target_option_current_node = cur_tree; arm_configure_build_target (&arm_active_target, TREE_TARGET_OPTION (cur_tree), false); + arm_option_reconfigure_globals (); } /* Update macros if target_node changes. The global state will be restored diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index b2dd58d8751..273202ac2fd 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -3058,6 +3058,7 @@ arm_option_restore (struct gcc_options */* opts */, struct cl_target_option *ptr) { arm_configure_build_target (&arm_active_target, ptr, false); + arm_option_reconfigure_globals (); } /* Reset options between modes that the user has specified. */ @@ -3441,7 +3442,6 @@ arm_configure_build_target (struct arm_build_target *target, target->tune_flags = tune_data->tune_flags; target->tune = tune_data->tune; target->tune_core = tune_data->scheduler; - arm_option_reconfigure_globals (); } /* Fix up any incompatible options that the user has specified. */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] 2021-08-05 11:57 [committed v2 0/3] arm: fix problems when targetting extended FPUs [PR101723] Richard Earnshaw 2021-08-05 11:57 ` [committed v2 1/3] arm: ensure the arch_name is always set for the build target Richard Earnshaw 2021-08-05 11:57 ` [committed v2 2/3] arm: Don't reconfigure globals in arm_configure_build_target Richard Earnshaw @ 2021-08-05 11:57 ` Richard Earnshaw 2021-08-06 14:08 ` Christophe Lyon 2 siblings, 1 reply; 10+ messages in thread From: Richard Earnshaw @ 2021-08-05 11:57 UTC (permalink / raw) To: gcc-patches; +Cc: Richard Earnshaw, christophe.lyon.oss [-- Attachment #1: Type: text/plain, Size: 4241 bytes --] 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. --- 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(-) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: v2-0003-arm-reorder-assembler-architecture-directives-PR1.patch --] [-- Type: text/x-patch; name="v2-0003-arm-reorder-assembler-architecture-directives-PR1.patch", Size: 23185 bytes --] diff --git a/gcc/config/arm/arm-cpus.in b/gcc/config/arm/arm-cpus.in index ab4b6acf5ea..249995a6bca 100644 --- a/gcc/config/arm/arm-cpus.in +++ b/gcc/config/arm/arm-cpus.in @@ -1080,6 +1080,7 @@ begin cpu generic-armv7-a cname genericv7a tune flags LDSCHED architecture armv7-a+fp + isa quirk_no_asmcpu option mp add mp option sec add sec option vfpv3-d16 add VFPv3 FP_DBL diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 273202ac2fd..11dafc70067 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -79,10 +79,6 @@ typedef struct minipool_node Mnode; typedef struct minipool_fixup Mfix; -/* The last .arch and .fpu assembly strings that we printed. */ -static std::string arm_last_printed_arch_string; -static std::string arm_last_printed_fpu_string; - void (*arm_lang_output_object_attributes_hook)(void); struct four_ints @@ -334,6 +330,7 @@ static rtx_insn *thumb1_md_asm_adjust (vec<rtx> &, vec<rtx> &, vec<machine_mode> &, vec<const char *> &, vec<rtx> &, HARD_REG_SET &, location_t); +static const char *arm_identify_fpu_from_isa (sbitmap); \f /* Table of machine attributes. */ static const struct attribute_spec arm_attribute_table[] = @@ -3411,6 +3408,11 @@ arm_configure_build_target (struct arm_build_target *target, bitmap_ior (target->isa, target->isa, fpu_bits); } + /* If we have the soft-float ABI, clear any feature bits relating to use of + floating-point operations. They'll just confuse things later on. */ + if (arm_float_abi == ARM_FLOAT_ABI_SOFT) + bitmap_and_compl (target->isa, target->isa, isa_all_fpbits); + /* There may be implied bits which we still need to enable. These are non-named features which are needed to complete other sets of features, but cannot be enabled from arm-cpus.in due to being shared between @@ -28096,20 +28098,65 @@ arm_print_tune_info (void) (int) current_tune->sched_autopref); } +/* The last set of target options used to emit .arch directives, etc. This + could be a function-local static if it were not required to expose it as a + root to the garbage collector. */ +static GTY(()) cl_target_option *last_asm_targ_options = NULL; + /* Print .arch and .arch_extension directives corresponding to the current architecture configuration. */ static void -arm_print_asm_arch_directives () +arm_print_asm_arch_directives (FILE *stream, cl_target_option *targ_options) { + arm_build_target build_target; + /* If the target options haven't changed since the last time we were called + there is nothing to do. This should be sufficient to suppress the + majority of redundant work. */ + if (last_asm_targ_options == targ_options) + return; + + last_asm_targ_options = targ_options; + + build_target.isa = sbitmap_alloc (isa_num_bits); + arm_configure_build_target (&build_target, targ_options, false); + + if (build_target.core_name + && !bitmap_bit_p (build_target.isa, isa_bit_quirk_no_asmcpu)) + { + const char* truncated_name + = arm_rewrite_selected_cpu (build_target.core_name); + asm_fprintf (stream, "\t.cpu %s\n", truncated_name); + } + const arch_option *arch = arm_parse_arch_option_name (all_architectures, "-march", - arm_active_target.arch_name); + build_target.arch_name); auto_sbitmap opt_bits (isa_num_bits); gcc_assert (arch); - asm_fprintf (asm_out_file, "\t.arch %s\n", arm_active_target.arch_name); - arm_last_printed_arch_string = arm_active_target.arch_name; + if (strcmp (build_target.arch_name, "armv7ve") == 0) + { + /* Keep backward compatability for assemblers which don't support + armv7ve. Fortunately, none of the following extensions are reset + by a .fpu directive. */ + asm_fprintf (stream, "\t.arch armv7-a\n"); + asm_fprintf (stream, "\t.arch_extension virt\n"); + asm_fprintf (stream, "\t.arch_extension idiv\n"); + asm_fprintf (stream, "\t.arch_extension sec\n"); + asm_fprintf (stream, "\t.arch_extension mp\n"); + } + else + asm_fprintf (stream, "\t.arch %s\n", build_target.arch_name); + + /* The .fpu directive will reset any architecture extensions from the + assembler that relate to the fp/vector extensions. So put this out before + any .arch_extension directives. */ + const char *fpu_name = (TARGET_SOFT_FLOAT + ? "softvfp" + : arm_identify_fpu_from_isa (build_target.isa)); + asm_fprintf (stream, "\t.fpu %s\n", fpu_name); + if (!arch->common.extensions) return; @@ -28135,13 +28182,12 @@ arm_print_asm_arch_directives () && !TARGET_HAVE_MVE_FLOAT)) continue; - /* If every feature bit of this option is set in the target - ISA specification, print out the option name. However, - don't print anything if all the bits are part of the - FPU specification. */ - if (bitmap_subset_p (opt_bits, arm_active_target.isa) + /* If every feature bit of this option is set in the target ISA + specification, print out the option name. However, don't print + anything if all the bits are part of the FPU specification. */ + if (bitmap_subset_p (opt_bits, build_target.isa) && !bitmap_subset_p (opt_bits, isa_all_fpubits_internal)) - asm_fprintf (asm_out_file, "\t.arch_extension %s\n", opt->name); + asm_fprintf (stream, "\t.arch_extension %s\n", opt->name); } } } @@ -28151,46 +28197,23 @@ arm_file_start (void) { int val; + arm_print_asm_arch_directives + (asm_out_file, TREE_TARGET_OPTION (target_option_default_node)); + if (TARGET_BPABI) { - /* We don't have a specified CPU. Use the architecture to - generate the tags. - - Note: it might be better to do this unconditionally, then the - assembler would not need to know about all new CPU names as - they are added. */ - if (!arm_active_target.core_name) - { - /* armv7ve doesn't support any extensions. */ - if (strcmp (arm_active_target.arch_name, "armv7ve") == 0) - { - /* Keep backward compatability for assemblers - which don't support armv7ve. */ - asm_fprintf (asm_out_file, "\t.arch armv7-a\n"); - asm_fprintf (asm_out_file, "\t.arch_extension virt\n"); - asm_fprintf (asm_out_file, "\t.arch_extension idiv\n"); - asm_fprintf (asm_out_file, "\t.arch_extension sec\n"); - asm_fprintf (asm_out_file, "\t.arch_extension mp\n"); - arm_last_printed_arch_string = "armv7ve"; - } - else - arm_print_asm_arch_directives (); - } - else if (startswith (arm_active_target.core_name, "generic")) - { - asm_fprintf (asm_out_file, "\t.arch %s\n", - arm_active_target.core_name + 8); - arm_last_printed_arch_string = arm_active_target.core_name + 8; - } - else + /* If we have a named cpu, but we the assembler does not support that + name via .cpu, put out a cpu name attribute; but don't do this if the + name starts with the fictitious prefix, 'generic'. */ + if (arm_active_target.core_name + && bitmap_bit_p (arm_active_target.isa, isa_bit_quirk_no_asmcpu) + && !startswith (arm_active_target.core_name, "generic")) { const char* truncated_name = arm_rewrite_selected_cpu (arm_active_target.core_name); if (bitmap_bit_p (arm_active_target.isa, isa_bit_quirk_no_asmcpu)) asm_fprintf (asm_out_file, "\t.eabi_attribute 5, \"%s\"\n", truncated_name); - else - asm_fprintf (asm_out_file, "\t.cpu %s\n", truncated_name); } if (print_tune_info) @@ -28255,6 +28278,13 @@ arm_file_end (void) { int regno; + /* Just in case the last function output in the assembler had non-default + architecture directives, we force the assembler state back to the default + set, so that any 'calculated' build attributes are based on the default + options rather than the special options for that function. */ + arm_print_asm_arch_directives + (asm_out_file, TREE_TARGET_OPTION (target_option_default_node)); + if (NEED_INDICATE_EXEC_STACK) /* Add .note.GNU-stack. */ file_end_indicate_exec_stack (); @@ -33265,58 +33295,7 @@ arm_declare_function_name (FILE *stream, const char *name, tree decl) targ_options = TREE_TARGET_OPTION (target_option_current_node); gcc_assert (targ_options); - /* Only update the assembler .arch string if it is distinct from the last - such string we printed. arch_to_print is set conditionally in case - targ_options->x_arm_arch_string is NULL which can be the case - when cc1 is invoked directly without passing -march option. */ - std::string arch_to_print; - if (targ_options->x_arm_arch_string) - arch_to_print = targ_options->x_arm_arch_string; - - if (arch_to_print != arm_last_printed_arch_string) - { - std::string arch_name - = arch_to_print.substr (0, arch_to_print.find ("+")); - asm_fprintf (asm_out_file, "\t.arch %s\n", arch_name.c_str ()); - const arch_option *arch - = arm_parse_arch_option_name (all_architectures, "-march", - targ_options->x_arm_arch_string); - auto_sbitmap opt_bits (isa_num_bits); - - gcc_assert (arch); - if (arch->common.extensions) - { - for (const struct cpu_arch_extension *opt = arch->common.extensions; - opt->name != NULL; - opt++) - { - if (!opt->remove) - { - arm_initialize_isa (opt_bits, opt->isa_bits); - /* For the cases "-march=armv8.1-m.main+mve -mfloat-abi=soft" - and "-march=armv8.1-m.main+mve.fp -mfloat-abi=soft" MVE and - MVE with floating point instructions is disabled. So the - following check restricts the printing of ".arch_extension - mve" and ".arch_extension fp" (for mve.fp) in the assembly - file. MVE needs this special behaviour because the - feature bit "mve" and "mve_float" are not part of - "fpu bits", so they are not cleared when -mfloat-abi=soft - (i.e nofp) but the marco TARGET_HAVE_MVE and - TARGET_HAVE_MVE_FLOAT are disabled. */ - if ((bitmap_bit_p (opt_bits, isa_bit_mve) && !TARGET_HAVE_MVE) - || (bitmap_bit_p (opt_bits, isa_bit_mve_float) - && !TARGET_HAVE_MVE_FLOAT)) - continue; - if (bitmap_subset_p (opt_bits, arm_active_target.isa) - && !bitmap_subset_p (opt_bits, isa_all_fpubits_internal)) - asm_fprintf (asm_out_file, "\t.arch_extension %s\n", - opt->name); - } - } - } - - arm_last_printed_arch_string = arch_to_print; - } + arm_print_asm_arch_directives (stream, targ_options); fprintf (stream, "\t.syntax unified\n"); @@ -33334,17 +33313,6 @@ arm_declare_function_name (FILE *stream, const char *name, tree decl) else fprintf (stream, "\t.arm\n"); - std::string fpu_to_print - = TARGET_SOFT_FLOAT - ? "softvfp" : arm_identify_fpu_from_isa (arm_active_target.isa); - - if (!(!strcmp (fpu_to_print.c_str (), "softvfp") && TARGET_VFP_BASE) - && (fpu_to_print != arm_last_printed_arch_string)) - { - asm_fprintf (asm_out_file, "\t.fpu %s\n", fpu_to_print.c_str ()); - arm_last_printed_fpu_string = fpu_to_print; - } - if (TARGET_POKE_FUNCTION_NAME) arm_poke_function_name (stream, (const char *) name); } diff --git a/gcc/testsuite/gcc.target/arm/attr-neon.c b/gcc/testsuite/gcc.target/arm/attr-neon.c index 225fb8dc3db..e8e3086247d 100644 --- a/gcc/testsuite/gcc.target/arm/attr-neon.c +++ b/gcc/testsuite/gcc.target/arm/attr-neon.c @@ -1,7 +1,10 @@ /* { dg-do compile } */ /* { dg-require-effective-target arm_neon_ok } */ /* { dg-options "-O2 -ftree-vectorize" } */ -/* { dg-add-options arm_neon arm_v8_vfp } */ /* The arm_v8_vfp adds -mfpu=fp-armv8 to the command line, overriding any -mfpu= option set by arm_neon, thus ensuring that the attributes below really are checked for correct fpu selection. */ +/* { dg-add-options arm_neon arm_v8_vfp } */ +/* The arm_v8_vfp adds -mfpu=fp-armv8 to the command line, overriding any + -mfpu= option set by arm_neon, thus ensuring that the attributes below + really are checked for correct fpu selection. */ /* Verify that neon instructions are emitted once. */ void __attribute__ ((target("fpu=neon"))) @@ -18,6 +21,6 @@ f3(int n, int x[], int y[]) { y[i] = x[i] << 3; } -/* { dg-final { scan-assembler-times "\.fpu vfp" 1 } } */ -/* { dg-final { scan-assembler-times "\.fpu neon" 1 } } */ +/* { dg-final { scan-assembler-times "\.fpu\\s+vfp\n" 1 } } */ +/* { dg-final { scan-assembler-times "\.fpu\\s+neon\n" 1 } } */ /* { dg-final { scan-assembler-times "vshl" 1 } } */ diff --git a/gcc/testsuite/gcc.target/arm/attr-neon2.c b/gcc/testsuite/gcc.target/arm/attr-neon2.c index 29668256cf5..a7a72dac379 100644 --- a/gcc/testsuite/gcc.target/arm/attr-neon2.c +++ b/gcc/testsuite/gcc.target/arm/attr-neon2.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-require-effective-target arm_neon_ok } */ /* { dg-require-effective-target arm_fp_ok } */ -/* { dg-options "-O2" } */ +/* { dg-options "-Ofast" } */ /* { dg-add-options arm_fp } */ /* Reset fpu to a value compatible with the next pragmas. */ @@ -12,23 +12,36 @@ #include <arm_neon.h> /* Check that pragma target is used. */ -int8x8_t -my (int8x8_t __a, int8x8_t __b) +/* +**my: +** ... +** vadd.f32 d[0-9]+, d[0-9]+, d[0-9]+ +** ... +** bx lr +*/ +float32x2_t +my (float32x2_t __a, float32x2_t __b) { return __a + __b; } #pragma GCC pop_options -/* Check that command line option is restored. */ -int8x8_t -my1 (int8x8_t __a, int8x8_t __b) +/* Check that fpu=vfp is restored. */ +/* +**my1: +** ... +** vadd.f32 s[0-9]+, s[0-9]+, s[0-9]+ +** vadd.f32 s[0-9]+, s[0-9]+, s[0-9]+ +** ... +** bx lr +*/ +float32x2_t +my1 (float32x2_t __a, float32x2_t __b) { return __a + __b; } -/* { dg-final { scan-assembler-times "\.fpu vfp" 1 } } */ -/* { dg-final { scan-assembler-times "\.fpu neon" 1 } } */ -/* { dg-final { scan-assembler "vadd" } } */ - - +/* { dg-final { scan-assembler "\.fpu\\s+vfp\n" } } */ +/* { dg-final { scan-assembler "\.fpu\\s+neon\n" } } */ +/* { dg-final { check-function-bodies "**" "" } } */ diff --git a/gcc/testsuite/gcc.target/arm/attr-neon3.c b/gcc/testsuite/gcc.target/arm/attr-neon3.c index 17e429ad739..0fbce6e4cd4 100644 --- a/gcc/testsuite/gcc.target/arm/attr-neon3.c +++ b/gcc/testsuite/gcc.target/arm/attr-neon3.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-require-effective-target arm_crypto_ok } */ /* { dg-require-effective-target arm_fp_ok } */ -/* { dg-options "-O2" } */ +/* { dg-options "-Ofast" } */ /* { dg-add-options arm_fp } */ /* Reset fpu to a value compatible with the next pragmas. */ @@ -11,28 +11,54 @@ #include <arm_neon.h> /* Check that neon is used. */ -int8x8_t __attribute__ ((target("fpu=neon"))) -my (int8x8_t __a, int8x8_t __b) +/* +**my: +** ... +** vadd.f32 d[0-9]+, d[0-9]+, d[0-9]+ +** ... +** bx lr +*/ +float32x2_t __attribute__ ((target("fpu=neon"))) +my (float32x2_t __a, float32x2_t __b) { return __a + __b; } /* Check that crypto builtins are recognized. */ +/* +**foo: +** ... +** ( +** vld1.64 {d[0-9]+-d[0-9]+}, \[r[0-9]+:64\] +** | +** vld1.64 {d[0-9]+}, \[r[0-9]+:64\]! +** vld1.64 {d[0-9]+}, \[r[0-9]+:64\] +** } +** ... +** bx lr +*/ + poly128_t __attribute__ ((target("fpu=crypto-neon-fp-armv8"))) foo (poly128_t* ptr) { return vldrq_p128 (ptr); } -/* Check that default mode is restored. */ -int8x8_t -my1 (int8x8_t __a, int8x8_t __b) +/* Check that fpu=vfp is restored. */ +/* +**my1: +** ... +** vadd.f32 s[0-9]+, s[0-9]+, s[0-9]+ +** vadd.f32 s[0-9]+, s[0-9]+, s[0-9]+ +** ... +** bx lr +*/float32x2_t +my1 (float32x2_t __a, float32x2_t __b) { return __a + __b; } -/* { dg-final { scan-assembler-times "\.fpu vfp" 1 } } */ -/* { dg-final { scan-assembler-times "\.fpu neon" 1 } } */ -/* { dg-final { scan-assembler-times "\.fpu crypto-neon-fp-armv8" 1 } } */ -/* { dg-final { scan-assembler-times "vld1" 1 } } */ -/* { dg-final { scan-assembler-times "vadd" 1} } */ +/* { dg-final { scan-assembler "\.fpu\\s+vfp\n" } } */ +/* { dg-final { scan-assembler "\.fpu\\s+neon\n" } } */ +/* { dg-final { scan-assembler "\.fpu\\s+crypto-neon-fp-armv8\n" } } */ +/* { dg-final { check-function-bodies "**" "" } } */ diff --git a/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-hard.c b/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-hard.c index 6a92dedcc59..e0fb307ac30 100644 --- a/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-hard.c +++ b/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-hard.c @@ -1,12 +1,12 @@ /* { dg-do assemble } */ /* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */ /* { dg-additional-options "-mcpu=cortex-m55+nofp -mthumb -mfloat-abi=hard -mfpu=auto --save-temps" } */ +/* { dg-final { scan-assembler "\.fpu softvfp" } } */ /* { dg-final { scan-assembler "\.arch_extension mve" } } */ /* { dg-final { scan-assembler "\.arch_extension dsp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension fp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension fp.dp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension mve.fp" } } */ -/* { dg-final { scan-assembler-not "\.fpu" } } */ int f () diff --git a/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-softfp.c b/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-softfp.c index 25e80e9bd64..50645e8cd0f 100644 --- a/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-softfp.c +++ b/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-flag-softfp.c @@ -1,12 +1,12 @@ /* { dg-do assemble } */ /* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */ /* { dg-additional-options "-mcpu=cortex-m55+nofp -mthumb -mfloat-abi=softfp -mfpu=auto --save-temps" } */ +/* { dg-final { scan-assembler "\.fpu softvfp" } } */ /* { dg-final { scan-assembler "\.arch_extension mve" } } */ /* { dg-final { scan-assembler "\.arch_extension dsp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension fp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension fp.dp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension mve.fp" } } */ -/* { dg-final { scan-assembler-not "\.fpu" } } */ int f () diff --git a/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-nomve-flag-softfp.c b/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-nomve-flag-softfp.c index 38042cc490c..948f622633c 100644 --- a/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-nomve-flag-softfp.c +++ b/gcc/testsuite/gcc.target/arm/cortex-m55-nofp-nomve-flag-softfp.c @@ -1,12 +1,12 @@ /* { dg-do assemble } */ /* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */ /* { dg-additional-options "-mcpu=cortex-m55+nomve+nofp -mthumb -mfloat-abi=softfp -mfpu=auto --save-temps" } */ +/* { dg-final { scan-assembler "\.fpu softvfp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension mve" } } */ /* { dg-final { scan-assembler-not "\.arch_extension mve.fp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension fp" } } */ /* { dg-final { scan-assembler-not "\.arch_extension fp.dp" } } */ /* { dg-final { scan-assembler "\.arch_extension dsp" } } */ -/* { dg-final { scan-assembler-not "\.fpu" } } */ int f () diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu1.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu1.c index 611097ec956..c5acdb5e4e5 100644 --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu1.c +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu1.c @@ -1,3 +1,4 @@ +/* { dg-do assemble } */ /* { dg-require-effective-target arm_v8_1m_mve_ok } */ /* { dg-require-effective-target arm_hard_ok } */ /* { dg-add-options arm_v8_1m_mve } */ @@ -8,8 +9,6 @@ int8x16_t foo1 (int8x16_t value) { - int8x16_t b = value; + int8x16_t b = -value; return b; } - -/* { dg-final { scan-assembler-not "\.fpu softvfp" } } */ diff --git a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu2.c b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu2.c index b8e105111de..907db5e1572 100644 --- a/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu2.c +++ b/gcc/testsuite/gcc.target/arm/mve/intrinsics/mve_fpu2.c @@ -1,3 +1,4 @@ +/* { dg-do assemble } */ /* { dg-require-effective-target arm_v8_1m_mve_ok } */ /* { dg-require-effective-target arm_softfp_ok } */ /* { dg-add-options arm_v8_1m_mve } */ @@ -8,8 +9,6 @@ int8x16_t foo1 (int8x16_t value) { - int8x16_t b = value; + int8x16_t b = -value; return b; } - -/* { dg-final { scan-assembler-not "\.fpu softvfp" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pr69245.c b/gcc/testsuite/gcc.target/arm/pr69245.c index bd505187728..34b97a22e15 100644 --- a/gcc/testsuite/gcc.target/arm/pr69245.c +++ b/gcc/testsuite/gcc.target/arm/pr69245.c @@ -23,4 +23,8 @@ void fn2 () d = b * c + a; } -/* { dg-final { scan-assembler-times "\.fpu vfp" 1 } } */ +/* 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" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pr98636.c b/gcc/testsuite/gcc.target/arm/pr98636.c index c4d235cb43d..559f9a26c1e 100644 --- a/gcc/testsuite/gcc.target/arm/pr98636.c +++ b/gcc/testsuite/gcc.target/arm/pr98636.c @@ -1,5 +1,6 @@ /* { dg-do compile } */ -/* { dg-options "-mfp16-format=alternative" } */ +/* { dg-require-effective-target arm_softfp_ok } */ +/* { dg-options "-mfp16-format=alternative -mfloat-abi=softfp" } */ #pragma GCC push_options # pragma GCC target ("arch=armv8.2-a+fp16") /* { dg-error "selected fp16 options are incompatible" } */ diff --git a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c index 174be85f3f7..7e63cf53013 100644 --- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c +++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute.c @@ -22,5 +22,8 @@ uint32_t restored () return bar(); } -/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */ -/* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */ +/* 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 {\.fpu\s+vfpv4\n} } } */ +/* { dg-final { scan-assembler {\.fpu\s+vfpv3-d16\n} } } */ 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 add40ddc6b8..3d33b04b787 100644 --- a/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c +++ b/gcc/testsuite/gcc.target/arm/pragma_fpu_attribute_2.c @@ -25,5 +25,8 @@ uint32_t restored () return bar(); } -/* { dg-final { scan-assembler-times {\.fpu\s+vfpv4} 1 } } */ -/* { dg-final { scan-assembler-times {\.fpu\s+vfpv3-d16} 1 } } */ +/* 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} } } */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] 2021-08-05 11:57 ` [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] Richard Earnshaw @ 2021-08-06 14:08 ` Christophe Lyon 2021-08-06 14:21 ` Christophe Lyon ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Christophe Lyon @ 2021-08-06 14:08 UTC (permalink / raw) To: Richard Earnshaw; +Cc: GCC Patches On Thu, Aug 5, 2021 at 1:58 PM Richard Earnshaw <rearnsha@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.lyon@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.lyon@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 */ 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(-) > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] 2021-08-06 14:08 ` Christophe Lyon @ 2021-08-06 14:21 ` Christophe Lyon 2021-08-06 14:28 ` Richard Earnshaw 2021-08-06 14:25 ` Richard Earnshaw 2021-08-09 17:47 ` Christophe Lyon 2 siblings, 1 reply; 10+ messages in thread From: Christophe Lyon @ 2021-08-06 14:21 UTC (permalink / raw) To: Richard Earnshaw; +Cc: GCC Patches 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 <rearnsha@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.lyon@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.lyon@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 > */ > 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 <christophe.lyon@foss.st.com> 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" } } */ 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(-) >> >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] 2021-08-06 14:21 ` Christophe Lyon @ 2021-08-06 14:28 ` Richard Earnshaw 2021-08-06 14:43 ` Christophe Lyon 0 siblings, 1 reply; 10+ messages in thread From: Richard Earnshaw @ 2021-08-06 14:28 UTC (permalink / raw) To: Christophe Lyon, Richard Earnshaw; +Cc: GCC Patches 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 <rearnsha@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.lyon@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.lyon@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 >> */ >> 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 <christophe.lyon@foss.st.com> > > > > > > > > > > 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. Argh! the tcl difference between "regexp" and {regexp} strikes again. I'll trust you on this. OK! 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(-) >>> >>> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] 2021-08-06 14:28 ` Richard Earnshaw @ 2021-08-06 14:43 ` Christophe Lyon 0 siblings, 0 replies; 10+ messages in thread From: Christophe Lyon @ 2021-08-06 14:43 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Richard Earnshaw, GCC Patches 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 <rearnsha@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.lyon@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.lyon@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 > >> */ > >> 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 <christophe.lyon@foss.st.com> > > > > > > > > > > > > > > > > > > > > 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(-) > >>> > >>> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] 2021-08-06 14:08 ` Christophe Lyon 2021-08-06 14:21 ` Christophe Lyon @ 2021-08-06 14:25 ` Richard Earnshaw 2021-08-09 17:47 ` Christophe Lyon 2 siblings, 0 replies; 10+ messages in thread From: Richard Earnshaw @ 2021-08-06 14:25 UTC (permalink / raw) To: Christophe Lyon, Richard Earnshaw; +Cc: GCC Patches On 06/08/2021 15:08, Christophe Lyon via Gcc-patches wrote: > On Thu, Aug 5, 2021 at 1:58 PM Richard Earnshaw <rearnsha@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.lyon@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.lyon@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 > */ > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] 2021-08-06 14:08 ` Christophe Lyon 2021-08-06 14:21 ` Christophe Lyon 2021-08-06 14:25 ` Richard Earnshaw @ 2021-08-09 17:47 ` Christophe Lyon 2 siblings, 0 replies; 10+ messages in thread From: Christophe Lyon @ 2021-08-09 17:47 UTC (permalink / raw) To: Richard Earnshaw; +Cc: GCC Patches Hi Richard, 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 <rearnsha@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.lyon@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.lyon@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(-) >> >> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-08-09 17:47 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-05 11:57 [committed v2 0/3] arm: fix problems when targetting extended FPUs [PR101723] Richard Earnshaw 2021-08-05 11:57 ` [committed v2 1/3] arm: ensure the arch_name is always set for the build target Richard Earnshaw 2021-08-05 11:57 ` [committed v2 2/3] arm: Don't reconfigure globals in arm_configure_build_target Richard Earnshaw 2021-08-05 11:57 ` [committed v2 3/3] arm: reorder assembler architecture directives [PR101723] Richard Earnshaw 2021-08-06 14:08 ` Christophe Lyon 2021-08-06 14:21 ` Christophe Lyon 2021-08-06 14:28 ` Richard Earnshaw 2021-08-06 14:43 ` Christophe Lyon 2021-08-06 14:25 ` Richard Earnshaw 2021-08-09 17:47 ` Christophe Lyon
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).