Patch for -mcaller-super-interworking & stack arguments

Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

-mcaller-super-interworking causes all indirect calls to use an
_interwork_call_via_rN stub.  If the target address is an ARM function,
this stub will push the (thumb) return address onto the stack and get
the ARM function to return to _arm_return instead.

Unfortunately, pushing the return address onto the stack means that thumb
functions can't pass stack arguments to ARM functions.  The suggested fix
is to define alternative interworking functions that store the return
address at [r7, #-4] instead.  This of course requires the caller to
use a frame pointer and to provide appropriate scratch space there.

If the caller used a frame pointer anyway, I think the overhead of this
approach should be very low.  We just need to bump up the size of the
frame and fiddle with the elimination offsets.  The only source of
extra insns should be from frame accesses that are on the borderline
between being in-range and out-of-range.

On the other hand, if the function _didn't_ need a frame pointer for
anything else, then there is definitely a big overhead.  The patch
therefore only uses these new functions if there are some outgoing
stack arguments.  (As per the comment in the patch, I don't think
we can really refine the condition much more than that.)

This patch has been regression tested on arm-elf, test pattern:

    arm-sim{,-mthumb,-mthumb/-mcaller-super-interworking}

I admit this doesn't exercise thumb->ARM calls that much since the
third pattern will link against the thumb multilibs.  Trying to test
against a --disable-multilib build is useless because of all the extra
linker warnings.

Even so, the thumb->ARM route does get some testing from compiler-generated
trampolines and thunks, and the patch does fix gcc.dg/trampoline-1.c and
g++.old-deja/g++.jason/thunk3.C.  There were no regressions.  OK to install?

Richard


	* config/arm/arm.h (CALLER_INTERWORKING_SLOT_SIZE): New macro.
	(FRAME_POINTER_REQUIRED): True if CALLER_INTERWORKING_SLOT_SIZE > 0.
	* config/arm/arm.c (arm_get_frame_offsets): Add
	CALLER_INTERWORKING_SLOT_SIZE bytes to the soft frame pointer offset.
	* config/arm/arm.md (*call_reg_thumb, *call_value_reg_thumb): Use
	_interwork_fp_call_via_rN if some arguments are passed on the stack.
	* config/arm/lib1funcs.asm (_arm_fp_return): New function.
	(interwork): Add _interwork_fp_call_via_rN() functions.

Index: config/arm/lib1funcs.asm
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/lib1funcs.asm,v
retrieving revision 1.31
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.31 lib1funcs.asm
*** config/arm/lib1funcs.asm	15 May 2004 17:31:51 -0000	1.31
--- config/arm/lib1funcs.asm	5 Aug 2004 06:43:18 -0000
*************** 1:
*** 1035,1041 ****
     the target code cannot be relied upon to return via a BX instruction, so
     instead we have to store the resturn address on the stack and allow the
     called function to return here instead.  Upon return we recover the real
!    return address and use a BX to get back to Thumb mode.  */
  	
  	.text
  	.align 0
--- 1035,1049 ----
     the target code cannot be relied upon to return via a BX instruction, so
     instead we have to store the resturn address on the stack and allow the
     called function to return here instead.  Upon return we recover the real
!    return address and use a BX to get back to Thumb mode.
! 
!    There are two variations of this code.  The first, _interwork_call_via_rN(),
!    will push the return address onto the stack and pop it in _arm_return().
!    It should only be used if all arguments are passed in registers.
! 
!    The second, _interwork_fp_call_via_rN(), instead stores the return
!    address at [r7, #-4].  It is the caller's responsibility to ensure
!    that this address is valid and contains no useful data.  */
  	
  	.text
  	.align 0
*************** 1:
*** 1044,1050 ****
  	.globl _arm_return
  _arm_return:
  	RETLDM
! 	.code   16
  
  .macro interwork register
  	.code	16
--- 1052,1062 ----
  	.globl _arm_return
  _arm_return:
  	RETLDM
! 
! 	.globl _arm_fp_return
! _arm_fp_return:
! 	ldr	lr, [r7, #-4]
! 	bx	lr
  
  .macro interwork register
  	.code	16
*************** LSYM(Lchange_\register):
*** 1063,1068 ****
--- 1075,1097 ----
  	bx	\register
  
  	SIZE	(_interwork_call_via_\register)
+ 
+ 	.code	16
+ 
+ 	THUMB_FUNC_START _interwork_fp_call_via_\register
+ 
+ 	bx	pc
+ 	nop
+ 
+ 	.code	32
+ 	.globl LSYM(Lfp_change_\register)
+ LSYM(Lfp_change_\register):
+ 	tst	\register, #1
+ 	streq	lr, [r7, #-4]
+ 	adreq	lr, _arm_fp_return
+ 	bx	\register
+ 
+ 	SIZE	(_interwork_fp_call_via_\register)
  .endm
  	
  	interwork r0
Index: config/arm/arm.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.245
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.245 arm.h
*** config/arm/arm.h	14 Jul 2004 17:51:18 -0000	1.245
--- config/arm/arm.h	5 Aug 2004 06:43:21 -0000
*************** #define FIRST_PSEUDO_REGISTER   96
*** 1043,1048 ****
--- 1043,1049 ----
     functions, or simple tail call functions.  */
  #define FRAME_POINTER_REQUIRED					\
    (current_function_has_nonlocal_label				\
+    || CALLER_INTERWORKING_SLOT_SIZE > 0				\
     || (TARGET_ARM && TARGET_APCS_FRAME && ! leaf_function_p ()))
  
  /* Return number of consecutive hard regs needed starting at reg REGNO
*************** #define STACK_GROWS_DOWNWARD  1
*** 1501,1506 ****
--- 1502,1522 ----
     goes at a more negative offset in the frame.  */
  #define FRAME_GROWS_DOWNWARD 1
  
+ /* The amount of scratch space needed by _interwork_fp_call_via_rN().
+    When present, it is one word in size, and sits at the top of the frame,
+    between FRAME_POINTER_REGNUM and THUMB_HARD_FRAME_POINTER_REGNUM.
+ 
+    We only need _interwork_fp_call_via_rN() for -mcaller-super-interworking,
+    and only then if some outgoing arguments are passed on the stack.
+    It would be tempting to also check whether the stack arguments are
+    passed by indirect calls, but there seems to be no reason in principle
+    why a post-reload pass couldn't convert a direct call into an indirect
+    one.  */
+ #define CALLER_INTERWORKING_SLOT_SIZE			\
+   (!TARGET_CALLER_INTERWORKING ? 0			\
+    : current_function_outgoing_args_size == 0 ? 0	\
+    : UNITS_PER_WORD)
+ 
  /* Offset within stack frame to start allocating local variables at.
     If FRAME_GROWS_DOWNWARD, this is the offset to the END of the
     first local allocated.  Otherwise, it is the offset to the BEGINNING
Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.381
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.381 arm.c
*** config/arm/arm.c	3 Aug 2004 14:30:46 -0000	1.381
--- config/arm/arm.c	5 Aug 2004 06:43:35 -0000
*************** arm_get_frame_offsets (void)
*** 10116,10122 ****
  
    /* Saved registers include the stack frame.  */
    offsets->saved_regs = offsets->saved_args + saved;
!   offsets->soft_frame = offsets->saved_regs;
    /* A leaf function does not need any stack alignment if it has nothing
       on the stack.  */
    if (leaf && frame_size == 0)
--- 10116,10122 ----
  
    /* Saved registers include the stack frame.  */
    offsets->saved_regs = offsets->saved_args + saved;
!   offsets->soft_frame = offsets->saved_regs + CALLER_INTERWORKING_SLOT_SIZE;
    /* A leaf function does not need any stack alignment if it has nothing
       on the stack.  */
    if (leaf && frame_size == 0)
Index: config/arm/arm.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.md,v
retrieving revision 1.174
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.174 arm.md
*** config/arm/arm.md	3 Aug 2004 13:27:02 -0000	1.174
--- config/arm/arm.md	5 Aug 2004 06:43:46 -0000
*************** (define_insn "*call_reg_thumb"
*** 7430,7439 ****
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (TARGET_CALLER_INTERWORKING)
        return \"bl\\t%__interwork_call_via_%0\";
      else
!       return \"bl\\t%__call_via_%0\";
    }"
    [(set_attr "type" "call")]
  )
--- 7430,7441 ----
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (!TARGET_CALLER_INTERWORKING)
!       return \"bl\\t%__call_via_%0\";
!     else if (operands[1] == const0_rtx)
        return \"bl\\t%__interwork_call_via_%0\";
      else
!       return \"bl\\t%__interwork_fp_call_via_%0\";
    }"
    [(set_attr "type" "call")]
  )
*************** (define_insn "*call_value_reg_thumb"
*** 7520,7529 ****
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (TARGET_CALLER_INTERWORKING)
        return \"bl\\t%__interwork_call_via_%1\";
      else
!       return \"bl\\t%__call_via_%1\";
    }"
    [(set_attr "type" "call")]
  )
