[Bug bootstrap/65150] [5 Regression] r220875 causes bootstrap failure on x86_64 darwin

["iains at gcc dot gnu.org" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65150

--- Comment #7 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> So, patch for discussions:
> 1) for DECL_VIRTUAL_P trying redirect_callers will unlikely help (not sure
> about that too)
> 2) don't remove alias even if it is virtual or has any non-callers
> references (what is the best way to express this condition)?
> 3) fall thru info create_thunk case if we didn't alias->remove ()
> 4) typo fix
> 5) use return false instead of return 0
> 
> --- gcc/ipa-icf.c.jj	2015-02-20 17:42:54.000000000 +0100
> +++ gcc/ipa-icf.c	2015-02-23 12:16:27.669089598 +0100
> @@ -662,6 +662,7 @@ sem_function::merge (sem_item *alias_ite
>        redirect_callers
>  	= (!original_discardable
>  	   && !DECL_COMDAT_GROUP (alias->decl)
> +	   && !DECL_VIRTUAL_P (alias->decl)
>  	   && alias->get_availability () > AVAIL_INTERPOSABLE
>  	   && original->get_availability () > AVAIL_INTERPOSABLE
>  	   && !alias->instrumented_version);
> @@ -724,15 +725,18 @@ sem_function::merge (sem_item *alias_ite
>  
>        /* The alias function is removed if symbol address
>           does not matter.  */
> -      if (!alias_address_matters)
> -	alias->remove ();
> +      if (!alias->externally_visible && !alias->address_taken)
> +	{
> +	  alias->remove ();
>  
> -      if (dump_file && redirected)
> -	fprintf (dump_file, "Callgraph local calls have been redirected.\n\n");
> +	  if (dump_file && redirected)
> +	    fprintf (dump_file, "Callgraph local calls have been redirected.\n\n");
> +	  return true;
> +	}
>      }

here we fall through to the case were we make a thunk - IFF we were not able to
rmove the alias.  However, potentially, we already moved all the callers from
the alias to the original.

(a) is that safe?
(b) we might as well avoid the work by testing whether the alias will be
removable before we set redirect_callers?


> -  /* If the condtion above is not met, we are lucky and can turn the
> +  /* If the condition above is not met, we are lucky and can turn the
>       function into real alias.  */
> -  else if (create_alias)
> +  if (create_alias)
>      {
>        alias->icf_merged = true;
>        if (local_original->lto_file_data
> @@ -762,7 +766,7 @@ sem_function::merge (sem_item *alias_ite
>  	  if (dump_file)
>  	    fprintf (dump_file, "Callgraph thunk cannot be created because of
> COMDAT\n");
>  
> -	  return 0;
> +	  return false;
>  	}
>  
>        if (DECL_STATIC_CHAIN (alias->decl))
> @@ -770,7 +774,7 @@ sem_function::merge (sem_item *alias_ite
>           if (dump_file)
>             fprintf (dump_file, "Thunk creation is risky for static-chain
> functions.\n\n");
>  
> -         return 0;
> +         return false;
>          }
>  
>        alias->icf_merged = true;

===

on x86_64-darwin12, Java and Ada look "normal".

there are regressions on (at least) the following C cases:

FAIL: gcc.dg/attr-noinline.c scan-assembler function_declaration_both_after

FAIL: gcc.dg/ipa/iinline-5.c scan-ipa-dump-not inline
"wrong_target[^\\\\n]*inline copy in"

however, ISTM that these might be more general issues being exposed by the
changes - since (at least) the iinline-5 shows up on Linux.

Essentially, I think we still need to address my points (b, c) in comment#23 on
pr63892 .. (I think Martin is looking, at least, at point b).