[PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jakub Jelinek]

Hi!

The following testcase ICEs, because the ctors during cc1plus all have
!opt_for_fn (decl, flag_semantic_interposition) - they have NULL
DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) and optimization_default_node
is for -Ofast and so has flag_semantic_interposition cleared.
During free lang data, we set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
for the ctor which has body (or for thunks), but don't touch it for
aliases.
During lto1 optimization_default_node reflects the lto1 flags which
are -O2 rather than -Ofast and so has flag_semantic_interposition
set, for the ctor which has body that makes no difference, but as the
alias doesn't still have DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) set,
we now trigger this verification check.

The following patch just doesn't verify it for aliases during lto1.
Another possibility would be to set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
during free lang data even for aliases.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-04-28  Jakub Jelinek  <jakub@redhat.com>

	PR lto/105399
	* cgraph.cc (cgraph_node::verify_node): Don't verify
	semantic_interposition flag against
	opt_for_fn (decl, flag_semantic_interposition) for aliases in lto1.

	* g++.dg/lto/pr105399_0.C: New test.

--- gcc/cgraph.cc.jj	2022-04-20 09:24:12.194579146 +0200
+++ gcc/cgraph.cc	2022-04-27 11:53:52.102173154 +0200
@@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
 	     "returns a pointer");
       error_found = true;
     }
-  if (definition && externally_visible
+  if (definition
+      && externally_visible
+      && (!alias || thunk || !in_lto_p)
       && semantic_interposition
 	 != opt_for_fn (decl, flag_semantic_interposition))
     {
--- gcc/testsuite/g++.dg/lto/pr105399_0.C.jj	2022-04-27 11:54:25.659703199 +0200
+++ gcc/testsuite/g++.dg/lto/pr105399_0.C	2022-04-27 11:48:31.387664565 +0200
@@ -0,0 +1,9 @@
+// PR lto/105399
+// { dg-lto-do link }
+// { dg-lto-options { { -fPIC -flto -Ofast } } }
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
+// { dg-extra-ld-options "-shared -O2" }
+
+struct S { S (); };
+S::S () {}

	Jakub

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Richard Biener]

On Thu, 28 Apr 2022, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs, because the ctors during cc1plus all have
> !opt_for_fn (decl, flag_semantic_interposition) - they have NULL
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) and optimization_default_node
> is for -Ofast and so has flag_semantic_interposition cleared.
> During free lang data, we set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> for the ctor which has body (or for thunks), but don't touch it for
> aliases.
> During lto1 optimization_default_node reflects the lto1 flags which
> are -O2 rather than -Ofast and so has flag_semantic_interposition
> set, for the ctor which has body that makes no difference, but as the
> alias doesn't still have DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) set,
> we now trigger this verification check.
> 
> The following patch just doesn't verify it for aliases during lto1.
> Another possibility would be to set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> during free lang data even for aliases.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-04-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR lto/105399
> 	* cgraph.cc (cgraph_node::verify_node): Don't verify
> 	semantic_interposition flag against
> 	opt_for_fn (decl, flag_semantic_interposition) for aliases in lto1.
> 
> 	* g++.dg/lto/pr105399_0.C: New test.
> 
> --- gcc/cgraph.cc.jj	2022-04-20 09:24:12.194579146 +0200
> +++ gcc/cgraph.cc	2022-04-27 11:53:52.102173154 +0200
> @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
>  	     "returns a pointer");
>        error_found = true;
>      }
> -  if (definition && externally_visible
> +  if (definition
> +      && externally_visible
> +      && (!alias || thunk || !in_lto_p)

Can you add a comment above this?

OK with this if Honza doesn't have any suggestions today.

Richard.

>        && semantic_interposition
>  	 != opt_for_fn (decl, flag_semantic_interposition))
>      {
> --- gcc/testsuite/g++.dg/lto/pr105399_0.C.jj	2022-04-27 11:54:25.659703199 +0200
> +++ gcc/testsuite/g++.dg/lto/pr105399_0.C	2022-04-27 11:48:31.387664565 +0200
> @@ -0,0 +1,9 @@
> +// PR lto/105399
> +// { dg-lto-do link }
> +// { dg-lto-options { { -fPIC -flto -Ofast } } }
> +// { dg-require-effective-target shared }
> +// { dg-require-effective-target fpic }
> +// { dg-extra-ld-options "-shared -O2" }
> +
> +struct S { S (); };
> +S::S () {}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jan Hubicka]

Hello,
> Hi!
> 
> The following testcase ICEs, because the ctors during cc1plus all have
> !opt_for_fn (decl, flag_semantic_interposition) - they have NULL
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) and optimization_default_node
> is for -Ofast and so has flag_semantic_interposition cleared.
> During free lang data, we set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> for the ctor which has body (or for thunks), but don't touch it for
> aliases.

Hmm, this is bit interesting.  We definitly want to preserve original
semantic interposition behaviour of aliases and it works here since we
move the flag to symbol table, but it is bit of accident (I added the
flag because variables does not have their ovn
DECL_VAR_SPECIFIC_OPTIMIZATION so it needs to be part of symbol table).

