When initialising intrinsics with `#pragma GCC aarch64 "arm_*.h"`, we often set an explicit target, but currently leave current_target_pragma unchanged. This results in the target pragma being applied to each simulated intrinsic on top of our explicit target, which is clearly undesirable.
As far as I can tell this doesn't cause any bugs at the moment, because none of the behaviour for builtin functions depends upon the function specific target. However, the unintended target feature combinations led to unwanted behaviour in an under-developement patch. This patch resolves the issue by extending aarch64_simd_switcher to explicitly unset the current_target_pragma, and adapting it for to support handle_arm_acle_h as well. I've also renamed the switcher classes and instances, because I think the new names a slightly clearer. The chosen sets of features for arm_sve.h and arm_sme.h are not normally valid, because they exclude FCMA and BF16. However, I don't think that matters for the usage here. Alternatively, aarch64_target_switcher could be modified to enable all the dependent features as well. Bootstrapped and regression tested on aarch64. Ok for master (to enable the dependant WIP patch)? gcc/ChangeLog: * config/aarch64/aarch64-builtins.cc (aarch64_simd_switcher::aarch64_simd_switcher): Rename to... (aarch64_target_switcher::aarch64_target_switcher): ...this, remove default simd flags and save current_target_pragma. (aarch64_simd_switcher::~aarch64_simd_switcher): Rename to... (aarch64_target_switcher::~aarch64_target_switcher): ...this, and restore current_target_pragma. (handle_arm_acle_h): Use aarch64_target_switcher. (handle_arm_neon_h): Rename switcher and pass explicit flags. (aarch64_general_init_builtins): Ditto. * config/aarch64/aarch64-protos.h (class aarch64_simd_switcher): Rename to... (class aarch64_target_switcher): ...this, and add pragma member. * config/aarch64/aarch64-sve-builtins.cc (sve_switcher::sve_switcher): Rename to... (sve_target_switcher::sve_target_switcher): ...this. (sve_switcher::~sve_switcher): Rename to... (sve_target_switcher::~sve_target_switcher): ...this. (init_builtins): Rename switcher. (handle_arm_sve_h): Ditto. (handle_arm_neon_sve_bridge_h): Ditto. (handle_arm_sme_h): Ditto. * config/aarch64/aarch64-sve-builtins.h (class sve_switcher): Rename to... (class sve_target_switcher): ...this. (class sme_switcher): Rename to... (class sme_target_switcher): ...this. diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc index 128cc365d3d585e01cb69668f285318ee56a36fc..c1cb6cdcc81c6b45c0132250589bba0be42f195d 100644 --- a/gcc/config/aarch64/aarch64-builtins.cc +++ b/gcc/config/aarch64/aarch64-builtins.cc @@ -1877,23 +1877,25 @@ aarch64_scalar_builtin_type_p (aarch64_simd_type t) return (t == Poly8_t || t == Poly16_t || t == Poly64_t || t == Poly128_t); } -/* Enable AARCH64_FL_* flags EXTRA_FLAGS on top of the base Advanced SIMD - set. */ -aarch64_simd_switcher::aarch64_simd_switcher (aarch64_feature_flags extra_flags) +/* Temporarily set FLAGS as the enabled target features. */ +aarch64_target_switcher::aarch64_target_switcher (aarch64_feature_flags flags) : m_old_asm_isa_flags (aarch64_asm_isa_flags), - m_old_general_regs_only (TARGET_GENERAL_REGS_ONLY) + m_old_general_regs_only (TARGET_GENERAL_REGS_ONLY), + m_old_target_pragma (current_target_pragma) { /* Changing the ISA flags should be enough here. We shouldn't need to pay the compile-time cost of a full target switch. */ global_options.x_target_flags &= ~MASK_GENERAL_REGS_ONLY; - aarch64_set_asm_isa_flags (AARCH64_FL_FP | AARCH64_FL_SIMD | extra_flags); + aarch64_set_asm_isa_flags (flags); + current_target_pragma = NULL_TREE; } -aarch64_simd_switcher::~aarch64_simd_switcher () +aarch64_target_switcher::~aarch64_target_switcher () { if (m_old_general_regs_only) global_options.x_target_flags |= MASK_GENERAL_REGS_ONLY; aarch64_set_asm_isa_flags (m_old_asm_isa_flags); + current_target_pragma = m_old_target_pragma; } /* Implement #pragma GCC aarch64 "arm_neon.h". @@ -1903,7 +1905,7 @@ aarch64_simd_switcher::~aarch64_simd_switcher () void handle_arm_neon_h (void) { - aarch64_simd_switcher simd; + aarch64_target_switcher switcher (AARCH64_FL_FP | AARCH64_FL_SIMD); /* Register the AdvSIMD vector tuple types. */ for (unsigned int i = 0; i < ARM_NEON_H_TYPES_LAST; i++) @@ -2353,6 +2355,8 @@ aarch64_init_data_intrinsics (void) void handle_arm_acle_h (void) { + aarch64_target_switcher switcher; + aarch64_init_ls64_builtins (); aarch64_init_tme_builtins (); aarch64_init_memtag_builtins (); @@ -2446,7 +2450,7 @@ aarch64_general_init_builtins (void) aarch64_init_bf16_types (); { - aarch64_simd_switcher simd; + aarch64_target_switcher switcher (AARCH64_FL_FP | AARCH64_FL_SIMD); aarch64_init_simd_builtins (); } diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index 4235f4a0ca51af49c2852a420f1056727b24f345..cbae258a5bbb3feee80f14add6567f2440cc6927 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -733,15 +733,16 @@ const unsigned int AARCH64_BUILTIN_CLASS = (1 << AARCH64_BUILTIN_SHIFT) - 1; /* RAII class for enabling enough features to define built-in types and implement the arm_neon.h pragma. */ -class aarch64_simd_switcher +class aarch64_target_switcher { public: - aarch64_simd_switcher (aarch64_feature_flags extra_flags = 0); - ~aarch64_simd_switcher (); + aarch64_target_switcher (aarch64_feature_flags flags = 0); + ~aarch64_target_switcher (); private: aarch64_feature_flags m_old_asm_isa_flags; bool m_old_general_regs_only; + tree m_old_target_pragma; }; /* Represents the ISA requirements of an intrinsic function, or of some diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h b/gcc/config/aarch64/aarch64-sve-builtins.h index 54d213dfe6e0e1cd95e932fc4a04e9cd360f15f5..ea19cfe47bec042e0fb0b4f3c3820b2d37bb222f 100644 --- a/gcc/config/aarch64/aarch64-sve-builtins.h +++ b/gcc/config/aarch64/aarch64-sve-builtins.h @@ -824,11 +824,11 @@ public: /* RAII class for enabling enough SVE features to define the built-in types and implement the arm_sve.h pragma. */ -class sve_switcher : public aarch64_simd_switcher +class sve_target_switcher : public aarch64_target_switcher { public: - sve_switcher (aarch64_feature_flags = 0); - ~sve_switcher (); + sve_target_switcher (aarch64_feature_flags = 0); + ~sve_target_switcher (); private: unsigned int m_old_maximum_field_alignment; @@ -836,10 +836,10 @@ private: }; /* Extends sve_switch enough for defining arm_sme.h. */ -class sme_switcher : public sve_switcher +class sme_target_switcher : public sve_target_switcher { public: - sme_switcher () : sve_switcher (AARCH64_FL_SME) {} + sme_target_switcher () : sve_target_switcher (AARCH64_FL_SME) {} }; extern const type_suffix_info type_suffixes[NUM_TYPE_SUFFIXES + 1]; diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc index 5d2062726d6bab31652bc9fa4bbd597704ef46e5..0e3887db74a64326d036f0ef70a35f31dcb971b0 100644 --- a/gcc/config/aarch64/aarch64-sve-builtins.cc +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc @@ -1296,8 +1296,12 @@ registered_function_hasher::equal (value_type value, const compare_type &key) return value->instance == key; } -sve_switcher::sve_switcher (aarch64_feature_flags flags) - : aarch64_simd_switcher (AARCH64_FL_F16 | AARCH64_FL_SVE | flags) +sve_target_switcher::sve_target_switcher (aarch64_feature_flags flags) + : aarch64_target_switcher (AARCH64_FL_FP + | AARCH64_FL_SIMD + | AARCH64_FL_F16 + | AARCH64_FL_SVE + | flags) { /* 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 @@ -1312,7 +1316,7 @@ sve_switcher::sve_switcher (aarch64_feature_flags flags) have_regs_of_mode[i] = true; } -sve_switcher::~sve_switcher () +sve_target_switcher::~sve_target_switcher () { memcpy (have_regs_of_mode, m_old_have_regs_of_mode, sizeof (have_regs_of_mode)); @@ -4726,7 +4730,7 @@ register_builtin_types () void init_builtins () { - sve_switcher sve; + sve_target_switcher switcher; register_builtin_types (); if (in_lto_p) { @@ -4842,7 +4846,7 @@ handle_arm_sve_h (bool function_nulls_p) return; } - sve_switcher sve; + sve_target_switcher switcher; /* Define the vector and tuple types. */ for (unsigned int type_i = 0; type_i < NUM_VECTOR_TYPES; ++type_i) @@ -4873,6 +4877,8 @@ handle_arm_neon_sve_bridge_h (bool function_nulls_p) if (initial_indexes[arm_sme_handle] == 0) handle_arm_sme_h (true); + aarch64_target_switcher switcher; + /* Define the functions. */ function_builder builder (arm_neon_sve_handle, function_nulls_p); for (unsigned int i = 0; i < ARRAY_SIZE (neon_sve_function_groups); ++i) @@ -4900,7 +4906,7 @@ handle_arm_sme_h (bool function_nulls_p) return; } - sme_switcher sme; + sme_target_switcher switcher; function_builder builder (arm_sme_handle, function_nulls_p); for (unsigned int i = 0; i < ARRAY_SIZE (sme_function_groups); ++i)