Re: [PATCH] IPA: do not release body if still needed

["Martin Liška" <mliska@suse.cz>]

On 1/14/23 22:36, Jan Hubicka wrote:
>> Hi.
>>
>> Noticed during building of libbackend.a with the LTO partial linking.
>>
>> The function release_body is called even if clone_of is a clone
>> of a another function and thus it shares tree declaration. We should
>> preserve it in that situation.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> 	PR ipa/107944
>>
>> gcc/ChangeLog:
>>
>> 	* cgraph.cc (cgraph_node::remove): Do not release body
>> 	if a node is clone of another node.
>> ---
>>  gcc/cgraph.cc | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
>> index f15cb47c8b8..2e7d77ffd6c 100644
>> --- a/gcc/cgraph.cc
>> +++ b/gcc/cgraph.cc
>> @@ -1893,7 +1893,7 @@ cgraph_node::remove (void)
>>    else if (clone_of)
>>      {
>>        clone_of->clones = next_sibling_clone;
>> -      if (!clone_of->analyzed && !clone_of->clones && !clones)
>> +      if (!clone_of->analyzed && !clone_of->clones && !clones && !clone_of->clone_of)
>>  	clone_of->release_body ();
> 
> It is interesting that the problem reproduced only after almost 20
> years.  But I suppose it is because we materialize clones in parituclar
> order.

Well, it started with r13-48-g27ee75dbe81bb7 where Martin add a new code
that calls the release_body function. So it's pretty new.

> 
> I think there are two ways to fix it.  Either declare release_body to be
> applicable only to the master clone and avoid calling it here (as you
> do) or make release_body do nothing when called on a clone.
> I guess it makes sense to keep your approach but please add sanity check
> to release_body that clone_of == NULL with a comment.

I do support Martin's enhanced version of the patch.

Cheers,
Martin

> 
> OK with that change.
> Honza
>>      }
>>    if (next_sibling_clone)
>> -- 
>> 2.38.1
>>