Andrew Carlotti <andrew.carlo...@arm.com> writes:
> This adds initial support for function multiversioning on aarch64 using
> the target_version and target_clones attributes. This loosely follows
> the Beta specification in the ACLE [1], although with some differences
> that still need to be resolved (possibly as follow-up patches).
>
> Existing function multiversioning implementations are broken in various
> ways when used across translation units. This includes placing
> resolvers in the wrong translation units, and using symbol mangling that
> callers to unintentionally bypass the resolver in some circumstances.
> Fixing these issues for aarch64 will require modifications to our ACLE
> specification. It will also require further adjustments to existing
> middle end code, to facilitate different mangling and resolver
> placement while preserving existing target behaviours.
>
> The list of function multiversioning features specified in the ACLE is
> also inconsistent with the list of features supported in target option
> extensions. I intend to resolve some or all of these inconsistencies at
> a later stage.
>
> The target_version attribute is currently only supported in C++, since
> this is the only frontend with existing support for multiversioning
> using the target attribute. On the other hand, this patch happens to
> enable multiversioning with the target_clones attribute in Ada and D, as
> well as the entire C family, using their existing frontend support.
>
> This patch also does not support the following aspects of the Beta
> specification:
>
> - The target_clones attribute should allow an implicit unlisted
> "default" version.
> - There should be an option to disable function multiversioning at
> compile time.
> - Unrecognised target names in a target_clones attribute should be
> ignored (with an optional warning). This current patch raises an
> error instead.
>
> [1]
> https://github.com/ARM-software/acle/blob/main/main/acle.md#function-multi-versioning
>
> ---
>
> I believe the support present in this patch correctly handles function
> multiversioning within a single translation unit for all features in the ACLE
> specification with option extension support.
>
> Is it ok to push this patch in its current state? I'd then continue working on
> incremental improvements to the supported feature extensions and the ABI
> issues
> in followup patches, along with corresponding changes and improvements to the
> ACLE specification.
>
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-feature-deps.h (fmv_deps_<FEAT_NAME>):
> Define aarch64_feature_flags mask foreach FMV feature.
> * config/aarch64/aarch64-option-extensions.def: Use new macros
> to define FMV feature extensions.
> * config/aarch64/aarch64.cc (aarch64_option_valid_attribute_p):
> Check for target_version attribute after processing target
> attribute.
> (aarch64_fmv_feature_data): New.
> (aarch64_parse_fmv_features): New.
> (aarch64_process_target_version_attr): New.
> (aarch64_option_valid_version_attribute_p): New.
> (get_feature_mask_for_version): New.
> (compare_feature_masks): New.
> (aarch64_compare_version_priority): New.
> (build_ifunc_arg_type): New.
> (make_resolver_func): New.
> (add_condition_to_bb): New.
> (dispatch_function_versions): New.
> (aarch64_generate_version_dispatcher_body): New.
> (aarch64_get_function_versions_dispatcher): New.
> (aarch64_common_function_versions): New.
> (aarch64_mangle_decl_assembler_name): New.
> (TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P): New implementation.
> (TARGET_OPTION_EXPANDED_CLONES_ATTRIBUTE): New implementation.
> (TARGET_OPTION_FUNCTION_VERSIONS): New implementation.
> (TARGET_COMPARE_VERSION_PRIORITY): New implementation.
> (TARGET_GENERATE_VERSION_DISPATCHER_BODY): New implementation.
> (TARGET_GET_FUNCTION_VERSIONS_DISPATCHER): New implementation.
> (TARGET_MANGLE_DECL_ASSEMBLER_NAME): New implementation.
> * config/aarch64/aarch64.h (TARGET_HAS_FMV_TARGET_ATTRIBUTE):
> Set target macro.
> * config/arm/aarch-common.h (enum aarch_parse_opt_result): Add
> new value to report duplicate FMV feature.
> * common/config/aarch64/cpuinfo.h: New file.
>
> libgcc/ChangeLog:
>
> * config/aarch64/cpuinfo.c (enum CPUFeatures): Move to shared
> copy in gcc/common
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/options_set_17.c: Reorder expected flags.
> * gcc.target/aarch64/cpunative/native_cpu_0.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_13.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_16.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_17.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_18.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_19.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_20.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_21.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_22.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_6.c: Ditto.
> * gcc.target/aarch64/cpunative/native_cpu_7.c: Ditto.
Looks good, thanks. OK for trunk with one trivial fix:
> [...]
> +/* Implement TARGET_OPTION_VALID_VERSION_ATTRIBUTE_P. This is used to
> + process attribute ((target_version ("..."))). */
> +
> +static bool
> +aarch64_option_valid_version_attribute_p (tree fndecl, tree, tree args, int)
> +{
> + struct cl_target_option cur_target;
> + bool ret;
> + tree new_target;
> + tree existing_target = DECL_FUNCTION_SPECIFIC_TARGET (fndecl);
> +
> + /* Save the current target options to restore at the end. */
> + cl_target_option_save (&cur_target, &global_options, &global_options_set);
> +
> + /* If fndecl already has some target attributes applied to it, unpack
> + them so that we add this attribute on top of them, rather than
> + overwriting them. */
> + if (existing_target)
> + {
> + struct cl_target_option *existing_options
> + = TREE_TARGET_OPTION (existing_target);
> +
> + if (existing_options)
> + cl_target_option_restore (&global_options, &global_options_set,
> + existing_options);
> + }
> + else
> + cl_target_option_restore (&global_options, &global_options_set,
> + TREE_TARGET_OPTION (target_option_current_node));
> +
> + ret = aarch64_process_target_version_attr (args);
> +
> + /* Set up any additional state. */
> + if (ret)
> + {
> + aarch64_override_options_internal (&global_options);
> + new_target = build_target_option_node (&global_options,
> + &global_options_set);
> + }
> + else
> + new_target = NULL;
> +
> + if (fndecl && ret)
> + {
> + DECL_FUNCTION_SPECIFIC_TARGET (fndecl) = new_target;
> + }
Formatting nit: should be:
if (fndecl && ret)
DECL_FUNCTION_SPECIFIC_TARGET (fndecl) = new_target;
without the braces.
Richard
