Re: [PATCH/RFC] PR target/15130 SH: A tail call optimization

[Joern Rennecke <joern dot rennecke at superh dot com>]

> Sounds reasonable.  How about the appended patch?
> It scans live registers if it's for sibcall epilogues and uses (4)
> when the scan fails in SH1-4 case.
> For SHmedia, it first tries to find the candidate for a temporary
> from the usable registers and then from live registers.  It would be
> enough for SHmedia.
> It's regtested on mainline for sh4-/sh64-linux. The bootstrap on
> 3.4.0 isn't complete yet, but compilers have been built successfully.

> @@ -4631,8 +4631,9 @@ static int extra_push;
>  
>  /* Adjust the stack by SIZE bytes.  REG holds the rtl of the register to be
>     adjusted.  If epilogue_p is zero, this is for a prologue; otherwise, it's
> -   for an epilogue.  If LIVE_REGS_MASK is nonzero, it points to a HARD_REG_SET 
> -   of all the registers that are about to be restored, and hence dead.  */
> +   for an epilogue and the minus value means that it's for sibcall.  If

'and the minus value ...' seems to imply that there is always one 'minus'
value.  I think ths should rather be:
'and a negative value means that it's for a sibcall epilogue.'

> +	  if (temp < 0 && ! current_function_interrupt
> +	      && (TARGET_SHMEDIA || epilogue_p >= 0))
>  	    {
>  	      HARD_REG_SET temps;
>  	      COPY_HARD_REG_SET (temps, call_used_reg_set);
>  	      AND_COMPL_HARD_REG_SET (temps, call_fixed_reg_set);
>  	      if (epilogue_p)

This should become
	      if (epilogue_p > 0)

>  		{
> -		  for (i = 0; i < HARD_REGNO_NREGS (FIRST_RET_REG, DImode); i++)
> +		  int nreg = 0;
> +		  if (current_function_return_rtx)

...
> @@ -4699,7 +4713,35 @@ output_stack_adjust (int size, rtx reg, 
>  	  if (temp < 0 && live_regs_mask)
>  	    temp = scavenge_reg (live_regs_mask);
>  	  if (temp < 0)
> -	    abort ();
> +	    {
> +	      /* If we reached here, the most likely case is the (sibcall)
> +		 epilogue for non SHmedia.  Put a special push/pop sequence
> +		 for such case as the last resort.  This looks lengthy but
> +		 would not be problem because it seems to be very rare.  */
> +	      if (! TARGET_SHMEDIA && epilogue_p)
> +		{
> +		  rtx adj_reg = gen_rtx_REG (GET_MODE (reg), 4);
> +		  rtx tmp_reg = gen_rtx_REG (GET_MODE (reg), 5);

There is still the slight possibility that r4 or r5 have been reserved as
fixed registers or assigned as global registers, and they change during
an interrupt.

Possible ways to handle this:
- If we are adjusting the frame pointer (r14), we can do with a single
  temp register and an ordinary push / pop on the stack.
- Grab any call-used or call-saved registers (i.e. not fixed or globals)
  for the temps we need.  We might also grab r14 if we are adjusting the
  stack pointer.  If we can't find enough available registers, issue a
  diagnostic and abort - the user bust have reserved way too many registers.

- Since all this is rather unlikely to happen and would require extra testing,
  it is reasonable for now to just abort if r4 / r5 are not available,
  putting a comment there what might be going on and how to fix it.
  (For SH1..SH4, r4 and r5 are the first integer argument passing registers,
   so code that is compiled with r5 reserved would have to do with no more
   than one integer argument being passed.  If r4 is also reserved, none
   could be passed.  Still, it is possible to do that.  For SHcompact, you
   could pass two or three arguments before getting a conflict.
   Since some code cannot possibly work with some argument passing
   registers being reserved, it seems reasonable to make the compiler
   fail for some more code with sibling call optimization enabled.
   But we should avoid silently generating code that sporadically fails.)

Otherwise, the patch is OK.