[v2] arm: Fix MVE intrinsics support with LTO (PR target/110268)
Checks
Commit Message
After the recent MVE intrinsics re-implementation, LTO stopped working
because the intrinsics would no longer be defined.
The main part of the patch is simple and similar to what we do for
AArch64:
- call handle_arm_mve_h() from arm_init_mve_builtins to declare the
intrinsics when the compiler is in LTO mode
- actually implement arm_builtin_decl for MVE.
It was just a bit tricky to handle __ARM_MVE_PRESERVE_USER_NAMESPACE:
its value in the user code cannot be guessed at LTO time, so we always
have to assume that it was not defined. The led to a few fixes in the
way we register MVE builtins as placeholders or not. Without this
patch, we would just omit some versions of the inttrinsics when
__ARM_MVE_PRESERVE_USER_NAMESPACE is true. In fact, like for the C/C++
placeholders, we need to always keep entries for all of them to ensure
that we have a consistent numbering scheme.
2023-06-26 Christophe Lyon <christophe.lyon@linaro.org>
PR target/110268
gcc/
* config/arm/arm-builtins.cc (arm_init_mve_builtins): Handle LTO.
(arm_builtin_decl): Hahndle MVE builtins.
* config/arm/arm-mve-builtins.cc (builtin_decl): New function.
(add_unique_function): Fix handling of
__ARM_MVE_PRESERVE_USER_NAMESPACE.
(add_overloaded_function): Likewise.
* config/arm/arm-protos.h (builtin_decl): New declaration.
gcc/testsuite/
* gcc.target/arm/pr110268-1.c: New test.
* gcc.target/arm/pr110268-2.c: New test.
---
gcc/config/arm/arm-builtins.cc | 11 +++-
gcc/config/arm/arm-mve-builtins.cc | 61 ++++++++++++-----------
gcc/config/arm/arm-protos.h | 1 +
gcc/testsuite/gcc.target/arm/pr110268-1.c | 12 +++++
gcc/testsuite/gcc.target/arm/pr110268-2.c | 23 +++++++++
5 files changed, 78 insertions(+), 30 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/arm/pr110268-1.c
create mode 100644 gcc/testsuite/gcc.target/arm/pr110268-2.c
Comments
> -----Original Message-----
> From: Christophe Lyon <christophe.lyon@linaro.org>
> Sent: Monday, July 10, 2023 2:09 PM
> To: gcc-patches@gcc.gnu.org; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>;
> Richard Earnshaw <Richard.Earnshaw@arm.com>
> Cc: Christophe Lyon <christophe.lyon@linaro.org>
> Subject: [PATCH v2] arm: Fix MVE intrinsics support with LTO (PR
> target/110268)
>
> After the recent MVE intrinsics re-implementation, LTO stopped working
> because the intrinsics would no longer be defined.
>
> The main part of the patch is simple and similar to what we do for
> AArch64:
> - call handle_arm_mve_h() from arm_init_mve_builtins to declare the
> intrinsics when the compiler is in LTO mode
> - actually implement arm_builtin_decl for MVE.
>
> It was just a bit tricky to handle __ARM_MVE_PRESERVE_USER_NAMESPACE:
> its value in the user code cannot be guessed at LTO time, so we always
> have to assume that it was not defined. The led to a few fixes in the
> way we register MVE builtins as placeholders or not. Without this
> patch, we would just omit some versions of the inttrinsics when
> __ARM_MVE_PRESERVE_USER_NAMESPACE is true. In fact, like for the C/C++
> placeholders, we need to always keep entries for all of them to ensure
> that we have a consistent numbering scheme.
Ok.
Thanks,
Kyrill
>
> 2023-06-26 Christophe Lyon <christophe.lyon@linaro.org>
>
> PR target/110268
> gcc/
> * config/arm/arm-builtins.cc (arm_init_mve_builtins): Handle LTO.
> (arm_builtin_decl): Hahndle MVE builtins.
> * config/arm/arm-mve-builtins.cc (builtin_decl): New function.
> (add_unique_function): Fix handling of
> __ARM_MVE_PRESERVE_USER_NAMESPACE.
> (add_overloaded_function): Likewise.
> * config/arm/arm-protos.h (builtin_decl): New declaration.
>
> gcc/testsuite/
> * gcc.target/arm/pr110268-1.c: New test.
> * gcc.target/arm/pr110268-2.c: New test.
> ---
> gcc/config/arm/arm-builtins.cc | 11 +++-
> gcc/config/arm/arm-mve-builtins.cc | 61 ++++++++++++-----------
> gcc/config/arm/arm-protos.h | 1 +
> gcc/testsuite/gcc.target/arm/pr110268-1.c | 12 +++++
> gcc/testsuite/gcc.target/arm/pr110268-2.c | 23 +++++++++
> 5 files changed, 78 insertions(+), 30 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arm/pr110268-1.c
> create mode 100644 gcc/testsuite/gcc.target/arm/pr110268-2.c
>
> diff --git a/gcc/config/arm/arm-builtins.cc b/gcc/config/arm/arm-builtins.cc
> index 36365e40a5b..fca7dcaf565 100644
> --- a/gcc/config/arm/arm-builtins.cc
> +++ b/gcc/config/arm/arm-builtins.cc
> @@ -1918,6 +1918,15 @@ arm_init_mve_builtins (void)
> arm_builtin_datum *d = &mve_builtin_data[i];
> arm_init_builtin (fcode, d, "__builtin_mve");
> }
> +
> + if (in_lto_p)
> + {
> + arm_mve::handle_arm_mve_types_h ();
> + /* Under LTO, we cannot know whether
> + __ARM_MVE_PRESERVE_USER_NAMESPACE was defined, so assume
> it
> + was not. */
> + arm_mve::handle_arm_mve_h (false);
> + }
> }
>
> /* Set up all the NEON builtins, even builtins for instructions that are not
> @@ -2723,7 +2732,7 @@ arm_builtin_decl (unsigned code, bool initialize_p
> ATTRIBUTE_UNUSED)
> case ARM_BUILTIN_GENERAL:
> return arm_general_builtin_decl (subcode);
> case ARM_BUILTIN_MVE:
> - return error_mark_node;
> + return arm_mve::builtin_decl (subcode);
> default:
> gcc_unreachable ();
> }
> diff --git a/gcc/config/arm/arm-mve-builtins.cc b/gcc/config/arm/arm-mve-
> builtins.cc
> index 7033e41a571..413d8100607 100644
> --- a/gcc/config/arm/arm-mve-builtins.cc
> +++ b/gcc/config/arm/arm-mve-builtins.cc
> @@ -493,6 +493,16 @@ handle_arm_mve_h (bool
> preserve_user_namespace)
> preserve_user_namespace);
> }
>
> +/* Return the function decl with MVE function subcode CODE, or
> error_mark_node
> + if no such function exists. */
> +tree
> +builtin_decl (unsigned int code)
> +{
> + if (code >= vec_safe_length (registered_functions))
> + return error_mark_node;
> + return (*registered_functions)[code]->decl;
> +}
> +
> /* Return true if CANDIDATE is equivalent to MODEL_TYPE for overloading
> purposes. */
> static bool
> @@ -849,7 +859,6 @@ function_builder::add_function (const
> function_instance &instance,
> ? integer_zero_node
> : simulate_builtin_function_decl (input_location, name, fntype,
> code, NULL, attrs);
> -
> registered_function &rfn = *ggc_alloc <registered_function> ();
> rfn.instance = instance;
> rfn.decl = decl;
> @@ -889,15 +898,12 @@ function_builder::add_unique_function (const
> function_instance &instance,
> gcc_assert (!*rfn_slot);
> *rfn_slot = &rfn;
>
> - /* Also add the non-prefixed non-overloaded function, if the user
> namespace
> - does not need to be preserved. */
> - if (!preserve_user_namespace)
> - {
> - char *noprefix_name = get_name (instance, false, false);
> - tree attrs = get_attributes (instance);
> - add_function (instance, noprefix_name, fntype, attrs, requires_float,
> - false, false);
> - }
> + /* Also add the non-prefixed non-overloaded function, as placeholder
> + if the user namespace does not need to be preserved. */
> + char *noprefix_name = get_name (instance, false, false);
> + attrs = get_attributes (instance);
> + add_function (instance, noprefix_name, fntype, attrs, requires_float,
> + false, preserve_user_namespace);
>
> /* Also add the function under its overloaded alias, if we want
> a separate decl for each instance of an overloaded function. */
> @@ -905,20 +911,17 @@ function_builder::add_unique_function (const
> function_instance &instance,
> if (strcmp (name, overload_name) != 0)
> {
> /* Attribute lists shouldn't be shared. */
> - tree attrs = get_attributes (instance);
> + attrs = get_attributes (instance);
> bool placeholder_p = !(m_direct_overloads || force_direct_overloads);
> add_function (instance, overload_name, fntype, attrs,
> requires_float, false, placeholder_p);
>
> - /* Also add the non-prefixed overloaded function, if the user namespace
> - does not need to be preserved. */
> - if (!preserve_user_namespace)
> - {
> - char *noprefix_overload_name = get_name (instance, false, true);
> - tree attrs = get_attributes (instance);
> - add_function (instance, noprefix_overload_name, fntype, attrs,
> - requires_float, false, placeholder_p);
> - }
> + /* Also add the non-prefixed overloaded function, as placeholder
> + if the user namespace does not need to be preserved. */
> + char *noprefix_overload_name = get_name (instance, false, true);
> + attrs = get_attributes (instance);
> + add_function (instance, noprefix_overload_name, fntype, attrs,
> + requires_float, false, preserve_user_namespace ||
> placeholder_p);
> }
>
> obstack_free (&m_string_obstack, name);
> @@ -948,15 +951,15 @@ function_builder::add_overloaded_function (const
> function_instance &instance,
> = add_function (instance, name, m_overload_type, NULL_TREE,
> requires_float, true, m_direct_overloads);
> m_overload_names.put (name, &rfn);
> - if (!preserve_user_namespace)
> - {
> - char *noprefix_name = get_name (instance, false, true);
> - registered_function &noprefix_rfn
> - = add_function (instance, noprefix_name, m_overload_type,
> - NULL_TREE, requires_float, true,
> - m_direct_overloads);
> - m_overload_names.put (noprefix_name, &noprefix_rfn);
> - }
> +
> + /* Also add the non-prefixed function, as placeholder if the
> + user namespace does not need to be preserved. */
> + char *noprefix_name = get_name (instance, false, true);
> + registered_function &noprefix_rfn
> + = add_function (instance, noprefix_name, m_overload_type,
> + NULL_TREE, requires_float, true,
> + preserve_user_namespace || m_direct_overloads);
> + m_overload_names.put (noprefix_name, &noprefix_rfn);
> }
> }
>
> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
> index 7d73c66a15d..6186921011e 100644
> --- a/gcc/config/arm/arm-protos.h
> +++ b/gcc/config/arm/arm-protos.h
> @@ -232,6 +232,7 @@ const unsigned int ARM_BUILTIN_CLASS = (1 <<
> ARM_BUILTIN_SHIFT) - 1;
> namespace arm_mve {
> void handle_arm_mve_types_h ();
> void handle_arm_mve_h (bool);
> + tree builtin_decl (unsigned);
> tree resolve_overloaded_builtin (location_t, unsigned int,
> vec<tree, va_gc> *);
> bool check_builtin_call (location_t, vec<location_t>, unsigned int,
> diff --git a/gcc/testsuite/gcc.target/arm/pr110268-1.c
> b/gcc/testsuite/gcc.target/arm/pr110268-1.c
> new file mode 100644
> index 00000000000..1243e4b14b6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr110268-1.c
> @@ -0,0 +1,12 @@
> +/* { dg-do link } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_link } */ /* Make
> sure we have suitable multilibs to link successfully. */
> +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> +/* { dg-add-options arm_v8_1m_mve } */
> +/* { dg-additional-options "-O2 -flto" } */
> +
> +#include <arm_mve.h>
> +
> +int main(int argc, char* argv[])
> +{
> + return vaddvq(__arm_vdupq_n_s8 (argc));
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/pr110268-2.c
> b/gcc/testsuite/gcc.target/arm/pr110268-2.c
> new file mode 100644
> index 00000000000..823e708f8fd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr110268-2.c
> @@ -0,0 +1,23 @@
> +/* { dg-do link } */
> +/* { dg-require-effective-target arm_arch_v8_1m_main_link } */ /* Make
> sure we have suitable multilibs to link successfully. */
> +/* { dg-require-effective-target arm_v8_1m_mve_ok } */
> +/* { dg-add-options arm_v8_1m_mve } */
> +/* { dg-additional-options "-O2 -flto" } */
> +
> +/* Check MVE intrinsics with LTO with
> __ARM_MVE_PRESERVE_USER_NAMESPACE and a
> + user-overridden intrinsic. */
> +
> +#define __ARM_MVE_PRESERVE_USER_NAMESPACE
> +#include <arm_mve.h>
> +
> +int global_int;
> +int32_t vaddvq(int8x16_t x)
> +{
> + return global_int + __arm_vgetq_lane_s8 (x, 0);
> +}
> +
> +int main(int argc, char* argv[])
> +{
> + global_int = argc;
> + return vaddvq(__arm_vdupq_n_s8 (argc));
> +}
> --
> 2.34.1
@@ -1918,6 +1918,15 @@ arm_init_mve_builtins (void)
arm_builtin_datum *d = &mve_builtin_data[i];
arm_init_builtin (fcode, d, "__builtin_mve");
}
+
+ if (in_lto_p)
+ {
+ arm_mve::handle_arm_mve_types_h ();
+ /* Under LTO, we cannot know whether
+ __ARM_MVE_PRESERVE_USER_NAMESPACE was defined, so assume it
+ was not. */
+ arm_mve::handle_arm_mve_h (false);
+ }
}
/* Set up all the NEON builtins, even builtins for instructions that are not
@@ -2723,7 +2732,7 @@ arm_builtin_decl (unsigned code, bool initialize_p ATTRIBUTE_UNUSED)
case ARM_BUILTIN_GENERAL:
return arm_general_builtin_decl (subcode);
case ARM_BUILTIN_MVE:
- return error_mark_node;
+ return arm_mve::builtin_decl (subcode);
default:
gcc_unreachable ();
}
@@ -493,6 +493,16 @@ handle_arm_mve_h (bool preserve_user_namespace)
preserve_user_namespace);
}
+/* Return the function decl with MVE function subcode CODE, or error_mark_node
+ if no such function exists. */
+tree
+builtin_decl (unsigned int code)
+{
+ if (code >= vec_safe_length (registered_functions))
+ return error_mark_node;
+ return (*registered_functions)[code]->decl;
+}
+
/* Return true if CANDIDATE is equivalent to MODEL_TYPE for overloading
purposes. */
static bool
@@ -849,7 +859,6 @@ function_builder::add_function (const function_instance &instance,
? integer_zero_node
: simulate_builtin_function_decl (input_location, name, fntype,
code, NULL, attrs);
-
registered_function &rfn = *ggc_alloc <registered_function> ();
rfn.instance = instance;
rfn.decl = decl;
@@ -889,15 +898,12 @@ function_builder::add_unique_function (const function_instance &instance,
gcc_assert (!*rfn_slot);
*rfn_slot = &rfn;
- /* Also add the non-prefixed non-overloaded function, if the user namespace
- does not need to be preserved. */
- if (!preserve_user_namespace)
- {
- char *noprefix_name = get_name (instance, false, false);
- tree attrs = get_attributes (instance);
- add_function (instance, noprefix_name, fntype, attrs, requires_float,
- false, false);
- }
+ /* Also add the non-prefixed non-overloaded function, as placeholder
+ if the user namespace does not need to be preserved. */
+ char *noprefix_name = get_name (instance, false, false);
+ attrs = get_attributes (instance);
+ add_function (instance, noprefix_name, fntype, attrs, requires_float,
+ false, preserve_user_namespace);
/* Also add the function under its overloaded alias, if we want
a separate decl for each instance of an overloaded function. */
@@ -905,20 +911,17 @@ function_builder::add_unique_function (const function_instance &instance,
if (strcmp (name, overload_name) != 0)
{
/* Attribute lists shouldn't be shared. */
- tree attrs = get_attributes (instance);
+ attrs = get_attributes (instance);
bool placeholder_p = !(m_direct_overloads || force_direct_overloads);
add_function (instance, overload_name, fntype, attrs,
requires_float, false, placeholder_p);
- /* Also add the non-prefixed overloaded function, if the user namespace
- does not need to be preserved. */
- if (!preserve_user_namespace)
- {
- char *noprefix_overload_name = get_name (instance, false, true);
- tree attrs = get_attributes (instance);
- add_function (instance, noprefix_overload_name, fntype, attrs,
- requires_float, false, placeholder_p);
- }
+ /* Also add the non-prefixed overloaded function, as placeholder
+ if the user namespace does not need to be preserved. */
+ char *noprefix_overload_name = get_name (instance, false, true);
+ attrs = get_attributes (instance);
+ add_function (instance, noprefix_overload_name, fntype, attrs,
+ requires_float, false, preserve_user_namespace || placeholder_p);
}
obstack_free (&m_string_obstack, name);
@@ -948,15 +951,15 @@ function_builder::add_overloaded_function (const function_instance &instance,
= add_function (instance, name, m_overload_type, NULL_TREE,
requires_float, true, m_direct_overloads);
m_overload_names.put (name, &rfn);
- if (!preserve_user_namespace)
- {
- char *noprefix_name = get_name (instance, false, true);
- registered_function &noprefix_rfn
- = add_function (instance, noprefix_name, m_overload_type,
- NULL_TREE, requires_float, true,
- m_direct_overloads);
- m_overload_names.put (noprefix_name, &noprefix_rfn);
- }
+
+ /* Also add the non-prefixed function, as placeholder if the
+ user namespace does not need to be preserved. */
+ char *noprefix_name = get_name (instance, false, true);
+ registered_function &noprefix_rfn
+ = add_function (instance, noprefix_name, m_overload_type,
+ NULL_TREE, requires_float, true,
+ preserve_user_namespace || m_direct_overloads);
+ m_overload_names.put (noprefix_name, &noprefix_rfn);
}
}
@@ -232,6 +232,7 @@ const unsigned int ARM_BUILTIN_CLASS = (1 << ARM_BUILTIN_SHIFT) - 1;
namespace arm_mve {
void handle_arm_mve_types_h ();
void handle_arm_mve_h (bool);
+ tree builtin_decl (unsigned);
tree resolve_overloaded_builtin (location_t, unsigned int,
vec<tree, va_gc> *);
bool check_builtin_call (location_t, vec<location_t>, unsigned int,
new file mode 100644
@@ -0,0 +1,12 @@
+/* { dg-do link } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_link } */ /* Make sure we have suitable multilibs to link successfully. */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -flto" } */
+
+#include <arm_mve.h>
+
+int main(int argc, char* argv[])
+{
+ return vaddvq(__arm_vdupq_n_s8 (argc));
+}
new file mode 100644
@@ -0,0 +1,23 @@
+/* { dg-do link } */
+/* { dg-require-effective-target arm_arch_v8_1m_main_link } */ /* Make sure we have suitable multilibs to link successfully. */
+/* { dg-require-effective-target arm_v8_1m_mve_ok } */
+/* { dg-add-options arm_v8_1m_mve } */
+/* { dg-additional-options "-O2 -flto" } */
+
+/* Check MVE intrinsics with LTO with __ARM_MVE_PRESERVE_USER_NAMESPACE and a
+ user-overridden intrinsic. */
+
+#define __ARM_MVE_PRESERVE_USER_NAMESPACE
+#include <arm_mve.h>
+
+int global_int;
+int32_t vaddvq(int8x16_t x)
+{
+ return global_int + __arm_vgetq_lane_s8 (x, 0);
+}
+
+int main(int argc, char* argv[])
+{
+ global_int = argc;
+ return vaddvq(__arm_vdupq_n_s8 (argc));
+}