--- 7522,7533 ----
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (!TARGET_CALLER_INTERWORKING)
!       return \"bl\\t%__call_via_%1\";
!     else if (operands[2] == const0_rtx)
        return \"bl\\t%__interwork_call_via_%1\";
      else
!       return \"bl\\t%__interwork_fp_call_via_%1\";
    }"
    [(set_attr "type" "call")]
  )

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Earnshaw]

Richard,

Sorry for not replying to this sooner, I've been pondering the situation
'cos I'm not 100% happy about any of this...

On Mon, 2004-08-09 at 08:03, Richard Sandiford wrote:
> -mcaller-super-interworking causes all indirect calls to use an
> _interwork_call_via_rN stub.  If the target address is an ARM function,
> this stub will push the (thumb) return address onto the stack and get
> the ARM function to return to _arm_return instead.
> 
> Unfortunately, pushing the return address onto the stack means that thumb
> functions can't pass stack arguments to ARM functions.  The suggested fix
> is to define alternative interworking functions that store the return
> address at [r7, #-4] instead.  This of course requires the caller to
> use a frame pointer and to provide appropriate scratch space there.
> 
> If the caller used a frame pointer anyway, I think the overhead of this
> approach should be very low.  We just need to bump up the size of the
> frame and fiddle with the elimination offsets.  The only source of
> extra insns should be from frame accesses that are on the borderline
> between being in-range and out-of-range.
> 

Agreed.

> On the other hand, if the function _didn't_ need a frame pointer for
> anything else, then there is definitely a big overhead.  The patch
> therefore only uses these new functions if there are some outgoing
> stack arguments.  (As per the comment in the patch, I don't think
> we can really refine the condition much more than that.)
> 

Yep, that's reasonable.

Another approach would be to use r11 (fp) to hold the 'frame pointer' in
the case where we didn't really need a frame (and continue to pretend
that the function was frameless).  It would mean a further function stub
in libgcc (unless you always made it work that way) but would mean that
we didn't loose a low register.  You'd end up with a prologue sequence
something like

	push	{r4-r7, lr}
	mov	r4, fp
	push	{r4}
	sub	sp, sp, #4
	mov	fp, sp

The interwork stub would be similar to the one in your proposed patch,
but use fp instead of r7.

To make this work you'd probably have to make r11 a fixed register for
-mthumb -mcaller-super-interworking, but that wouldn't hurt very much.

> This patch has been regression tested on arm-elf, test pattern:
> 
>     arm-sim{,-mthumb,-mthumb/-mcaller-super-interworking}
> 
> I admit this doesn't exercise thumb->ARM calls that much since the
> third pattern will link against the thumb multilibs.  Trying to test
> against a --disable-multilib build is useless because of all the extra
> linker warnings.
> 
> Even so, the thumb->ARM route does get some testing from compiler-generated
> trampolines and thunks, and the patch does fix gcc.dg/trampoline-1.c and
> g++.old-deja/g++.jason/thunk3.C.  There were no regressions.  OK to install?
> 
> Richard
> 
> 
> 	* config/arm/arm.h (CALLER_INTERWORKING_SLOT_SIZE): New macro.
> 	(FRAME_POINTER_REQUIRED): True if CALLER_INTERWORKING_SLOT_SIZE > 0.
> 	* config/arm/arm.c (arm_get_frame_offsets): Add
> 	CALLER_INTERWORKING_SLOT_SIZE bytes to the soft frame pointer offset.
> 	* config/arm/arm.md (*call_reg_thumb, *call_value_reg_thumb): Use
> 	_interwork_fp_call_via_rN if some arguments are passed on the stack.
> 	* config/arm/lib1funcs.asm (_arm_fp_return): New function.
> 	(interwork): Add _interwork_fp_call_via_rN() functions.

OK.  But can you give some thought to my suggestion.

R.

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

Thanks for the feedback.

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> Sorry for not replying to this sooner, I've been pondering the situation
> 'cos I'm not 100% happy about any of this...

I'm not sure whether "this" means my patch in particular or just
-mcaller-super-interworking in general ;)  If the former,  I'm certainly
willing to try alternatives.

