public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: PATCH for sibcalls on i386
@ 2002-09-30  9:21 John David Anglin
  2002-09-30 13:29 ` Hans-Peter Nilsson
  0 siblings, 1 reply; 26+ messages in thread
From: John David Anglin @ 2002-09-30  9:21 UTC (permalink / raw)
  To: gcc-patches

> Perhaps, in view of the comment,
> 
>         /* Sibcalls, stubs, and elf sections don't play well.  */
> 
> the macro should be named TARGET_HAS_STUBS_AND_ELF_SECTIONS
> rather than TARGET_PA_LINUX?

That seems reasonable.  Sibcalls are not ok on hppa64-hp-hpux* for
the same reason.  This was expressed as "! TARGET_64BIT" in the define
for FUNCTION_OK_FOR_SIBCALL in pa.h.  This should be changed to
use the above macro if the patch is approved.

All the new sibcall tests fail on hppa64-hp-hpux*.  The last two
fail on hppa-linux.  For sibcall-1.c, I see that the statement
"Self-recursion tail calls are optimized for all targets, regardless
of presence of sibcall patterns." is incorrect when it comes to
hppa64-hp-hpux*, although it does happen under hppa-linux.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-30  9:21 PATCH for sibcalls on i386 John David Anglin
@ 2002-09-30 13:29 ` Hans-Peter Nilsson
  2002-09-30 14:08   ` John David Anglin
  0 siblings, 1 reply; 26+ messages in thread
From: Hans-Peter Nilsson @ 2002-09-30 13:29 UTC (permalink / raw)
  To: John David Anglin; +Cc: gcc-patches

On Mon, 30 Sep 2002, John David Anglin wrote:
> All the new sibcall tests fail on hppa64-hp-hpux*.  The last two
> fail on hppa-linux.  For sibcall-1.c, I see that the statement
> "Self-recursion tail calls are optimized for all targets, regardless
> of presence of sibcall patterns." is incorrect when it comes to
> hppa64-hp-hpux*, although it does happen under hppa-linux.

I think it's just the "are optimized for all targets" bit that's
not correct.  Would you agree with "Self-recursion tail call
optimization happen regardless of presence of sibcall patterns"?
(That's what I saw.)

If you don't agree, why not (as in "how comes that a sibcall
pattern matter in your case")?

brgds, H-P

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-30 13:29 ` Hans-Peter Nilsson
@ 2002-09-30 14:08   ` John David Anglin
  2002-09-30 15:00     ` Hans-Peter Nilsson
  2002-09-30 17:06     ` Richard Henderson
  0 siblings, 2 replies; 26+ messages in thread
From: John David Anglin @ 2002-09-30 14:08 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: gcc-patches

> If you don't agree, why not (as in "how comes that a sibcall
> pattern matter in your case")?

I've been looking and I don't have a full explanation yet.  I see in
expand call that try_tail_call is 0 and try_tail_recursion is 1.  It
looks like try_tail_call == 1 is necessary for sibcall generation
(see line 2623 in calls.c).  try_tail_call is 0 because
FUNCTION_OK_FOR_SIBCALL always is 0 for TARGET_64BIT.  What I don't
understand is why we get a sibcall on hppa-linux which defines
FUNCTION_OK_FOR_SIBCALL to be 0.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-30 14:08   ` John David Anglin
@ 2002-09-30 15:00     ` Hans-Peter Nilsson
  2002-09-30 17:06     ` Richard Henderson
  1 sibling, 0 replies; 26+ messages in thread
From: Hans-Peter Nilsson @ 2002-09-30 15:00 UTC (permalink / raw)
  To: John David Anglin; +Cc: gcc-patches

On Mon, 30 Sep 2002, John David Anglin wrote:
> > If you don't agree, why not (as in "how comes that a sibcall
> > pattern matter in your case")?
>
> I've been looking and I don't have a full explanation yet.  I see in
> expand call that try_tail_call is 0 and try_tail_recursion is 1.  It
> looks like try_tail_call == 1 is necessary for sibcall generation
> (see line 2623 in calls.c).  try_tail_call is 0 because
> FUNCTION_OK_FOR_SIBCALL always is 0 for TARGET_64BIT.

Seems like a bug.  For the test-case, some well-placed "should"s
should (!) fix its misconception.

brgds, H-P

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-30 14:08   ` John David Anglin
  2002-09-30 15:00     ` Hans-Peter Nilsson
@ 2002-09-30 17:06     ` Richard Henderson
  2002-09-30 17:44       ` John David Anglin
  1 sibling, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2002-09-30 17:06 UTC (permalink / raw)
  To: John David Anglin; +Cc: Hans-Peter Nilsson, gcc-patches

On Mon, Sep 30, 2002 at 05:07:58PM -0400, John David Anglin wrote:
> I've been looking and I don't have a full explanation yet.  I see in
> expand call that try_tail_call is 0 and try_tail_recursion is 1.  It
> looks like try_tail_call == 1 is necessary for sibcall generation
> (see line 2623 in calls.c).  try_tail_call is 0 because
> FUNCTION_OK_FOR_SIBCALL always is 0 for TARGET_64BIT.  What I don't
> understand is why we get a sibcall on hppa-linux which defines
> FUNCTION_OK_FOR_SIBCALL to be 0.

Do you get a sibcall?  try_tail_recursion does not rely on
the sibcall machinery, and is independent of it.  It should
not be shut off by FUNCTION_OK_FOR_SIBCALL.


r~

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-30 17:06     ` Richard Henderson
@ 2002-09-30 17:44       ` John David Anglin
  0 siblings, 0 replies; 26+ messages in thread
From: John David Anglin @ 2002-09-30 17:44 UTC (permalink / raw)
  To: Richard Henderson; +Cc: hp, gcc-patches

> Do you get a sibcall?  try_tail_recursion does not rely on

No.

> the sibcall machinery, and is independent of it.  It should
> not be shut off by FUNCTION_OK_FOR_SIBCALL.

The main difference between the 32-bit ports (e.g., hppa-linux)
and hppa64-hpux is that calls on hppa64-hpux always pass a pointer
to the first argument on on the stack (arg8) as one of the arguments.
Possibly, this is the reason that there is no sibcall.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
       [not found] <no.id>
@ 2002-09-30 21:03 ` John David Anglin
  0 siblings, 0 replies; 26+ messages in thread
From: John David Anglin @ 2002-09-30 21:03 UTC (permalink / raw)
  To: John David Anglin; +Cc: rth, hp, gcc-patches

> > the sibcall machinery, and is independent of it.  It should
> > not be shut off by FUNCTION_OK_FOR_SIBCALL.
> 
> The main difference between the 32-bit ports (e.g., hppa-linux)
> and hppa64-hpux is that calls on hppa64-hpux always pass a pointer
> to the first argument on on the stack (arg8) as one of the arguments.
> Possibly, this is the reason that there is no sibcall.

This appears to be the reason why we don't get recursive tail calls
on hppa64-hpux.  sequence_uses_addressof returns nonzero because
current_function_internal_arg_pointer is found.  This causes
no_sibcalls_this_function to be set.  Thus, I think that all the
sibcall tests need to be xfailed for this target.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-30  1:43             ` Andreas Bauer
@ 2002-09-30  8:11               ` Fergus Henderson
  0 siblings, 0 replies; 26+ messages in thread
From: Fergus Henderson @ 2002-09-30  8:11 UTC (permalink / raw)
  To: Andreas Bauer; +Cc: Richard Henderson, gcc-patches, pizka, jason.ozolins

On 30-Sep-2002, Andreas Bauer <baueran@in.tum.de> wrote:
> > > +++ pa/pa-linux.h	25 Sep 2002 04:01:52 -0000
> > > @@ -189,3 +185,7 @@ Boston, MA 02111-1307, USA.  */
> > >  /* Linux always uses gas.  */
> > >  #undef TARGET_GAS
> > >  #define TARGET_GAS 1
> > > +
> > > +/* Sibcalls, stubs, and elf sections don't play well.  */
> > > +#undef TARGET_FUNCTION_OK_FOR_SIBCALL
> > > +#define TARGET_FUNCTION_OK_FOR_SIBCALL hook_tree_tree_bool_false
> > [...]
> > > +++ pa/pa.c	25 Sep 2002 04:02:18 -0000
> > > @@ -194,6 +195,9 @@ static size_t n_deferred_plabels = 0;
> > >  #undef TARGET_STRIP_NAME_ENCODING
> > >  #define TARGET_STRIP_NAME_ENCODING pa_strip_name_encoding
> > >  
> > > +#undef TARGET_FUNCTION_OK_FOR_SIBCALL
> > > +#define TARGET_FUNCTION_OK_FOR_SIBCALL pa_function_ok_for_sibcall
> > 
> > The pa-linux definition is no longer in effect.
> 
> What would be the appropriate way to express that it's not ok to do any
> sibcalls when the target is pa-linux?  I'm sure pa.c needs to be changed
> as it now contains the definition for pa_function_ok_for_sibcall, but I'm
> hesitant towards How.
> 
> Something like this, inside pa_function_ok_for_sibcall, would probably
> work:
> 
>    if (TARGET_PA_LINUX)
>       return false;

Well, you could put something like

    #define TARGET_PA_LINUX 1

in config/pa/pa-linux.h, and put

    #ifdef TARGET_PA_LINUX
       return false;
    #endif

in config/pa/pa.c (together with some appropriate comments in both cases).

That would have the desired effect of preserving the current behaviour.
Maybe that is the right thing to do for now.

However, I'm not really satisfied with this as a long term solution,
because testing for TARGET_PA_LINUX seems like the wrong thing to do.
It should instead be testing for some lower-level property.

Perhaps, in view of the comment,

	/* Sibcalls, stubs, and elf sections don't play well.  */

the macro should be named TARGET_HAS_STUBS_AND_ELF_SECTIONS
rather than TARGET_PA_LINUX?

-- 
Fergus Henderson <fjh@cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: < http://www.cs.mu.oz.au/~fjh >  |     -- the last words of T. S. Garp.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-25 16:47           ` Richard Henderson
@ 2002-09-30  1:43             ` Andreas Bauer
  2002-09-30  8:11               ` Fergus Henderson
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Bauer @ 2002-09-30  1:43 UTC (permalink / raw)
  To: Richard Henderson, gcc-patches, pizka, jason.ozolins

Sorry, for a late reply, but incorporating comments and criticism into
the patch took me longer than expected.

> > +++ pa/pa-linux.h	25 Sep 2002 04:01:52 -0000
> > @@ -189,3 +185,7 @@ Boston, MA 02111-1307, USA.  */
> >  /* Linux always uses gas.  */
> >  #undef TARGET_GAS
> >  #define TARGET_GAS 1
> > +
> > +/* Sibcalls, stubs, and elf sections don't play well.  */
> > +#undef TARGET_FUNCTION_OK_FOR_SIBCALL
> > +#define TARGET_FUNCTION_OK_FOR_SIBCALL hook_tree_tree_bool_false
> [...]
> > +++ pa/pa.c	25 Sep 2002 04:02:18 -0000
> > @@ -194,6 +195,9 @@ static size_t n_deferred_plabels = 0;
> >  #undef TARGET_STRIP_NAME_ENCODING
> >  #define TARGET_STRIP_NAME_ENCODING pa_strip_name_encoding
> >  
> > +#undef TARGET_FUNCTION_OK_FOR_SIBCALL
> > +#define TARGET_FUNCTION_OK_FOR_SIBCALL pa_function_ok_for_sibcall
> 
> The pa-linux definition is no longer in effect.

What would be the appropriate way to express that it's not ok to do any
sibcalls when the target is pa-linux?  I'm sure pa.c needs to be changed
as it now contains the definition for pa_function_ok_for_sibcall, but I'm
hesitant towards How.

Something like this, inside pa_function_ok_for_sibcall, would probably
work:

   if (TARGET_PA_LINUX)
      return false;

Any hints would be greatly appreciated.

Cheers,
Andi.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-24 22:24         ` Andreas Bauer
  2002-09-24 23:28           ` Fergus Henderson
@ 2002-09-25 16:47           ` Richard Henderson
  2002-09-30  1:43             ` Andreas Bauer
  1 sibling, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2002-09-25 16:47 UTC (permalink / raw)
  To: Andreas Bauer; +Cc: gcc-patches, pizka, jason.ozolins

