Message ID | 52ACBC2A.6070806@mentor.com |
---|---|
State | New |
Headers | show |
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
2013-12-14 Tom de Vries <tom@codesourcery.com> * calls.c (expand_call): Fix REG_PARM_STACK_SPACE comparison. diff --git a/gcc/calls.c b/gcc/calls.c index 2226e78..501474b 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -2595,7 +2595,7 @@ expand_call (tree exp, rtx target, int ignore) /* 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)) + || (reg_parm_stack_space != REG_PARM_STACK_SPACE (current_function_decl)) #endif /* Check whether the target is able to optimize the call into a sibcall. */