> Another approach would be to use r11 (fp) to hold the 'frame pointer' in
> the case where we didn't really need a frame (and continue to pretend
> that the function was frameless).  It would mean a further function stub
> in libgcc (unless you always made it work that way) but would mean that
> we didn't loose a low register.

Good idea.  Will give it a go.  I won't commit my original patch
until I see how the new one works out.

Btw, as a heads-up, I was just looking into how the new patch might be
written, and I noticed what seems to be a little thinko in the recent
changes to thumb_compute_save_reg_mask():

  if (flag_pic && !TARGET_SINGLE_PIC_BASE)
    mask |= PIC_OFFSET_TABLE_REGNUM;

Looks like it should be "mask |= 1 << ...".  I'm not really
set up to test that change myself though.

Richard

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Earnshaw]

On Fri, 2004-08-27 at 14:33, Richard Sandiford wrote:
> I'm not sure whether "this" means my patch in particular or just
> -mcaller-super-interworking in general ;)  If the former,  I'm certainly
> willing to try alternatives.

Well it started of with -mcaller-super-interworking in general, because
it just doesn't work for far too many cases, but then I realised your
patch was addressing most, if not all, of those failures.

Then I didn't like the costs of your proposed solution, until I realised
you'd thought about most of those cases too... and the one case you
really ended up losing on, I think we now have a conceptual approach
for.  So I guess I'm not as unhappy now as I was when I first started
reading your proposal :-)

> 
> > Another approach would be to use r11 (fp) to hold the 'frame pointer' in
> > the case where we didn't really need a frame (and continue to pretend
> > that the function was frameless).  It would mean a further function stub
> > in libgcc (unless you always made it work that way) but would mean that
> > we didn't loose a low register.
> 
> Good idea.  Will give it a go.  I won't commit my original patch
> until I see how the new one works out.
> 
> Btw, as a heads-up, I was just looking into how the new patch might be
> written, and I noticed what seems to be a little thinko in the recent
> changes to thumb_compute_save_reg_mask():
> 
>   if (flag_pic && !TARGET_SINGLE_PIC_BASE)
>     mask |= PIC_OFFSET_TABLE_REGNUM;
> 
> Looks like it should be "mask |= 1 << ...".  I'm not really
> set up to test that change myself though.

You're right.  I think that even comes under the 'obvious' rule.

Well spotted.

R.

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
>> Btw, as a heads-up, I was just looking into how the new patch might be
>> written, and I noticed what seems to be a little thinko in the recent
>> changes to thumb_compute_save_reg_mask():
>> 
>>   if (flag_pic && !TARGET_SINGLE_PIC_BASE)
>>     mask |= PIC_OFFSET_TABLE_REGNUM;
>> 
>> Looks like it should be "mask |= 1 << ...".  I'm not really
>> set up to test that change myself though.
>
> You're right.  I think that even comes under the 'obvious' rule.

Yeah, I thought it might.  But like I say, I'm not really set
up to test thumb PIC (at least, not knowingly) so I thought I'd
better leave it to somehow who is.

Richard

Re: Patch for -mcaller-super-interworking & stack arguments

[Daniel Jacobowitz]