On Wed, Sep 25, 2002 at 03:28:51PM +1000, Andreas Bauer wrote:
> 	* calls.c (expand_call): Removed the `no indirect check'
> 	for sibcall optimization, and replaced it by making use
> 	of the new target hook "function_ok_for_sibcall".
> 	* hooks.c: Added function hook_tree_tree_bool_false to
> 	disable all sibcall optimization on all targets by
> 	default, so that certain targets can later override this
> 	setting with their individual hooks.
> 	* hooks.h: Declared hook_tree_tree_bool_false.
> 	* target-def.h: Defined and added TARGET_FUNCTION_OK_FOR_SIBCALL
> 	to TARGET_INITIALIZER
> 	* target.h: Declared the function_ok_for_sibcall
> 	function.

Again, note that changelogs should describe "what" not "why".  Thus

	* calls.c (expand_call): Remove the `no indirect check'
	for sibcall optimization; use function_ok_for_sibcall
	target hook.
	* hooks.c (hook_tree_tree_bool_false): New.
	* hooks.h (hook_tree_tree_bool_false): Declare.
	* target-def.h (TARGET_FUNCTION_OK_FOR_SIBCALL): New.
	(TARGET_INITIALIZER): Add it.
	* target.h (struct gcc_target): Add function_ok_for_sibcall.

"Why" belongs in comments inside the code.

> +/* Return true if a function is ok to be called as a sibcall.  */
> +static int
> +frv_function_ok_for_sibcall (decl, exp)
> +     tree decl;
> +     tree exp;
> +{
> +  return 0;
>  }

Note that this is identical to the default hook.

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

They aren't implemented _anywhere_ yet.  Your x86 code won't
have been merged yet.

> +++ pa/pa-linux.h	25 Sep 2002 04:01:52 -0000
> @@ -189,3 +185,7 @@ Boston, MA 02111-1307, USA.  */
>  /* Linux always uses gas.  */
>  #undef TARGET_GAS
>  #define TARGET_GAS 1
> +
> +/* Sibcalls, stubs, and elf sections don't play well.  */
> +#undef TARGET_FUNCTION_OK_FOR_SIBCALL
> +#define TARGET_FUNCTION_OK_FOR_SIBCALL hook_tree_tree_bool_false
[...]
> +++ pa/pa.c	25 Sep 2002 04:02:18 -0000
> @@ -194,6 +195,9 @@ static size_t n_deferred_plabels = 0;
>  #undef TARGET_STRIP_NAME_ENCODING
>  #define TARGET_STRIP_NAME_ENCODING pa_strip_name_encoding
>  
> +#undef TARGET_FUNCTION_OK_FOR_SIBCALL
> +#define TARGET_FUNCTION_OK_FOR_SIBCALL pa_function_ok_for_sibcall

The pa-linux definition is no longer in effect.

> +static int
> +rs6000_function_ok_for_sibcall (fndecl, exp)
>      tree fndecl;
> +    tree exp;

Need ATTRIBUTE_UNUSED.  Likewise for the other functions you added.
In general, you'll want to configure a cross-compiler for each target
you touch, build cc1 and look for warnings you may have introduced.

> +static int
> +xtensa_function_ok_for_sibcall (decl, exp)
> +     tree decl;
> +     tree exp;
> +{
> +  return 0;
> +}

Default hook.


r~

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-24 22:24         ` Andreas Bauer
@ 2002-09-24 23:28           ` Fergus Henderson
  2002-09-25 16:47           ` Richard Henderson
  1 sibling, 0 replies; 26+ messages in thread
From: Fergus Henderson @ 2002-09-24 23:28 UTC (permalink / raw)
  To: Andreas Bauer; +Cc: Richard Henderson, gcc-patches, pizka, jason.ozolins

On 25-Sep-2002, Andreas Bauer <baueran@in.tum.de> wrote:

> Index: alpha/alpha.c
...
> +static int
> +alpha_function_ok_for_sibcall (decl, exp)
> +     tree decl;
> +     tree exp;

That should be

     tree exp ATTRIBUTE_UNUSED;

to avoid warnings about unused parameters.
Likewise for the other targets.

Also the return type here should be bool, not int, to match the prototype in
target.h.  Likewise for the other targets.

Most of your changes to config/i386/i386.[ch] don't belong in this patch.
Just do the same as you did with the other targets, and leave the
additional changes for i386 to a separate patch.

> Index: sh/sh.c
...
> +/* FIXME: This is overly conservative.  A SHcompact function that
> +   receives arguments ``by reference'' will have them stored in its
> +   own stack frame, so it must not pass pointers or references to
> +   these arguments to other functions by means of sibling calls.  */
> +static int
> +sh_function_ok_for_sibcall (decl, exp)
> +     tree decl;
> +     tree exp;
> +{
> +  if (! TARGET_SHCOMPACT || current_function_args_info.stack_regs == 0)
> +    return 1;
> +  else
> +    return 0;
> +}

This should check that decl != NULL.

> Index: target.h
> ===================================================================
> RCS file: /cvsroot/gcc/gcc/gcc/target.h,v
> retrieving revision 1.33.2.3
> diff -u -p -r1.33.2.3 target.h
> --- target.h	17 Sep 2002 22:58:47 -0000	1.33.2.3
> +++ target.h	25 Sep 2002 04:51:03 -0000
> @@ -244,6 +244,10 @@ struct gcc_target
>       not, at the current point in the compilation.  */
>    bool (* cannot_modify_jumps_p) PARAMS ((void));
>  
> +  /* True if function defined in either the function declaration, or
> +     pointed to via function pointer is ok to be sibcall optimized.  */
> +  bool (*function_ok_for_sibcall) PARAMS ((tree, tree));

The declaration here should include parameter names.
The documentation should document the meaning of the parameters.
E.g.

  /* True if it is OK to do sibling call optimization for the specified
     call expression EXP.  FNDECL will be the called function,
     or NULL if this is an indirect call.  */
  bool (*function_ok_for_sibcall) PARAMS ((tree fndecl, tree exp));

> @@ -2443,17 +2439,12 @@ expand_call (exp, target, ignore)
>  	 It does not seem worth the effort since few optimizable
>  	 sibling calls will return a structure.  */
>        || structure_value_addr != NULL_RTX
> -      /* If the register holding the address is a callee saved
> -	 register, then we lose.  We have no way to prevent that,
> -	 so we only allow calls to named functions.  */
> -      /* ??? This could be done by having the insn constraints
> -	 use a register class that is all call-clobbered.  Any
> -	 reload insns generated to fix things up would appear
> -	 before the sibcall_epilogue.  */
> -      || fndecl == NULL_TREE
> +      /* Check for indirect calls.  Not all targets have register
> +	 classes to support sibcall optimization for indirect calls,
> +	 but some like ix86, do.  */
> +      || (*targetm.function_ok_for_sibcall) (fndecl, exp)
>        || (flags & (ECF_RETURNS_TWICE | ECF_LONGJMP))
> -      || TREE_THIS_VOLATILE (fndecl)
> -      || !FUNCTION_OK_FOR_SIBCALL (fndecl)
> +      || (fndecl != NULL_TREE && TREE_THIS_VOLATILE (fndecl))

The check for no-return functions also needs to be done for indirect calls.

For indirect calls, it needs to look at the volatile attribute on the
type of the function.

-- 
Fergus Henderson <fjh@cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 22:24       ` Richard Henderson
@ 2002-09-24 22:24         ` Andreas Bauer
  2002-09-24 23:28           ` Fergus Henderson
  2002-09-25 16:47           ` Richard Henderson
  0 siblings, 2 replies; 26+ messages in thread
From: Andreas Bauer @ 2002-09-24 22:24 UTC (permalink / raw)
  To: Richard Henderson, gcc-patches, pizka, jason.ozolins

[-- Attachment #1: Type: text/plain, Size: 3817 bytes --]

Dear all,

I have been following Richard's and Zack's insightful comments and points
of criticism and want to submit a new patch incorporating these new ideas.

Both patches are attached and the ChangeLog for these is in clear text
(as required from the page "Contributing to GCC").  The first entry for
"gcc.diff" reflects changes made inside the gcc/ directory:

ChangeLog:

2002-09-25  Andreas Bauer  <baueran@in.tum.de>

	* calls.c (expand_call): Removed the `no indirect check'
	for sibcall optimization, and replaced it by making use
	of the new target hook "function_ok_for_sibcall".
	* hooks.c: Added function hook_tree_tree_bool_false to
	disable all sibcall optimization on all targets by
	default, so that certain targets can later override this
	setting with their individual hooks.
	* hooks.h: Declared hook_tree_tree_bool_false.
	* target-def.h: Defined and added TARGET_FUNCTION_OK_FOR_SIBCALL
	to TARGET_INITIALIZER
	* target.h: Declared the function_ok_for_sibcall
	function.

The second entry for "config.diff" reflects all changes made inside the
config/ directory:

ChangeLog:

2002-09-25  Andreas Bauer  <baueran@in.tum.de>

	* alpha.c: Added static function alpha_function_ok_for_sibcall
	and according TARGET_FUNCTION_OK_FOR_SIBCALL definition.
	* alpha.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
        * i386.c: Added static function ix86_function_ok_for_sibcall
        and according TARGET_FUNCTION_OK_FOR_SIBCALL definition. 
        * i386.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
        * i386.c: Added static function ix86_function_ok_for_sibcall
        and according TARGET_FUNCTION_OK_FOR_SIBCALL definition. 
        * sparc.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
        * sparc.c: Added static function sparc_function_ok_for_sibcall
        and according TARGET_FUNCTION_OK_FOR_SIBCALL definition. 
        * arm.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
        * arm.c: Made function arm_function_ok_for_sibcall static and
	accept yet another parameter.
	* arm-protos.h: Removed definition for arm_function_ok_for_sibcall.
        * rs6000.c: Added static function rs6000_function_ok_for_sibcall
	and according TARGET_FUNCTION_OK_FOR_SIBCALL definition.
	* rs6000.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
	* rs6000-protos.h: Removed definition for function_ok_for_sibcall.
        * pa.c: Added static function pa_function_ok_for_sibcall
        and according TARGET_FUNCTION_OK_FOR_SIBCALL definition. 
        * pa.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
	* pa-linux.h: Removed FUNCTION_OK_FOR_SIBCALL macro and replaced
	it with by defining TARGET_FUNCTION_OK_FOR_SIBCALL to
	hook_tree_tree_bool_false.
        * sh.c: Added static function sh_function_ok_for_sibcall
        and according TARGET_FUNCTION_OK_FOR_SIBCALL definition. 
        * sh.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
        * xtensia.c: Added static function xtensia_function_ok_for_sibcall
        and according TARGET_FUNCTION_OK_FOR_SIBCALL definition. 
        * xtensia.h: Removed FUNCTION_OK_FOR_SIBCALL macro.
        * frv.c: Added static function frv_function_ok_for_sibcall
        and according TARGET_FUNCTION_OK_FOR_SIBCALL definition. 
        * frv.h: Removed FUNCTION_OK_FOR_SIBCALL macro.

I would appreciate feedback on the two patches very much.  I hope, I got
everything complete and working this time.  There is, however, one issue
I am not 100% convinced I did right:  pa-linux.h used to redefine
FUNCTION_OK_FOR_SIBCALL to `false' and I simply redefined
TARGET_FUNCTION_OK_FOR_SIBCALL to hook_tree_tree_bool_false which also
resolves to `false'.  Was that ok?

Other than that, could someone point out how I should update the
documentation to reflect these rather substantial changes?  All comments
and ideas are highly appreciated.

Thank you in advance.

Cheers,
Andi.

