[Patch][AVR]: Support tail calls

[Georg-Johann Lay]

Richard Henderson schrieb:
> On 03/11/2011 05:43 AM, Georg-Johann Lay wrote:
>> I did not find a way to make this work together with -mcall-prologues.
>> Please let me know if you have suggestion on how call prologues can be
>> combine with tail calls.
> 
> You need a new symbol in libgcc for this.  It should be easy enough to have
> the sibcall epilogue load up Z+EIND before jumping to the new symbol
> (perhaps called __sibcall_restores__).  This new symbol would be just like
> the existing __epilogue_restores__ except that it would finish with an
> eijmp/ijmp instruction (depending on multilib) instead of a ret instruction.

The point is that the intent of -mprologue-saves is to save program
memory space (flash). If we load the callee's address in the epilogue,
the epilogue's size will increase. Moreover, adding an other lengthy
function besides __epilogue_restores__ to libgcc, that will increase
code size because in many cases we will see both epilogue flavors
being dragged into application.

Note that, in comparison with non-tailcall, a tailcall without
prologue-saves saves one instruction, i.e. the ret. This is no more
true in presence of prologue-saves, i.e. even with tail-calling the
code would be neither faster nor smaller. (It might still save some
stack space, though)

>> The implementation uses struct machine_function to pass information
>> around, i.e. from avr_function_arg_advance to avr_function_ok_for_sibcall.
> 
> Look at how the s390 port handles this exact problem.
> 
>   /* Register 6 on s390 is available as an argument register but unfortunately
>      "caller saved". This makes functions needing this register for arguments
>      not suitable for sibcalls.  */
>   return !s390_call_saved_register_used (exp);
> 
> I'll admit that it would be helpful if the cumulative_args pointer was passed
> into the ok_for_sibcall hook, but it's not *that* hard to recreate that value
> by hand.  This is what the s390_call_saved_register_used function does.

See also discussion in

http://gcc.gnu.org/ml/gcc/2011-03/msg00048.html

I must admit that I did not try to copy-paste all code in calls.c that
deals with preparation of arguments and mapping trees to rtx and see
how it deflates when applying all the #if and #ifdef and avr backend
implications...

Maybe we see Joern's work being committed soon so that on top of this
a patch adding cumulative args to function_of_for_sibcall will be
approved.

Presumably, even s390 backend guys would prefer reusing information
instead of recomputing them.

> 
>> +      || (avr_OS_task_function_p (decl_callee) ^ avr_OS_task_function_p (current_function_decl))
> 
> Please just use != instead of ^ here.  Also, needs line wrapping.

Done in the patch attached.

In the cases of OS_task resp.OS_main, I think I am a bit
over-conservative when rejecting tail-calls. This is basically because
there is *no* documentation of these attributes. Presumably they
implement "naked+ret" so that tail-calling from OS_task resp. OS_main
should be ok?

> I do like very much how you've cleaned up the call patterns.  IMO this should
> be committed as a separate patch; I'll let the AVR maintainers approve it though.

If anyone desires an explicit, intermediate step I will provide
according patch, of course.

New patch attached. Changes with respect to original patch:
- use != instead of ^ to ensure caller/callee epilogue compatibility
  for functions attributed OS_task resp OS_main
- add some more comment to md

Johann

2011-03-16  Georg-Johann Lay  <avr@gjlay.de>

	* config/avr/avr-protos.h (expand_epilogue): Change prototype
	* config/avr/avr.h (struct machine_function): Add field
	sibcall_fails.
	* config/avr/avr.c (init_cumulative_args,
	avr_function_arg_advance): Use it.
	* config/avr/avr.c (expand_epilogue): Add bool parameter. Handle
	sibcall	epilogues.
	(TARGET_FUNCTION_OK_FOR_SIBCALL): Define to...
	(avr_function_ok_for_sibcall): ...this new function.
	(avr_lookup_function_attribute1): New static Function.
	(avr_naked_function_p, interrupt_function_p,
	signal_function_p, avr_OS_task_function_p,
	avr_OS_main_function_p): Use it.
	* config/avr/avr.md ("sibcall", "sibcall_value",
	"sibcall_epilogue"): New expander.
	("*call_insn", "*call_value_insn"): New insn.
	("call_insn", "call_value_insn"): Remove
	("call", "call_value", "epilogue"): Change expander to handle
	sibling calls.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sibcall-v3.diff
Type: text/x-patch
Size: 15009 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110316/aff850ac/attachment.bin>