On Fri, Aug 27, 2004 at 12:04:55PM +0100, Richard Earnshaw wrote:
> On Mon, 2004-08-09 at 08:03, Richard Sandiford wrote:
> > -mcaller-super-interworking causes all indirect calls to use an
> > _interwork_call_via_rN stub.  If the target address is an ARM function,
> > this stub will push the (thumb) return address onto the stack and get
> > the ARM function to return to _arm_return instead.
> > 
> > Unfortunately, pushing the return address onto the stack means that thumb
> > functions can't pass stack arguments to ARM functions.  The suggested fix
> > is to define alternative interworking functions that store the return
> > address at [r7, #-4] instead.  This of course requires the caller to
> > use a frame pointer and to provide appropriate scratch space there.

I was curious how non-indirect calls handled this situation, and whether this
change would affect the linker, so I took a look.  My interpretation is that:

  (A) the --support-old-code functionality of the COFF and PE backends
      would need a matching update.  I don't know how this would work
      since Richard's patch decides when compiling the caller whether
      to stash it at [r7, #-4] or not.

  (B) None of this has ever worked for ELF, since GNU ld has only about
      half of the code necessary to build the right stubs.

So right now the only options are to use -mcaller-super-interworking and
restrict all calls to be via function pointers, or fix these shortcoming
of the linker.  Am I reading the code right?

-- 
Daniel Jacobowitz

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Earnshaw]

On Fri, 2004-08-27 at 16:35, Daniel Jacobowitz wrote:
> On Fri, Aug 27, 2004 at 12:04:55PM +0100, Richard Earnshaw wrote:
> > On Mon, 2004-08-09 at 08:03, Richard Sandiford wrote:
> > > -mcaller-super-interworking causes all indirect calls to use an
> > > _interwork_call_via_rN stub.  If the target address is an ARM function,
> > > this stub will push the (thumb) return address onto the stack and get
> > > the ARM function to return to _arm_return instead.
> > > 
> > > Unfortunately, pushing the return address onto the stack means that thumb
> > > functions can't pass stack arguments to ARM functions.  The suggested fix
> > > is to define alternative interworking functions that store the return
> > > address at [r7, #-4] instead.  This of course requires the caller to
> > > use a frame pointer and to provide appropriate scratch space there.
> 
> I was curious how non-indirect calls handled this situation, and whether this
> change would affect the linker, so I took a look.  My interpretation is that:
> 
>   (A) the --support-old-code functionality of the COFF and PE backends
>       would need a matching update.  I don't know how this would work
>       since Richard's patch decides when compiling the caller whether
>       to stash it at [r7, #-4] or not.
> 
>   (B) None of this has ever worked for ELF, since GNU ld has only about
>       half of the code necessary to build the right stubs.
> 
> So right now the only options are to use -mcaller-super-interworking and
> restrict all calls to be via function pointers, or fix these shortcoming
> of the linker.  Am I reading the code right?

Quite likely.  My recent experiments with Thumb code on NetBSD/ARM
showed that I could only get things working correctly if I compiled all
Thumb code with -march=armv5t -mthumb -mlong-calls.  If I didn't do that
the linker would not insert correct interworking veneers.  I'd assumed
that this was because a mix of objects that were non-interworking and
interworking (being armv5 objects there's actually no difference, but
the linker's not smart enough to know that).

R.

Re: Patch for -mcaller-super-interworking & stack arguments

[Daniel Jacobowitz]

On Fri, Aug 27, 2004 at 04:54:40PM +0100, Richard Earnshaw wrote:
> Quite likely.  My recent experiments with Thumb code on NetBSD/ARM
> showed that I could only get things working correctly if I compiled all
> Thumb code with -march=armv5t -mthumb -mlong-calls.  If I didn't do that
> the linker would not insert correct interworking veneers.  I'd assumed
> that this was because a mix of objects that were non-interworking and
> interworking (being armv5 objects there's actually no difference, but
> the linker's not smart enough to know that).

Ah, because -mlong-calls causes all calls to be indirect.  I see.  I
hadn't tried that; on GNU/Linux I was only able to get the right
behavior by building the entire system with -march=armv5t
-mthumb-interwork.

-- 
Daniel Jacobowitz

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> Another approach would be to use r11 (fp) to hold the 'frame pointer' in
> the case where we didn't really need a frame (and continue to pretend
> that the function was frameless).  It would mean a further function stub
> in libgcc (unless you always made it work that way) but would mean that
> we didn't loose a low register.  You'd end up with a prologue sequence
> something like
>
> 	push	{r4-r7, lr}
> 	mov	r4, fp
> 	push	{r4}
> 	sub	sp, sp, #4
> 	mov	fp, sp
>
> The interwork stub would be similar to the one in your proposed patch,
> but use fp instead of r7.
>
> To make this work you'd probably have to make r11 a fixed register for
> -mthumb -mcaller-super-interworking, but that wouldn't hurt very much.

OK, here's a patch to do that.  In most other respects, it's the same
as the patch I posted before.

The main difficulty with using r11 is that we need to make it fixed
while at the same time:

  (a) making sure it is treated as call-saved rather than call-clobbered and
  (b) making sure that every CALL_INSN is treated as using r11.

(a) is easy enough to do with CALL_REALLY_USED_REGISTERS, but I believe
the standard way to achieve (b) is to either:

  (1) add (use r11) to the call patterns or
  (2) add (use r11) to CALL_INSN_FUNCTION_USAGE.

(1) would mean adding new patterns or complicating the existing ones.
(2) would mean changing the call expanders so that they do something like:

        insn = emit_call_insn (gen_... (...));
        if (...)
          use_reg (&CALL_INSN_FUNCTION_USAGE (insn), gen_rtx_REG (Pmode, 11));
        DONE;

Both seemed too invasive for what is (I suspect) such a little-used feature.

The patch therefore just makes r11 global, which achieves (a) and (b) above,
and also has the benefit of warning if the user tries to use r11 as a true
global register.

In his review, Nick asked me to add a testcase to the testsuite.
This is a bit difficult because something like:

    /* { dg-options "-mthumb -mcaller-super-interworking" } */

will cause lots of linker warnings if run on a non-thumb multilib.
For example, testglue (which arm-sim still uses) will not have been
compiled for thumb.

I think the only reliable way of testing -mcaller-super-interworking is
to add it to the --target_board list.  If you do that, then like I said
in my original posting, you'll get a failure in gcc.dg/trampoline-1.c
that is fixed by this patch.

Tested on arm-elf with {,-mthumb,-mthumb/-mcaller-super-interworking}.
OK to install?

Richard


	* config/arm/arm.h (CONDITIONAL_REGISTER_USAGE): Make r11 fixed and
	global for -mcaller-super-interworking.
	(CALLER_INTERWORKING_SLOT_SIZE): New macro.
	* config/arm/arm.c (thumb_compute_save_reg_mask): Save r11 if
	CALLER_INTERWORKING_SLOT_SIZE is nonzero and the function does
	not need a frame pointer.
	(arm_get_frame_offsets): Add CALLER_INTERWORKING_SLOT_SIZE bytes to
	the soft frame pointer offset.
	(thumb_expand_prologue): Set up r11 for -mcaller-super-interworking.
	* config/arm/arm.md (*call_reg_thumb, *call_value_reg_thumb): Use
	_interwork_{r7,r11}_call_via_rN if some arguments are passed on
	the stack.  Use frame_pointer_needed to choose between them.
	* config/arm/lib1funcs.asm (_arm_return_{r7,r11}): New functions.
	(interwork_with_frame): New macro.
	(interwork): Add _interwork_{r7,r11}_call_via_rN().

Index: config/arm/arm.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.255
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.255 arm.h
*** config/arm/arm.h	29 Aug 2004 22:18:25 -0000	1.255
--- config/arm/arm.h	1 Sep 2004 14:10:04 -0000
*************** #define CONDITIONAL_REGISTER_USAGE				\
*** 917,926 ****
        fixed_regs[10]     = 1;					\
        call_used_regs[10] = 1;					\
      }								\
!   if (TARGET_APCS_FRAME)					\
      {								\
        fixed_regs[ARM_HARD_FRAME_POINTER_REGNUM] = 1;		\
        call_used_regs[ARM_HARD_FRAME_POINTER_REGNUM] = 1;	\
      }								\
    SUBTARGET_CONDITIONAL_REGISTER_USAGE				\
  }
--- 917,932 ----
        fixed_regs[10]     = 1;					\
        call_used_regs[10] = 1;					\
      }								\
!   /* -mcaller-super-interworking reserves r11 for calls to	\
!      _interwork_r11_call_via_rN().  Making the register global	\
!      is an easy way of ensuring that it remains valid for all	\
!      calls.  */							\
!   if (TARGET_APCS_FRAME || TARGET_CALLER_INTERWORKING)		\
      {								\
        fixed_regs[ARM_HARD_FRAME_POINTER_REGNUM] = 1;		\
        call_used_regs[ARM_HARD_FRAME_POINTER_REGNUM] = 1;	\
+       if (TARGET_CALLER_INTERWORKING)				\
+ 	global_regs[ARM_HARD_FRAME_POINTER_REGNUM] = 1;		\
      }								\
    SUBTARGET_CONDITIONAL_REGISTER_USAGE				\
  }
*************** #define STACK_GROWS_DOWNWARD  1
*** 1517,1522 ****
--- 1523,1542 ----
     goes at a more negative offset in the frame.  */
  #define FRAME_GROWS_DOWNWARD 1
  