Looking for current uses of opt_for_fn we seems to care about properties
of the actual function bodies, not aliases, but it feels a bit fragile.
Next stage1 I should make opt_for_fn to bomb on all aliases so we know
we are not using it wrongly.

> During lto1 optimization_default_node reflects the lto1 flags which
> are -O2 rather than -Ofast and so has flag_semantic_interposition
> set, for the ctor which has body that makes no difference, but as the
> alias doesn't still have DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl) set,
> we now trigger this verification check.
> 
> The following patch just doesn't verify it for aliases during lto1.
> Another possibility would be to set DECL_FUNCTION_SPECIFIC_OPTIMIZATION (decl)
> during free lang data even for aliases.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2022-04-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR lto/105399
> 	* cgraph.cc (cgraph_node::verify_node): Don't verify
> 	semantic_interposition flag against
> 	opt_for_fn (decl, flag_semantic_interposition) for aliases in lto1.
> 
> 	* g++.dg/lto/pr105399_0.C: New test.
> 
> --- gcc/cgraph.cc.jj	2022-04-20 09:24:12.194579146 +0200
> +++ gcc/cgraph.cc	2022-04-27 11:53:52.102173154 +0200
> @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
>  	     "returns a pointer");
>        error_found = true;
>      }
> -  if (definition && externally_visible
> +  if (definition
> +      && externally_visible
> +      && (!alias || thunk || !in_lto_p)

The alias check seems OK to me.
Why we need thunk?  Here thunk may eventually gain a gimple body and we
will be interested in its opt_for_fn flags.  So if we do not set
DECL_FUNCTION_SPECIFIC_OPTIMIZATION we may get unexpected surprises
(such as blocked inlining).
>        && semantic_interposition
>  	 != opt_for_fn (decl, flag_semantic_interposition))
>      {
> --- gcc/testsuite/g++.dg/lto/pr105399_0.C.jj	2022-04-27 11:54:25.659703199 +0200
> +++ gcc/testsuite/g++.dg/lto/pr105399_0.C	2022-04-27 11:48:31.387664565 +0200
> @@ -0,0 +1,9 @@
> +// PR lto/105399
> +// { dg-lto-do link }
> +// { dg-lto-options { { -fPIC -flto -Ofast } } }
> +// { dg-require-effective-target shared }
> +// { dg-require-effective-target fpic }
> +// { dg-extra-ld-options "-shared -O2" }
> +
> +struct S { S (); };
> +S::S () {}
> 
> 	Jakub
> 

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jakub Jelinek]

On Thu, Apr 28, 2022 at 01:54:51PM +0200, Jan Hubicka wrote:
> > --- gcc/cgraph.cc.jj	2022-04-20 09:24:12.194579146 +0200
> > +++ gcc/cgraph.cc	2022-04-27 11:53:52.102173154 +0200
> > @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
> >  	     "returns a pointer");
> >        error_found = true;
> >      }
> > -  if (definition && externally_visible
> > +  if (definition
> > +      && externally_visible
> > +      && (!alias || thunk || !in_lto_p)
> 
> The alias check seems OK to me.
> Why we need thunk?  Here thunk may eventually gain a gimple body and we
> will be interested in its opt_for_fn flags.  So if we do not set
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION we may get unexpected surprises
> (such as blocked inlining).

I've added || thunk because free_lang_data sets DECL_FUNCTION_SPECIFIC_OPTIMIZATION
for thunks, the guarding condition is:
if (gimple_has_body_p (decl) || (node && node->thunk))
I wasn't sure if node->alias and node->thunk can be both set or not.
If they can't, I can take it out.

	Jakub

Re: [PATCH] cgraph: Don't verify semantic_interposition flag for aliases [PR105399]

[Jan Hubicka]

> On Thu, Apr 28, 2022 at 01:54:51PM +0200, Jan Hubicka wrote:
> > > --- gcc/cgraph.cc.jj	2022-04-20 09:24:12.194579146 +0200
> > > +++ gcc/cgraph.cc	2022-04-27 11:53:52.102173154 +0200
> > > @@ -3488,7 +3488,9 @@ cgraph_node::verify_node (void)
> > >  	     "returns a pointer");
> > >        error_found = true;
> > >      }
> > > -  if (definition && externally_visible
> > > +  if (definition
> > > +      && externally_visible
> > > +      && (!alias || thunk || !in_lto_p)
> > 
> > The alias check seems OK to me.
> > Why we need thunk?  Here thunk may eventually gain a gimple body and we
> > will be interested in its opt_for_fn flags.  So if we do not set
> > DECL_FUNCTION_SPECIFIC_OPTIMIZATION we may get unexpected surprises
> > (such as blocked inlining).
> 
> I've added || thunk because free_lang_data sets DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> for thunks, the guarding condition is:
> if (gimple_has_body_p (decl) || (node && node->thunk))
> I wasn't sure if node->alias and node->thunk can be both set or not.
> If they can't, I can take it out.

Aha, I misread is as !alias || !thunk.
But node can not be both alias and thunk, so I would leave it out.

Honza

> 
> 	Jakub
>