On Mon, 26 May 2025 at 18:35, Christophe Lyon
<[email protected]> wrote:
>
> We get lots of error messages when compiling arm_neon.h under
> e.g. -mcpu=cortex-m55, because Neon builtins are enabled only when
> !TARGET_HAVE_MVE. This has been the case since MVE support was
> introduced.
>
> This patch uses an approach similar to what we do on aarch64, but only
> partially since Neon intrinsics do not use the "new" framework.
>
> We register all types and Neon intrinsics, whether MVE is enabled or
> not, which enables to compile arm_neon.h. However, we need to
> introduce a "switcher" similar to aarch64's to avoid ICEs when LTO is
> enabled: in that case, since we have to enable the MVE intrinsics, we
> temporarily change arm_active_target.isa to enable MVE bits. This
> enables hooks like arm_vector_mode_supported_p and arm_array_mode to
> behave as expected by the MVE intrinsics framework. We switch patch
s/patch/back/ :-)
> to the previous arm_active_target.isa immediately after.
>
> There is no impact on the testsuite results, except that gcc.log is no
> longer full of errors messages when trying to compile arm_neon.h if
> MVE is forced somehow.
>
> gcc/ChangeLog:
>
> * config/arm/arm-builtins.cc (arm_init_simd_builtin_types): Remove
> TARGET_HAVE_MVE condition.
> (arm_init_mve_builtins): Remove calls to
> arm_init_simd_builtin_types and
> arm_init_simd_builtin_scalar_types. Switch to MVE isa flags.
> (arm_init_neon_builtins): Remove calls to
> arm_init_simd_builtin_types and
> arm_init_simd_builtin_scalar_types.
> (arm_target_switcher::arm_target_switcher): New.
> (arm_target_switcher::~arm_target_switcher): New.
> (arm_init_builtins): Call arm_init_simd_builtin_scalar_types and
> arm_init_simd_builtin_types. Always call arm_init_mve_builtins
> and arm_init_neon_builtins.
> * config/arm/arm-protos.h (class arm_target_switcher): New.
> ---
> gcc/config/arm/arm-builtins.cc | 131 ++++++++++++++++++++++-----------
> gcc/config/arm/arm-protos.h | 15 ++++
> 2 files changed, 101 insertions(+), 45 deletions(-)
>
> diff --git a/gcc/config/arm/arm-builtins.cc b/gcc/config/arm/arm-builtins.cc
> index 3bb2566f9a2..2e4f3595ed2 100644
> --- a/gcc/config/arm/arm-builtins.cc
> +++ b/gcc/config/arm/arm-builtins.cc
> @@ -48,6 +48,7 @@
> #include "basic-block.h"
> #include "gimple.h"
> #include "ssa.h"
> +#include "regs.h"
>
> #define SIMD_MAX_BUILTIN_ARGS 7
>
> @@ -1105,37 +1106,35 @@ arm_init_simd_builtin_types (void)
> an entry in our mangling table, consequently, they get default
> mangling. As a further gotcha, poly8_t and poly16_t are signed
> types, poly64_t and poly128_t are unsigned types. */
> - if (!TARGET_HAVE_MVE)
> - {
> - arm_simd_polyQI_type_node
> - = build_distinct_type_copy (intQI_type_node);
> - (*lang_hooks.types.register_builtin_type) (arm_simd_polyQI_type_node,
> - "__builtin_neon_poly8");
> - arm_simd_polyHI_type_node
> - = build_distinct_type_copy (intHI_type_node);
> - (*lang_hooks.types.register_builtin_type) (arm_simd_polyHI_type_node,
> - "__builtin_neon_poly16");
> - arm_simd_polyDI_type_node
> - = build_distinct_type_copy (unsigned_intDI_type_node);
> - (*lang_hooks.types.register_builtin_type) (arm_simd_polyDI_type_node,
> - "__builtin_neon_poly64");
> - arm_simd_polyTI_type_node
> - = build_distinct_type_copy (unsigned_intTI_type_node);
> - (*lang_hooks.types.register_builtin_type) (arm_simd_polyTI_type_node,
> - "__builtin_neon_poly128");
> - /* Init poly vector element types with scalar poly types. */
> - arm_simd_types[Poly8x8_t].eltype = arm_simd_polyQI_type_node;
> - arm_simd_types[Poly8x16_t].eltype = arm_simd_polyQI_type_node;
> - arm_simd_types[Poly16x4_t].eltype = arm_simd_polyHI_type_node;
> - arm_simd_types[Poly16x8_t].eltype = arm_simd_polyHI_type_node;
> - /* Note: poly64x2_t is defined in arm_neon.h, to ensure it gets default
> - mangling. */
> -
> - /* Prevent front-ends from transforming poly vectors into string
> - literals. */
> - TYPE_STRING_FLAG (arm_simd_polyQI_type_node) = false;
> - TYPE_STRING_FLAG (arm_simd_polyHI_type_node) = false;
> - }
> + arm_simd_polyQI_type_node
> + = build_distinct_type_copy (intQI_type_node);
> + (*lang_hooks.types.register_builtin_type) (arm_simd_polyQI_type_node,
> + "__builtin_neon_poly8");
> + arm_simd_polyHI_type_node
> + = build_distinct_type_copy (intHI_type_node);
> + (*lang_hooks.types.register_builtin_type) (arm_simd_polyHI_type_node,
> + "__builtin_neon_poly16");
> + arm_simd_polyDI_type_node
> + = build_distinct_type_copy (unsigned_intDI_type_node);
> + (*lang_hooks.types.register_builtin_type) (arm_simd_polyDI_type_node,
> + "__builtin_neon_poly64");
> + arm_simd_polyTI_type_node
> + = build_distinct_type_copy (unsigned_intTI_type_node);
> + (*lang_hooks.types.register_builtin_type) (arm_simd_polyTI_type_node,
> + "__builtin_neon_poly128");
> + /* Init poly vector element types with scalar poly types. */
> + arm_simd_types[Poly8x8_t].eltype = arm_simd_polyQI_type_node;
> + arm_simd_types[Poly8x16_t].eltype = arm_simd_polyQI_type_node;
> + arm_simd_types[Poly16x4_t].eltype = arm_simd_polyHI_type_node;
> + arm_simd_types[Poly16x8_t].eltype = arm_simd_polyHI_type_node;
> + /* Note: poly64x2_t is defined in arm_neon.h, to ensure it gets default
> + mangling. */
> +
> + /* Prevent front-ends from transforming poly vectors into string
> + literals. */
> + TYPE_STRING_FLAG (arm_simd_polyQI_type_node) = false;
> + TYPE_STRING_FLAG (arm_simd_polyHI_type_node) = false;
> +
> /* Init all the element types built by the front-end. */
> arm_simd_types[Int8x8_t].eltype = get_typenode_from_name (INT8_TYPE);
> arm_simd_types[Int8x16_t].eltype = get_typenode_from_name (INT8_TYPE);
> @@ -1451,8 +1450,8 @@ arm_init_mve_builtins (void)
> {
> volatile unsigned int i, fcode = ARM_BUILTIN_MVE_PATTERN_START;
>
> - arm_init_simd_builtin_scalar_types ();
> - arm_init_simd_builtin_types ();
> + enum isa_feature mve_flags[] = { ISA_MVE_FP, isa_nobit };
> + arm_target_switcher switcher (mve_flags);
>
> /* Add support for __builtin_{get,set}_fpscr_nzcvqc, used by MVE intrinsics
> that read and/or write the carry bit. */
> @@ -1496,14 +1495,6 @@ arm_init_neon_builtins (void)
> {
> unsigned int i, fcode = ARM_BUILTIN_NEON_PATTERN_START;
>
> - arm_init_simd_builtin_types ();
> -
> - /* Strong-typing hasn't been implemented for all AdvSIMD builtin
> intrinsics.
> - Therefore we need to preserve the old __builtin scalar types. It can be
> - removed once all the intrinsics become strongly typed using the
> qualifier
> - system. */
> - arm_init_simd_builtin_scalar_types ();
> -
> for (i = 0; i < ARRAY_SIZE (neon_builtin_data); i++, fcode++)
> {
> arm_builtin_datum *d = &neon_builtin_data[i];
> @@ -1690,6 +1681,50 @@ arm_init_fp16_builtins (void)
> "__fp16");
> }
>
> +/* Temporarily set FLAGS as the enabled target features. */
> +arm_target_switcher::arm_target_switcher (const enum isa_feature *flags)
> + : m_old_general_regs_only (TARGET_GENERAL_REGS_ONLY),
> + m_old_target_pragma (current_target_pragma)
> +{
> + m_old_arm_active_target_isa = sbitmap_alloc (isa_num_bits);
> + bitmap_copy (m_old_arm_active_target_isa, arm_active_target.isa);
> +
> + /* Changing the ISA flags and have_regs_of_mode should be enough here. We
> + shouldn't need to pay the compile-time cost of a full target switch. */
> + if (! TARGET_SOFT_FLOAT)
> + global_options.x_target_flags &= ~MASK_GENERAL_REGS_ONLY;
> +
> + arm_initialize_isa (arm_active_target.isa, flags);
> +
> + /* Target pragmas are irrelevant when defining intrinsics artificially. */
> + current_target_pragma = NULL_TREE;
> +
> + /* Ensure SIMD / VFP regs are available if Neon or MVE is enabled. */
> + memcpy (m_old_have_regs_of_mode, have_regs_of_mode, sizeof
> + (have_regs_of_mode));
> +
> + for (int i = 0; i < NUM_MACHINE_MODES; ++i)
> + if ((bitmap_bit_p (arm_active_target.isa, isa_bit_mve)
> + && (VALID_MVE_MODE ((machine_mode) i)
> + || VALID_MVE_STRUCT_MODE ((machine_mode) i)))
> + || (bitmap_bit_p (arm_active_target.isa, isa_bit_neon)
> + && (VALID_NEON_QREG_MODE ((machine_mode) i)
> + || VALID_NEON_DREG_MODE ((machine_mode) i))))
> + have_regs_of_mode[i] = true;
> +}
> +
> +arm_target_switcher::~arm_target_switcher ()
> +{
> + if (m_old_general_regs_only)
> + global_options.x_target_flags |= MASK_GENERAL_REGS_ONLY;
> + bitmap_copy (arm_active_target.isa, m_old_arm_active_target_isa);
> + sbitmap_free (m_old_arm_active_target_isa);
> + current_target_pragma = m_old_target_pragma;
> +
> + memcpy (have_regs_of_mode, m_old_have_regs_of_mode,
> + sizeof (have_regs_of_mode));
> +}
> +
> void
> arm_init_builtins (void)
> {
> @@ -1709,10 +1744,16 @@ arm_init_builtins (void)
> = arm_general_add_builtin_function ("__builtin_arm_lane_check",
> lane_check_fpr,
> ARM_BUILTIN_SIMD_LANE_CHECK);
> - if (TARGET_HAVE_MVE)
> - arm_init_mve_builtins ();
> - else
> - arm_init_neon_builtins ();
> +
> + /* Strong-typing hasn't been implemented for all AdvSIMD builtin
> + intrinsics. Therefore we need to preserve the old __builtin scalar
> + types. It can be removed once all the intrinsics become strongly
> + typed using the qualifier system. */
> + arm_init_simd_builtin_scalar_types ();
> + arm_init_simd_builtin_types ();
> + arm_init_neon_builtins ();
> + arm_init_mve_builtins ();
> +
> arm_init_vfp_builtins ();
> arm_init_crypto_builtins ();
> }
> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
> index ff7e7658f91..b95e16db363 100644
> --- a/gcc/config/arm/arm-protos.h
> +++ b/gcc/config/arm/arm-protos.h
> @@ -611,4 +611,19 @@ bool arm_mve_immediate_check(rtx, machine_mode, bool);
>
> opt_machine_mode arm_mve_data_mode (scalar_mode, poly_uint64);
>
> +/* RAII class for enabling enough features to define built-in types
> + and implement the arm_mve.h pragma. */
> +class arm_target_switcher
> +{
> +public:
> + arm_target_switcher (const enum isa_feature *flags);
> + ~arm_target_switcher ();
> +
> +private:
> + sbitmap m_old_arm_active_target_isa;
> + bool m_old_general_regs_only;
> + tree m_old_target_pragma;
> + bool m_old_have_regs_of_mode[MAX_MACHINE_MODE];
> +};
> +
> #endif /* ! GCC_ARM_PROTOS_H */
> --
> 2.34.1
>
