question about REG_PARM_STACK_SPACE usage in expand_call

On 13-12-13 06:43, Jeff Law wrote:
>> Was this meant perhaps?
>> ...
>>        || (reg_parm_stack_space != REG_PARM_STACK_SPACE
>> (current_function_decl))
> I think you're probably right.
>
> sibcall/tailcall basically re-use the current function's stack

Jeff,

Right, say we have function a tail-calling function b. If the sibcall 
optimization is done, function a deallocates it's stack frame before calling 
function b.

> so if there's a
> difference between REG_PARM_STACK_SPACE (fndecl) and REG_PARM_STACK_SPACE
> (current_function_decl), then we can't perform a sibcall/tailcall optimization.
>
> So the pattern that we want to check a property of fndecl and
> current_function_decl and if they don't match, then don't do a sibcall is
> repeated in a few places.
>

I wonder if OUTGOING_REG_PARM_STACK_SPACE makes a difference here.

If OUTGOING_REG_PARM_STACK_SPACE == 0, it is the responsibility of the callee to 
allocate the area reserved for arguments passed in registers. AFAIU, both 
functions a and b would do that in their own stack frame, and there's no need to 
test for reg_parm_stack_space != REG_PARM_STACK_SPACE (current_function_decl).

If OUTGOING_REG_PARM_STACK_SPACE != 0, it is the responsibility of the caller to 
allocate the area reserved for arguments passed in registers. Which means that 
function a and b share the space allocated by the caller of function a.
AFAIU, what is required is reg_parm_stack_space <= REG_PARM_STACK_SPACE 
(current_function_decl).

So this might be a more precise fix:
...
...

But probably not a stage3 fix.

> Assuming you bootstrap & test successfully, consider a patch which fixes this
> pre-approved.
>

Bootstrapped and reg-tested attached patch on x86_64. Committed to trunk.

Thanks,
- Tom

> jeff

On Sat, Dec 14, 2013 at 09:14:34PM +0100, Tom de Vries wrote:
> I wonder if OUTGOING_REG_PARM_STACK_SPACE makes a difference here.
> 
> If OUTGOING_REG_PARM_STACK_SPACE == 0, it is the responsibility of
> the callee to allocate the area reserved for arguments passed in
> registers. AFAIU, both functions a and b would do that in their own
> stack frame, and there's no need to test for reg_parm_stack_space !=
> REG_PARM_STACK_SPACE (current_function_decl).
> 
> If OUTGOING_REG_PARM_STACK_SPACE != 0, it is the responsibility of
> the caller to allocate the area reserved for arguments passed in
> registers. Which means that function a and b share the space
> allocated by the caller of function a.
> AFAIU, what is required is reg_parm_stack_space <=
> REG_PARM_STACK_SPACE (current_function_decl).

Hi Tom, I happened to be looking at this code a few weeks ago as part
of the PowerPC64 ELFv2 ABI work.  I missed seeing the fndecl /
current_function_decl bug, but came to the same conclusion as
you do above regarding reg_parm_stack_space.  In fact, I think you can
go a little further.  Not all changes in OUTGOING_REG_PARM_STACK_SPACE
are bad.  If the current function has OUTGOING_REG_PARM_STACK_SPACE
non-zero then a sibcall to a function with
OUTGOING_REG_PARM_STACK_SPACE zero ought to be OK.  So..

 #ifdef REG_PARM_STACK_SPACE
-      /* If outgoing reg parm stack space changes, we can not do sibcall.  */
-      || (OUTGOING_REG_PARM_STACK_SPACE (funtype)
-	  != OUTGOING_REG_PARM_STACK_SPACE (TREE_TYPE (current_function_decl)))
-      || (reg_parm_stack_space != REG_PARM_STACK_SPACE (fndecl))
+      || (OUTGOING_REG_PARM_STACK_SPACE (funtype)
+	  && (!OUTGOING_REG_PARM_STACK_SPACE (TREE_TYPE (current_function_decl))
+	      || (reg_parm_stack_space
+		  > REG_PARM_STACK_SPACE (current_function_decl))))
 #endif

