Re: Fix for Thumb compiling va-arg-11.c

[Nick Clifton <nickc at cygnus dot com>]

Hi Richard,

: 3) We only need to use this if we have eliminated afp into the stack 
: pointer or the frame pointer: that is, afp was a live register before 
: reload ran -- if the function doesn't have any arguments on the stack, 
: then it doesn't matter whether or not we subsequently eliminate the push 
: of lr, because there are no offsets dependent on this.
: 
: Using this method there would still be a small number of functions where 
: we would unnecessarily push lr in the prologue, but they would be very 
: rare.  I doubt that we would really find many cases where we were 
: pessimistic.

Well the test case va-arg-11.c triggers this result.

: Determining that afp was live before reload should be fairly straight 
: forward (and probably quicker than testing the size of the function) -- 
: something in INITIAL_ELIMINATION_OFFSET like
: 
: 	afp_was_live |= regs_ever_live[afp_regnum]
: 
: should do the trick (we could clear this at the end of each function).  
: The OR is important since once we have eliminated afp into a real register 
: it won't be marked live any more (I think).
: 
: The final part, of course, is that we always push lr in the prologue if 
: afp_was_live is true and the code potentially contains a far_jump.

So here is a reworked version of the patch following the suggestions
that you made.  I have tested it and it does fix the bug, and it does
not introduce any new failures, although I think that we may end up
with the redundant save and restore of LR more often than you think.

Shall I apply this version ?

Cheers
	Nick

2000-03-22  Nick Clifton  <nickc@cygnus.com>

	* config/arm/arm.h (THUMB_INITIAL_ELIMINATION_OFFSET): Pass 0
	to thumb_far_jump_used_p.

	* config/arm/arm-protos.h (thumb_far_jump_used_p): Take a
	single integer parameter.

	* config/arm/arm.c (struct machine_function): Add two new
	fields, 'far_jump_used' and 'arg_pointer_live'.
	(thumb_far_jump_used_p): Once the decision has been made that
	far jumps might be used, always return true.
	If being called from the initial elimination offset macro then
	do not bother to perform the test if the arg pointer is not
	being used.
	(thumb_unexpand_epilogue): Pass 1 to thumb_far_jump_used_p().
	(output_thumb_prologue): Pass 1 to thumb_far_jump_used_p().

Index: config/arm/arm.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/arm/arm.h,v
retrieving revision 1.49.2.13
diff -p -r1.49.2.13 arm.h
*** arm.h	2000/03/16 22:47:36	1.49.2.13
--- arm.h	2000/03/22 21:18:28
*************** typedef struct
*** 1622,1628 ****
        for (regno = 0; regno <= LAST_LO_REGNUM; regno ++)		\
  	if (regs_ever_live[regno] && ! call_used_regs[regno])		\
  	  count_regs ++;						\
!       if (count_regs || ! leaf_function_p () || thumb_far_jump_used_p ())\
  	(OFFSET) += 4 * (count_regs + 1);				\
        if (TARGET_BACKTRACE)						\
          {								\
--- 1622,1628 ----
        for (regno = 0; regno <= LAST_LO_REGNUM; regno ++)		\
  	if (regs_ever_live[regno] && ! call_used_regs[regno])		\
  	  count_regs ++;						\
!       if (count_regs || ! leaf_function_p () || thumb_far_jump_used_p (0))\
  	(OFFSET) += 4 * (count_regs + 1);				\
        if (TARGET_BACKTRACE)						\
          {								\

Index: config/arm/arm-protos.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/arm/arm-protos.h,v
retrieving revision 1.1.2.6
diff -p -r1.1.2.6 arm-protos.h
*** arm-protos.h	2000/02/29 01:41:18	1.1.2.6
--- arm-protos.h	2000/03/22 21:18:28
*************** extern void   aof_dump_imports		PARAMS (
*** 152,158 ****
  
  /* Thumb functions */
  extern void   arm_init_expanders	PARAMS ((void));
! extern int    thumb_far_jump_used_p	PARAMS ((void));
  extern char * thumb_unexpanded_epilogue	PARAMS ((void));
  extern void   thumb_expand_prologue	PARAMS ((void));
  extern void   thumb_expand_epilogue	PARAMS ((void));
--- 152,158 ----
  
  /* Thumb functions */
  extern void   arm_init_expanders	PARAMS ((void));
! extern int    thumb_far_jump_used_p	PARAMS ((int));
  extern char * thumb_unexpanded_epilogue	PARAMS ((void));
  extern void   thumb_expand_prologue	PARAMS ((void));
  extern void   thumb_expand_epilogue	PARAMS ((void));

Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/arm/arm.c,v
retrieving revision 1.68.2.23
diff -p -r1.68.2.23 arm.c
*** arm.c	2000/03/15 22:07:12	1.68.2.23
--- arm.c	2000/03/22 21:18:28
*************** char * arm_condition_codes[] =
*** 205,214 ****
    "hi", "ls", "ge", "lt", "gt", "le", "al", "nv"
  };
  
! /* Data for propagating the return address.  */
  struct machine_function
  {
!   rtx ra_rtx;
  };
  
  #define streq(string1, string2) (strcmp (string1, string2) == 0)
--- 205,216 ----
    "hi", "ls", "ge", "lt", "gt", "le", "al", "nv"
  };
  
! /* Per function data.  */
  struct machine_function
  {
!   rtx ra_rtx;		/* Records return address.  */
!   int far_jump_used;	/* Records if LR has to be saved for far jumps.  */
!   int arg_pointer_live;	/* Records if ARG_POINTER was ever live.  */
  };
  
  #define streq(string1, string2) (strcmp (string1, string2) == 0)
*************** thumb_shiftable_const (val)
*** 8290,8301 ****
    return 0;
  }
  
