Alfie Richards <alfie.richa...@arm.com> writes:
> On 01/08/2025 11:46, Richard Sandiford wrote:
>> Sorry, I think I missed the multiple_targets.cc changes in my
>> previous review.
>>
>> Alfie Richards <alfie.richa...@arm.com> writes:
>>> +
>>> + tree attrs = remove_attribute ("target_clones",
>>> + DECL_ATTRIBUTES (node->decl));
>>> + tree assembler_name = node_v->assembler_name;
>>> +
>>> + /* Change the current node into the default node. */
>>> + if (num_defaults == 1)
>>> {
>>> - char *attr = attrs[i];
>>> + /* Setting new attribute to initial function. */
>>> + tree attributes = make_attribute (new_attr_name, "default", attrs);
>>> + DECL_ATTRIBUTES (node->decl) = attributes;
>>> + DECL_FUNCTION_VERSIONED (node->decl) = true;
>>> +
>>> + node->is_target_clone = true;
>>> + node->local = false;
>>> +
>>> + /* Remangle base node after new target version string set. */
>>> + tree id = targetm.mangle_decl_assembler_name (node->decl,
>>> assembler_name);
>>> + symtab->change_decl_assembler_name (node->decl, id);
>>> + }
>>> + else
>>> + {
>>> + /* Target clones without a default are only allowed for
>>> target_version
>>> + semantics where we can have target_clones/target_version mixing. */
>>> + gcc_assert (!TARGET_HAS_FMV_TARGET_ATTRIBUTE);
>>> +
>>> + /* If there isn't a default version, can safely remove this version.
>>> + The node itself gets removed after the other versions are created. */
>>> + cgraph_function_version_info *temp = node_v;
>>> + node_v = node_v->next ? node_v->next : node_v->prev;
>>> + cgraph_node::delete_function_version (temp);
>>> + }
>>
>> In terms of staging, would it be worth putting the else block in patch 8
>> instead, along with the corresponding !node_v and num_defaults == 0
>> handling? As it stands, this code can't be reached.
>>
>> I don't mind keeping it as-is if you prefer though. I just found it a
>> little hard to follow in this context.
> You're right that this and patch 8 are really intertwined. I developed
> them simultaneously and didn't want to split up this logic as it would
> have involved writing new logic to delete later.
>
> I can do that if you would like. But I'd rather not personally :)>
No, no need to add temporary logic. But just so I understand:
I was thinking that the num_defaults == 0 paths would be purely
additive. Is that not the case?
Richard
