aboutsummaryrefslogtreecommitdiff
path: root/gcc/config
diff options
context:
space:
mode:
authorRichard Sandiford <richard.sandiford@arm.com>2022-04-05 17:31:35 +0100
committerRichard Sandiford <richard.sandiford@arm.com>2022-04-05 17:31:35 +0100
commit14814e20161d7b6a4e9cac244c7013fa56f71f55 (patch)
tree318912f70236fbabbff538bebc1aa62955087ec8 /gcc/config
parente8bc70cc17b0105e2c463a98c9ff76039d03ff60 (diff)
downloadgcc-14814e20161d7b6a4e9cac244c7013fa56f71f55.zip
gcc-14814e20161d7b6a4e9cac244c7013fa56f71f55.tar.gz
gcc-14814e20161d7b6a4e9cac244c7013fa56f71f55.tar.bz2
aarch64: Fix -fpack-struct + <arm_neon.h> [PR103147]
This PR is about -fpack-struct causing a crash when <arm_neon.h> is included. The new register_tuple_type code was expecting a normal unpacked structure layout instead of a packed one. For SVE we got around this by temporarily suppressing -fpack-struct, so that the tuple types always have their normal ABI. However: (a) The SVE ACLE tuple types are defined to be abstract. The fact that GCC uses structures is an internal implementation detail. (b) In contrast, the ACLE explicitly defines the Advanced SIMD tuple types to be particular structures. (c) Clang and previous versions of GCC are consistent in applying -fpack-struct to these tuple structures. This patch therefore honours -fpack-struct and -fpack-struct=. It also adds tests for some other combinations, such as -mgeneral-regs-only and -fpack-struct -mstrict-align. gcc/ PR target/103147 * config/aarch64/aarch64-protos.h (aarch64_simd_switcher): New class. * config/aarch64/aarch64-sve-builtins.h (sve_switcher): Inherit from aarch64_simd_switcher. * config/aarch64/aarch64-builtins.cc (aarch64_simd_tuple_modes): New variable. (aarch64_lookup_simd_builtin_type): Use it instead of TYPE_MODE. (register_tuple_type): Add more asserts. Expect the alignment of the structure to be subject to flag_pack_struct and maximum_field_alignment. Set aarch64_simd_tuple_modes. (aarch64_simd_switcher::aarch64_simd_switcher): New function. (aarch64_simd_switcher::~aarch64_simd_switcher): Likewise. (handle_arm_neon_h): Hold an aarch64_simd_switcher throughout. (aarch64_general_init_builtins): Hold an aarch64_simd_switcher while calling aarch64_init_simd_builtins. * config/aarch64/aarch64-sve-builtins.cc (sve_switcher::sve_switcher) (sve_switcher::~sve_switcher): Remove code now performed by aarch64_simd_switcher. gcc/testsuite/ PR target/103147 * gcc.target/aarch64/pr103147-1.c: New test. * gcc.target/aarch64/pr103147-2.c: Likewise. * gcc.target/aarch64/pr103147-3.c: Likewise. * gcc.target/aarch64/pr103147-4.c: Likewise. * gcc.target/aarch64/pr103147-5.c: Likewise. * gcc.target/aarch64/pr103147-6.c: Likewise. * gcc.target/aarch64/pr103147-7.c: Likewise. * gcc.target/aarch64/pr103147-8.c: Likewise. * gcc.target/aarch64/pr103147-9.c: Likewise. * gcc.target/aarch64/pr103147-10.c: Likewise. * g++.target/aarch64/pr103147-1.C: Likewise. * g++.target/aarch64/pr103147-2.C: Likewise. * g++.target/aarch64/pr103147-3.C: Likewise. * g++.target/aarch64/pr103147-4.C: Likewise. * g++.target/aarch64/pr103147-5.C: Likewise. * g++.target/aarch64/pr103147-6.C: Likewise. * g++.target/aarch64/pr103147-7.C: Likewise. * g++.target/aarch64/pr103147-8.C: Likewise. * g++.target/aarch64/pr103147-9.C: Likewise. * g++.target/aarch64/pr103147-10.C: Likewise.
Diffstat (limited to 'gcc/config')
-rw-r--r--gcc/config/aarch64/aarch64-builtins.cc49
-rw-r--r--gcc/config/aarch64/aarch64-protos.h13
-rw-r--r--gcc/config/aarch64/aarch64-sve-builtins.cc11
-rw-r--r--gcc/config/aarch64/aarch64-sve-builtins.h4
4 files changed, 52 insertions, 25 deletions
diff --git a/gcc/config/aarch64/aarch64-builtins.cc b/gcc/config/aarch64/aarch64-builtins.cc
index 5217dbd..6ebeee7 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -716,6 +716,7 @@ static GTY(()) struct aarch64_simd_type_info aarch64_simd_types [] = {
};
#undef ENTRY
+static machine_mode aarch64_simd_tuple_modes[ARM_NEON_H_TYPES_LAST][3];
static GTY(()) tree aarch64_simd_tuple_types[ARM_NEON_H_TYPES_LAST][3];
static GTY(()) tree aarch64_simd_intOI_type_node = NULL_TREE;
@@ -844,7 +845,7 @@ aarch64_lookup_simd_builtin_type (machine_mode mode,
return aarch64_simd_types[i].itype;
if (aarch64_simd_tuple_types[i][0] != NULL_TREE)
for (int j = 0; j < 3; j++)
- if (TYPE_MODE (aarch64_simd_tuple_types[i][j]) == mode
+ if (aarch64_simd_tuple_modes[i][j] == mode
&& aarch64_simd_types[i].q == q)
return aarch64_simd_tuple_types[i][j];
}
@@ -1297,8 +1298,10 @@ register_tuple_type (unsigned int num_vectors, unsigned int type_index)
}
unsigned int alignment
- = (known_eq (GET_MODE_SIZE (type->mode), 16) ? 128 : 64);
- gcc_assert (TYPE_MODE_RAW (array_type) == TYPE_MODE (array_type)
+ = known_eq (GET_MODE_SIZE (type->mode), 16) ? 128 : 64;
+ machine_mode tuple_mode = TYPE_MODE_RAW (array_type);
+ gcc_assert (VECTOR_MODE_P (tuple_mode)
+ && TYPE_MODE (array_type) == tuple_mode
&& TYPE_ALIGN (array_type) == alignment);
tree field = build_decl (input_location, FIELD_DECL,
@@ -1309,14 +1312,13 @@ register_tuple_type (unsigned int num_vectors, unsigned int type_index)
make_array_slice (&field,
1));
gcc_assert (TYPE_MODE_RAW (t) == TYPE_MODE (t)
- && TYPE_ALIGN (t) == alignment);
-
- if (num_vectors == 2)
- aarch64_simd_tuple_types[type_index][0] = t;
- else if (num_vectors == 3)
- aarch64_simd_tuple_types[type_index][1] = t;
- else if (num_vectors == 4)
- aarch64_simd_tuple_types[type_index][2] = t;
+ && (flag_pack_struct
+ || maximum_field_alignment
+ || (TYPE_MODE_RAW (t) == tuple_mode
+ && TYPE_ALIGN (t) == alignment)));
+
+ aarch64_simd_tuple_modes[type_index][num_vectors - 2] = tuple_mode;
+ aarch64_simd_tuple_types[type_index][num_vectors - 2] = t;
}
static bool
@@ -1325,10 +1327,31 @@ 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 (unsigned int extra_flags)
+ : m_old_isa_flags (aarch64_isa_flags),
+ m_old_general_regs_only (TARGET_GENERAL_REGS_ONLY)
+{
+ /* Changing the ISA flags should be enough here. We shouldn't need to
+ pay the compile-time cost of a full target switch. */
+ aarch64_isa_flags = AARCH64_FL_FP | AARCH64_FL_SIMD | extra_flags;
+ global_options.x_target_flags &= ~MASK_GENERAL_REGS_ONLY;
+}
+
+aarch64_simd_switcher::~aarch64_simd_switcher ()
+{
+ if (m_old_general_regs_only)
+ global_options.x_target_flags |= MASK_GENERAL_REGS_ONLY;
+ aarch64_isa_flags = m_old_isa_flags;
+}
+
/* Implement #pragma GCC aarch64 "arm_neon.h". */
void
handle_arm_neon_h (void)
{
+ aarch64_simd_switcher simd;
+
/* Register the AdvSIMD vector tuple types. */
for (unsigned int i = 0; i < ARM_NEON_H_TYPES_LAST; i++)
for (unsigned int count = 2; count <= 4; ++count)
@@ -1703,8 +1726,10 @@ aarch64_general_init_builtins (void)
aarch64_init_bf16_types ();
- if (TARGET_SIMD)
+ {
+ aarch64_simd_switcher simd;
aarch64_init_simd_builtins ();
+ }
aarch64_init_crc32_builtins ();
aarch64_init_builtin_rsqrt ();
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 46bade2..c6f13ee 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -746,6 +746,19 @@ const unsigned int AARCH64_BUILTIN_SHIFT = 1;
/* Mask that selects the aarch64_builtin_class part of a function code. */
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
+{
+public:
+ aarch64_simd_switcher (unsigned int extra_flags = 0);
+ ~aarch64_simd_switcher ();
+
+private:
+ unsigned long m_old_isa_flags;
+ bool m_old_general_regs_only;
+};
+
void aarch64_post_cfi_startproc (void);
poly_int64 aarch64_initial_elimination_offset (unsigned, unsigned);
int aarch64_get_condition_code (rtx);
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index 5d1348a..9d78b27 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -871,20 +871,14 @@ registered_function_hasher::equal (value_type value, const compare_type &key)
}
sve_switcher::sve_switcher ()
- : m_old_isa_flags (aarch64_isa_flags)
+ : aarch64_simd_switcher (AARCH64_FL_F16 | AARCH64_FL_SVE)
{
/* 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. */
- aarch64_isa_flags = (AARCH64_FL_FP | AARCH64_FL_SIMD | AARCH64_FL_F16
- | AARCH64_FL_SVE);
-
m_old_maximum_field_alignment = maximum_field_alignment;
maximum_field_alignment = 0;
- m_old_general_regs_only = TARGET_GENERAL_REGS_ONLY;
- global_options.x_target_flags &= ~MASK_GENERAL_REGS_ONLY;
-
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)
@@ -896,9 +890,6 @@ sve_switcher::~sve_switcher ()
{
memcpy (have_regs_of_mode, m_old_have_regs_of_mode,
sizeof (have_regs_of_mode));
- if (m_old_general_regs_only)
- global_options.x_target_flags |= MASK_GENERAL_REGS_ONLY;
- aarch64_isa_flags = m_old_isa_flags;
maximum_field_alignment = m_old_maximum_field_alignment;
}
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h b/gcc/config/aarch64/aarch64-sve-builtins.h
index 48cae9a..24594d5 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.h
+++ b/gcc/config/aarch64/aarch64-sve-builtins.h
@@ -651,16 +651,14 @@ public:
/* RAII class for enabling enough SVE features to define the built-in
types and implement the arm_sve.h pragma. */
-class sve_switcher
+class sve_switcher : public aarch64_simd_switcher
{
public:
sve_switcher ();
~sve_switcher ();
private:
- unsigned long m_old_isa_flags;
unsigned int m_old_maximum_field_alignment;
- bool m_old_general_regs_only;
bool m_old_have_regs_of_mode[MAX_MACHINE_MODE];
};