[PATCH 1/3] Introduce can_implement_as_sibling_call_p

[Kyrill Tkachov]

Hi David,

On 17/05/16 23:01, David Malcolm wrote:
> This patch moves part of the logic for determining if tail
> call optimizations are possible to a new helper function.
>
> There are no functional changes.
>
> expand_call is 1300 lines long, so there's arguably a
> case for doing this on its own, but this change also
> enables the followup patch.
>
> The patch changes the logic from a big "if" with joined
> || clauses:
>
>    if (first_problem ()
>        ||second_problem ()
>        /* ...etc... */
>        ||final_problem ())
>       try_tail_call = 0;
>
> to a series of separate tests:
>
>    if (first_problem ())
>      return false;
>    if (second_problem ())
>      return false;
>    /* ...etc... */
>    if (final_problem ())
>      return false;
>
> I think the latter form has several advantages over the former:
> - IMHO it's easier to read
> - it makes it easy to put breakpoints on individual causes of failure
> - it makes it easy to put specific error messages on individual causes
>    of failure (as done in the followup patch).
>
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/ChangeLog:
> 	* calls.c (expand_call): Move "Rest of purposes for tail call
> 	optimizations to fail" to...
> 	(can_implement_as_sibling_call_p): ...this new function, and
> 	split into multiple "if" statements.
> ---
>   gcc/calls.c | 114 ++++++++++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 76 insertions(+), 38 deletions(-)
>
> diff --git a/gcc/calls.c b/gcc/calls.c
> index 6cc1fc7..ac8092c 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -2344,6 +2344,78 @@ avoid_likely_spilled_reg (rtx x)
>     return x;
>   }
>   
> +/* Helper function for expand_call.
> +   Return false is EXP is not implementable as a sibling call.  */
> +
> +static bool
> +can_implement_as_sibling_call_p (tree exp,
> +				 rtx structure_value_addr,
> +				 tree funtype,
> +				 int reg_parm_stack_space,
> +				 tree fndecl,
> +				 int flags,
> +				 tree addr,
> +				 const args_size &args_size)
> +{
> +  if (!targetm.have_sibcall_epilogue ())
> +    return false;
> +
> +  /* Doing sibling call optimization needs some work, since
> +     structure_value_addr can be allocated on the stack.
> +     It does not seem worth the effort since few optimizable
> +     sibling calls will return a structure.  */
> +  if (structure_value_addr != NULL_RTX)
> +    return false;
> +
> +#ifdef REG_PARM_STACK_SPACE
> +  /* If outgoing reg parm stack space changes, we can not do sibcall.  */
> +  if (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)))
> +    return false;
> +#endif
> +

REG_PARM_STACK_SPACE is not defined on arm, which makes reg_parm_stack_space
unused in this function and so breaks bootstrap on arm.
Can you please add an ATTRIBUTE_UNUSED to reg_parm_stack_space?

Thanks,
Kyrill

> +  /* Check whether the target is able to optimize the call
> +     into a sibcall.  */
> +  if (!targetm.function_ok_for_sibcall (fndecl, exp))
> +    return false;
> +
> +  /* Functions that do not return exactly once may not be sibcall
> +     optimized.  */
> +  if (flags & (ECF_RETURNS_TWICE | ECF_NORETURN))
> +    return false;
> +
> +  if (TYPE_VOLATILE (TREE_TYPE (TREE_TYPE (addr))))
> +    return false;
> +
> +  /* If the called function is nested in the current one, it might access
> +     some of the caller's arguments, but could clobber them beforehand if
> +     the argument areas are shared.  */
> +  if (fndecl && decl_function_context (fndecl) == current_function_decl)
> +    return false;
> +
> +  /* If this function requires more stack slots than the current
> +     function, we cannot change it into a sibling call.
> +     crtl->args.pretend_args_size is not part of the
> +     stack allocated by our caller.  */
> +  if (args_size.constant > (crtl->args.size - crtl->args.pretend_args_size))
> +    return false;
> +
> +  /* If the callee pops its own arguments, then it must pop exactly
> +     the same number of arguments as the current function.  */
> +  if (targetm.calls.return_pops_args (fndecl, funtype, args_size.constant)
> +      != targetm.calls.return_pops_args (current_function_decl,
> +					 TREE_TYPE (current_function_decl),
> +					 crtl->args.size))
> +    return false;
> +
> +  if (!lang_hooks.decls.ok_for_sibcall (fndecl))
> +    return false;
> +
> +  /* All checks passed.  */
> +  return true;
> +}
> +
>   /* Generate all the code for a CALL_EXPR exp
>      and return an rtx for its value.
>      Store the value in TARGET (specified as an rtx) if convenient.
> @@ -2740,44 +2812,10 @@ expand_call (tree exp, rtx target, int ignore)
>       try_tail_call = 0;
>   
>     /*  Rest of purposes for tail call optimizations to fail.  */
> -  if (!try_tail_call
> -      || !targetm.have_sibcall_epilogue ()
> -      /* Doing sibling call optimization needs some work, since
> -	 structure_value_addr can be allocated on the stack.
> -	 It does not seem worth the effort since few optimizable
> -	 sibling calls will return a structure.  */
> -      || structure_value_addr != NULL_RTX
> -#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 (current_function_decl))
> -#endif
> -      /* Check whether the target is able to optimize the call
> -	 into a sibcall.  */
> -      || !targetm.function_ok_for_sibcall (fndecl, exp)
> -      /* Functions that do not return exactly once may not be sibcall
> -	 optimized.  */
> -      || (flags & (ECF_RETURNS_TWICE | ECF_NORETURN))
> -      || TYPE_VOLATILE (TREE_TYPE (TREE_TYPE (addr)))
> -      /* If the called function is nested in the current one, it might access
> -	 some of the caller's arguments, but could clobber them beforehand if
> -	 the argument areas are shared.  */
> -      || (fndecl && decl_function_context (fndecl) == current_function_decl)
> -      /* If this function requires more stack slots than the current
> -	 function, we cannot change it into a sibling call.
> -	 crtl->args.pretend_args_size is not part of the
> -	 stack allocated by our caller.  */
> -      || args_size.constant > (crtl->args.size
> -			       - crtl->args.pretend_args_size)
> -      /* If the callee pops its own arguments, then it must pop exactly
> -	 the same number of arguments as the current function.  */
> -      || (targetm.calls.return_pops_args (fndecl, funtype, args_size.constant)
> -	  != targetm.calls.return_pops_args (current_function_decl,
> -					     TREE_TYPE (current_function_decl),
> -					     crtl->args.size))
> -      || !lang_hooks.decls.ok_for_sibcall (fndecl))
> -    try_tail_call = 0;
> +  if (try_tail_call)
> +    try_tail_call = can_implement_as_sibling_call_p (exp, structure_value_addr, funtype,
> +						     reg_parm_stack_space, fndecl,
> +						     flags, addr, args_size);
>   
>     /* Check if caller and callee disagree in promotion of function
>        return value.  */