Richard Sandiford <[email protected]> writes:
> Alfie Richards <[email protected]> writes:
>> ---
>> gcc/cgraph.cc | 39 +++++++++++++++++++++++-------
>> gcc/config/aarch64/aarch64.cc | 37 +++++++---------------------
>> gcc/config/i386/i386-features.cc | 33 ++++---------------------
>> gcc/config/riscv/riscv.cc | 41 +++++++-------------------------
>> gcc/config/rs6000/rs6000.cc | 35 +++++----------------------
>> 5 files changed, 58 insertions(+), 127 deletions(-)
>>
>> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
>> index d0b19ad850e..1ea38d16e56 100644
>> --- a/gcc/cgraph.cc
>> +++ b/gcc/cgraph.cc
>> @@ -236,37 +236,58 @@ cgraph_node::delete_function_version_by_decl (tree
>> decl)
>> void
>> cgraph_node::record_function_versions (tree decl1, tree decl2)
>> {
>> - cgraph_node *decl1_node = cgraph_node::get_create (decl1);
>> - cgraph_node *decl2_node = cgraph_node::get_create (decl2);
>> + cgraph_node *decl1_node;
>> + cgraph_node *decl2_node;
>> cgraph_function_version_info *decl1_v = NULL;
>> cgraph_function_version_info *decl2_v = NULL;
>> cgraph_function_version_info *before;
>> cgraph_function_version_info *after;
>> + cgraph_function_version_info *temp_node;
>> +
>> + decl1_node = cgraph_node::get_create (decl1);
>> + decl2_node = cgraph_node::get_create (decl2);
>>
>> gcc_assert (decl1_node != NULL && decl2_node != NULL);
>> decl1_v = decl1_node->function_version ();
>> decl2_v = decl2_node->function_version ();
>>
>> - if (decl1_v != NULL && decl2_v != NULL)
>> - return;
>> -
>
> Could you go into more detail about why this return needs to be removed?
> It seems like the assumption was that, if the two decls were already
> versioned, they were already versions of the same thing. For example,
> we wouldn't create a set of 4 versions and a set of 2 versions and
> only then merge them into a single set of 6 versions. Is that not
> the case with the new scheme?
>
> If we could keep the return, then we could add:
>
> if (is_function_default_version (decl2)
> || (!decl1_v && !is_function_default_version (decl1)))
On second thoughts, a slightly cleaner alternative might be:
if (decl2_v
? !is_function_default_version (decl1)
: is_function_default_version (decl2))
> {
> std::swap (decl1, decl2);
> std::swap (decl1_v, decl2_v);
> }
>
> after it and then proceed as before, on the basis that (a) decl1_v and
> decl2_v are individually canonical and (b) after the swap, any default
> must be decl1 or earlier in decl1_v. That would avoid a bit of extra
> pointer chasing.
