Re: cgraph: does node->inlined_to imply node->clones is non-empty?

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

Hi.

I'm CCing my more skilled colleagues who will know the right answer.

Cheers,
Martin

On 3/13/23 11:24, Arsen Arsenović via Gcc wrote:
> Hi,
> 
> I was debugging PR96059 and ran into a question which does not seem
> obvious from the code.
> 
> For the test case in the PR, in ipa.cc:remove_unreachable_nodes, GCC
> seems to try to remove an unreachable function that was already inlined
> into a different unreachable function.  When the original inline
> happens, ipa-inline-transform.cc:clone_inlined_nodes decides not to make
> a clone, since the function being cloned is a master clone but with no
> non-inline clones.
> 
> This decision later trips up the gcc_assert in:
> 
>   /* Inline clones might be kept around so their materializing allows further
>      cloning.  If the function the clone is inlined into is removed, we need
>      to turn it into normal cone.  */
>   FOR_EACH_FUNCTION (node)
>     {
>       if (node->inlined_to
> 	  && !node->callers)
> 	{
> 	  gcc_assert (node->clones);
> 	  node->inlined_to = NULL;
> 	  update_inlined_to_pointer (node, node);
> 	}
>       node->aux = NULL;
>     }
> 
> .. because it is expecting that an inlined function without callers
> (which is necessarily true here as this function is unreachable and so
> was ->remove ()'d earlier) has clones.
> 
> Either removing the assertion or making clone_inline_nodes clone when
> there are no existing clones 'fixes' (suppresses, but I haven't verified
> whether the results are correct) the problem.
> 
> Is this gcc_assert correct in that an inlined function without callers
> necessarily must have clones?
> 
> And as a side question, do all clone nodes have a ->clones pointing to
> the same list of all clones, or are they in a tree-ish arrangement,
> where clones of clones end up forming a tree, with the clone_of pointer
> being a pointer to the parent?  (in this instance, the node that trips
> the assert has a nullptr clone_of and clones value, which would AIUI
> imply that it is the original)
> 
> This train of thinking doesn't end up involving any devirtualization
> code, which the PR was originally reproduced with, but my current theory
> is that devirtualizing here just exposed an edge case that is decently
> well hidden, rather than it playing a crucial role.
> 
> Thanks in advance, have a lovely day.