Richard Ball <richard.b...@arm.com> writes:
> When using LTO, handling the pragma for sme before the pragma
> for the neon-sve-bridge caused the following error on svset_neonq,
> in the neon-sve-bridge.c test.
>
> error: ACLE function '0' can only be called when SME streaming mode is
> enabled.
>
> This has been resolved by changing the pragma handlers to accept two modes.
> One where they add functions normally and a second in which
> registered_functions
> is filled with a placeholder value.
>
> By using this, the ordering of the functions can be maintained.
>
> No regressions on aarch64-none-elf.
OK, thanks.
Richard
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-c.cc (aarch64_pragma_aarch64):
> Add functions_nulls parameter to pragma_handlers.
> * config/aarch64/aarch64-protos.h: Likewise.
> * config/aarch64/aarch64-sve-builtins.h
> (enum handle_pragma_index): Add enum to count
> number of pragmas to be handled.
> * config/aarch64/aarch64-sve-builtins.cc
> (GTY): Add global variable for initial indexes
> and change overload_names to an array.
> (function_builder::function_builder):
> Add pragma handler information.
> (function_builder::add_function):
> Add code for overwriting previous
> registered_functions entries.
> (add_unique_function):
> Use an array to register overload_names
> for both pragma handler modes.
> (add_overloaded_function): Likewise.
> (init_builtins):
> Add functions_nulls parameter to pragma_handlers.
> (handle_arm_sve_h):
> Initialize pragma handler information.
> (handle_arm_neon_sve_bridge_h): Likewise.
> (handle_arm_sme_h): Likewise.
>
> diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc
> index
> b5a6917d06db41a845681ed6f13f2800c70ede10..fe1a20e4e546a68e5f7eddff3bbb0d3e831fbd9b
> 100644
> --- a/gcc/config/aarch64/aarch64-c.cc
> +++ b/gcc/config/aarch64/aarch64-c.cc
> @@ -344,15 +344,15 @@ aarch64_pragma_aarch64 (cpp_reader *)
>
> const char *name = TREE_STRING_POINTER (x);
> if (strcmp (name, "arm_sve.h") == 0)
> - aarch64_sve::handle_arm_sve_h ();
> + aarch64_sve::handle_arm_sve_h (false);
> else if (strcmp (name, "arm_sme.h") == 0)
> - aarch64_sve::handle_arm_sme_h ();
> + aarch64_sve::handle_arm_sme_h (false);
> else if (strcmp (name, "arm_neon.h") == 0)
> handle_arm_neon_h ();
> else if (strcmp (name, "arm_acle.h") == 0)
> handle_arm_acle_h ();
> else if (strcmp (name, "arm_neon_sve_bridge.h") == 0)
> - aarch64_sve::handle_arm_neon_sve_bridge_h ();
> + aarch64_sve::handle_arm_neon_sve_bridge_h (false);
> else
> error ("unknown %<#pragma GCC aarch64%> option %qs", name);
> }
> diff --git a/gcc/config/aarch64/aarch64-protos.h
> b/gcc/config/aarch64/aarch64-protos.h
> index
> bd719b992a545630f9ad6a73753fad55c6ac5d7e..42639e9efcf1e0f9362f759ae63a31b8eeb0d581
> 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -1014,9 +1014,9 @@ bool aarch64_general_check_builtin_call (location_t,
> vec<location_t>,
>
> namespace aarch64_sve {
> void init_builtins ();
> - void handle_arm_sve_h ();
> - void handle_arm_sme_h ();
> - void handle_arm_neon_sve_bridge_h ();
> + void handle_arm_sve_h (bool);
> + void handle_arm_sme_h (bool);
> + void handle_arm_neon_sve_bridge_h (bool);
> tree builtin_decl (unsigned, bool);
> bool builtin_type_p (const_tree);
> bool builtin_type_p (const_tree, unsigned int *, unsigned int *);
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h
> b/gcc/config/aarch64/aarch64-sve-builtins.h
> index
> e66729ed63532811b3b16ab57ae11cb10518caca..3e0d2b8560c538d201c97fba3591ddf9893d664a
> 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.h
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.h
> @@ -123,6 +123,15 @@ enum units_index
> UNITS_vectors
> };
>
> +/* Enumerates the pragma handlers. */
> +enum handle_pragma_index
> +{
> + arm_sve_handle,
> + arm_sme_handle,
> + arm_neon_sve_handle,
> + NUM_PRAGMA_HANDLERS
> +};
> +
> /* Describes the various uses of a governing predicate. */
> enum predication_index
> {
> @@ -419,7 +428,7 @@ class registered_function;
> class function_builder
> {
> public:
> - function_builder ();
> + function_builder (handle_pragma_index, bool);
> ~function_builder ();
>
> void add_unique_function (const function_instance &, tree,
> @@ -453,6 +462,12 @@ private:
>
> /* Used for building up function names. */
> obstack m_string_obstack;
> +
> + /* Used to store the index for the current function. */
> + unsigned int m_function_index;
> +
> + /* Stores the mode of the current pragma handler. */
> + bool m_function_nulls;
> };
>
> /* A base class for handling calls to built-in functions. */
> diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc
> b/gcc/config/aarch64/aarch64-sve-builtins.cc
> index
> 11f5c5c500c8331094933cb1c1205a1360eca79b..924bfeb3e23bb1bbed79d36adf33286b87b52f85
> 100644
> --- a/gcc/config/aarch64/aarch64-sve-builtins.cc
> +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
> @@ -933,14 +933,19 @@ tree acle_svprfop;
> /* The list of all registered function decls, indexed by code. */
> static GTY(()) vec<registered_function *, va_gc> *registered_functions;
>
> +/* Stores the starting function index for each pragma handler. */
> +static unsigned int initial_indexes[NUM_PRAGMA_HANDLERS];
> +
> /* All registered function decls, hashed on the function_instance
> that they implement. This is used for looking up implementations of
> overloaded functions. */
> static hash_table<registered_function_hasher> *function_table;
>
> -/* Maps all overloaded function names that we've registered so far to
> - their associated function_instances. The map keys are IDENTIFIER_NODEs.
> */
> -static GTY(()) hash_map<tree, registered_function *> *overload_names;
> +/* Index 0 maps all overloaded function names that we've registered so far to
> + their associated function_instances. Index 1 does the same for functions
> + that we've skipped over without registering. In both cases, the map keys
> + are IDENTIFIER_NODEs. */
> +static GTY(()) hash_map<tree, registered_function *> *overload_names[2];
>
> /* True if we've already complained about attempts to use functions
> when the required extension is disabled. */
> @@ -1332,10 +1337,21 @@ sve_switcher::~sve_switcher ()
> maximum_field_alignment = m_old_maximum_field_alignment;
> }
>
> -function_builder::function_builder ()
> +function_builder::function_builder (handle_pragma_index pragma_index,
> + bool function_nulls)
> {
> m_overload_type = build_function_type (void_type_node, void_list_node);
> m_direct_overloads = lang_GNU_CXX ();
> +
> + if (initial_indexes[pragma_index] == 0)
> + {
> + unsigned int index = vec_safe_length (registered_functions);
> + initial_indexes[pragma_index] = index;
> + }
> +
> + m_function_index = initial_indexes[pragma_index];
> + m_function_nulls = function_nulls;
> +
> gcc_obstack_init (&m_string_obstack);
> }
>
> @@ -1499,9 +1515,8 @@ function_builder::add_function (const function_instance
> &instance,
> bool overloaded_p,
> bool placeholder_p)
> {
> - unsigned int code = vec_safe_length (registered_functions);
> - code = (code << AARCH64_BUILTIN_SHIFT) | AARCH64_BUILTIN_SVE;
> -
> + unsigned int length = vec_safe_length (registered_functions);
> + unsigned int code = (m_function_index << AARCH64_BUILTIN_SHIFT) |
> AARCH64_BUILTIN_SVE;
> /* 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.
> @@ -1515,7 +1530,7 @@ function_builder::add_function (const function_instance
> &instance,
> 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
> + tree decl = placeholder_p || m_function_nulls
> ? integer_zero_node
> : simulate_builtin_function_decl (input_location, name, fntype,
> code, NULL, attrs);
> @@ -1525,7 +1540,11 @@ function_builder::add_function (const
> function_instance &instance,
> rfn.decl = decl;
> rfn.required_extensions = required_extensions;
> rfn.overloaded_p = overloaded_p;
> - vec_safe_push (registered_functions, &rfn);
> + if (m_function_index >= length)
> + vec_safe_push (registered_functions, &rfn);
> + else
> + (*registered_functions)[m_function_index] = &rfn;
> + m_function_index++;
>
> return rfn;
> }
> @@ -1554,11 +1573,14 @@ add_unique_function (const function_instance
> &instance,
> required_extensions, false, false);
>
> /* Enter the function into the hash table. */
> - hashval_t hash = instance.hash ();
> - registered_function **rfn_slot
> - = function_table->find_slot_with_hash (instance, hash, INSERT);
> - gcc_assert (!*rfn_slot);
> - *rfn_slot = &rfn;
> + if (!m_function_nulls)
> + {
> + hashval_t hash = instance.hash ();
> + registered_function **rfn_slot
> + = function_table->find_slot_with_hash (instance, hash, INSERT);
> + gcc_assert (!*rfn_slot);
> + *rfn_slot = &rfn;
> + }
>
> /* Also add the function under its overloaded alias, if we want
> a separate decl for each instance of an overloaded function. */
> @@ -1589,12 +1611,13 @@ function_builder::
> add_overloaded_function (const function_instance &instance,
> aarch64_feature_flags required_extensions)
> {
> - if (!overload_names)
> - overload_names = hash_map<tree, registered_function *>::create_ggc ();
> + auto &name_map = overload_names[m_function_nulls];
> + if (!name_map)
> + name_map = hash_map<tree, registered_function *>::create_ggc ();
>
> char *name = get_name (instance, true);
> tree id = get_identifier (name);
> - if (registered_function **map_value = overload_names->get (id))
> + if (registered_function **map_value = name_map->get (id))
> gcc_assert ((*map_value)->instance == instance
> && ((*map_value)->required_extensions
> & ~required_extensions) == 0);
> @@ -1603,7 +1626,7 @@ add_overloaded_function (const function_instance
> &instance,
> registered_function &rfn
> = add_function (instance, name, m_overload_type, NULL_TREE,
> required_extensions, true, m_direct_overloads);
> - overload_names->put (id, &rfn);
> + name_map->put (id, &rfn);
> }
> obstack_free (&m_string_obstack, name);
> }
> @@ -4510,9 +4533,9 @@ init_builtins ()
> register_builtin_types ();
> if (in_lto_p)
> {
> - handle_arm_sve_h ();
> - handle_arm_sme_h ();
> - handle_arm_neon_sve_bridge_h ();
> + handle_arm_sve_h (false);
> + handle_arm_sme_h (false);
> + handle_arm_neon_sve_bridge_h (false);
> }
> }
>
> @@ -4614,7 +4637,7 @@ register_svprfop ()
>
> /* Implement #pragma GCC aarch64 "arm_sve.h". */
> void
> -handle_arm_sve_h ()
> +handle_arm_sve_h (bool function_nulls_p)
> {
> if (function_table)
> {
> @@ -4641,17 +4664,20 @@ handle_arm_sve_h ()
>
> /* Define the functions. */
> function_table = new hash_table<registered_function_hasher> (1023);
> - function_builder builder;
> + function_builder builder (arm_sve_handle, function_nulls_p);
> for (unsigned int i = 0; i < ARRAY_SIZE (function_groups); ++i)
> builder.register_function_group (function_groups[i]);
> }
>
> /* Implement #pragma GCC aarch64 "arm_neon_sve_bridge.h". */
> void
> -handle_arm_neon_sve_bridge_h ()
> +handle_arm_neon_sve_bridge_h (bool function_nulls_p)
> {
> + if (initial_indexes[arm_sme_handle] == 0)
> + handle_arm_sme_h (true);
> +
> /* Define the functions. */
> - function_builder builder;
> + function_builder builder (arm_neon_sve_handle, function_nulls_p);
> for (unsigned int i = 0; i < ARRAY_SIZE (neon_sve_function_groups); ++i)
> builder.register_function_group (neon_sve_function_groups[i]);
> }
> @@ -4668,7 +4694,7 @@ builtin_decl (unsigned int code, bool)
>
> /* Implement #pragma GCC aarch64 "arm_sme.h". */
> void
> -handle_arm_sme_h ()
> +handle_arm_sme_h (bool function_nulls_p)
> {
> if (!function_table)
> {
> @@ -4677,17 +4703,9 @@ handle_arm_sme_h ()
> return;
> }
>
> - static bool initialized_p;
> - if (initialized_p)
> - {
> - error ("duplicate definition of %qs", "arm_sme.h");
> - return;
> - }
> - initialized_p = true;
> -
> sme_switcher sme;
>
> - function_builder builder;
> + function_builder builder (arm_sme_handle, function_nulls_p);
> for (unsigned int i = 0; i < ARRAY_SIZE (sme_function_groups); ++i)
> builder.register_function_group (sme_function_groups[i]);
> }
