Re: PATCH for sibcalls on i386

[Zack Weinberg <zack at codesourcery dot com>]

On Mon, Sep 23, 2002 at 08:03:20PM +1000, Fergus Henderson wrote:
>
> P.S.  If you are concerned about not changing the target machine interface,
> then another way of handling it would be to add a new target macro
> CALL_OK_FOR_SIBCALL(exp) which would be used for indirect calls.
> That way the FUNCTION_OK_FOR_SIBCALL macro could remain unchanged,
> and the existing non-x86 targets could remain unchanged, you'd just
> need to add a default definition for it, and a definition for x86.
> 
> In fact thinking about it a bit more, this does seem like a better approach.

I'm not convinced.  Look at the code Andreas wrote for
i386_function_ok_for_sibcall: 

> + int
> + i386_function_ok_for_sibcall (decl, exp)
> +      tree decl, exp;
> + {
> +   /* Check whether it's legal to optimize this _direct_ call.  */
> +   if (decl)
> +     {
> +       if ((! flag_pic || ! TREE_PUBLIC (decl))
> + 	  && (! TARGET_FLOAT_RETURNS_IN_80387
> + 	      || ! FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (TREE_TYPE (decl))))
> + 	      || FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (TREE_TYPE (cfun->decl))))))
> + 	return 1;
> +     }
> +   /* Check whether it's legal to optimize this _indirect_ call.  */
> +   /* TODO: currently only 32 bit targets are supported by the machine
> +            descriptions.  */
> +   else
> +     {
> +       if (! TARGET_64BIT
> + 	  && (! flag_pic
> + 	      && (! TARGET_FLOAT_RETURNS_IN_80387
> + 		  || ! FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (exp)))
> + 		  || FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (TREE_TYPE (cfun->decl)))))))
> + 	return 1;
> +     }
> +   
> +   /* Function call can not be optimized.  */
> +   warning ("function not ok for tail call optimization");
> + 
> +   return 0;
> + }

The conditionals are practically identical.  And the type of the call
expression should always be the same as the return type of the called
function (this needs verifying), so you could collapse this down to

int
i386_function_ok_for_sibcall (decl, exp)
     tree decl, exp;
{
  /* If we are generating position-independent code, we cannot sibcall
     optimize any indirect call, or a direct call to a global
     function, as the PLT requires %ebx be live.  */
  if (flag_pic && (!decl || !TREE_PUBLIC (decl)))
    return 0;

  /* If we are returning floats on the 80387 register stack, we cannot
     make a sibcall from a function that doesn't return a float to a
     function that does; the necessary stack adjustment will not be
     executed.  */
  if (TARGET_FLOAT_RETURNS_IN_80387
      && FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (exp)))
      && !FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (TREE_TYPE (cfun->decl)))))
    return 0;

  /* TODO: Indirect sibcalls are not yet supported by the 64-bit call
     patterns.  */
  if (!decl && TARGET_64BIT)
    return 0;

  /* Otherwise okay.  */
  return 1;
}

If we had separate CALL_OK_FOR_SIBCALL and FUNCTION_OK_FOR_SIBCALL
hooks, that would force duplication of most of this logic.

I'm in agreement with all your other criticisms.

> So, for the next iteration:
> 	- delete the #ifdef __i386 test in calls.c
> 	- make `exp' a parameter to FUNCTION_OK_FOR_SIBCALL
> 	- update the documentation for FUNCTION_OK_FOR_SIBCALL in tm.texi
> 	- update all the definitions of FUNCTION_OK_FOR_SIBCALL
> 	  for the different targets, to accept (and ignore) the exp
> 	  parameter, and to return false if fndecl is null
> 	- update the patch for the gcc-3_4-basic-improvements branch
> 	- and then repost the patch

Andreas, may I suggest that you split the patch?  It will be easier
for people to review it, which means it will get committed faster, if
you make these changes in three separate stages.

First make the mechanical changes necessary to add the 'exp' parameter
to FUNCTION_OK_FOR_SIBCALL, and update the documentation.  Do not
change calls.c or any of the definitions at this stage.  Please
consider turning FUNCTION_OK_FOR_SIBCALL into a "target hook" with
this patch, too.

Second, change all the definitions of FUNCTION_OK_FOR_SIBCALL to
return false if fndecl is null, remove the hardwired 'decl != 0' check
from calls.c, and again update the documentation.

Third, change the i386 machine description and FUNCTION_OK_FOR_SIBCALL
hook to accept indirect sibcalls when possible.

zw