From e4005cf8717abe8c949f840c707e02e6c394c2e7 Mon Sep 17 00:00:00 2001 From: Alex Coplan Date: Mon, 29 Mar 2021 12:18:19 +0100 Subject: [PATCH] aarch64: Fix SVE ACLE builtins with LTO [PR99216] As discussed in the PR, we currently have two different numbering schemes for SVE builtins: one for C, and one for C++. This is problematic for LTO, where we end up getting confused about which intrinsic we're talking about. This patch inserts placeholders into the registered_functions vector to ensure that there is a consistent numbering scheme for both C and C++. We use integer_zero_node as a placeholder node instead of building a function decl. This is safe because the node is only returned by the TARGET_BUILTIN_DECL hook, which (on AArch64) is only used for validation when builtin decls are streamed into lto1. gcc/ChangeLog: PR target/99216 * config/aarch64/aarch64-sve-builtins.cc (function_builder::add_function): Add placeholder_p argument, use placeholder decls if this is set. (function_builder::add_unique_function): Instead of conditionally adding direct overloads, unconditionally add either a direct overload or a placeholder. (function_builder::add_overloaded_function): Set placeholder_p if we're using C++ overloads. Use the obstack for string storage instead of relying on the tree nodes. (function_builder::add_overloaded_functions): Don't return early for m_direct_overloads: we need to add placeholders. * config/aarch64/aarch64-sve-builtins.h (function_builder::add_function): Add placeholder_p argument. gcc/testsuite/ChangeLog: PR target/99216 * g++.target/aarch64/sve/pr99216.C: New test. --- gcc/config/aarch64/aarch64-sve-builtins.cc | 59 +++++++++++-------- gcc/config/aarch64/aarch64-sve-builtins.h | 3 +- .../g++.target/aarch64/sve/pr99216.C | 5 ++ 3 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 gcc/testsuite/g++.target/aarch64/sve/pr99216.C diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc index 25612d2ea52..f44f81f1375 100644 --- a/gcc/config/aarch64/aarch64-sve-builtins.cc +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc @@ -999,12 +999,29 @@ registered_function & function_builder::add_function (const function_instance &instance, const char *name, tree fntype, tree attrs, uint64_t required_extensions, - bool overloaded_p) + bool overloaded_p, + bool placeholder_p) { unsigned int code = vec_safe_length (registered_functions); code = (code << AARCH64_BUILTIN_SHIFT) | AARCH64_BUILTIN_SVE; - tree decl = simulate_builtin_function_decl (input_location, name, fntype, - code, NULL, attrs); + + /* We need to be able to generate placeholders to enusre that we have a + consistent numbering scheme for function codes between the C and C++ + frontends, so that everything ties up in LTO. + + Currently, tree-streamer-in.c:unpack_ts_function_decl_value_fields + validates that tree nodes returned by TARGET_BUILTIN_DECL are non-NULL and + some node other than error_mark_node. This is a holdover from when builtin + decls were streamed by code rather than by value. + + Ultimately, we should be able to remove this validation of BUILT_IN_MD + nodes and remove the target hook. For now, however, we need to appease the + validation and return a non-NULL, non-error_mark_node node, so we + arbitrarily choose integer_zero_node. */ + tree decl = placeholder_p + ? integer_zero_node + : simulate_builtin_function_decl (input_location, name, fntype, + code, NULL, attrs); registered_function &rfn = *ggc_alloc (); rfn.instance = instance; @@ -1036,7 +1053,7 @@ function_builder::add_unique_function (const function_instance &instance, argument_types.address ()); tree attrs = get_attributes (instance); registered_function &rfn = add_function (instance, name, fntype, attrs, - required_extensions, false); + required_extensions, false, false); /* Enter the function into the hash table. */ hashval_t hash = instance.hash (); @@ -1047,16 +1064,14 @@ function_builder::add_unique_function (const function_instance &instance, /* Also add the function under its overloaded alias, if we want a separate decl for each instance of an overloaded function. */ - if (m_direct_overloads || force_direct_overloads) + char *overload_name = get_name (instance, true); + if (strcmp (name, overload_name) != 0) { - char *overload_name = get_name (instance, true); - if (strcmp (name, overload_name) != 0) - { - /* Attribute lists shouldn't be shared. */ - tree attrs = get_attributes (instance); - add_function (instance, overload_name, fntype, attrs, - required_extensions, false); - } + /* Attribute lists shouldn't be shared. */ + tree attrs = get_attributes (instance); + bool placeholder_p = !(m_direct_overloads || force_direct_overloads); + add_function (instance, overload_name, fntype, attrs, + required_extensions, false, placeholder_p); } obstack_free (&m_string_obstack, name); @@ -1077,18 +1092,19 @@ function_builder::add_overloaded_function (const function_instance &instance, { char *name = get_name (instance, true); if (registered_function **map_value = m_overload_names.get (name)) - gcc_assert ((*map_value)->instance == instance - && ((*map_value)->required_extensions - & ~required_extensions) == 0); + { + gcc_assert ((*map_value)->instance == instance + && ((*map_value)->required_extensions + & ~required_extensions) == 0); + obstack_free (&m_string_obstack, name); + } else { registered_function &rfn = add_function (instance, name, m_overload_type, NULL_TREE, - required_extensions, true); - const char *permanent_name = IDENTIFIER_POINTER (DECL_NAME (rfn.decl)); - m_overload_names.put (permanent_name, &rfn); + required_extensions, true, m_direct_overloads); + m_overload_names.put (name, &rfn); } - obstack_free (&m_string_obstack, name); } /* If we are using manual overload resolution, add one function decl @@ -1098,9 +1114,6 @@ void function_builder::add_overloaded_functions (const function_group_info &group, mode_suffix_index mode) { - if (m_direct_overloads) - return; - unsigned int explicit_type0 = (*group.shape)->explicit_type_suffix_p (0); unsigned int explicit_type1 = (*group.shape)->explicit_type_suffix_p (1); for (unsigned int pi = 0; group.preds[pi] != NUM_PREDS; ++pi) diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h b/gcc/config/aarch64/aarch64-sve-builtins.h index 620e18841b2..b701f90ac1a 100644 --- a/gcc/config/aarch64/aarch64-sve-builtins.h +++ b/gcc/config/aarch64/aarch64-sve-builtins.h @@ -337,7 +337,8 @@ private: tree get_attributes (const function_instance &); registered_function &add_function (const function_instance &, - const char *, tree, tree, uint64_t, bool); + const char *, tree, tree, + uint64_t, bool, bool); /* The function type to use for functions that are resolved by function_resolver. */ diff --git a/gcc/testsuite/g++.target/aarch64/sve/pr99216.C b/gcc/testsuite/g++.target/aarch64/sve/pr99216.C new file mode 100644 index 00000000000..61a58a7fcf4 --- /dev/null +++ b/gcc/testsuite/g++.target/aarch64/sve/pr99216.C @@ -0,0 +1,5 @@ +/* { dg-do link { target aarch64_asm_sve_ok } } */ +/* { dg-additional-options "-flto" } */ +#include +bool a; +int main() { a = svaddv(svptrue_b8(), svdup_s8(0)); }