Richard Sandiford <richard.sandif...@arm.com> writes:
> Alfie Richards <alfie.richa...@arm.com> 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.

Reply via email to