[-- Attachment #2: config.diff --]
[-- Type: text/plain, Size: 28489 bytes --]

Index: alpha/alpha.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/alpha/alpha.c,v
retrieving revision 1.272.2.3
diff -u -p -r1.272.2.3 alpha.c
--- alpha/alpha.c	20 Sep 2002 01:29:08 -0000	1.272.2.3
+++ alpha/alpha.c	25 Sep 2002 03:59:03 -0000
@@ -118,6 +118,8 @@ int alpha_this_literal_sequence_number;
 int alpha_this_gpdisp_sequence_number;
 
 /* Declarations of static functions.  */
+static int alpha_function_ok_for_sibcall
+  PARAMS ((tree, tree));
 static int tls_symbolic_operand_1
   PARAMS ((rtx, enum machine_mode, int, int));
 static enum tls_model tls_symbolic_operand_type
@@ -292,6 +294,9 @@ static void unicosmk_unique_section PARA
 #undef  TARGET_EXPAND_BUILTIN
 #define TARGET_EXPAND_BUILTIN alpha_expand_builtin
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL alpha_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 /* Parse target option strings.  */
@@ -2267,6 +2272,22 @@ alpha_legitimize_address (x, scratch, mo
 
     return plus_constant (x, low);
   }
+}
+
+/* We do not allow indirect calls to be optimized into sibling calls, nor
+   can we allow a call to a function in a different compilation unit to
+   be optimized into a sibcall.  */
+static int
+alpha_function_ok_for_sibcall (decl, exp)
+     tree decl;
+     tree exp;
+{
+  if (decl
+      && (! TREE_PUBLIC (decl)
+	  || (TREE_ASM_WRITTEN (decl) && (*targetm.binds_local_p) (decl))))
+    return 1;
+  else
+    return 0;
 }
 
 /* For TARGET_EXPLICIT_RELOCS, we don't obfuscate a SYMBOL_REF to a
Index: alpha/alpha.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/alpha/alpha.h,v
retrieving revision 1.176.4.7
diff -u -p -r1.176.4.7 alpha.h
--- alpha/alpha.h	23 Sep 2002 04:38:44 -0000	1.176.4.7
+++ alpha/alpha.h	25 Sep 2002 03:59:12 -0000
@@ -1165,14 +1165,6 @@ extern int alpha_memory_latency;
     }									\
 }
 
-/* We do not allow indirect calls to be optimized into sibling calls, nor
-   can we allow a call to a function in a different compilation unit to
-   be optimized into a sibcall.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL)			\
-  (DECL							\
-   && (! TREE_PUBLIC (DECL)				\
-       || (TREE_ASM_WRITTEN (DECL) && (*targetm.binds_local_p) (DECL))))
-
 /* Try to output insns to set TARGET equal to the constant C if it can be
    done in less than N insns.  Do all computations in MODE.  Returns the place
    where the output has been placed if it can be done and the insns have been
Index: arm/arm-protos.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/arm/arm-protos.h,v
retrieving revision 1.29.8.2
diff -u -p -r1.29.8.2 arm-protos.h
--- arm/arm-protos.h	17 Sep 2002 22:58:53 -0000	1.29.8.2
+++ arm/arm-protos.h	25 Sep 2002 03:59:14 -0000
@@ -41,7 +41,6 @@ extern unsigned int  arm_compute_initial
 #ifdef TREE_CODE
 extern int    arm_return_in_memory	PARAMS ((tree));
 extern void   arm_encode_call_attribute	PARAMS ((tree, int));
-extern int    arm_function_ok_for_sibcall PARAMS ((tree));
 #endif
 #ifdef RTX_CODE
 extern int    arm_hard_regno_mode_ok	PARAMS ((unsigned int, enum machine_mode));
Index: arm/arm.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.223.2.5
diff -u -p -r1.223.2.5 arm.c
--- arm/arm.c	20 Sep 2002 01:29:09 -0000	1.223.2.5
+++ arm/arm.c	25 Sep 2002 03:59:48 -0000
@@ -117,6 +117,7 @@ static void	 arm_set_default_type_attrib
 static int	 arm_adjust_cost		PARAMS ((rtx, rtx, rtx, int));
 static int	 count_insns_for_constant	PARAMS ((HOST_WIDE_INT, int));
 static int	 arm_get_strip_length		PARAMS ((int));
+static int       arm_function_ok_for_sibcall    PARAMS ((tree, tree));
 #ifdef OBJECT_FORMAT_ELF
 static void	 arm_elf_asm_named_section	PARAMS ((const char *, unsigned int));
 #endif
@@ -192,6 +193,9 @@ static void	 arm_internal_label		PARAMS 
 #undef TARGET_ASM_INTERNAL_LABEL
 #define TARGET_ASM_INTERNAL_LABEL arm_internal_label
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL arm_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 /* Obstack for minipool constant handling.  */
@@ -2266,9 +2270,10 @@ arm_is_longcall_p (sym_ref, call_cookie,
 
 /* Return nonzero if it is ok to make a tail-call to DECL.  */
 
-int
-arm_function_ok_for_sibcall (decl)
+static int
+arm_function_ok_for_sibcall (decl, exp)
      tree decl;
+     tree exp;
 {
   int call_type = TARGET_LONG_CALLS ? CALL_LONG : CALL_NORMAL;
 
Index: arm/arm.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.155.4.5
diff -u -p -r1.155.4.5 arm.h
--- arm/arm.h	20 Sep 2002 01:29:10 -0000	1.155.4.5
+++ arm/arm.h	25 Sep 2002 04:00:00 -0000
@@ -1502,12 +1502,6 @@ typedef struct
 #define FUNCTION_ARG_REGNO_P(REGNO)	(IN_RANGE ((REGNO), 0, 3))
 
 \f
-/* Tail calling.  */
-
-/* A C expression that evaluates to true if it is ok to perform a sibling
-   call to DECL.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) arm_function_ok_for_sibcall ((DECL))
-
 /* Perform any actions needed for a function that is receiving a variable
    number of arguments.  CUM is as above.  MODE and TYPE are the mode and type
    of the current parameter.  PRETEND_SIZE is a variable that should be set to
Index: frv/frv.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/frv/frv.c,v
retrieving revision 1.2.10.3
diff -u -p -r1.2.10.3 frv.c
--- frv/frv.c	20 Sep 2002 01:29:12 -0000	1.2.10.3
+++ frv/frv.c	25 Sep 2002 04:00:30 -0000
@@ -279,6 +279,7 @@ static void frv_encode_section_info		PAR
 static void frv_init_builtins			PARAMS ((void));
 static rtx frv_expand_builtin			PARAMS ((tree, rtx, rtx, enum machine_mode, int));
 static bool frv_in_small_data_p			PARAMS ((tree));
+static int  frv_function_ok_for_sibcall         PARAMS ((tree, tree));
 \f
 /* Initialize the GCC target structure.  */
 #undef  TARGET_ASM_FUNCTION_PROLOGUE
@@ -297,6 +298,8 @@ static bool frv_in_small_data_p			PARAMS
 #define TARGET_EXPAND_BUILTIN frv_expand_builtin
 #undef TARGET_IN_SMALL_DATA_P
 #define TARGET_IN_SMALL_DATA_P frv_in_small_data_p
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL frv_function_ok_for_sibcall
 
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
@@ -9773,4 +9776,13 @@ frv_in_small_data_p (decl)
 
   return symbol_ref_small_data_p (XEXP (DECL_RTL (decl), 0))
     && size > 0 && size <= g_switch_value;
+}
+
+/* Return true if a function is ok to be called as a sibcall.  */
+static int
+frv_function_ok_for_sibcall (decl, exp)
+     tree decl;
+     tree exp;
+{
+  return 0;
 }
Index: frv/frv.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/frv/frv.h,v
retrieving revision 1.3.2.5
diff -u -p -r1.3.2.5 frv.h
--- frv/frv.h	20 Sep 2002 01:29:12 -0000	1.3.2.5
+++ frv/frv.h	25 Sep 2002 04:00:48 -0000
@@ -3539,9 +3539,6 @@ frv_ifcvt_modify_multiple_tests (CE_INFO
    scheduling.  */
 #define FIRST_CYCLE_MULTIPASS_SCHEDULING_LOOKAHEAD frv_sched_lookahead
 
-/* Return true if a function is ok to be called as a sibcall.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) 0
-
 enum frv_builtins
 {
   FRV_BUILTIN_MAND,
Index: i386/i386.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/i386/i386.c,v
retrieving revision 1.447.2.3
diff -u -p -r1.447.2.3 i386.c
--- i386/i386.c	17 Sep 2002 22:58:57 -0000	1.447.2.3
+++ i386/i386.c	25 Sep 2002 04:01:35 -0000
@@ -742,6 +742,7 @@ static int ix86_save_reg PARAMS ((unsign
 static void ix86_compute_frame_layout PARAMS ((struct ix86_frame *));
 static int ix86_comp_type_attributes PARAMS ((tree, tree));
 const struct attribute_spec ix86_attribute_table[];
+static int ix86_function_ok_for_sibcall PARAMS ((tree, tree));
 static tree ix86_handle_cdecl_attribute PARAMS ((tree *, tree, tree, int, bool *));
 static tree ix86_handle_regparm_attribute PARAMS ((tree *, tree, tree, int, bool *));
 static int ix86_value_regno PARAMS ((enum machine_mode));
@@ -843,6 +844,9 @@ static enum x86_64_reg_class merge_class
 #define TARGET_SCHED_FIRST_CYCLE_MULTIPASS_DFA_LOOKAHEAD \
   ia32_multipass_dfa_lookahead
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL ix86_function_ok_for_sibcall
+
 #ifdef HAVE_AS_TLS
 #undef TARGET_HAVE_TLS
 #define TARGET_HAVE_TLS true
@@ -1286,6 +1290,41 @@ const struct attribute_spec ix86_attribu
 #endif
   { NULL,        0, 0, false, false, false, NULL }
 };
+
+/* If PIC, we cannot make sibling calls to global functions
+   because the PLT requires %ebx live.
+   If we are returning floats on the register stack, we cannot make
+   sibling calls to functions that return floats.  (The stack adjust
+   instruction will wind up after the sibcall jump, and not be executed.)  */
+
+static int
+ix86_function_ok_for_sibcall (decl, exp)
+     tree decl;
+     tree 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;
+}
 
 /* Handle a "cdecl" or "stdcall" attribute;
    arguments as in struct attribute_spec.handler.  */
Index: i386/i386.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/i386/i386.h,v
retrieving revision 1.280.4.5
diff -u -p -r1.280.4.5 i386.h
--- i386/i386.h	23 Sep 2002 04:38:45 -0000	1.280.4.5
+++ i386/i386.h	25 Sep 2002 04:01:49 -0000
@@ -1674,17 +1674,9 @@ typedef struct ix86_args {
 
 #define FUNCTION_ARG_PARTIAL_NREGS(CUM, MODE, TYPE, NAMED) 0
 
-/* If PIC, we cannot make sibling calls to global functions
-   because the PLT requires %ebx live.
-   If we are returning floats on the register stack, we cannot make
-   sibling calls to functions that return floats.  (The stack adjust
-   instruction will wind up after the sibcall jump, and not be executed.) */
-#define FUNCTION_OK_FOR_SIBCALL(DECL)					\
-  ((DECL)								\
-   && (! 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))))))
+/* Checks whether the call can be sibcall optimized.  More information
+   can be found in i386.c  */
+/* #define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) ix86_function_ok_for_sibcall ((DECL, EXP)) */
 
 /* Perform any needed actions needed for a function that is receiving a
    variable number of arguments.
Index: pa/pa-linux.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa-linux.h,v
retrieving revision 1.24.2.1
diff -u -p -r1.24.2.1 pa-linux.h
--- pa/pa-linux.h	2 Sep 2002 02:54:02 -0000	1.24.2.1
+++ pa/pa-linux.h	25 Sep 2002 04:01:52 -0000
@@ -81,10 +81,6 @@ Boston, MA 02111-1307, USA.  */
       %{!dynamic-linker:-dynamic-linker /lib/ld.so.1}} \
       %{static:-static}}"
 
-/* Sibcalls, stubs, and elf sections don't play well.  */
-#undef FUNCTION_OK_FOR_SIBCALL
-#define FUNCTION_OK_FOR_SIBCALL(x) 0
-
 /* glibc's profiling functions don't need gcc to allocate counters.  */
 #define NO_PROFILE_COUNTERS 1
 
@@ -189,3 +185,7 @@ Boston, MA 02111-1307, USA.  */
 /* Linux always uses gas.  */
 #undef TARGET_GAS
 #define TARGET_GAS 1