On 15-12-13 04:37, Alan Modra wrote:
> On Sat, Dec 14, 2013 at 09:14:34PM +0100, Tom de Vries wrote:
>> I wonder if OUTGOING_REG_PARM_STACK_SPACE makes a difference here.
>>
>> If OUTGOING_REG_PARM_STACK_SPACE == 0, it is the responsibility of
>> the callee to allocate the area reserved for arguments passed in
>> registers. AFAIU, both functions a and b would do that in their own
>> stack frame, and there's no need to test for reg_parm_stack_space !=
>> REG_PARM_STACK_SPACE (current_function_decl).
>>
>> If OUTGOING_REG_PARM_STACK_SPACE != 0, it is the responsibility of
>> the caller to allocate the area reserved for arguments passed in
>> registers. Which means that function a and b share the space
>> allocated by the caller of function a.
>> AFAIU, what is required is reg_parm_stack_space <=
>> REG_PARM_STACK_SPACE (current_function_decl).
>
> Hi Tom, I happened to be looking at this code a few weeks ago as part
> of the PowerPC64 ELFv2 ABI work.  I missed seeing the fndecl /
> current_function_decl bug, but came to the same conclusion as
> you do above regarding reg_parm_stack_space.  In fact, I think you can
> go a little further.  Not all changes in OUTGOING_REG_PARM_STACK_SPACE
> are bad.  If the current function has OUTGOING_REG_PARM_STACK_SPACE
> non-zero then a sibcall to a function with
> OUTGOING_REG_PARM_STACK_SPACE zero ought to be OK.  So..
>
>   #ifdef REG_PARM_STACK_SPACE
> -      /* If outgoing reg parm stack space changes, we can not do sibcall.  */
> -      || (OUTGOING_REG_PARM_STACK_SPACE (funtype)
> -	  != OUTGOING_REG_PARM_STACK_SPACE (TREE_TYPE (current_function_decl)))
> -      || (reg_parm_stack_space != REG_PARM_STACK_SPACE (fndecl))
> +      || (OUTGOING_REG_PARM_STACK_SPACE (funtype)
> +	  && (!OUTGOING_REG_PARM_STACK_SPACE (TREE_TYPE (current_function_decl))
> +	      || (reg_parm_stack_space
> +		  > REG_PARM_STACK_SPACE (current_function_decl))))
>   #endif
>

Hi Alan,

I agree.

FWIW, AFAIU you could take it even further with IPA. Say you have function a 
tail-calling function b, and function b tail-calling function c, and 
OUTGOING_REG_PARM_STACK_SPACEs 1 (a), 0 (b), and 1 (c).

The snippet above allows the call to function b to be a sibling call, but not 
the call to function c.

Still the call to function c could be a sibling call, if the call to function b 
is translated as a sibling call. The space needed by function c is then provided 
by the caller of function a (if REG_PARM_STACK_SPACE (c) <= REG_PARM_STACK_SPACE 
(a)).

Thanks,
  Tom

On 12/14/13 13:14, Tom de Vries wrote:
> I wonder if OUTGOING_REG_PARM_STACK_SPACE makes a difference here.
If I'm reading the archives correctly is the caller/callee may have 
different ABIs (by way of sysv/ms_abi attributes).  Presumably there's 
some magic to deal with the differences in the ABIs to make that work.

So, at least in theory you could have a caller/callee which differ 
materially in OUTGOING_REG_PARM_STACK_SPACE (in particular I'd be 
worried anytime the caller's value is less than the callee's value.





> If OUTGOING_REG_PARM_STACK_SPACE == 0, it is the responsibility of the
> callee to allocate the area reserved for arguments passed in registers.
> AFAIU, both functions a and b would do that in their own stack frame,
> and there's no need to test for reg_parm_stack_space !=
> REG_PARM_STACK_SPACE (current_function_decl).
Seems reasonable.  I probably came to similar conclusions when I first 
wrote the tail/sibling call stuff.  I'd certainly want to hear from Jan 
and/or Kai since they added the change and presumably would have some 
state on exactly what these cross-abi calls can do.

>
> If OUTGOING_REG_PARM_STACK_SPACE != 0, it is the responsibility of the
> caller to allocate the area reserved for arguments passed in registers.
> Which means that function a and b share the space allocated by the
> caller of function a.
> AFAIU, what is required is reg_parm_stack_space <= REG_PARM_STACK_SPACE
> (current_function_decl).
>
> So this might be a more precise fix:
[ ... ]
> ...
>
> But probably not a stage3 fix.
Agreed on both statements.

jeff