Fix ICE with thunks taking scalars passed by reference

On Wed, Apr 01, 2015 at 08:32:04AM +0200, Jan Hubicka wrote:
> this patch solves ICE in the attached testcase on mingw32.  The problem is that
> on Windows API long double is passed & returned by reference and while expanidng
> the tunk tail call, we get lost because we turn the parameter into SSA name and
> later need its address to pass it further.
> 
> The patch extends hack https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00423.html
> to handle not only non-registers but also registers.
> 
> Bootstrapped/regtested ppc64-linux. OK?

I think it might be better to handle thunks the same way as other tail
calls, but
long double func1 (long double x);

long double
func2 (long double x)
{
  return func1 (x);
}
on both mingw and x86_64-linux) doesn't really work, because calls.c treats
calls in thunks differently from other calls - only in the thunks it takes
ADDR_EXPR of the parameter/result.  Thus your patch is ok
for trunk, but please add the testcase into the testsuite.

> 	PR ipa/65540
> 	* calls.c (initialize_argument_information): When producing tail
> 	call also turn SSA_NAMES passed by references to original PARM_DECLs
> Index: calls.c
> ===================================================================
> --- calls.c	(revision 221805)
> +++ calls.c	(working copy)
> @@ -1321,6 +1321,15 @@ initialize_argument_information (int num
>  		  && TREE_CODE (base) != SSA_NAME
>  		  && (!DECL_P (base) || MEM_P (DECL_RTL (base)))))
>  	    {
> +	      /* We may have turned the parameter value into an SSA name.
> +		 Go back to the original parameter so we can take the
> +		 address.  */
> +	      if (TREE_CODE (args[i].tree_value) == SSA_NAME)
> +		{
> +		  gcc_assert (SSA_NAME_IS_DEFAULT_DEF (args[i].tree_value));
> +		  args[i].tree_value = SSA_NAME_VAR (args[i].tree_value);
> +		  gcc_assert (TREE_CODE (args[i].tree_value) == PARM_DECL);
> +		}
>  	      /* Argument setup code may have copied the value to register.  We
>  		 revert that optimization now because the tail call code must
>  		 use the original location.  */

	Jakub

> On Wed, Apr 01, 2015 at 08:32:04AM +0200, Jan Hubicka wrote:
> > this patch solves ICE in the attached testcase on mingw32.  The problem is that
> > on Windows API long double is passed & returned by reference and while expanidng
> > the tunk tail call, we get lost because we turn the parameter into SSA name and
> > later need its address to pass it further.
> > 
> > The patch extends hack https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00423.html
> > to handle not only non-registers but also registers.
> > 
> > Bootstrapped/regtested ppc64-linux. OK?
> 
> I think it might be better to handle thunks the same way as other tail
> calls, but
> --- gcc/cgraphunit.c	2015-04-03 15:32:31.000000000 +0200
> +++ gcc/cgraphunit.c	2015-04-07 13:02:16.537723725 +0200
> @@ -1767,7 +1767,17 @@ cgraph_node::expand_thunk (bool output_a
>  
>  	  /* Build return value.  */
>  	  if (!DECL_BY_REFERENCE (resdecl))
> -	    ret = gimple_build_return (restmp);
> +	    {
> +	      if (is_gimple_reg_type (restype)
> +		  && aggregate_value_p (resdecl, TREE_TYPE (thunk_fndecl)))
> +		{
> +		  gimple stmt = gimple_build_assign (resdecl, restmp);
> +		  gsi_insert_after (&bsi, stmt, GSI_NEW_STMT);
> +		  ret = gimple_build_return (resdecl);
> +		}
> +	      else
> +		ret = gimple_build_return (restmp);
> +	    }
>  	  else
>  	    ret = gimple_build_return (resdecl);
> (which generates the same GIMPLE code as say
> long double func1 (long double x);
> 
> long double
> func2 (long double x)
> {
>   return func1 (x);
> }
> on both mingw and x86_64-linux) doesn't really work, because calls.c treats
> calls in thunks differently from other calls - only in the thunks it takes
> ADDR_EXPR of the parameter/result.  Thus your patch is ok

Yes, unforutnately thunk tail calls are quite special because we know we can
actually use the space occupied by incomming parameters because the signature
is not changed.

Implementing this trick in general may be nice, but with current gimple representation
not exposing ABI details very well it would be hard.
> for trunk, but please add the testcase into the testsuite.

Will do.
> 
> > 	PR ipa/65540
> > 	* calls.c (initialize_argument_information): When producing tail
> > 	call also turn SSA_NAMES passed by references to original PARM_DECLs
> > Index: calls.c
> > ===================================================================
> > --- calls.c	(revision 221805)
> > +++ calls.c	(working copy)
> > @@ -1321,6 +1321,15 @@ initialize_argument_information (int num
> >  		  && TREE_CODE (base) != SSA_NAME
> >  		  && (!DECL_P (base) || MEM_P (DECL_RTL (base)))))
> >  	    {
> > +	      /* We may have turned the parameter value into an SSA name.
> > +		 Go back to the original parameter so we can take the
> > +		 address.  */
> > +	      if (TREE_CODE (args[i].tree_value) == SSA_NAME)
> > +		{
> > +		  gcc_assert (SSA_NAME_IS_DEFAULT_DEF (args[i].tree_value));
> > +		  args[i].tree_value = SSA_NAME_VAR (args[i].tree_value);
> > +		  gcc_assert (TREE_CODE (args[i].tree_value) == PARM_DECL);
> > +		}
> >  	      /* Argument setup code may have copied the value to register.  We
> >  		 revert that optimization now because the tail call code must
> >  		 use the original location.  */
> 
> 	Jakub