+
+/* Sibcalls, stubs, and elf sections don't play well.  */
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL hook_tree_tree_bool_false
Index: pa/pa.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.c,v
retrieving revision 1.177.2.3
diff -u -p -r1.177.2.3 pa.c
--- pa/pa.c	17 Sep 2002 22:59:03 -0000	1.177.2.3
+++ pa/pa.c	25 Sep 2002 04:02:18 -0000
@@ -116,6 +116,7 @@ static void pa_select_section PARAMS ((t
      ATTRIBUTE_UNUSED;
 static void pa_encode_section_info PARAMS ((tree, int));
 static const char *pa_strip_name_encoding PARAMS ((const char *));
+static int pa_function_ok_for_sibcall PARAMS ((tree, tree));
 static void pa_globalize_label PARAMS ((FILE *, const char *))
      ATTRIBUTE_UNUSED;
 
@@ -194,6 +195,9 @@ static size_t n_deferred_plabels = 0;
 #undef TARGET_STRIP_NAME_ENCODING
 #define TARGET_STRIP_NAME_ENCODING pa_strip_name_encoding
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL pa_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 void
@@ -6631,6 +6635,43 @@ pa_asm_output_mi_thunk (file, thunk_fnde
       function_section (thunk_fndecl);
     }
   current_thunk_number++;
+}
+
+/* Only direct calls to static functions are allowed to be sibling (tail)
+   call optimized.
+
+   This restriction is necessary because some linker generated stubs will
+   store return pointers into rp' in some cases which might clobber a
+   live value already in rp'.
+
+   In a sibcall the current function and the target function share stack
+   space.  Thus if the path to the current function and the path to the
+   target function save a value in rp', they save the value into the
+   same stack slot, which has undesirable consequences.
+
+   Because of the deferred binding nature of shared libraries any function
+   with external scope could be in a different load module and thus require
+   rp' to be saved when calling that function.  So sibcall optimizations
+   can only be safe for static function.
+
+   Note that GCC never needs return value relocations, so we don't have to
+   worry about static calls with return value relocations (which require
+   saving rp').
+
+   It is safe to perform a sibcall optimization when the target function
+   will never return.  */
+static int
+pa_function_ok_for_sibcall (decl, exp)
+     tree decl;
+     tree exp;
+{
+  if (decl
+      && ! TARGET_PORTABLE_RUNTIME
+      && ! TARGET_64BIT
+      && ! TREE_PUBLIC (decl))
+    return 1;
+  else
+    return 0;
 }
 
 /* Returns 1 if the 6 operands specified in OPERANDS are suitable for
Index: pa/pa.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.h,v
retrieving revision 1.166.2.4
diff -u -p -r1.166.2.4 pa.h
--- pa/pa.h	17 Sep 2002 22:59:03 -0000	1.166.2.4
+++ pa/pa.h	25 Sep 2002 04:02:26 -0000
@@ -1831,35 +1831,6 @@ do { 									\
 /* The number of Pmode words for the setjmp buffer.  */
 #define JMP_BUF_SIZE 50
 
-/* Only direct calls to static functions are allowed to be sibling (tail)
-   call optimized.
-
-   This restriction is necessary because some linker generated stubs will
-   store return pointers into rp' in some cases which might clobber a
-   live value already in rp'.
-
-   In a sibcall the current function and the target function share stack
-   space.  Thus if the path to the current function and the path to the
-   target function save a value in rp', they save the value into the
-   same stack slot, which has undesirable consequences.
-
-   Because of the deferred binding nature of shared libraries any function
-   with external scope could be in a different load module and thus require
-   rp' to be saved when calling that function.  So sibcall optimizations
-   can only be safe for static function.
-
-   Note that GCC never needs return value relocations, so we don't have to
-   worry about static calls with return value relocations (which require
-   saving rp').
-
-   It is safe to perform a sibcall optimization when the target function
-   will never return.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) \
-  (DECL \
-   && ! TARGET_PORTABLE_RUNTIME \
-   && ! TARGET_64BIT \
-   && ! TREE_PUBLIC (DECL))
-
 #define PREDICATE_CODES							\
   {"reg_or_0_operand", {SUBREG, REG, CONST_INT}},			\
   {"call_operand_address", {LABEL_REF, SYMBOL_REF, CONST_INT,		\
Index: rs6000/rs6000-protos.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/rs6000/rs6000-protos.h,v
retrieving revision 1.43.4.1
diff -u -p -r1.43.4.1 rs6000-protos.h
--- rs6000/rs6000-protos.h	17 Sep 2002 22:59:05 -0000	1.43.4.1
+++ rs6000/rs6000-protos.h	25 Sep 2002 04:02:28 -0000
@@ -151,7 +151,6 @@ extern void setup_incoming_varargs PARAM
 					    int *, int));
 extern struct rtx_def *rs6000_va_arg PARAMS ((tree, tree));
 extern void output_mi_thunk PARAMS ((FILE *, tree, int, tree));
-extern int function_ok_for_sibcall PARAMS ((tree));
 #ifdef ARGS_SIZE_RTX
 /* expr.h defines ARGS_SIZE_RTX and `enum direction' */
 extern enum direction function_arg_padding PARAMS ((enum machine_mode, tree));
Index: rs6000/rs6000.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/rs6000/rs6000.c,v
retrieving revision 1.366.2.5
diff -u -p -r1.366.2.5 rs6000.c
--- rs6000/rs6000.c	20 Sep 2002 01:29:19 -0000	1.366.2.5
+++ rs6000/rs6000.c	25 Sep 2002 04:03:13 -0000
@@ -165,6 +165,7 @@ struct builtin_description
   const enum rs6000_builtins code;
 };
 
+static int rs6000_function_ok_for_sibcall PARAMS ((tree, tree));
 static void rs6000_add_gc_roots PARAMS ((void));
 static int num_insns_constant_wide PARAMS ((HOST_WIDE_INT));
 static void validate_condition_mode 
@@ -376,6 +377,9 @@ static const char alt_reg_names[][8] =
 /* The VRSAVE bitmask puts bit %v0 as the most significant bit.  */
 #define ALTIVEC_REG_BIT(REGNO) (0x80000000 >> ((REGNO) - FIRST_ALTIVEC_REGNO))
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL rs6000_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 /* Override command line options.  Mostly we process the processor
@@ -9403,9 +9407,10 @@ rs6000_return_addr (count, frame)
    vector parameters are required to have a prototype, so the argument
    type info must be available here.  (The tail recursion case can work
    with vector parameters, but there's no way to distinguish here.) */
-int
-function_ok_for_sibcall (fndecl)
+static int
+rs6000_function_ok_for_sibcall (fndecl, exp)
     tree fndecl;
+    tree exp;
 {
   tree type;
   if (fndecl)
Index: rs6000/rs6000.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/rs6000/rs6000.h,v
retrieving revision 1.224.4.6
diff -u -p -r1.224.4.6 rs6000.h
--- rs6000/rs6000.h	23 Sep 2002 04:38:47 -0000	1.224.4.6
+++ rs6000/rs6000.h	25 Sep 2002 04:03:27 -0000
@@ -1804,10 +1804,6 @@ typedef struct rs6000_args
    argument is passed depends on whether or not it is a named argument.  */
 #define STRICT_ARGUMENT_NAMING 1
 
-/* We do not allow indirect calls to be optimized into sibling calls, nor
-   do we allow calls with vector parameters.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) function_ok_for_sibcall ((DECL))
-
 /* Output assembler code to FILE to increment profiler label # LABELNO
    for profiling a function entry.  */
 
Index: sh/sh.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/sh/sh.c,v
retrieving revision 1.169.4.4
diff -u -p -r1.169.4.4 sh.c
--- sh/sh.c	20 Sep 2002 01:29:21 -0000	1.169.4.4
+++ sh/sh.c	25 Sep 2002 04:03:51 -0000
@@ -199,6 +199,7 @@ static void sh_insert_attributes PARAMS 
 static int sh_adjust_cost PARAMS ((rtx, rtx, rtx, int));
 static int sh_use_dfa_interface PARAMS ((void));
 static int sh_issue_rate PARAMS ((void));
+static int sh_function_ok_for_sibcall PARAMS ((tree, tree));
 
 static bool sh_cannot_modify_jumps_p PARAMS ((void));
 static bool sh_ms_bitfield_layout_p PARAMS ((tree));
@@ -259,6 +260,9 @@ static void flow_dependent_p_1 PARAMS ((
 #undef TARGET_EXPAND_BUILTIN
 #define TARGET_EXPAND_BUILTIN sh_expand_builtin
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL sh_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 /* Print the operand address in x to the stream.  */
@@ -7383,6 +7387,20 @@ sh_initialize_trampoline (tramp, fnaddr,
     }
 }
 
+/* FIXME: This is overly conservative.  A SHcompact function that
+   receives arguments ``by reference'' will have them stored in its
+   own stack frame, so it must not pass pointers or references to
+   these arguments to other functions by means of sibling calls.  */
+static int
+sh_function_ok_for_sibcall (decl, exp)
+     tree decl;
+     tree exp;
+{
+  if (! TARGET_SHCOMPACT || current_function_args_info.stack_regs == 0)
+    return 1;
+  else
+    return 0;
+}
 \f
 /* Machine specific built-in functions.  */
 
Index: sh/sh.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/sh/sh.h,v
retrieving revision 1.166.4.6
diff -u -p -r1.166.4.6 sh.h
--- sh/sh.h	23 Sep 2002 04:38:48 -0000	1.166.4.6
+++ sh/sh.h	25 Sep 2002 04:04:05 -0000
@@ -1706,13 +1706,6 @@ struct sh_args {
     (CUM).outgoing = 0;						\
   } while (0)
  
-/* FIXME: This is overly conservative.  A SHcompact function that
-   receives arguments ``by reference'' will have them stored in its
-   own stack frame, so it must not pass pointers or references to
-   these arguments to other functions by means of sibling calls.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) \
-  (! TARGET_SHCOMPACT || current_function_args_info.stack_regs == 0)
-
 /* Update the data in CUM to advance over an argument
    of mode MODE and data type TYPE.
    (TYPE is null for libcalls where that information may not be
Index: sparc/sparc.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/sparc/sparc.c,v
retrieving revision 1.226.4.4
diff -u -p -r1.226.4.4 sparc.c
--- sparc/sparc.c	20 Sep 2002 01:29:21 -0000	1.226.4.4
+++ sparc/sparc.c	25 Sep 2002 04:04:47 -0000
@@ -176,6 +176,8 @@ static void emit_soft_tfmode_cvt PARAMS 
 static void emit_hard_tfmode_operation PARAMS ((enum rtx_code, rtx *));
 
 static void sparc_encode_section_info PARAMS ((tree, int));
+
+static int sparc_function_ok_for_sibcall PARAMS ((tree, tree));
 \f
 /* Option handling.  */
 
@@ -239,6 +241,9 @@ enum processor_type sparc_cpu;
 #undef TARGET_ENCODE_SECTION_INFO
 #define TARGET_ENCODE_SECTION_INFO sparc_encode_section_info
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL sparc_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 /* Validate and override various options, and do some machine dependent
@@ -8021,6 +8026,35 @@ sparc_elf_asm_named_section (name, flags
   fputc ('\n', asm_out_file);
 }
 #endif /* OBJECT_FORMAT_ELF */
+
+/* We do not allow sibling calls if -mflat, nor
+   we do not allow indirect calls to be optimized into sibling calls.
+   
+   Also, on sparc 32-bit we cannot emit a sibling call when the
+   current function returns a structure.  This is because the "unimp
+   after call" convention would cause the callee to return to the
+   wrong place.  The generic code already disallows cases where the
+   function being called returns a structure.
+
+   It may seem strange how this last case could occur.  Usually there
+   is code after the call which jumps to epilogue code which dumps the
+   return value into the struct return area.  That ought to invalidate
+   the sibling call right?  Well, in the c++ case we can end up passing
+   the pointer to the struct return area to a constructor (which returns
+   void) and then nothing else happens.  Such a sibling call would look
+   valid without the added check here.  */
+static int
+sparc_function_ok_for_sibcall (decl, exp)
+     tree decl;
+     tree exp;
+{
+  if (decl
+      && ! TARGET_FLAT
+      && (TARGET_ARCH64 || ! current_function_returns_struct))
+    return 1;
+  else
+    return 0;
+}
 
 /* ??? Similar to the standard section selection, but force reloc-y-ness
    if SUNOS4_SHARED_LIBRARIES.  Unclear why this helps (as opposed to
Index: sparc/sparc.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/sparc/sparc.h,v
retrieving revision 1.207.4.6
diff -u -p -r1.207.4.6 sparc.h
--- sparc/sparc.h	23 Sep 2002 04:38:48 -0000	1.207.4.6
+++ sparc/sparc.h	25 Sep 2002 04:05:00 -0000
@@ -1934,27 +1934,6 @@ do {									\
 
 #define STRICT_ARGUMENT_NAMING TARGET_V9
 
-/* We do not allow sibling calls if -mflat, nor
-   we do not allow indirect calls to be optimized into sibling calls.
-
-   Also, on sparc 32-bit we cannot emit a sibling call when the
-   current function returns a structure.  This is because the "unimp
-   after call" convention would cause the callee to return to the
-   wrong place.  The generic code already disallows cases where the
-   function being called returns a structure.
-
-   It may seem strange how this last case could occur.  Usually there
-   is code after the call which jumps to epilogue code which dumps the
-   return value into the struct return area.  That ought to invalidate
-   the sibling call right?  Well, in the c++ case we can end up passing
-   the pointer to the struct return area to a constructor (which returns
-   void) and then nothing else happens.  Such a sibling call would look
-   valid without the added check here.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) \
-	(DECL \
-	 && ! TARGET_FLAT \
-	 && (TARGET_ARCH64 || ! current_function_returns_struct))
-
 /* Generate RTL to flush the register windows so as to make arbitrary frames
    available.  */
 #define SETUP_FRAME_ADDRESSES()		\
Index: xtensa/xtensa.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/xtensa/xtensa.c,v
retrieving revision 1.18.4.1
diff -u -p -r1.18.4.1 xtensa.c
--- xtensa/xtensa.c	5 Sep 2002 17:47:31 -0000	1.18.4.1
+++ xtensa/xtensa.c	25 Sep 2002 04:05:08 -0000
@@ -202,6 +202,7 @@ static unsigned int xtensa_multibss_sect
 static void xtensa_select_rtx_section
   PARAMS ((enum machine_mode, rtx, unsigned HOST_WIDE_INT));
 static void xtensa_encode_section_info PARAMS ((tree, int));
+static int xtensa_function_ok_for_sibcall PARAMS ((tree, tree));
 
 static rtx frame_size_const;
 static int current_function_arg_words;
@@ -238,6 +239,9 @@ static const int reg_nonleaf_alloc_order
 #undef TARGET_ENCODE_SECTION_INFO
 #define TARGET_ENCODE_SECTION_INFO  xtensa_encode_section_info
 
+#undef TARGET_FUNCTION_OK_FOR_SIBCALL
+#define TARGET_FUNCTION_OK_FOR_SIBCALL xtensa_function_ok_for_sibcall
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 
@@ -1624,6 +1628,16 @@ xtensa_emit_loop_end (insn, operands)
   output_asm_insn ("# loop end for %0", operands);
 }
 
+/* A C expression that evaluates to true if it is ok to perform a
+   sibling call to DECL.  */
+/* TODO: fix this up to allow at least some sibcalls */
+static int
+xtensa_function_ok_for_sibcall (decl, exp)
+     tree decl;
+     tree exp;
+{
+  return 0;
+}
 
 char *
 xtensa_emit_call (callop, operands)
Index: xtensa/xtensa.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/xtensa/xtensa.h,v
retrieving revision 1.20.4.1
diff -u -p -r1.20.4.1 xtensa.h
--- xtensa/xtensa.h	16 Sep 2002 17:38:28 -0000	1.20.4.1
+++ xtensa/xtensa.h	25 Sep 2002 04:05:15 -0000
@@ -1287,11 +1287,6 @@ typedef struct xtensa_args {
    indexing purposes) so give the MEM rtx a words's mode.  */
 #define FUNCTION_MODE SImode
 
-/* A C expression that evaluates to true if it is ok to perform a
-   sibling call to DECL.  */
-/* TODO: fix this up to allow at least some sibcalls */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) 0
-
 /* Xtensa constant costs.  */
 #define CONST_COSTS(X, CODE, OUTER_CODE)				\
   case CONST_INT:							\

