* 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
[parent not found: <no.id>]
* 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
* 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
* 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
* 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-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 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 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 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 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: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: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 ` (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 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-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-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-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-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-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-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-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
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).