+ /* The amount of scratch space needed by _interwork_{r7,r11}_call_via_rN().
+    When present, it is one word in size, and sits at the top of the frame,
+    between the soft frame pointer and either r7 or r11.
+ 
+    We only need _interwork_rM_call_via_rN() for -mcaller-super-interworking,
+    and only then if some outgoing arguments are passed on the stack.  It would
+    be tempting to also check whether the stack arguments are passed by indirect
+    calls, but there seems to be no reason in principle why a post-reload pass
+    couldn't convert a direct call into an indirect one.  */
+ #define CALLER_INTERWORKING_SLOT_SIZE			\
+   (!TARGET_CALLER_INTERWORKING ? 0			\
+    : current_function_outgoing_args_size == 0 ? 0	\
+    : UNITS_PER_WORD)
+ 
  /* Offset within stack frame to start allocating local variables at.
     If FRAME_GROWS_DOWNWARD, this is the offset to the END of the
     first local allocated.  Otherwise, it is the offset to the BEGINNING
Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.399
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.399 arm.c
*** config/arm/arm.c	25 Aug 2004 09:52:09 -0000	1.399
--- config/arm/arm.c	1 Sep 2004 14:10:18 -0000
*************** thumb_compute_save_reg_mask (void)
*** 8729,8734 ****
--- 8729,8737 ----
      mask |= PIC_OFFSET_TABLE_REGNUM;
    if (TARGET_SINGLE_PIC_BASE)
      mask &= ~(1 << arm_pic_register);
+   /* See if we might need r11 for calls to _interwork_r11_call_via_rN().  */
+   if (!frame_pointer_needed && CALLER_INTERWORKING_SLOT_SIZE > 0)
+     mask |= 1 << ARM_HARD_FRAME_POINTER_REGNUM;
  
    /* lr will also be pushed if any lo regs are pushed.  */
    if (mask & 0xff || thumb_force_lr_save ())
*************** arm_get_frame_offsets (void)
*** 9808,9814 ****
  
    /* Saved registers include the stack frame.  */
    offsets->saved_regs = offsets->saved_args + saved;
!   offsets->soft_frame = offsets->saved_regs;
    /* A leaf function does not need any stack alignment if it has nothing
       on the stack.  */
    if (leaf && frame_size == 0)
--- 9811,9817 ----
  
    /* Saved registers include the stack frame.  */
    offsets->saved_regs = offsets->saved_args + saved;
!   offsets->soft_frame = offsets->saved_regs + CALLER_INTERWORKING_SLOT_SIZE;
    /* A leaf function does not need any stack alignment if it has nothing
       on the stack.  */
    if (leaf && frame_size == 0)
*************** thumb_expand_prologue (void)
*** 12903,12908 ****
--- 12906,12914 ----
  				   stack_pointer_rtx));
        RTX_FRAME_RELATED_P (insn) = 1;
      }
+   else if (CALLER_INTERWORKING_SLOT_SIZE > 0)
+     emit_move_insn (gen_rtx_REG (Pmode, ARM_HARD_FRAME_POINTER_REGNUM),
+ 		    stack_pointer_rtx);
  
    live_regs_mask = thumb_compute_save_reg_mask ();
    amount = offsets->outgoing_args - offsets->saved_regs;
Index: config/arm/arm.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.md,v
retrieving revision 1.181
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.181 arm.md
*** config/arm/arm.md	24 Aug 2004 20:16:34 -0000	1.181
--- config/arm/arm.md	1 Sep 2004 14:10:29 -0000
*************** (define_insn "*call_reg_thumb"
*** 7427,7436 ****
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (TARGET_CALLER_INTERWORKING)
        return \"bl\\t%__interwork_call_via_%0\";
      else
!       return \"bl\\t%__call_via_%0\";
    }"
    [(set_attr "type" "call")]
  )
--- 7427,7440 ----
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (!TARGET_CALLER_INTERWORKING)
!       return \"bl\\t%__call_via_%0\";
!     else if (operands[1] == const0_rtx)
        return \"bl\\t%__interwork_call_via_%0\";
+     else if (frame_pointer_needed)
+       return \"bl\\t%__interwork_r7_call_via_%0\";
      else
!       return \"bl\\t%__interwork_r11_call_via_%0\";
    }"
    [(set_attr "type" "call")]
  )
*************** (define_insn "*call_value_reg_thumb"
*** 7517,7526 ****
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (TARGET_CALLER_INTERWORKING)
        return \"bl\\t%__interwork_call_via_%1\";
      else
!       return \"bl\\t%__call_via_%1\";
    }"
    [(set_attr "type" "call")]
  )
--- 7521,7534 ----
    "TARGET_THUMB && !arm_arch5"
    "*
    {
!     if (!TARGET_CALLER_INTERWORKING)
!       return \"bl\\t%__call_via_%1\";
!     else if (operands[2] == const0_rtx)
        return \"bl\\t%__interwork_call_via_%1\";
+     else if (frame_pointer_needed)
+       return \"bl\\t%__interwork_r7_call_via_%1\";
      else
!       return \"bl\\t%__interwork_r11_call_via_%1\";
    }"
    [(set_attr "type" "call")]
  )
Index: config/arm/lib1funcs.asm
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/lib1funcs.asm,v
retrieving revision 1.33
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.33 lib1funcs.asm
*** config/arm/lib1funcs.asm	12 Aug 2004 16:14:52 -0000	1.33
--- config/arm/lib1funcs.asm	1 Sep 2004 14:10:30 -0000
*************** 1:
*** 1096,1102 ****
     the target code cannot be relied upon to return via a BX instruction, so
     instead we have to store the resturn address on the stack and allow the
     called function to return here instead.  Upon return we recover the real
!    return address and use a BX to get back to Thumb mode.  */
  	
  	.text
  	.align 0
--- 1096,1115 ----
     the target code cannot be relied upon to return via a BX instruction, so
     instead we have to store the resturn address on the stack and allow the
     called function to return here instead.  Upon return we recover the real