[-- Attachment #3: gcc.diff --]
[-- Type: text/plain, Size: 4540 bytes --]

Index: hooks.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/hooks.c,v
retrieving revision 1.5
diff -u -p -r1.5 hooks.c
--- hooks.c	21 Aug 2002 02:41:44 -0000	1.5
+++ hooks.c	25 Sep 2002 04:51:01 -0000
@@ -62,3 +62,13 @@ hook_FILEptr_constcharptr_void (a, b)
      const char *b ATTRIBUTE_UNUSED;
 {
 }
+
+/* Hook that takes a function definition and an expression node and
+   returns false.  */
+bool
+hook_tree_tree_bool_false (a, b)
+     tree a ATTRIBUTE_UNUSED;
+     tree b ATTRIBUTE_UNUSED;
+{
+  return false;
+}
Index: hooks.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/hooks.h,v
retrieving revision 1.5
diff -u -p -r1.5 hooks.h
--- hooks.h	21 Aug 2002 02:41:44 -0000	1.5
+++ hooks.h	25 Sep 2002 04:51:01 -0000
@@ -27,5 +27,5 @@ bool hook_tree_bool_false PARAMS ((tree)
 void hook_tree_int_void PARAMS ((tree, int));
 void hook_void_void PARAMS ((void));
 void hook_FILEptr_constcharptr_void PARAMS ((FILE *, const char *));
-
+bool hook_tree_tree_bool_false PARAMS ((tree, tree));
 #endif
Index: target-def.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/target-def.h,v
retrieving revision 1.30.2.3
diff -u -p -r1.30.2.3 target-def.h
--- target-def.h	17 Sep 2002 22:58:47 -0000	1.30.2.3
+++ target-def.h	25 Sep 2002 04:51:02 -0000
@@ -245,6 +245,7 @@ Foundation, 59 Temple Place - Suite 330,
 
 /* In hook.c.  */
 #define TARGET_CANNOT_MODIFY_JUMPS_P hook_void_bool_false
+#define TARGET_FUNCTION_OK_FOR_SIBCALL hook_tree_tree_bool_false
 
 #ifndef TARGET_IN_SMALL_DATA_P
 #define TARGET_IN_SMALL_DATA_P hook_tree_bool_false
@@ -271,6 +272,7 @@ Foundation, 59 Temple Place - Suite 330,
   TARGET_EXPAND_BUILTIN,			\
   TARGET_SECTION_TYPE_FLAGS,			\
   TARGET_CANNOT_MODIFY_JUMPS_P,			\
+  TARGET_FUNCTION_OK_FOR_SIBCALL,		\
   TARGET_IN_SMALL_DATA_P,			\
   TARGET_BINDS_LOCAL_P,				\
   TARGET_ENCODE_SECTION_INFO,			\
Index: target.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/target.h,v
retrieving revision 1.33.2.3
diff -u -p -r1.33.2.3 target.h
--- target.h	17 Sep 2002 22:58:47 -0000	1.33.2.3
+++ target.h	25 Sep 2002 04:51:03 -0000
@@ -244,6 +244,10 @@ struct gcc_target
      not, at the current point in the compilation.  */
   bool (* cannot_modify_jumps_p) PARAMS ((void));
 
+  /* True if function defined in either the function declaration, or
+     pointed to via function pointer is ok to be sibcall optimized.  */
+  bool (*function_ok_for_sibcall) PARAMS ((tree, tree));
+
   /* True if EXP should be placed in a "small data" section.  */
   bool (* in_small_data_p) PARAMS ((tree));
 
Index: calls.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/calls.c,v
retrieving revision 1.231.4.5
diff -u -p -r1.231.4.5 calls.c
--- calls.c	20 Sep 2002 01:29:06 -0000	1.231.4.5
+++ calls.c	25 Sep 2002 04:51:20 -0000
@@ -36,10 +36,6 @@ Software Foundation, 59 Temple Place - S
 #include "langhooks.h"
 #include "target.h"
 
-#if !defined FUNCTION_OK_FOR_SIBCALL
-#define FUNCTION_OK_FOR_SIBCALL(DECL) 1
-#endif
-
 /* Decide whether a function's arguments should be processed
    from first to last or from last to first.
 
@@ -2443,17 +2439,12 @@ expand_call (exp, target, ignore)
 	 It does not seem worth the effort since few optimizable
 	 sibling calls will return a structure.  */
       || structure_value_addr != NULL_RTX
-      /* If the register holding the address is a callee saved
-	 register, then we lose.  We have no way to prevent that,
-	 so we only allow calls to named functions.  */
-      /* ??? This could be done by having the insn constraints
-	 use a register class that is all call-clobbered.  Any
-	 reload insns generated to fix things up would appear
-	 before the sibcall_epilogue.  */
-      || fndecl == NULL_TREE
+      /* Check for indirect calls.  Not all targets have register
+	 classes to support sibcall optimization for indirect calls,
+	 but some like ix86, do.  */
+      || (*targetm.function_ok_for_sibcall) (fndecl, exp)
       || (flags & (ECF_RETURNS_TWICE | ECF_LONGJMP))
-      || TREE_THIS_VOLATILE (fndecl)
-      || !FUNCTION_OK_FOR_SIBCALL (fndecl)
+      || (fndecl != NULL_TREE && TREE_THIS_VOLATILE (fndecl))
       /* If this function requires more stack slots than the current
 	 function, we cannot change it into a sibling call.  */
       || args_size.constant > current_function_args_size

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 21:27     ` Andreas Bauer
                         ` (2 preceding siblings ...)
  2002-09-23 22:14       ` Richard Henderson
@ 2002-09-23 22:24       ` Richard Henderson
  2002-09-24 22:24         ` Andreas Bauer
  3 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2002-09-23 22:24 UTC (permalink / raw)
  To: Andreas Bauer
  Cc: Zack Weinberg, Fergus Henderson, gcc-patches, pizka, jason.ozolins

On Tue, Sep 24, 2002 at 02:31:40PM +1000, Andreas Bauer wrote:
> > Please consider turning FUNCTION_OK_FOR_SIBCALL into a "target hook"
> > with this patch, too.
> 
> Please, send me more information on this request, if it's important or
> relevant to you.

In target.h, there is a structure struct gcc_target.  Add a 
new member

	bool (*function_ok_for_sibcall) PARAMS ((tree, tree));

just after cannot_modify_jumps_p.  In hooks.c, add a new
function

	bool
	hook_tree_tree_bool_false (a, b)
	     tree a ATTRIBUTE_UNUSED;
	     tree b ATTRIBUTE_UNUSED;
	{
	  return false;
	}

In hooks.h, declare it.  In target-def.h add

	#define TARGET_FUNCTION_OK_FOR_SIBCALL hook_tree_tree_bool_false

just after TARGET_CANNOT_MODIFY_JUMPS_P.  Do not add ifndef wrappers.  

For every target that implements FUNCTION_OK_FOR_SIBCALL, if the
implementation is inline, copy it to a new static function
<cpu>_function_ok_for_sibcall in the cpu.c file.  If the existing
implementation was a function, change it to be static and remove
the declaration in cpu-protos.h.  In either case, add 

	#undef TARGET_FUNCTION_OK_FOR_SIBCALL
	#define TARGET_FUNCTION_OK_FOR_SIBCALL cpu_function_ok_for_sibcall

in some likely looking place before

	struct gcc_target targetm = TARGET_INITIALIZER;

in the cpu.c file.

Now change calls.c to do

	(*targetm.function_ok_for_sibcall) (decl, exp)


r~

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 21:27     ` Andreas Bauer
  2002-09-23 21:37       ` Andrew Pinski
  2002-09-23 21:48       ` Zack Weinberg
@ 2002-09-23 22:14       ` Richard Henderson
  2002-09-23 22:24       ` Richard Henderson
  3 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2002-09-23 22:14 UTC (permalink / raw)
  To: Andreas Bauer
  Cc: Zack Weinberg, Fergus Henderson, gcc-patches, pizka, jason.ozolins

On Tue, Sep 24, 2002 at 02:31:40PM +1000, Andreas Bauer wrote:
> -#define FUNCTION_OK_FOR_SIBCALL(DECL) arm_function_ok_for_sibcall ((DECL))
> +#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) arm_function_ok_for_sibcall ((DECL, EXP))

This won't work.  You're calling a function of one parameter
with two parameters.


r~

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 21:27     ` Andreas Bauer
  2002-09-23 21:37       ` Andrew Pinski
@ 2002-09-23 21:48       ` Zack Weinberg
  2002-09-23 22:14       ` Richard Henderson
  2002-09-23 22:24       ` Richard Henderson
  3 siblings, 0 replies; 26+ messages in thread
From: Zack Weinberg @ 2002-09-23 21:48 UTC (permalink / raw)
  To: Andreas Bauer; +Cc: Fergus Henderson, gcc-patches, pizka, jason.ozolins

On Tue, Sep 24, 2002 at 02:31:40PM +1000, Andreas Bauer wrote:
> Hi Zack and others,
> 
> I have followed your advise and attached the first rather "mechanical"
> patch of my series of patches.  However, I did have to make a _small_
> change to calls.c in order to let it use the new interface to
> FUNCTION_OK_FOR_SIBCALL:
...
> -      || !FUNCTION_OK_FOR_SIBCALL (fndecl)
> +      || !FUNCTION_OK_FOR_SIBCALL (fndecl, exp)

Yes, that's fine.  But did you consider the target hook conversion
which I suggested earlier?

> Attached is a rather large patch which adds the 'exp' to all the targets
> in the config/ directory.  This alone, of course, does not offer the new
> functionality, but it extends the interfaces, compiles & doesn't break
> anything, and (hopefully) let's us continue with Step 2.

As Andrew pointed out, these 

> -#define FUNCTION_OK_FOR_SIBCALL(DECL) arm_function_ok_for_sibcall ((DECL))
> +#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) arm_function_ok_for_sibcall ((DECL, EXP))

are incorrect.  I don't know why the double parentheses were there in
the first place, but they should be removed.  Every time you see a
definition of this form, you need to add the second argument to the
function on the right-hand side, too.

zw

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 21:37       ` Andrew Pinski
@ 2002-09-23 21:44         ` Andreas Bauer
  0 siblings, 0 replies; 26+ messages in thread
From: Andreas Bauer @ 2002-09-23 21:44 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches, pizka, jason.ozolins

> Index: arm/arm.h
> ===================================================================
> ...
> -#define FUNCTION_OK_FOR_SIBCALL(DECL) arm_function_ok_for_sibcall 
> ((DECL))
> +#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) arm_function_ok_for_sibcall 
> ((DECL, EXP))
> ...
> and this part:
> 
> Index: rs6000/rs6000.h
> ===================================================================
> ....
> -#define FUNCTION_OK_FOR_SIBCALL(DECL) function_ok_for_sibcall ((DECL))
> +#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) function_ok_for_sibcall 
> ((DECL, EXP))
> ...
> 
> looks totally wrong, I do not you want to be passing exp to those 
> functions.

Argh, that was my cvs-mistake.  Sorry!  I will repost the patch with the
necessary changes to those files.  However, "exp" must be passed, whether
you use it or not is another question...  (Why does it matter to you?)

Andi.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 21:27     ` Andreas Bauer
@ 2002-09-23 21:37       ` Andrew Pinski
  2002-09-23 21:44         ` Andreas Bauer
  2002-09-23 21:48       ` Zack Weinberg
                         ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Andrew Pinski @ 2002-09-23 21:37 UTC (permalink / raw)
  To: Andreas Bauer
  Cc: Zack Weinberg, Fergus Henderson, gcc-patches, pizka, jason.ozolins

One major problem

This part of the patch for config:

Index: arm/arm.h
===================================================================
...
-#define FUNCTION_OK_FOR_SIBCALL(DECL) arm_function_ok_for_sibcall 
((DECL))
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) arm_function_ok_for_sibcall 
((DECL, EXP))
...
and this part:

Index: rs6000/rs6000.h
===================================================================
....
-#define FUNCTION_OK_FOR_SIBCALL(DECL) function_ok_for_sibcall ((DECL))
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) function_ok_for_sibcall 
((DECL, EXP))
...

looks totally wrong, I do not you want to be passing exp to those 
functions.


Thanks,
Andrew Pinski

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 12:25   ` Zack Weinberg
  2002-09-23 16:52     ` Andreas Bauer
  2002-09-23 17:10     ` Fergus Henderson
@ 2002-09-23 21:27     ` Andreas Bauer
  2002-09-23 21:37       ` Andrew Pinski
                         ` (3 more replies)
  2 siblings, 4 replies; 26+ messages in thread
From: Andreas Bauer @ 2002-09-23 21:27 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Andreas Bauer, Fergus Henderson, gcc-patches, pizka, jason.ozolins

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]

Hi Zack and others,

I have followed your advise and attached the first rather "mechanical"
patch of my series of patches.  However, I did have to make a _small_
change to calls.c in order to let it use the new interface to
FUNCTION_OK_FOR_SIBCALL:

Index: calls.c
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/calls.c,v
retrieving revision 1.231.4.5
diff -u -p -r1.231.4.5 calls.c
--- calls.c     20 Sep 2002 01:29:06 -0000      1.231.4.5
+++ calls.c     24 Sep 2002 04:07:43 -0000
@@ -37,7 +37,7 @@ Software Foundation, 59 Temple Place - S
 #include "target.h"
 
 #if !defined FUNCTION_OK_FOR_SIBCALL
-#define FUNCTION_OK_FOR_SIBCALL(DECL) 1
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) 1
 #endif
 
 /* Decide whether a function's arguments should be processed
@@ -2453,7 +2453,7 @@ expand_call (exp, target, ignore)
       || fndecl == NULL_TREE
       || (flags & (ECF_RETURNS_TWICE | ECF_LONGJMP))
       || TREE_THIS_VOLATILE (fndecl)
-      || !FUNCTION_OK_FOR_SIBCALL (fndecl)
+      || !FUNCTION_OK_FOR_SIBCALL (fndecl, exp)
       /* If this function requires more stack slots than the current
         function, we cannot change it into a sibling call.  */
       || args_size.constant > current_function_args_size

> 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.

Attached is a rather large patch which adds the 'exp' to all the targets
in the config/ directory.  This alone, of course, does not offer the new
functionality, but it extends the interfaces, compiles & doesn't break
anything, and (hopefully) let's us continue with Step 2.

> Please consider turning FUNCTION_OK_FOR_SIBCALL into a "target hook"
> with this patch, too.

Please, send me more information on this request, if it's important or
relevant to you.

Cheers,
Andi.

P.S. also look carefully at my documentation change, as I'm not a native
English speaker.  :-)

[-- Attachment #2: config.diff --]
[-- Type: text/plain, Size: 6964 bytes --]

Index: alpha/alpha.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/alpha/alpha.h,v
retrieving revision 1.176.4.7
diff -u -p -r1.176.4.7 alpha.h
--- alpha/alpha.h	23 Sep 2002 04:38:44 -0000	1.176.4.7
+++ alpha/alpha.h	24 Sep 2002 04:08:55 -0000
@@ -1168,7 +1168,7 @@ extern int alpha_memory_latency;
 /* We do not allow indirect calls to be optimized into sibling calls, nor
    can we allow a call to a function in a different compilation unit to
    be optimized into a sibcall.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL)			\
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP)		\
   (DECL							\
    && (! TREE_PUBLIC (DECL)				\
        || (TREE_ASM_WRITTEN (DECL) && (*targetm.binds_local_p) (DECL))))
Index: arm/arm.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.155.4.5
diff -u -p -r1.155.4.5 arm.h
--- arm/arm.h	20 Sep 2002 01:29:10 -0000	1.155.4.5
+++ arm/arm.h	24 Sep 2002 04:09:07 -0000
@@ -1506,7 +1506,7 @@ typedef struct
 
 /* A C expression that evaluates to true if it is ok to perform a sibling
    call to DECL.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) arm_function_ok_for_sibcall ((DECL))
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) arm_function_ok_for_sibcall ((DECL, EXP))
 
 /* Perform any actions needed for a function that is receiving a variable
    number of arguments.  CUM is as above.  MODE and TYPE are the mode and type
Index: frv/frv.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/frv/frv.h,v
retrieving revision 1.3.2.5
diff -u -p -r1.3.2.5 frv.h
--- frv/frv.h	20 Sep 2002 01:29:12 -0000	1.3.2.5
+++ frv/frv.h	24 Sep 2002 04:09:26 -0000
@@ -3540,7 +3540,7 @@ frv_ifcvt_modify_multiple_tests (CE_INFO
 #define FIRST_CYCLE_MULTIPASS_SCHEDULING_LOOKAHEAD frv_sched_lookahead
 
 /* Return true if a function is ok to be called as a sibcall.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) 0
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) 0
 
 enum frv_builtins
 {
Index: i386/i386.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/i386/i386.h,v
retrieving revision 1.280.4.5
diff -u -p -r1.280.4.5 i386.h
--- i386/i386.h	23 Sep 2002 04:38:45 -0000	1.280.4.5
+++ i386/i386.h	24 Sep 2002 04:09:41 -0000
@@ -1679,7 +1679,7 @@ typedef struct ix86_args {
    If we are returning floats on the register stack, we cannot make
    sibling calls to functions that return floats.  (The stack adjust
    instruction will wind up after the sibcall jump, and not be executed.) */
-#define FUNCTION_OK_FOR_SIBCALL(DECL)					\
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP)		       		\
   ((DECL)								\
    && (! flag_pic || ! TREE_PUBLIC (DECL))				\
    && (! TARGET_FLOAT_RETURNS_IN_80387					\
Index: pa/pa-linux.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa-linux.h,v
retrieving revision 1.24.2.1
diff -u -p -r1.24.2.1 pa-linux.h
--- pa/pa-linux.h	2 Sep 2002 02:54:02 -0000	1.24.2.1
+++ pa/pa-linux.h	24 Sep 2002 04:09:44 -0000
@@ -83,7 +83,7 @@ Boston, MA 02111-1307, USA.  */
 
 /* Sibcalls, stubs, and elf sections don't play well.  */
 #undef FUNCTION_OK_FOR_SIBCALL
-#define FUNCTION_OK_FOR_SIBCALL(x) 0
+#define FUNCTION_OK_FOR_SIBCALL(x, y) 0
 
 /* glibc's profiling functions don't need gcc to allocate counters.  */
 #define NO_PROFILE_COUNTERS 1
Index: pa/pa.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/pa/pa.h,v
retrieving revision 1.166.2.4
diff -u -p -r1.166.2.4 pa.h
--- pa/pa.h	17 Sep 2002 22:59:03 -0000	1.166.2.4
+++ pa/pa.h	24 Sep 2002 04:09:53 -0000
@@ -1854,7 +1854,7 @@ do { 									\
 
    It is safe to perform a sibcall optimization when the target function
    will never return.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) \
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) \
   (DECL \
    && ! TARGET_PORTABLE_RUNTIME \
    && ! TARGET_64BIT \
Index: rs6000/rs6000.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/rs6000/rs6000.h,v
retrieving revision 1.224.4.6
diff -u -p -r1.224.4.6 rs6000.h
--- rs6000/rs6000.h	23 Sep 2002 04:38:47 -0000	1.224.4.6
+++ rs6000/rs6000.h	24 Sep 2002 04:10:07 -0000
@@ -1806,7 +1806,7 @@ typedef struct rs6000_args
 
 /* We do not allow indirect calls to be optimized into sibling calls, nor
    do we allow calls with vector parameters.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) function_ok_for_sibcall ((DECL))
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) function_ok_for_sibcall ((DECL, EXP))
 
 /* Output assembler code to FILE to increment profiler label # LABELNO
    for profiling a function entry.  */
Index: sh/sh.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/sh/sh.h,v
retrieving revision 1.166.4.6
diff -u -p -r1.166.4.6 sh.h
--- sh/sh.h	23 Sep 2002 04:38:48 -0000	1.166.4.6
+++ sh/sh.h	24 Sep 2002 04:10:22 -0000
@@ -1710,7 +1710,7 @@ struct sh_args {
    receives arguments ``by reference'' will have them stored in its
    own stack frame, so it must not pass pointers or references to
    these arguments to other functions by means of sibling calls.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) \
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) \
   (! TARGET_SHCOMPACT || current_function_args_info.stack_regs == 0)
 
 /* Update the data in CUM to advance over an argument
Index: sparc/sparc.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/sparc/sparc.h,v
retrieving revision 1.207.4.6
diff -u -p -r1.207.4.6 sparc.h
--- sparc/sparc.h	23 Sep 2002 04:38:48 -0000	1.207.4.6
+++ sparc/sparc.h	24 Sep 2002 04:10:35 -0000
@@ -1950,7 +1950,7 @@ do {									\
    the pointer to the struct return area to a constructor (which returns
    void) and then nothing else happens.  Such a sibling call would look
    valid without the added check here.  */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) \
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) \
 	(DECL \
 	 && ! TARGET_FLAT \
 	 && (TARGET_ARCH64 || ! current_function_returns_struct))
Index: xtensa/xtensa.h
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/config/xtensa/xtensa.h,v
retrieving revision 1.20.4.1
diff -u -p -r1.20.4.1 xtensa.h
--- xtensa/xtensa.h	16 Sep 2002 17:38:28 -0000	1.20.4.1
+++ xtensa/xtensa.h	24 Sep 2002 04:10:43 -0000
@@ -1290,7 +1290,7 @@ typedef struct xtensa_args {
 /* A C expression that evaluates to true if it is ok to perform a
    sibling call to DECL.  */
 /* TODO: fix this up to allow at least some sibcalls */
-#define FUNCTION_OK_FOR_SIBCALL(DECL) 0
+#define FUNCTION_OK_FOR_SIBCALL(DECL, EXP) 0
 
 /* Xtensa constant costs.  */
 #define CONST_COSTS(X, CODE, OUTER_CODE)				\

[-- Attachment #3: tm.texi.diff --]
[-- Type: text/plain, Size: 886 bytes --]

Index: tm.texi
===================================================================
RCS file: /cvsroot/gcc/gcc/gcc/doc/tm.texi,v
retrieving revision 1.159.2.6
diff -u -p -r1.159.2.6 tm.texi
--- tm.texi	23 Sep 2002 00:17:16 -0000	1.159.2.6
+++ tm.texi	24 Sep 2002 04:13:08 -0000
@@ -4235,9 +4235,10 @@ the function prologue.  Normally, the pr
 
 @table @code
 @findex FUNCTION_OK_FOR_SIBCALL
-@item FUNCTION_OK_FOR_SIBCALL (@var{decl})
+@item FUNCTION_OK_FOR_SIBCALL (@var{decl}, @var{exp})
 A C expression that evaluates to true if it is ok to perform a sibling
-call to @var{decl} from the current function.
+call from the current function to a function described by @var{decl} or,
+in case of indirect calls, via the expression node @var{exp}.
 
 It is not uncommon for limitations of calling conventions to prevent
 tail calls to functions outside the current unit of translation, or

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 12:25   ` Zack Weinberg
  2002-09-23 16:52     ` Andreas Bauer
@ 2002-09-23 17:10     ` Fergus Henderson
  2002-09-23 21:27     ` Andreas Bauer
  2 siblings, 0 replies; 26+ messages in thread
From: Fergus Henderson @ 2002-09-23 17:10 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Andreas Bauer, gcc-patches, pizka, jason.ozolins

On 23-Sep-2002, Zack Weinberg <zack@codesourcery.com> wrote:
> 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: 
...
> The conditionals are practically identical.
> If we had separate CALL_OK_FOR_SIBCALL and FUNCTION_OK_FOR_SIBCALL
> hooks, that would force duplication of most of this logic.

Not really.

	#define CALL_OK_FOR_SIBCALL(exp) \
		i386_ok_for_sibcall(NULL, TREE_TYPE (exp))

	#define FUNCTION_OK_FOR_SIBCALL(fndecl) \
		i386_ok_for_sibcall(fndecl, TREE_TYPE (TREE_TYPE (fndecl)))

	...

	int
	ix86_ok_for_sibcall(fndecl, return_type)
	     ...
	{
	  ...
	}

The drawback is that the interface is a little more complicated.
The advantage is that the interface remains stable -- we're only
adding new target macros, not changing the interface of existing ones.

As a front-end maintainer, I have first-hand experience of the pain that
interface changes cause.  So I'm inclined to favour interface stability
over interface simplicity in this case.

On the other hand, if the interface is already churning, then maybe one more 
interface change isn't such a big deal ;-)

-- 
Fergus Henderson <fjh@cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 16:46   ` Andreas Bauer
@ 2002-09-23 17:09     ` Richard Henderson
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2002-09-23 17:09 UTC (permalink / raw)
  To: Andreas Bauer; +Cc: Fergus Henderson, gcc-patches, pizka, jason.ozolins

On Tue, Sep 24, 2002 at 09:50:48AM +1000, Andreas Bauer wrote:
> Ok, this is a rather huge change.  Will I feed each target as an
> individual patch?

No.  Large, but entirely mechanical patches are easy to 
deal with.  But that is why it makes it easier for us if
each patch does exactly one thing, if possible.


r~

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23 12:25   ` Zack Weinberg
@ 2002-09-23 16:52     ` Andreas Bauer
  2002-09-23 17:10     ` Fergus Henderson
  2002-09-23 21:27     ` Andreas Bauer
  2 siblings, 0 replies; 26+ messages in thread
From: Andreas Bauer @ 2002-09-23 16:52 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Andreas Bauer, Fergus Henderson, gcc-patches, pizka, jason.ozolins

> 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;
>
> [...]
>
>   /* Otherwise okay.  */
>   return 1;
> }

I agree, indeed this is much nicer.  I will incorporate the change into
my next series of patches.

> 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.

Yes, I will try to come up with smaller patches. (I'm still somewhat new
to the gcc business, sorry.)

> 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.

Sounds all good to me.  Thank you a lot for the useful comments.

Cheers,
Andi.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23  3:03 ` Fergus Henderson
  2002-09-23 12:25   ` Zack Weinberg
@ 2002-09-23 16:46   ` Andreas Bauer
  2002-09-23 17:09     ` Richard Henderson
  1 sibling, 1 reply; 26+ messages in thread
From: Andreas Bauer @ 2002-09-23 16:46 UTC (permalink / raw)
  To: Fergus Henderson; +Cc: Andreas Bauer, gcc-patches, pizka, jason.ozolins

Hi Fergus (and others),

Thank you a lot for the comments.

> It's probably best to develop such patches against the
> gcc-3_4-basic-improvements branch.

Ok.

> It would be much cleaner to do as Richard Henderson suggested,
> and check for fndecl in FUNCTION_OK_FOR_SIBCALL for all
> targets, not just x86.  For the non-x86 targets, just
> have FUNCTION_OK_FOR_SIBCALL return false if fndecl is null.

That check is already extant in FUNCTION_OK_FOR_SIBCALL, for all targets,
as far as I know.  The reason why fndecl is being checked in calls.c
again is that a few other macros expect it to have a sensible value.

> > +       /* Additional call patterns in i386.md allow i386 to do
> > + 	 optimized indirect calls.  Other platforms may/should
> > + 	 follow this example.
> > + 	 In fact, it is sufficient on i386 to merely test with the
> > + 	 FUNCTION_OK_FOR_SIBCALL macro, as it contains certain
> > + 	 conditionals for handling indirect calls.  */
> > + 
> > + #ifndef __i386
> >         || fndecl == NULL_TREE
> > + #endif
> 
> This test for __i386 is wrong.  Whether or not __i386 is defined here
> will depend on whether the host system is an x86, but whether or not
> indirect calls matter depends on whether the *target* system is an x86.
> So this will break cross-compilation.

Ouch.  I could have checked it, but didn't think of it.  :-(

I think, I was indeed to tentative here and should have removed this
check and the conditions around it at once.  Didn't occur to me at the
time.

> This warning may be useful for debugging your changes,
> but it is not suitable for inclusion in the FSF's GCC sources.

Obviously, I did not expect the patch to be approved as is.  I was aware
that some lines in it were rather naughty and that's the reason why I was
looking for advice.  Of course, such warnings would _not_ be part of
anything "official" and if people mind them, will be removed from future
patches/discussions.

> > ! #define FUNCTION_OK_FOR_SIBCALL(DECL) i386_function_ok_for_sibcall (DECL, exp)
> 
> Where did `exp' come from?
> 
> Having a macro refer to a value which happens to be in scope like this,
> without explicitly passing it or documenting that it must be in scope,
> is a very nasty coding style.  That sort of thing would be bound to
> cause trouble or confusion later.
> 
> It would be better to make `exp' a parameter to FUNCTION_OK_FOR_SIBCALL.

...which means having to change it for _all_ the targets gcc supports.
Again, I was too tentative and wanted to discuss first, before I do
something like this.

> So, for the next iteration:
> 	- delete the #ifdef __i386 test in calls.c

Ok.

> 	- 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

Ok, this is a rather huge change.  Will I feed each target as an
individual patch?

> 	- update the patch for the gcc-3_4-basic-improvements branch

Ok.

> 	- and then repost the patch

Will happily do so.

Cheers,
Andi.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-22 21:11 Andreas Bauer
  2002-09-23  3:03 ` Fergus Henderson
@ 2002-09-23 15:12 ` Richard Henderson
  1 sibling, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2002-09-23 15:12 UTC (permalink / raw)
  To: Andreas Bauer; +Cc: gcc-patches, pizka, jason.ozolins

I'll only comment on the md change, since Fergus and Zack
have adequately addressed the generic change that the proper
way to attack this problem.

On Mon, Sep 23, 2002 at 02:10:54PM +1000, Andreas Bauer wrote:
> + (define_insn "*sibcall_0"
> +   [(call (mem:QI (match_operand 0 "constant_call_address_operand" "s,c,d,a"))
> + 	 (match_operand 1 "" ""))]
> +   "SIBLING_CALL_P (insn)"
> + {
> +   if (SIBLING_CALL_P (insn))

Note that you're checking SIBLING_CALL_P here twice.  Note that
the call pattern from which you duplicated this now contains
references to SIBLING_CALL_P which should always be false.  All
of your patterns are doing this.

Note that constant_call_address_operand really means constant.
You cannot have register alternatives.

Don't be so tentative with respect to x86_64.  Yes, we _could_
do better than ecx/edx/eax, but those are a good start.

> + (define_insn "*sibcall_value_1"
> +   [(call (mem:QI (match_operand:SI 0 "call_insn_operand" "s,c,d,a"))
> + 	 (match_operand 1 "" ""))]

This is not a call value pattern.  There's no value.  Contrast
this with the sibcall_value_0 pattern which is correct.


r~

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-23  3:03 ` Fergus Henderson
@ 2002-09-23 12:25   ` Zack Weinberg
  2002-09-23 16:52     ` Andreas Bauer
                       ` (2 more replies)
  2002-09-23 16:46   ` Andreas Bauer
  1 sibling, 3 replies; 26+ messages in thread
From: Zack Weinberg @ 2002-09-23 12:25 UTC (permalink / raw)
  To: Andreas Bauer, Fergus Henderson; +Cc: gcc-patches, pizka, jason.ozolins

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: PATCH for sibcalls on i386
  2002-09-22 21:11 Andreas Bauer
@ 2002-09-23  3:03 ` Fergus Henderson
  2002-09-23 12:25   ` Zack Weinberg
  2002-09-23 16:46   ` Andreas Bauer
  2002-09-23 15:12 ` Richard Henderson
  1 sibling, 2 replies; 26+ messages in thread
From: Fergus Henderson @ 2002-09-23  3:03 UTC (permalink / raw)
  To: Andreas Bauer; +Cc: gcc-patches, pizka, jason.ozolins

Hi Andreas,

That looks like a good start, but there are still a few major issues
that need to be addressed before this patch could be acceptable.

On 23-Sep-2002, Andreas Bauer <baueran@in.tum.de> wrote:
> Attached is a patch to gcc-3.2 that adds a few new call patterns to the
> i386 machine description which allow sibcall optimisation for indirect
> calls.

It's probably best to develop such patches against the
gcc-3_4-basic-improvements branch.

> I'm overriding the "fndecl == NULL_TREE" check in "calls.c" and cover it
> in FUNCTION_OK_FOR_SIBCALL instead.  The macro will decide whether an
> indirect call is a candidate for sibcall optimisation and if so, the
> changed machine descriptions may be put into place at the end of
> compilation.  Non-i386 architectures, however, still check for fndecl in
> "calls.c", because they do not yet allow any indirect calls at all!

Putting a check for x86 in calls.c is messy -- it breaks the idea that
target-specific stuff should go in target-specific files.

It would be much cleaner to do as Richard Henderson suggested,
and check for fndecl in FUNCTION_OK_FOR_SIBCALL for all
targets, not just x86.  For the non-x86 targets, just
have FUNCTION_OK_FOR_SIBCALL return false if fndecl is null.

> So, the patch doesn't break anthing, as far as I know.

I'm afraid the patch does break things.
It breaks cross-compilation, and introduces unwanted warnings.

> I'm aware that the patch is too small to actually make a huge difference
> on gcc as a whole, but if the concept proves useful, then it could be
> applied to other architectures as well and we could remove the test
> "fndecl == NULL_TREE" totally from the file "calls.c".

You're being a bit too tentative here.
It would be better to go ahead and do this step now.

> +       /* Additional call patterns in i386.md allow i386 to do
> + 	 optimized indirect calls.  Other platforms may/should
> + 	 follow this example.
> + 	 In fact, it is sufficient on i386 to merely test with the
> + 	 FUNCTION_OK_FOR_SIBCALL macro, as it contains certain
> + 	 conditionals for handling indirect calls.  */
> + 
> + #ifndef __i386
>         || fndecl == NULL_TREE
> + #endif

This test for __i386 is wrong.  Whether or not __i386 is defined here
will depend on whether the host system is an x86, but whether or not
indirect calls matter depends on whether the *target* system is an x86.
So this will break cross-compilation.

The phrase ``elegance is not optional'' comes to mind ;-)
Doing it cleanly, in the way Richard Henderson suggested,
would avoid this bug.

> + int
> + i386_function_ok_for_sibcall (decl, exp)
> +      tree decl, exp;
> + {
...
> +   /* Function call can not be optimized.  */
> +   warning ("function not ok for tail call optimization");

This warning may be useful for debugging your changes,
but it is not suitable for inclusion in the FSF's GCC sources.

> ! #define FUNCTION_OK_FOR_SIBCALL(DECL) i386_function_ok_for_sibcall (DECL, exp)

Where did `exp' come from?

Having a macro refer to a value which happens to be in scope like this,
without explicitly passing it or documenting that it must be in scope,
is a very nasty coding style.  That sort of thing would be bound to
cause trouble or confusion later.

It would be better to make `exp' a parameter to FUNCTION_OK_FOR_SIBCALL.

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

I didn't review the changes to the `.md' file.

Cheers,
	Fergus.

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.

-- 
Fergus Henderson <fjh@cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* PATCH for sibcalls on i386
@ 2002-09-22 21:11 Andreas Bauer
  2002-09-23  3:03 ` Fergus Henderson
  2002-09-23 15:12 ` Richard Henderson
  0 siblings, 2 replies; 26+ messages in thread
From: Andreas Bauer @ 2002-09-22 21:11 UTC (permalink / raw)
  To: gcc-patches; +Cc: pizka, jason.ozolins

[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]

Dear gcc hackers,

This patch is a follow-up to a discussion I had with several people on
the main gcc list not too long ago.

    (See also http://gcc.gnu.org/ml/gcc/2002-09/threads.html#00298)

I'm trying to make sibcall optimisation more general and I'm starting out
with the very common i386 platform.

Attached is a patch to gcc-3.2 that adds a few new call patterns to the
i386 machine description which allow sibcall optimisation for indirect
calls.  For example, it is possible to optimise the following code:

 extern int bar (int, int);
 void (*ptr) (int, int);
 void foo (int a, int b)
 {
   ptr = bar;
   return (*ptr) (a, b);
 }

Also, possible is:

 extern int bar (int, int);
 int (*ptr) (int, int);
 int foo (int a, int b)
 {
   ptr = bar;
   return (*ptr) (a, b);
 }

Etc.  I hope the idea is clear.

BRIEF OUTLINE:
--------------
I'm overriding the "fndecl == NULL_TREE" check in "calls.c" and cover it
in FUNCTION_OK_FOR_SIBCALL instead.  The macro will decide whether an
indirect call is a candidate for sibcall optimisation and if so, the
changed machine descriptions may be put into place at the end of
compilation.  Non-i386 architectures, however, still check for fndecl in
"calls.c", because they do not yet allow any indirect calls at all!  So,
the patch doesn't break anthing, as far as I know.

I'm aware that the patch is too small to actually make a huge difference
on gcc as a whole, but if the concept proves useful, then it could be
applied to other architectures as well and we could remove the test
"fndecl == NULL_TREE" totally from the file "calls.c".

However, it's a starting point and hopefully someone's able to give me
at least some feedback on this.

Thanks in advance,
Andreas.

[-- Attachment #2: sibcall_patch.diff --]
[-- Type: text/plain, Size: 7661 bytes --]

diff -r -b -w -B -d -p -c /home/baueran/Development/gcc-3.2/gcc/calls.c /home/baueran/Development/gcc-3.2-testing/gcc/calls.c
*** /home/baueran/Development/gcc-3.2/gcc/calls.c	Fri Apr  5 09:28:47 2002
--- /home/baueran/Development/gcc-3.2-testing/gcc/calls.c	Mon Sep 23 13:24:57 2002
*************** expand_call (exp, target, ignore)
*** 2455,2463 ****
  	 use a register class that is all call-clobbered.  Any
  	 reload insns generated to fix things up would appear
  	 before the sibcall_epilogue.  */
        || fndecl == NULL_TREE
        || (flags & (ECF_RETURNS_TWICE | ECF_LONGJMP))
!       || TREE_THIS_VOLATILE (fndecl)
        || !FUNCTION_OK_FOR_SIBCALL (fndecl)
        /* If this function requires more stack slots than the current
  	 function, we cannot change it into a sibling call.  */
--- 2455,2474 ----
  	 use a register class that is all call-clobbered.  Any
  	 reload insns generated to fix things up would appear
  	 before the sibcall_epilogue.  */
+ 
+       /* Additional call patterns in i386.md allow i386 to do
+ 	 optimized indirect calls.  Other platforms may/should
+ 	 follow this example.
+ 	 In fact, it is sufficient on i386 to merely test with the
+ 	 FUNCTION_OK_FOR_SIBCALL macro, as it contains certain
+ 	 conditionals for handling indirect calls.  */
+ 
+ #ifndef __i386
        || fndecl == NULL_TREE
+ #endif
+ 
        || (flags & (ECF_RETURNS_TWICE | ECF_LONGJMP))
!       || (fndecl != NULL_TREE && TREE_THIS_VOLATILE (fndecl))
        || !FUNCTION_OK_FOR_SIBCALL (fndecl)
        /* If this function requires more stack slots than the current
  	 function, we cannot change it into a sibling call.  */
diff -r -b -w -B -d -p -c /home/baueran/Development/gcc-3.2/gcc/config/i386/i386.c /home/baueran/Development/gcc-3.2-testing/gcc/config/i386/i386.c
*** /home/baueran/Development/gcc-3.2/gcc/config/i386/i386.c	Thu Aug  8 04:10:57 2002
--- /home/baueran/Development/gcc-3.2-testing/gcc/config/i386/i386.c	Mon Sep 23 13:27:11 2002
*************** static enum x86_64_reg_class merge_class
*** 822,827 ****
--- 822,865 ----
  
  struct gcc_target targetm = TARGET_INITIALIZER;
  \f
+ /* If PIC, we cannot make sibling calls to global functions
+    because the PLT requires %ebx live.
+    If we are returning floats on the register stack, we cannot make
+    sibling calls to functions that return floats.  (The stack adjust
+    instruction will wind up after the sibcall jump, and not be executed.) */
+ 
+ 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;
+ }
+ 
  /* Sometimes certain combinations of command options do not make
     sense on a particular target machine.  You can define a macro
     `OVERRIDE_OPTIONS' to take account of this.  This macro, if
diff -r -b -w -B -d -p -c /home/baueran/Development/gcc-3.2/gcc/config/i386/i386.h /home/baueran/Development/gcc-3.2-testing/gcc/config/i386/i386.h
*** /home/baueran/Development/gcc-3.2/gcc/config/i386/i386.h	Mon Jul 15 16:54:36 2002
--- /home/baueran/Development/gcc-3.2-testing/gcc/config/i386/i386.h	Mon Sep 23 13:26:30 2002
*************** typedef struct ix86_args {
*** 1710,1726 ****
  
  #define FUNCTION_ARG_PARTIAL_NREGS(CUM, MODE, TYPE, NAMED) 0
  
! /* If PIC, we cannot make sibling calls to global functions
!    because the PLT requires %ebx live.
!    If we are returning floats on the register stack, we cannot make
!    sibling calls to functions that return floats.  (The stack adjust
!    instruction will wind up after the sibcall jump, and not be executed.) */
! #define FUNCTION_OK_FOR_SIBCALL(DECL)					\
!   ((DECL)								\
!    && (! 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))))))
  
  /* Perform any needed actions needed for a function that is receiving a
     variable number of arguments.
--- 1710,1719 ----
  
  #define FUNCTION_ARG_PARTIAL_NREGS(CUM, MODE, TYPE, NAMED) 0
  
! /* Conditions why sibcall optimization might fail as well as the function
!    to check, are defined in i386.c.  */
! 
! #define FUNCTION_OK_FOR_SIBCALL(DECL) i386_function_ok_for_sibcall (DECL, exp)
  
  /* Perform any needed actions needed for a function that is receiving a
     variable number of arguments.
diff -r -b -w -B -d -p -c /home/baueran/Development/gcc-3.2/gcc/config/i386/i386.md /home/baueran/Development/gcc-3.2-testing/gcc/config/i386/i386.md
*** /home/baueran/Development/gcc-3.2/gcc/config/i386/i386.md	Mon Jul 15 16:54:36 2002
--- /home/baueran/Development/gcc-3.2-testing/gcc/config/i386/i386.md	Mon Sep 23 13:28:25 2002
***************
*** 13573,13578 ****
--- 13573,13609 ----
  }
    [(set_attr "type" "call")])
  
+ (define_insn "*sibcall_0"
+   [(call (mem:QI (match_operand 0 "constant_call_address_operand" "s,c,d,a"))
+ 	 (match_operand 1 "" ""))]
+   "SIBLING_CALL_P (insn)"
+ {
+   if (SIBLING_CALL_P (insn))
+     return "jmp\t%P0";
+   else
+     return "call\t%P0";
+ }
+   [(set_attr "type" "call")])
+ 
+ (define_insn "*sibcall_1"
+   [(call (mem:QI (match_operand:SI 0 "call_insn_operand" "s,c,d,a"))
+ 	 (match_operand 1 "" ""))]
+   "SIBLING_CALL_P (insn) && !TARGET_64BIT"
+ {
+   if (constant_call_address_operand (operands[0], QImode))
+     {
+       if (SIBLING_CALL_P (insn))
+ 	return "jmp\t%P0";
+       else
+ 	return "call\t%P0";
+     }
+   if (SIBLING_CALL_P (insn))
+     return "jmp\t%A0";
+   else
+     return "call\t%A0";
+ }
+   [(set_attr "type" "call")])
+ 
  (define_insn "*call_1"
    [(call (mem:QI (match_operand:SI 0 "call_insn_operand" "rsm"))
  	 (match_operand 1 "" ""))]
***************
*** 17760,17765 ****
--- 17791,17828 ----
  }
    [(set_attr "type" "callv")])
  
+ (define_insn "*sibcall_value_0"
+   [(set (match_operand 0 "" "")
+ 	(call (mem:QI (match_operand:SI 1 "constant_call_address_operand" "s,c,d,a"))
+ 	      (match_operand:SI 2 "" "")))]
+   "SIBLING_CALL_P (insn) && !TARGET_64BIT"
+ {
+   if (SIBLING_CALL_P (insn))
+     return "jmp\t%P1";
+   else
+     return "call\t%P1";
+ }
+   [(set_attr "type" "callv")])
+ 
+ (define_insn "*sibcall_value_1"
+   [(call (mem:QI (match_operand:SI 0 "call_insn_operand" "s,c,d,a"))
+ 	 (match_operand 1 "" ""))]
+   "SIBLING_CALL_P (insn) && !TARGET_64BIT"
+ {
+   if (constant_call_address_operand (operands[0], QImode))
+     {
+       if (SIBLING_CALL_P (insn))
+ 	return "jmp\t%P0";
+       else
+ 	return "call\t%P0";
+     }
+   if (SIBLING_CALL_P (insn))
+     return "jmp\t%A0";
+   else
+     return "call\t%A0";
+ }
+   [(set_attr "type" "call")])
+ 
  (define_insn "*call_value_1"
    [(set (match_operand 0 "" "")
  	(call (mem:QI (match_operand:SI 1 "call_insn_operand" "rsm"))

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2002-09-30 21:03 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-09-30  9:21 PATCH for sibcalls on i386 John David Anglin
2002-09-30 13:29 ` Hans-Peter Nilsson
2002-09-30 14:08   ` John David Anglin
2002-09-30 15:00     ` Hans-Peter Nilsson
2002-09-30 17:06     ` Richard Henderson
2002-09-30 17:44       ` John David Anglin
     [not found] <no.id>
2002-09-30 21:03 ` John David Anglin
  -- strict thread matches above, loose matches on Subject: below --
2002-09-22 21:11 Andreas Bauer
2002-09-23  3:03 ` Fergus Henderson
2002-09-23 12:25   ` Zack Weinberg
2002-09-23 16:52     ` Andreas Bauer
2002-09-23 17:10     ` Fergus Henderson
2002-09-23 21:27     ` Andreas Bauer
2002-09-23 21:37       ` Andrew Pinski
2002-09-23 21:44         ` Andreas Bauer
2002-09-23 21:48       ` Zack Weinberg
2002-09-23 22:14       ` Richard Henderson
2002-09-23 22:24       ` Richard Henderson
2002-09-24 22:24         ` Andreas Bauer
2002-09-24 23:28           ` Fergus Henderson
2002-09-25 16:47           ` Richard Henderson
2002-09-30  1:43             ` Andreas Bauer
2002-09-30  8:11               ` Fergus Henderson
2002-09-23 16:46   ` Andreas Bauer
2002-09-23 17:09     ` Richard Henderson
2002-09-23 15:12 ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).