Compared to v2, this splits out the alignment switching into a new class and
merges the rest of the switching functionality into aarch64_target_switcher,
as agreed with Richard in the previous review discussion.
Bootstrapped and regression tested on aarch64. Is this ok for master?
---
Refactor the switcher classes into two separate classes:
- sve_alignment_switcher takes the alignment switching functionality,
and is used only for ABI correctness when defining sve structure
types.
- aarch64_target_switcher takes the rest of the functionality of
aarch64_simd_switcher and sve_switcher, and gates simd/sve specific
parts upon the specified feature flags.
Additionally, aarch64_target_switcher now adds dependencies of the
specified flags (which adds +fcma and +bf16 to some intrinsic
declarations), and unsets current_target_pragma.
This last change fixes an internal bug where we would sometimes add a
user specified target pragma (stored in current_target_pragma) on top of
an internally specified target architecture while initialising
intrinsics with `#pragma GCC aarch64 "arm_*.h"`. As far as I can tell, this
has no visible impact at the moment. However, the unintended target
feature combinations lead to unwanted behaviour in an under-development
patch.
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.cc
(struct aarch64_extension_info): Add field.
(aarch64_get_required_features): New.
* config/aarch64/aarch64-builtins.cc
(aarch64_simd_switcher::aarch64_simd_switcher): Rename to...
(aarch64_target_switcher::aarch64_target_switcher): ...this,
and extend to handle sve, nosimd and target pragmas.
(aarch64_simd_switcher::~aarch64_simd_switcher): Rename to...
(aarch64_target_switcher::~aarch64_target_switcher): ...this,
and extend to handle sve, nosimd and target pragmas.
(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 new members.
(aarch64_get_required_features): New prototype.
* config/aarch64/aarch64-sve-builtins.cc
(sve_switcher::sve_switcher): Delete
(sve_switcher::~sve_switcher): Delete
(sve_alignment_switcher::sve_alignment_switcher): New
(sve_alignment_switcher::~sve_alignment_switcher): New
(register_builtin_types): Use alignment switcher
(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): Delete.
(class sme_switcher): Delete.
(class sve_alignment_switcher): New.
diff --git a/gcc/common/config/aarch64/aarch64-common.cc
b/gcc/common/config/aarch64/aarch64-common.cc
index
ef4458fb69308d2bb6785e97be5be85226cf0ebb..500bf784983d851c54ea4ec59cf3cad29e5e309e
100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -157,6 +157,8 @@ struct aarch64_extension_info
aarch64_feature_flags flags_on;
/* If this feature is turned off, these bits also need to be turned off. */
aarch64_feature_flags flags_off;
+ /* If this feature remains enabled, these bits must also remain enabled. */
+ aarch64_feature_flags flags_required;
};
/* ISA extensions in AArch64. */
@@ -164,9 +166,10 @@ static constexpr aarch64_extension_info all_extensions[] =
{
#define AARCH64_OPT_EXTENSION(NAME, IDENT, C, D, E, FEATURE_STRING) \
{NAME, AARCH64_FL_##IDENT, feature_deps::IDENT ().explicit_on, \
- feature_deps::get_flags_off (feature_deps::root_off_##IDENT)},
+ feature_deps::get_flags_off (feature_deps::root_off_##IDENT), \
+ feature_deps::IDENT ().enable},
#include "config/aarch64/aarch64-option-extensions.def"
- {NULL, 0, 0, 0}
+ {NULL, 0, 0, 0, 0}
};
struct aarch64_arch_info
@@ -204,6 +207,18 @@ static constexpr aarch64_processor_info all_cores[] =
{NULL, aarch64_no_cpu, aarch64_no_arch, 0}
};
+/* Return the set of feature flags that are required to be enabled when the
+ features in FLAGS are enabled. */
+
+aarch64_feature_flags
+aarch64_get_required_features (aarch64_feature_flags flags)
+{
+ const struct aarch64_extension_info *opt;
+ for (opt = all_extensions; opt->name != NULL; opt++)
+ if (flags & opt->flag_canonical)
+ flags |= opt->flags_required;
+ return flags;
+}
/* Print a list of CANDIDATES for an argument, and try to suggest a specific
close match. */
diff --git a/gcc/config/aarch64/aarch64-builtins.cc
b/gcc/config/aarch64/aarch64-builtins.cc
index
128cc365d3d585e01cb69668f285318ee56a36fc..93f939a9c834c664fa8f081e6a484779071503eb
100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -43,6 +43,7 @@
#include "langhooks.h"
#include "gimple-iterator.h"
#include "case-cfn-macros.h"
+#include "regs.h"
#include "emit-rtl.h"
#include "stringpool.h"
#include "attribs.h"
@@ -1877,23 +1878,42 @@ 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);
+ /* Include all dependencies. */
+ flags = aarch64_get_required_features (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 switch. */
+ if (flags & AARCH64_FL_FP)
+ global_options.x_target_flags &= ~MASK_GENERAL_REGS_ONLY;
+ aarch64_set_asm_isa_flags (flags);
+
+ /* Target pragmas are irrelevant when defining intrinsics artificially. */
+ current_target_pragma = NULL_TREE;
+
+ /* Ensure SVE regs are available if SVE or SME is enabled. */
+ memcpy (m_old_have_regs_of_mode, have_regs_of_mode, sizeof
+ (have_regs_of_mode));
+ if (flags & (AARCH64_FL_SVE | AARCH64_FL_SME))
+ for (int i = 0; i < NUM_MACHINE_MODES; ++i)
+ if (aarch64_sve_mode_p ((machine_mode) i))
+ have_regs_of_mode[i] = true;
}
-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;
+
+ memcpy (have_regs_of_mode, m_old_have_regs_of_mode,
+ sizeof (have_regs_of_mode));
}
/* Implement #pragma GCC aarch64 "arm_neon.h".
@@ -1903,7 +1923,7 @@ aarch64_simd_switcher::~aarch64_simd_switcher ()
void
handle_arm_neon_h (void)
{
- aarch64_simd_switcher simd;
+ aarch64_target_switcher switcher (AARCH64_FL_SIMD);
/* Register the AdvSIMD vector tuple types. */
for (unsigned int i = 0; i < ARM_NEON_H_TYPES_LAST; i++)
@@ -2353,6 +2373,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 +2468,7 @@ aarch64_general_init_builtins (void)
aarch64_init_bf16_types ();
{
- aarch64_simd_switcher simd;
+ aarch64_target_switcher switcher (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..ecacbd307987c54575d546d6dd34c33dbf7e1c9b
100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -733,15 +733,17 @@ 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;
+ bool m_old_have_regs_of_mode[MAX_MACHINE_MODE];
};
/* Represents the ISA requirements of an intrinsic function, or of some
@@ -1190,6 +1192,7 @@ void aarch64_set_asm_isa_flags (aarch64_feature_flags);
void aarch64_set_asm_isa_flags (gcc_options *, aarch64_feature_flags);
bool aarch64_handle_option (struct gcc_options *, struct gcc_options *,
const struct cl_decoded_option *, location_t);
+aarch64_feature_flags aarch64_get_required_features (aarch64_feature_flags);
void aarch64_print_hint_for_extensions (const char *);
void aarch64_print_hint_for_arch (const char *);
void aarch64_print_hint_for_core (const char *);
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc
b/gcc/config/aarch64/aarch64-sve-builtins.cc
index
5d2062726d6bab31652bc9fa4bbd597704ef46e5..c62d0c2ea499f4c2eee3e70b41b6281ebd326a6d
100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -42,7 +42,6 @@
#include "emit-rtl.h"
#include "tree-vector-builder.h"
#include "stor-layout.h"
-#include "regs.h"
#include "alias.h"
#include "gimple-fold.h"
#include "langhooks.h"
@@ -1296,26 +1295,14 @@ 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_alignment_switcher::sve_alignment_switcher ()
{
- /* 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. */
m_old_maximum_field_alignment = maximum_field_alignment;
maximum_field_alignment = 0;
-
- 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 (aarch64_sve_mode_p ((machine_mode) i))
- have_regs_of_mode[i] = true;
}
-sve_switcher::~sve_switcher ()
+sve_alignment_switcher::~sve_alignment_switcher ()
{
- memcpy (have_regs_of_mode, m_old_have_regs_of_mode,
- sizeof (have_regs_of_mode));
maximum_field_alignment = m_old_maximum_field_alignment;
}
@@ -4652,6 +4639,8 @@ register_type_decl (tree type, const char *name)
static void
register_builtin_types ()
{
+ sve_alignment_switcher switcher;
+
#define DEF_SVE_TYPE(ACLE_NAME, NCHARS, ABI_NAME, SCALAR_TYPE) \
scalar_types[VECTOR_TYPE_ ## ACLE_NAME] = SCALAR_TYPE;
#include "aarch64-sve-builtins.def"
@@ -4726,7 +4715,7 @@ register_builtin_types ()
void
init_builtins ()
{
- sve_switcher sve;
+ aarch64_target_switcher switcher (AARCH64_FL_SVE);
register_builtin_types ();
if (in_lto_p)
{
@@ -4842,18 +4831,21 @@ handle_arm_sve_h (bool function_nulls_p)
return;
}
- sve_switcher sve;
+ aarch64_target_switcher switcher (AARCH64_FL_SVE);
- /* Define the vector and tuple types. */
- for (unsigned int type_i = 0; type_i < NUM_VECTOR_TYPES; ++type_i)
- {
- vector_type_index type = vector_type_index (type_i);
- register_vector_type (type);
- if (type != VECTOR_TYPE_svcount_t)
- for (unsigned int count = 2; count <= MAX_TUPLE_SIZE; ++count)
- if (type != VECTOR_TYPE_svbool_t || count == 2 || count == 4)
- register_tuple_type (count, type);
- }
+ {
+ /* Define the vector and tuple types. */
+ sve_alignment_switcher alignment_switcher;
+ for (unsigned int type_i = 0; type_i < NUM_VECTOR_TYPES; ++type_i)
+ {
+ vector_type_index type = vector_type_index (type_i);
+ register_vector_type (type);
+ if (type != VECTOR_TYPE_svcount_t)
+ for (unsigned int count = 2; count <= MAX_TUPLE_SIZE; ++count)
+ if (type != VECTOR_TYPE_svbool_t || count == 2 || count == 4)
+ register_tuple_type (count, type);
+ }
+ }
/* Define the enums. */
register_svpattern ();
@@ -4873,6 +4865,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 +4894,7 @@ handle_arm_sme_h (bool function_nulls_p)
return;
}
- sme_switcher sme;
+ aarch64_target_switcher switcher (AARCH64_FL_SME);
function_builder builder (arm_sme_handle, function_nulls_p);
for (unsigned int i = 0; i < ARRAY_SIZE (sme_function_groups); ++i)
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h
b/gcc/config/aarch64/aarch64-sve-builtins.h
index
54d213dfe6e0e1cd95e932fc4a04e9cd360f15f5..c145b8065ae3c32bc860b3d8def0380743537aa6
100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.h
+++ b/gcc/config/aarch64/aarch64-sve-builtins.h
@@ -822,24 +822,17 @@ public:
virtual bool check (function_checker &) const { return true; }
};
-/* 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
+/* RAII class for temporarily disabling the effect of any -fpack-struct option.
+ This is used to ensure that sve vector tuple types are defined with the
+ correct alignment. */
+class sve_alignment_switcher
{
public:
- sve_switcher (aarch64_feature_flags = 0);
- ~sve_switcher ();
+ sve_alignment_switcher ();
+ ~sve_alignment_switcher ();
private:
unsigned int m_old_maximum_field_alignment;
- bool m_old_have_regs_of_mode[MAX_MACHINE_MODE];
-};
-
-/* Extends sve_switch enough for defining arm_sme.h. */
-class sme_switcher : public sve_switcher
-{
-public:
- sme_switcher () : sve_switcher (AARCH64_FL_SME) {}
};
extern const type_suffix_info type_suffixes[NUM_TYPE_SUFFIXES + 1];
