On 04/02/2025 10:03, Richard Sandiford wrote:
Alfie Richards <alfie.richa...@arm.com> writes:
+ return id;
+ else if (cgraph_node::get_create (decl)->dispatcher_resolver_function)
+ id = clone_identifier (id, "resolver");
+ else if (DECL_FUNCTION_VERSIONED (decl))
{
- name += ".default";
- return get_identifier (name.c_str());
- }
-
- name += "._";
+ aarch64_fmv_feature_mask feature_mask
+ = get_feature_mask_for_version (decl);
- int num_features = ARRAY_SIZE (aarch64_fmv_feature_data);
- for (int i = 0; i < num_features; i++)
- {
- if (feature_mask & aarch64_fmv_feature_data[i].feature_mask)
+ if (feature_mask == 0ULL) // && not implemented!
Could you explain the // comment?
Apologies, this was a temporary marker for myself that slipped through.
{
- name += "M";
- name += aarch64_fmv_feature_data[i].name;
+ if (!DECL_INITIAL (decl))
+ return id;
+ return clone_identifier (id, "default");
This different handling of defined vs. undefined functions feels a
bit weird. It's not clear to me whether:
(1) The .default version is effectively internal to the translation unit,
and therefore other translation units cannot assume it exists.
(2) Other translation units can assume that the .default version exists
if they can see a target_version("default") or target_clones definition.
(3) Other translation units can assume that the .default version exists
if they can see a non-default target_version or a target_clones definition.
(4) Something else.
(2) would create a difference between implicit and explicit defaults
and doesn't seem to be what the series implements (mv-symbols6.C from
patch 14). (3) seems indirect, and IMO it's dangerous to assume that
the mere existence of a non-default version X in one TU implies that
version X will be visible in the TU that contains the resolver. I would
have expected that it would be valid for version X not to be visible
in the TU that contains the resolver, with the consequence that the
resolver won't use it.
(1) seems more appealing on the face of it, so that .default is more
like .resolver. But that doesn't seem to be what the spec says.
I would say its (4) Something else.
This code is the result of that and a discussion we had where we decided we
can avoid mangling the default version symbol if it is external. As we
know that
in the TU where the default is defined, the default
version will be mangled, and the dispatched symbol will take its name.
As the default is the only resolvable version, then all calls and
references
will already have the correct target.
This allows us to avoid making the dispatched symbol and redirecting
default decl calls/references to the dispatched symbol.
It's a bit of a hack, and we can instead always mangle the default,
create the dispatched symbol? Apologies if I misunderstood the earlier
conversation.
}
- }
- if (DECL_ASSEMBLER_NAME_SET_P (decl))
- SET_DECL_RTL (decl, NULL);
+ std::string suffix = "_";
+
+ int num_features = ARRAY_SIZE (aarch64_fmv_feature_data);
+ for (int i = 0; i < num_features; i++)
+ if (feature_mask & aarch64_fmv_feature_data[i].feature_mask)
+ {
+ suffix += "M";
+ suffix += aarch64_fmv_feature_data[i].name;
+ }
+
+ if (DECL_ASSEMBLER_NAME_SET_P (decl))
+ SET_DECL_RTL (decl, NULL);
Why is it necessary to conditionally reset the DECL_RTL for the
non-default case but not necessary when adding ".default"?
Thanks,
Richard
To be honest I don’t understand this code particularly well. This was here
previously, and I understood it to imply that the function needs to be
recompiled if it is non-default as it's architecture features will
have changed.I think it only really applies in the target_clones case
where the new
decl from expand_clones could have carried over the RTL of the default?
Thanks,
Alfie
