diff mbox

Fix ICF sem_function::merge (PR target/63892)

Message ID 7D7949B0-73B6-4A3D-A48F-868FB006F8FB@codesourcery.com
State New
Headers show

Commit Message

Iain Sandoe Feb. 21, 2015, 1:24 p.m. UTC 
Hi Jakub, Martin,

On 20 Feb 2015, at 16:42, Jeff Law wrote:

> On 02/20/15 07:48, Jakub Jelinek wrote:
>> Hi!
>> 
>> As written in the PR, the sibcall-3.c testcase fails on Darwin, because
>> !sem_item::target_supports_symbol_aliases_p () and we don't really try
>> redirect_callers, even when that is the best way to perform ICF (both
>> original and alias are local).
>> 
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> OK.

This caused a bootstrap fail on Darwin, because of what looks like a typo in the re-factoring of the code (unfortunately, doesn't show up until stage#2).

I've bootstrapped the following patch on x86_64-linux and x86_64-darwin12, OK for trunk if wider testing passes?

Iain

P.S. The patch does solve a problem with ADT/SmallVectorTests.cpp in llvm suite (with generation of a varargs thunk).
However, it does not appear to restore sibcall-3 for m32 darwin (see. pr63892 for updated analysis).

gcc 
	* ipa-icf.c (sem_function::merge): Do not try to redirect unless the target supports
	symbol aliases.

Comments

Jakub Jelinek Feb. 22, 2015, 11:13 a.m. UTC | #1 
On Sat, Feb 21, 2015 at 01:24:55PM +0000, Iain Sandoe wrote:
> P.S. The patch does solve a problem with ADT/SmallVectorTests.cpp in llvm suite (with generation of a varargs thunk).
> However, it does not appear to restore sibcall-3 for m32 darwin (see. pr63892 for updated analysis).
> 
> gcc 
> 	* ipa-icf.c (sem_function::merge): Do not try to redirect unless the target supports
> 	symbol aliases.

No, that looks wrong.  It is very much intentional, there should be no
reason why even without proper support of aliases you couldn't redirect
callers.  Callers redirection is done by just changing the IL, there is
really no need for any backend support for that.
Just look at the sibcall-3.c testcase.

> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index e1af8bf..f128494 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -662,6 +662,7 @@ sem_function::merge (sem_item *alias_item)
>        redirect_callers
>  	= (!original_discardable
>  	   && !DECL_COMDAT_GROUP (alias->decl)
> +	   && sem_item::target_supports_symbol_aliases_p ()
>  	   && alias->get_availability () > AVAIL_INTERPOSABLE
>  	   && original->get_availability () > AVAIL_INTERPOSABLE
>  	   && !alias->instrumented_version);

	Jakub
diff mbox

Patch

diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index e1af8bf..f128494 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -662,6 +662,7 @@  sem_function::merge (sem_item *alias_item)
       redirect_callers
 	= (!original_discardable
 	   && !DECL_COMDAT_GROUP (alias->decl)
+	   && sem_item::target_supports_symbol_aliases_p ()
 	   && alias->get_availability () > AVAIL_INTERPOSABLE
 	   && original->get_availability () > AVAIL_INTERPOSABLE
 	   && !alias->instrumented_version);