! /* Returns non-zero if the current function contains a far jump */
  int
! thumb_far_jump_used_p (void)
  {
    rtx insn;
    
    for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
      {
        if (GET_CODE (insn) == JUMP_INSN
--- 8292,8346 ----
    return 0;
  }
  
! /* Returns non-zero if the current function contains,
!    or might contain a far jump.  */
  int
! thumb_far_jump_used_p (int in_prologue)
  {
    rtx insn;
+ 
+   /* This test is only important for leaf functions.  */
+   /* assert (! leaf_function_p ()); */
    
+   /* If we have already decided that far jumps may be used,
+      do not bother checking again, and always return true even if
+      it turns out that they are not being used.  Once we have made
+      the decision that far jumps are present (and that hence the link
+      register will be pushed onto the stack) we cannot go back on it.  */
+   if (cfun->machine->far_jump_used)
+     return 1;
+ 
+   /* If this function is not being called from the prologue/epilogue
+      generation code then it must be being called from the
+      INITIAL_ELIMINATION_OFFSET macro.  */
+   if (! in_prologue)
+     {
+       /* In this case we know that we are being asked about the elimination
+ 	 of the arg pointer register.  If that register is not being used,
+ 	 then there are no arguments on the stack, and we do not have to
+ 	 worry that a far jump might force the prologue to push the link
+ 	 register, changing the stack offsets.  In this case we can just
+ 	 return false, since the presence of far jumps in the function will
+ 	 not affect stack offsets.
+ 
+ 	 If the arg pointer is live (or if it was live, but has now been
+ 	 eliminated and so set to dead) then we do have to test to see if
+ 	 the function might contain a far jump.  This test can lead to some
+ 	 false negatives, since before reload is completed, then length of
+ 	 branch instructions is not known, so gcc defaults to returning their
+ 	 longest length, which in turn sets the far jump attribute to true.
+ 
+ 	 A false negative will not result in bad code being generated, but it
+ 	 will result in a needless push and pop of the link register.  We
+ 	 hope that this does not occur too often.  */
+       if (regs_ever_live [ARG_POINTER_REGNUM])
+ 	cfun->machine->arg_pointer_live = 1;
+       else if (! cfun->machine->arg_pointer_live)
+ 	return 0;
+     }
+ 
+   /* Check to see if the function contains a branch
+      insn with the far jump attribute set.  */
    for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
      {
        if (GET_CODE (insn) == JUMP_INSN
*************** thumb_far_jump_used_p (void)
*** 8304,8312 ****
  	  && GET_CODE (PATTERN (insn)) != ADDR_DIFF_VEC
  	  && get_attr_far_jump (insn) == FAR_JUMP_YES
  	  )
! 	return 1;
      }
! 
    return 0;
  }
  
--- 8349,8362 ----
  	  && GET_CODE (PATTERN (insn)) != ADDR_DIFF_VEC
  	  && get_attr_far_jump (insn) == FAR_JUMP_YES
  	  )
! 	{
! 	  /* Record the fact that we have decied that
! 	     the function does use far jumps.  */
! 	  cfun->machine->far_jump_used = 1;
! 	  return 1;
! 	}
      }
!   
    return 0;
  }
  
*************** thumb_unexpanded_epilogue ()
*** 8437,8443 ****
      }
  
    had_to_push_lr = (live_regs_mask || ! leaf_function
! 		    || thumb_far_jump_used_p ());
    
    if (TARGET_BACKTRACE
        && (live_regs_mask & 0xFF) == 0
--- 8487,8493 ----
      }
  
    had_to_push_lr = (live_regs_mask || ! leaf_function
! 		    || thumb_far_jump_used_p (1));
    
    if (TARGET_BACKTRACE
        && (live_regs_mask & 0xFF) == 0
*************** output_thumb_prologue (f)
*** 8763,8769 ****
  	&& ! (TARGET_SINGLE_PIC_BASE && (regno == arm_pic_register)))
        live_regs_mask |= 1 << regno;
  
!   if (live_regs_mask || ! leaf_function_p () || thumb_far_jump_used_p ())
      live_regs_mask |= 1 << LR_REGNUM;
  
    if (TARGET_BACKTRACE)
--- 8813,8819 ----
  	&& ! (TARGET_SINGLE_PIC_BASE && (regno == arm_pic_register)))
        live_regs_mask |= 1 << regno;
  
!   if (live_regs_mask || ! leaf_function_p () || thumb_far_jump_used_p (1))
      live_regs_mask |= 1 << LR_REGNUM;
  
    if (TARGET_BACKTRACE)