Re: [PATCH] Do not remove ifunc_resolver in LTO.

2020-04-23 Thread Jan Hubicka
> On 4/22/20 8:11 PM, Jan Hubicka wrote:
> > > On Mon, 2020-04-20 at 11:34 +0200, Martin Liška wrote:
> > > > Hi.
> > > > 
> > > > The patch prevents a ifunc alias from removal in remove unreachable 
> > > > nodes.
> > > > Note that ifunc alias lives in a COMDAT section and so that
> > > > cgraph_node::can_remove_if_no_direct_calls_and_refs_p returned true for 
> > > > it.
> > > > 
> > > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > > > I was unable to create a lto test-case where a linked binary could be
> > > > scanned for assembly.
> > > > 
> > > > Ready to be installed?
> > > > Thanks,
> > > > Martin
> > > > 
> > > > gcc/ChangeLog:
> > > > 
> > > > 2020-04-20  Martin Liska  
> > > > 
> > > > PR lto/94659
> > > > * cgraph.h 
> > > > (cgraph_node::can_remove_if_no_direct_calls_and_refs_p):
> > > > Do not remove ifunc_resolvers in remove unreachable nodes in 
> > > > LTO.
> > > OK
> > Is it intended to keep the comdat group alive even when the function is
> > not used by the current translation unit?
> 
> Yes, if you take a look at the mentioned PR, the function is exported and used
> by a different TU.
> 
> Or do you have a particular test-case which you're talking about?

Well, first I think you want to use force_output flag when you create
the alias instead of modifying can_remove_if_no_direct_calls_and_refs_p.

I wonder what happens when your function is static and we optimize away
all uses of it - then I think GCC should optimize it away.
Similarly if the function is comdat:
__attribute__((target_clones("default,avx")))
inline
int f1()
{
return 2;
}
int
main()
{
  return f1();
}

We should avoid outputting it to every compilation unit that includes
the header.

I do not recall, why that function is comdat at first place?

Honza


Re: [PATCH] Do not remove ifunc_resolver in LTO.

2020-04-22 Thread Martin Liška

On 4/22/20 8:11 PM, Jan Hubicka wrote:

On Mon, 2020-04-20 at 11:34 +0200, Martin Liška wrote:

Hi.

The patch prevents a ifunc alias from removal in remove unreachable nodes.
Note that ifunc alias lives in a COMDAT section and so that
cgraph_node::can_remove_if_no_direct_calls_and_refs_p returned true for it.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
I was unable to create a lto test-case where a linked binary could be
scanned for assembly.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-04-20  Martin Liska  

PR lto/94659
* cgraph.h (cgraph_node::can_remove_if_no_direct_calls_and_refs_p):
Do not remove ifunc_resolvers in remove unreachable nodes in LTO.

OK

Is it intended to keep the comdat group alive even when the function is
not used by the current translation unit?


Yes, if you take a look at the mentioned PR, the function is exported and used
by a different TU.

Or do you have a particular test-case which you're talking about?

Martin



Honza

jeff





Re: [PATCH] Do not remove ifunc_resolver in LTO.

2020-04-22 Thread Jan Hubicka
> On Mon, 2020-04-20 at 11:34 +0200, Martin Liška wrote:
> > Hi.
> > 
> > The patch prevents a ifunc alias from removal in remove unreachable nodes.
> > Note that ifunc alias lives in a COMDAT section and so that
> > cgraph_node::can_remove_if_no_direct_calls_and_refs_p returned true for it.
> > 
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > I was unable to create a lto test-case where a linked binary could be
> > scanned for assembly.
> > 
> > Ready to be installed?
> > Thanks,
> > Martin
> > 
> > gcc/ChangeLog:
> > 
> > 2020-04-20  Martin Liska  
> > 
> > PR lto/94659
> > * cgraph.h (cgraph_node::can_remove_if_no_direct_calls_and_refs_p):
> > Do not remove ifunc_resolvers in remove unreachable nodes in LTO.
> OK
Is it intended to keep the comdat group alive even when the function is
not used by the current translation unit?

Honza
> jeff
> 


Re: [PATCH] Do not remove ifunc_resolver in LTO.

2020-04-22 Thread Jeff Law via Gcc-patches
On Mon, 2020-04-20 at 11:34 +0200, Martin Liška wrote:
> Hi.
> 
> The patch prevents a ifunc alias from removal in remove unreachable nodes.
> Note that ifunc alias lives in a COMDAT section and so that
> cgraph_node::can_remove_if_no_direct_calls_and_refs_p returned true for it.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> I was unable to create a lto test-case where a linked binary could be
> scanned for assembly.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2020-04-20  Martin Liska  
> 
>   PR lto/94659
>   * cgraph.h (cgraph_node::can_remove_if_no_direct_calls_and_refs_p):
>   Do not remove ifunc_resolvers in remove unreachable nodes in LTO.
OK
jeff



[PATCH] Do not remove ifunc_resolver in LTO.

2020-04-20 Thread Martin Liška

Hi.

The patch prevents a ifunc alias from removal in remove unreachable nodes.
Note that ifunc alias lives in a COMDAT section and so that
cgraph_node::can_remove_if_no_direct_calls_and_refs_p returned true for it.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
I was unable to create a lto test-case where a linked binary could be
scanned for assembly.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-04-20  Martin Liska  

PR lto/94659
* cgraph.h (cgraph_node::can_remove_if_no_direct_calls_and_refs_p):
Do not remove ifunc_resolvers in remove unreachable nodes in LTO.
---
 gcc/cgraph.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 43de3b4a8ac..5ddeb65269b 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -3162,7 +3162,7 @@ cgraph_node::can_remove_if_no_direct_calls_and_refs_p (void)
 return false;
   /* Only COMDAT functions can be removed if externally visible.  */
   if (externally_visible
-  && (!DECL_COMDAT (decl)
+  && ((!DECL_COMDAT (decl) || ifunc_resolver)
 	  || forced_by_abi
 	  || used_from_object_file_p ()))
 return false;