!    return address and use a BX to get back to Thumb mode.
! 
!    There are three variations of this code.  The first,
!    _interwork_call_via_rN(), will push the return address onto the
!    stack and pop it in _arm_return().  It should only be used if all
!    arguments are passed in registers.
! 
!    The second, _interwork_r7_call_via_rN(), instead stores the return
!    address at [r7, #-4].  It is the caller's responsibility to ensure
!    that this address is valid and contains no useful data.
! 
!    The third, _interwork_r11_call_via_rN(), works in the same way but
!    uses r11 instead of r7.  It is useful if the caller does not really
!    need a frame pointer.  */
  	
  	.text
  	.align 0
*************** 1:
*** 1105,1111 ****
  	.globl _arm_return
  _arm_return:
  	RETLDM
! 	.code   16
  
  .macro interwork register
  	.code	16
--- 1118,1150 ----
  	.globl _arm_return
  _arm_return:
  	RETLDM
! 
! 	.globl _arm_return_r7
! _arm_return_r7:
! 	ldr	lr, [r7, #-4]
! 	bx	lr
! 
! 	.globl _arm_return_r11
! _arm_return_r11:
! 	ldr	lr, [r11, #-4]
! 	bx	lr
! 
! .macro interwork_with_frame frame, register, name, return
! 	.code	16
! 
! 	THUMB_FUNC_START \name
! 
! 	bx	pc
! 	nop
! 
! 	.code	32
! 	tst	\register, #1
! 	streq	lr, [\frame, #-4]
! 	adreq	lr, _arm_return_\frame
! 	bx	\register
! 
! 	SIZE	(\name)
! .endm
  
  .macro interwork register
  	.code	16
*************** LSYM(Lchange_\register):
*** 1124,1129 ****
--- 1163,1171 ----
  	bx	\register
  
  	SIZE	(_interwork_call_via_\register)
+ 
+ 	interwork_with_frame r7,\register,_interwork_r7_call_via_\register
+ 	interwork_with_frame r11,\register,_interwork_r11_call_via_\register
  .endm
  	
  	interwork r0

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Earnshaw]

On Wed, 2004-09-01 at 15:14, Richard Sandiford wrote:
> Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> > Another approach would be to use r11 (fp) to hold the 'frame pointer' in
> > the case where we didn't really need a frame (and continue to pretend
> > that the function was frameless).  It would mean a further function stub
> > in libgcc (unless you always made it work that way) but would mean that
> > we didn't loose a low register.  You'd end up with a prologue sequence
> > something like
> >
> > 	push	{r4-r7, lr}
> > 	mov	r4, fp
> > 	push	{r4}
> > 	sub	sp, sp, #4
> > 	mov	fp, sp
> >
> > The interwork stub would be similar to the one in your proposed patch,
> > but use fp instead of r7.
> >
> > To make this work you'd probably have to make r11 a fixed register for
> > -mthumb -mcaller-super-interworking, but that wouldn't hurt very much.
> 
> OK, here's a patch to do that.  In most other respects, it's the same
> as the patch I posted before.
> 
> The main difficulty with using r11 is that we need to make it fixed
> while at the same time:
> 
>   (a) making sure it is treated as call-saved rather than call-clobbered and

r11 (fp) is *always* call saved.  Whatever made you think it was
otherwise?

>   (b) making sure that every CALL_INSN is treated as using r11.
> 

Again this seems wrong.

I think all you need to do is make sure that 
	fixed_regs [11] = call_used_regs[11] = 1
in environments where you want to play this trick and everything else
should just fall out in the wash.  The only slightly tricky bit is
ensuring that you save r11 correctly when it's needed for this purpose
and not otherwise, but I think you can work that all out given the other
variables you have to hand.

R.

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
>>   (a) making sure it is treated as call-saved rather than call-clobbered and
>
> r11 (fp) is *always* call saved.  Whatever made you think it was
> otherwise?

Err.. nothing made me think it was otherwise ;)  I think you misunderstood
what I was trying to say.

The point is that gcc only treats r11 as call-saved in ARM mode because
it is the hard frame pointer.  In thumb mode it's just any old register.
Thus if you set:

    fixed_regs[11] = call_used_regs[11] = 1

then gcc will believe what you say about call_used_regs[].

Like I say, CALL_REALLY_USED_REGISTERS is one way around that: we can
set call_really_used_regs[11] = 0.  But that's only half the story,
because we then need to tell gcc that calls actually care what's in r11.
If we don't, then gcc doesn't see how anything uses r11, and will treat
the prologue insn that sets it as dead.

> I think all you need to do is make sure that 
> 	fixed_regs [11] = call_used_regs[11] = 1
> in environments where you want to play this trick and everything else
> should just fall out in the wash.

No, this won't work as I tried to explain.  Obviously my message
wasn't very clear, but please read it again in light of the above ;)

Richard

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Earnshaw]

On Wed, 2004-09-01 at 16:55, Richard Sandiford wrote:
> Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> >>   (a) making sure it is treated as call-saved rather than call-clobbered and
> >
> > r11 (fp) is *always* call saved.  Whatever made you think it was
> > otherwise?
> 
> Err.. nothing made me think it was otherwise ;)  I think you misunderstood
> what I was trying to say.
> 

Maybe...

> The point is that gcc only treats r11 as call-saved in ARM mode because
> it is the hard frame pointer.  In thumb mode it's just any old register.

No, even in Thumb mode it's call-saved (ie the value on return from a
call is the same as then value on calling).  If it were otherwise then
interworking calls from arm->thumb would never work.

> Thus if you set:
> 
>     fixed_regs[11] = call_used_regs[11] = 1
> 
> then gcc will believe what you say about call_used_regs[].
> 

Gcc will never allocate variables to a register with 
	f_r = c_u_r = 1

A register is ONLY call-clobbered if F_R = 0, C_U_R =1.

> Like I say, CALL_REALLY_USED_REGISTERS is one way around that: we can
> set call_really_used_regs[11] = 0.  But that's only half the story,
> because we then need to tell gcc that calls actually care what's in r11.
> If we don't, then gcc doesn't see how anything uses r11, and will treat
> the prologue insn that sets it as dead.

I'd missed this bit, I'll have to think about that...

R.

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
>> The point is that gcc only treats r11 as call-saved in ARM mode because
>> it is the hard frame pointer.  In thumb mode it's just any old register.
>
> No, even in Thumb mode it's call-saved (ie the value on return from a
> call is the same as then value on calling).

Yes I know, and the whole point is how we tell gcc that.  But..

>> Thus if you set:
>> 
>>     fixed_regs[11] = call_used_regs[11] = 1
>> 
>> then gcc will believe what you say about call_used_regs[].
>> 
>
> Gcc will never allocate variables to a register with 
> 	f_r = c_u_r = 1
>
> A register is ONLY call-clobbered if F_R = 0, C_U_R =1.

...my understanding has always been that this is false.  See e.g. how
reg_invalidated_by_call is initialised in regclass.c:init_reg_sets_1().
Certain registers (like the hard frame pointer, as I say) are known not
be invalidated by a call.  But there's no opt-out for all fixed registers.
After all, some targets _do_ have call-clobbered fixed registers.

So what I'm trying to say is that:

      call_used_regs[11] = 1

is OK in ARM mode, because gcc knows that the hard frame pointer is special.
But if we do the same thing in thumb mode, where r11 doesn't correspond to
anything special, then r11 really is treated as call-clobbered.

