Fix ICE with thunks taking argument passed by reference

Hi,
PR63573 is about ICE when expanding thunk call for function taking as a
parameter structure passed by reference.  This structure in fact contains only
one integer and thus it is promoted to register by argument setup in function.c
(as an optimization). This is an sensible optimization, but when expanding the tailcall
we need memory location to store the value into to pass it again by reference.
Because we lost the original memory location we ICE because we have addressable flag
set on decl whose DECL_RTL is register.

This patch fixes it up by reverting to original memory location in calls.c.
This is of course not safe in general because the register value may be
hcanged, but the path is executed only when THUNK flag of the call statement is
set and that is set only in thunks where the values are not updated. Ugly hack
but I can not think of better way to fix the ugly hacks already in there to make
thunk expansion happen in tailcall.

This fixed boostrap on ppc64-linux and the set of testsuite failures match ones before
the bug was introduced.

OK?

	PR bootstrap/63573
	* calls.c (initialize_argument_information): When emitting thunk call use original memory
	placement of the argument.

On Thu, 6 Nov 2014, Jan Hubicka wrote:

> Hi,
> PR63573 is about ICE when expanding thunk call for function taking as a
> parameter structure passed by reference.  This structure in fact contains only
> one integer and thus it is promoted to register by argument setup in function.c
> (as an optimization). This is an sensible optimization, but when expanding the tailcall
> we need memory location to store the value into to pass it again by reference.
> Because we lost the original memory location we ICE because we have addressable flag
> set on decl whose DECL_RTL is register.
> 
> This patch fixes it up by reverting to original memory location in calls.c.
> This is of course not safe in general because the register value may be
> hcanged, but the path is executed only when THUNK flag of the call statement is
> set and that is set only in thunks where the values are not updated. Ugly hack
> but I can not think of better way to fix the ugly hacks already in there to make
> thunk expansion happen in tailcall.
> 
> This fixed boostrap on ppc64-linux and the set of testsuite failures match ones before
> the bug was introduced.
> 
> OK?
> 
> 	PR bootstrap/63573
> 	* calls.c (initialize_argument_information): When emitting thunk call use original memory
> 	placement of the argument.
> Index: calls.c
> ===================================================================
> --- calls.c	(revision 216942)
> +++ calls.c	(working copy)
> @@ -1210,6 +1211,15 @@ initialize_argument_information (int num
>  		  && TREE_CODE (base) != SSA_NAME
>  		  && (!DECL_P (base) || MEM_P (DECL_RTL (base)))))
>  	    {
> +	      /* Argument setup code may have copied the value to register.  We
> +		 that optimization now because the tail call code must use
> +		 the original location.  */

The last sentence probably misses some words.

> +	      if (TREE_CODE (args[i].tree_value) == PARM_DECL
> +		  && !MEM_P (DECL_RTL (args[i].tree_value))
> +		  && DECL_INCOMING_RTL (args[i].tree_value)
> +		  && MEM_P (DECL_INCOMING_RTL (args[i].tree_value)))
> +		set_decl_rtl (args[i].tree_value, DECL_INCOMING_RTL (args[i].tree_value));

Please fix the long line.

I don't know calls.c so no further comments or approval from me.

Sorry,
Richard.

On 11/06/14 03:36, Jan Hubicka wrote:
> Hi,
> PR63573 is about ICE when expanding thunk call for function taking as a
> parameter structure passed by reference.  This structure in fact contains only
> one integer and thus it is promoted to register by argument setup in function.c
> (as an optimization). This is an sensible optimization, but when expanding the tailcall
> we need memory location to store the value into to pass it again by reference.
> Because we lost the original memory location we ICE because we have addressable flag
> set on decl whose DECL_RTL is register.
>
> This patch fixes it up by reverting to original memory location in calls.c.
> This is of course not safe in general because the register value may be
> hcanged, but the path is executed only when THUNK flag of the call statement is
> set and that is set only in thunks where the values are not updated. Ugly hack
> but I can not think of better way to fix the ugly hacks already in there to make
> thunk expansion happen in tailcall.
>
> This fixed boostrap on ppc64-linux and the set of testsuite failures match ones before
> the bug was introduced.
>
> OK?
>
> 	PR bootstrap/63573
> 	* calls.c (initialize_argument_information): When emitting thunk call use original memory
> 	placement of the argument.
> Index: calls.c
> ===================================================================
> --- calls.c	(revision 216942)
> +++ calls.c	(working copy)
> @@ -1210,6 +1211,15 @@ initialize_argument_information (int num
>   		  && TREE_CODE (base) != SSA_NAME
>   		  && (!DECL_P (base) || MEM_P (DECL_RTL (base)))))
>   	    {
> +	      /* Argument setup code may have copied the value to register.  We
> +		 that optimization now because the tail call code must use
> +		 the original location.  */
Second sentence in comment doesn't parse :-)  Figure you meant "We undo" 
or "We revert"

OK with comment fixed.  And yes, this is clearly a hack.

Jeff