Traditionally, gcc has required all fixed_regs[] to be call_used_regs[]
as well.  The point of CALL_REALLY_USED_REGISTERS was to say "this
register is fixed but not call clobbered".

Richard

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Earnshaw]

On Wed, 2004-09-01 at 18:15, Richard Sandiford wrote:
> Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> >> The point is that gcc only treats r11 as call-saved in ARM mode because
> >> it is the hard frame pointer.  In thumb mode it's just any old register.
> >
> > No, even in Thumb mode it's call-saved (ie the value on return from a
> > call is the same as then value on calling).
> 
> Yes I know, and the whole point is how we tell gcc that.  But..
> 
> >> Thus if you set:
> >> 
> >>     fixed_regs[11] = call_used_regs[11] = 1
> >> 
> >> then gcc will believe what you say about call_used_regs[].
> >> 
> >
> > Gcc will never allocate variables to a register with 
> > 	f_r = c_u_r = 1
> >
> > A register is ONLY call-clobbered if F_R = 0, C_U_R =1.
> 
> ...my understanding has always been that this is false.  See e.g. how
> reg_invalidated_by_call is initialised in regclass.c:init_reg_sets_1().
> Certain registers (like the hard frame pointer, as I say) are known not
> be invalidated by a call.  But there's no opt-out for all fixed registers.
> After all, some targets _do_ have call-clobbered fixed registers.
> 
> So what I'm trying to say is that:
> 
>       call_used_regs[11] = 1
> 
> is OK in ARM mode, 

Only when F_R = 1.  At other times (eg when there's no frame pointer) it
*must* be zero or the compiler may use it as a scratch.

> because gcc knows that the hard frame pointer is special.
> But if we do the same thing in thumb mode, where r11 doesn't correspond to
> anything special, then r11 really is treated as call-clobbered.
> 
> Traditionally, gcc has required all fixed_regs[] to be call_used_regs[]
> as well.  The point of CALL_REALLY_USED_REGISTERS was to say "this
> register is fixed but not call clobbered".

The documentation doesn't say that.  Actually, the documentation doesn't
say anything at all about CALL_REALLY_USED_REGISTERS that is useful,
except that it doesn't have to be a superset of FIXED_REGISTERS.

I don't see how you can conclude the above specification from:

@defmac CALL_REALLY_USED_REGISTERS
Like @code{CALL_USED_REGISTERS} except this macro doesn't require
that the entire set of @code{FIXED_REGISTERS} be included.
(@code{CALL_USED_REGISTERS} must be a superset of @code{FIXED_REGISTERS}).
This macro is optional.  If not specified, it defaults to the value
of @code{CALL_USED_REGISTERS}.
@end defmac

It seems to me that a better way to handle this would be to add r11 to
the CALL_INSN_FUNCTION_USAGE chain for those calls that need it (ie that
will end up calling the library stub).

R.

R.

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
>> So what I'm trying to say is that:
>> 
>>       call_used_regs[11] = 1
>> 
>> is OK in ARM mode, 
>
> Only when F_R = 1.  At other times (eg when there's no frame pointer) it
> *must* be zero or the compiler may use it as a scratch.

Right.  Sorry if I was unclear.  I assumed it was taken as given that
we talking about making r11 fixed.  What I'm trying to describe is the
repercussions of also setting call_used_regs[11] at the same time (which
is required, but doesn't do what we want in thumb mode).

Richard

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Earnshaw]

On Wed, 2004-09-01 at 15:14, Richard Sandiford wrote:
> Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> > Another approach would be to use r11 (fp) to hold the 'frame pointer' in
> > the case where we didn't really need a frame (and continue to pretend
> > that the function was frameless).  It would mean a further function stub
> > in libgcc (unless you always made it work that way) but would mean that
> > we didn't loose a low register.  You'd end up with a prologue sequence
> > something like
> >
> > 	push	{r4-r7, lr}
> > 	mov	r4, fp
> > 	push	{r4}
> > 	sub	sp, sp, #4
> > 	mov	fp, sp
> >
> > The interwork stub would be similar to the one in your proposed patch,
> > but use fp instead of r7.
> >
> > To make this work you'd probably have to make r11 a fixed register for
> > -mthumb -mcaller-super-interworking, but that wouldn't hurt very much.
> 
> OK, here's a patch to do that.  In most other respects, it's the same
> as the patch I posted before.
> 
> The main difficulty with using r11 is that we need to make it fixed
> while at the same time:
> 
>   (a) making sure it is treated as call-saved rather than call-clobbered and
>   (b) making sure that every CALL_INSN is treated as using r11.
> 
> (a) is easy enough to do with CALL_REALLY_USED_REGISTERS, but I believe
> the standard way to achieve (b) is to either:
> 
>   (1) add (use r11) to the call patterns or
>   (2) add (use r11) to CALL_INSN_FUNCTION_USAGE.
> 
> (1) would mean adding new patterns or complicating the existing ones.
> (2) would mean changing the call expanders so that they do something like:
> 
>         insn = emit_call_insn (gen_... (...));
>         if (...)
>           use_reg (&CALL_INSN_FUNCTION_USAGE (insn), gen_rtx_REG (Pmode, 11));
>         DONE;
> 
> Both seemed too invasive for what is (I suspect) such a little-used feature.
> 
> The patch therefore just makes r11 global, which achieves (a) and (b) above,
> and also has the benefit of warning if the user tries to use r11 as a true
> global register.
> 
> In his review, Nick asked me to add a testcase to the testsuite.
> This is a bit difficult because something like:
> 
>     /* { dg-options "-mthumb -mcaller-super-interworking" } */
> 
> will cause lots of linker warnings if run on a non-thumb multilib.
> For example, testglue (which arm-sim still uses) will not have been
> compiled for thumb.
> 
> I think the only reliable way of testing -mcaller-super-interworking is
> to add it to the --target_board list.  If you do that, then like I said
> in my original posting, you'll get a failure in gcc.dg/trampoline-1.c
> that is fixed by this patch.
> 
> Tested on arm-elf with {,-mthumb,-mthumb/-mcaller-super-interworking}.
> OK to install?
> 
> Richard
> 
> 
> 	* config/arm/arm.h (CONDITIONAL_REGISTER_USAGE): Make r11 fixed and
> 	global for -mcaller-super-interworking.
> 	(CALLER_INTERWORKING_SLOT_SIZE): New macro.
> 	* config/arm/arm.c (thumb_compute_save_reg_mask): Save r11 if
> 	CALLER_INTERWORKING_SLOT_SIZE is nonzero and the function does
> 	not need a frame pointer.
> 	(arm_get_frame_offsets): Add CALLER_INTERWORKING_SLOT_SIZE bytes to
> 	the soft frame pointer offset.
> 	(thumb_expand_prologue): Set up r11 for -mcaller-super-interworking.
> 	* config/arm/arm.md (*call_reg_thumb, *call_value_reg_thumb): Use
> 	_interwork_{r7,r11}_call_via_rN if some arguments are passed on
> 	the stack.  Use frame_pointer_needed to choose between them.
> 	* config/arm/lib1funcs.asm (_arm_return_{r7,r11}): New functions.
> 	(interwork_with_frame): New macro.
> 	(interwork): Add _interwork_{r7,r11}_call_via_rN().

Having thought about this for a while, I now think this is probably the
best solution.  However, can you make the following change before
committing?

> *************** #define STACK_GROWS_DOWNWARD  1
> *** 1517,1522 ****
> --- 1523,1542 ----
>      goes at a more negative offset in the frame.  */
>   #define FRAME_GROWS_DOWNWARD 1
>   
> + /* The amount of scratch space needed by _interwork_{r7,r11}_call_via_rN().
> +    When present, it is one word in size, and sits at the top of the frame,
> +    between the soft frame pointer and either r7 or r11.
> + 
> +    We only need _interwork_rM_call_via_rN() for -mcaller-super-interworking,
> +    and only then if some outgoing arguments are passed on the stack.  It would
> +    be tempting to also check whether the stack arguments are passed by indirect
> +    calls, but there seems to be no reason in principle why a post-reload pass
> +    couldn't convert a direct call into an indirect one.  */
> + #define CALLER_INTERWORKING_SLOT_SIZE			\
> +   (!TARGET_CALLER_INTERWORKING ? 0			\
> +    : current_function_outgoing_args_size == 0 ? 0	\
> +    : UNITS_PER_WORD)
> + 
>   /* Offset within stack frame to start allocating local variables at.
>      If FRAME_GROWS_DOWNWARD, this is the offset to the END of the
>      first local allocated.  Otherwise, it is the offset to the BEGINNING

This would be clearer if expressed as

 ((TARGET_CALLER_INTERWORKING
   && current_funciton_outgoing_args_size != 0)
  ? UNITS_PER_WORD : 0)

R.

Re: Patch for -mcaller-super-interworking & stack arguments

[Richard Sandiford]

Richard Earnshaw <rearnsha@gcc.gnu.org> writes:
> On Wed, 2004-09-01 at 15:14, Richard Sandiford wrote:
>> 	* config/arm/arm.h (CONDITIONAL_REGISTER_USAGE): Make r11 fixed and
>> 	global for -mcaller-super-interworking.
>> 	(CALLER_INTERWORKING_SLOT_SIZE): New macro.
>> 	* config/arm/arm.c (thumb_compute_save_reg_mask): Save r11 if
>> 	CALLER_INTERWORKING_SLOT_SIZE is nonzero and the function does
>> 	not need a frame pointer.
>> 	(arm_get_frame_offsets): Add CALLER_INTERWORKING_SLOT_SIZE bytes to
>> 	the soft frame pointer offset.
>> 	(thumb_expand_prologue): Set up r11 for -mcaller-super-interworking.
>> 	* config/arm/arm.md (*call_reg_thumb, *call_value_reg_thumb): Use
>> 	_interwork_{r7,r11}_call_via_rN if some arguments are passed on
>> 	the stack.  Use frame_pointer_needed to choose between them.
>> 	* config/arm/lib1funcs.asm (_arm_return_{r7,r11}): New functions.
>> 	(interwork_with_frame): New macro.
>> 	(interwork): Add _interwork_{r7,r11}_call_via_rN().
>
> Having thought about this for a while, I now think this is probably the
> best solution.

Thanks.  Installed with your suggested change after retesting on arm-elf
(same test pattern, 'arm-sim{,-mthumb,-mthumb/-mcaller-super-interworking}').

Richard

> However, can you make the following change before
> committing?
>
>> *************** #define STACK_GROWS_DOWNWARD  1
>> *** 1517,1522 ****
>> --- 1523,1542 ----
>>      goes at a more negative offset in the frame.  */
>>   #define FRAME_GROWS_DOWNWARD 1
>>   
>> + /* The amount of scratch space needed by _interwork_{r7,r11}_call_via_rN().
>> +    When present, it is one word in size, and sits at the top of the frame,
>> +    between the soft frame pointer and either r7 or r11.
>> + 
>> +    We only need _interwork_rM_call_via_rN() for -mcaller-super-interworking,
>> +    and only then if some outgoing arguments are passed on the stack.  It would
>> +    be tempting to also check whether the stack arguments are passed by indirect
>> +    calls, but there seems to be no reason in principle why a post-reload pass
>> +    couldn't convert a direct call into an indirect one.  */
>> + #define CALLER_INTERWORKING_SLOT_SIZE			\
>> +   (!TARGET_CALLER_INTERWORKING ? 0			\
>> +    : current_function_outgoing_args_size == 0 ? 0	\
>> +    : UNITS_PER_WORD)
>> + 
>>   /* Offset within stack frame to start allocating local variables at.
>>      If FRAME_GROWS_DOWNWARD, this is the offset to the END of the
>>      first local allocated.  Otherwise, it is the offset to the BEGINNING
>
> This would be clearer if expressed as
>
>  ((TARGET_CALLER_INTERWORKING
>    && current_funciton_outgoing_args_size != 0)
>   ? UNITS_PER_WORD : 0)

Re: Patch for -mcaller-super-interworking & stack arguments

[Nick Clifton]

Hi Richard,

>	* config/arm/arm.h (CALLER_INTERWORKING_SLOT_SIZE): New macro.
>	(FRAME_POINTER_REQUIRED): True if CALLER_INTERWORKING_SLOT_SIZE > 0.
>	* config/arm/arm.c (arm_get_frame_offsets): Add
>	CALLER_INTERWORKING_SLOT_SIZE bytes to the soft frame pointer offset.
>	* config/arm/arm.md (*call_reg_thumb, *call_value_reg_thumb): Use
>	_interwork_fp_call_via_rN if some arguments are passed on the stack.
>	* config/arm/lib1funcs.asm (_arm_fp_return): New function.
>	(interwork): Add _interwork_fp_call_via_rN() functions.

Approved - please apply.

Note - it should be possible to create a test in say gcc.misc-tests
which checks this functionality and makes sure that it continues to
work.  What do you think ?

Cheers
  Nick