Re: [rfc/rfa:arm] rewrite arm_push_arguments() as a single two pass loop

> Hello,
>
> The code to push arguments onto the stack, for many ABI's, involves two passes:
>
> 1. iterate over the arguments. compute the location/offset of each, accumulate 
> results (to compute the size of the stack frame).
>
> 2. iterate over the arguments, compute the location/offset of each, store 
> accordingly
>
> The attached patch rewrites arm_push_arguments() so that the two passes are 
> combined into a single two-pass for-loop.  The only difference between the 
> first and second pass of the loop is that the second pass stores values.  The 
> intent is to make the maintenance of the function easier - e.g. adding FP 
> support would now involve one addition (currently it involves two).
>
> Thoughts, on the general approach?
>
> It also adds the command ``set debug arm'' which will cause the code to print 
> out the arguments as they are pushed.
>
> Ok for arm?
> Andrew 

Hmm, with patch ...

2002-05-29  Andrew Cagney  <ac131313@xxxxxxxxxx>
 
        * arm-tdep.c (arm_push_arguments): Rewrite using a two-pass loop.
        (arm_debug): New static variable.
        (_initialize_arm_tdep): Add ``set debug arm'' command.
 
Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.62
diff -p -r1.62 arm-tdep.c
*** arm-tdep.c  21 May 2002 15:36:02 -0000      1.62
--- arm-tdep.c  30 May 2002 02:30:47 -0000
***************
*** 41,46 ****
--- 41,48 ----
  #include "coff/internal.h"
  #include "elf/arm.h"
  
+ static int arm_debug;
+ 
  /* Each OS has a different mechanism for accessing the various
     registers stored in the sigcontext structure.
  
*************** static CORE_ADDR
*** 1407,1510 ****
  arm_push_arguments (int nargs, struct value **args, CORE_ADDR sp,
                    int struct_return, CORE_ADDR struct_addr)
  {
!   char *fp;
!   int argnum, argreg, nstack_size;
  
    /* Walk through the list of args and determine how large a temporary
       stack is required.  Need to take care here as structs may be
!      passed on the stack, and we have to to push them.  */
!   nstack_size = -4 * REGISTER_SIZE;   /* Some arguments go into A1-A4.  */
!   if (struct_return)                  /* The struct address goes in A1.  */
!     nstack_size += REGISTER_SIZE;
! 
!   /* Walk through the arguments and add their size to nstack_size.  */
!   for (argnum = 0; argnum < nargs; argnum++)
!     {
!       int len;
!       struct type *arg_type;
! 
!       arg_type = check_typedef (VALUE_TYPE (args[argnum]));
!       len = TYPE_LENGTH (arg_type);
! 
!       nstack_size += len;
!     }
! 
!   /* Allocate room on the stack, and initialize our stack frame
!      pointer.  */
!   fp = NULL;
!   if (nstack_size > 0)
!     {
!       sp -= nstack_size;
!       fp = (char *) sp;
!     }
! 
!   /* Initialize the integer argument register pointer.  */
!   argreg = ARM_A1_REGNUM;
! 
!   /* The struct_return pointer occupies the first parameter passing
!      register.  */
!   if (struct_return)
!     write_register (argreg++, struct_addr);
! 
!   /* Process arguments from left to right.  Store as many as allowed
!      in the parameter passing registers (A1-A4), and save the rest on
!      the temporary stack.  */
!   for (argnum = 0; argnum < nargs; argnum++)
!     {
!       int len;
!       char *val;
!       CORE_ADDR regval;
!       enum type_code typecode;
!       struct type *arg_type, *target_type;
! 
!       arg_type = check_typedef (VALUE_TYPE (args[argnum]));
!       target_type = TYPE_TARGET_TYPE (arg_type);
!       len = TYPE_LENGTH (arg_type);
!       typecode = TYPE_CODE (arg_type);
!       val = (char *) VALUE_CONTENTS (args[argnum]);
! 
! #if 1
!       /* I don't know why this code was disable. The only logical use
!          for a function pointer is to call that function, so setting
!          the mode bit is perfectly fine.  FN */
!       /* If the argument is a pointer to a function, and it is a Thumb
!          function, set the low bit of the pointer.  */
!       if (TYPE_CODE_PTR == typecode
!         && NULL != target_type
!         && TYPE_CODE_FUNC == TYPE_CODE (target_type))
!       {
!         CORE_ADDR regval = extract_address (val, len);
!         if (arm_pc_is_thumb (regval))
!           store_address (val, len, MAKE_THUMB_ADDR (regval));
!       }
! #endif
!       /* Copy the argument to general registers or the stack in
!          register-sized pieces.  Large arguments are split between
!          registers and stack.  */
!       while (len > 0)
        {
!         int partial_len = len < REGISTER_SIZE ? len : REGISTER_SIZE;
  
!         if (argreg <= ARM_LAST_ARG_REGNUM)
            {
!             /* It's an argument being passed in a general register.  */
!             regval = extract_address (val, partial_len);
!             write_register (argreg++, regval);
            }
!         else
            {
!             /* Push the arguments onto the stack.  */
!             write_memory ((CORE_ADDR) fp, val, REGISTER_SIZE);
!             fp += REGISTER_SIZE;
            }
  
-         len -= partial_len;
-         val += partial_len;
        }
      }
  
!   /* Return adjusted stack pointer.  */
!   return sp;
  }
  
  /* Pop the current frame.  So long as the frame info has been
--- 1409,1533 ----
  arm_push_arguments (int nargs, struct value **args, CORE_ADDR sp,
                    int struct_return, CORE_ADDR struct_addr)
  {
!   CORE_ADDR fp;
!   int argnum;
!   int argreg;
!   int nstack;
!   int simd_argreg;
!   int second_pass;
!   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
  
    /* Walk through the list of args and determine how large a temporary
       stack is required.  Need to take care here as structs may be
!      passed on the stack, and we have to to push them.  On the second
!      pass, do the store.  */
!   nstack = 0;
!   fp = sp;
!   for (second_pass = 0; second_pass < 2; second_pass++)
!     {
!       /* Compute the FP using the information computed during the
!          first pass.  */
!       if (second_pass)
!       fp = sp - nstack;
! 
!       simd_argreg = 0;
!       argreg = ARM_A1_REGNUM;
!       nstack = 0;
! 
!       /* The struct_return pointer occupies the first parameter
!        passing register.  */
!       if (struct_return)
        {
!         if (second_pass)
!           {
!             if (arm_debug)
!               fprintf_unfiltered (gdb_stdlog,
!                                   "struct return in %s = 0x%s\n",
!                                   REGISTER_NAME (argreg),
!                                   paddr (struct_addr));
!             write_register (argreg, struct_addr);
!           }
!         argreg++;
!       }
  
!       for (argnum = 0; argnum < nargs; argnum++)
!       {
!         int len;
!         struct type *arg_type;
!         struct type *target_type;
!         enum type_code typecode;
!         char *val;
!         
!         arg_type = check_typedef (VALUE_TYPE (args[argnum]));
!         len = TYPE_LENGTH (arg_type);
!         target_type = TYPE_TARGET_TYPE (arg_type);
!         typecode = TYPE_CODE (arg_type);
!         val = VALUE_CONTENTS (args[argnum]);
!         
!         /* If the argument is a pointer to a function, and it is a
!            Thumb function, create a LOCAL copy of the value and set
!            the THUMB bit in it.  */
!         if (second_pass
!             && TYPE_CODE_PTR == typecode
!             && target_type != NULL
!             && TYPE_CODE_FUNC == TYPE_CODE (target_type))
            {
!             CORE_ADDR regval = extract_address (val, len);
!             if (arm_pc_is_thumb (regval))
!               {
!                 val = alloca (len);
!                 store_address (val, len, MAKE_THUMB_ADDR (regval));
!               }
            }
! 
!         /* Copy the argument to general registers or the stack in
!            register-sized pieces.  Large arguments are split between
!            registers and stack.  */
!         while (len > 0)
            {
!             int partial_len = len < REGISTER_SIZE ? len : REGISTER_SIZE;
!             
!             if (argreg <= ARM_LAST_ARG_REGNUM)
!               {
!                 /* The argument is being passed in a general purpose
!                    register.  */
!                 if (second_pass)
!                   {
!                     CORE_ADDR regval = extract_address (val,
!                                                         partial_len);
!                     if (arm_debug)
!                       fprintf_unfiltered (gdb_stdlog,
!                                           "arg %d in %s = 0x%s\n",
!                                           argnum,
!                                           REGISTER_NAME (argreg),
!                                           phex (regval, REGISTER_SIZE));
!                     write_register (argreg, regval);
!                   }
!                 argreg++;
!               }
!             else
!               {
!                 if (second_pass)
!                   {
!                     /* Push the arguments onto the stack.  */
!                     if (arm_debug)
!                       fprintf_unfiltered (gdb_stdlog,
!                                           "arg %d @ 0x%s + %d\n",
!                                           argnum, paddr (fp), nstack);
!                     write_memory (fp + nstack, val, REGISTER_SIZE);
!                   }
!                 nstack += REGISTER_SIZE;
!               }
!             
!             len -= partial_len;
!             val += partial_len;
            }
  
        }
      }
  
!   /* Return the botom of the argument list (pointed to by fp).  */
!   return fp;
  }
  
  /* Pop the current frame.  So long as the frame info has been
*************** The valid values are:\n");
*** 3076,3079 ****
--- 3099,3108 ----
    prologue_cache.saved_regs = NULL;
    prologue_cache.extra_info = (struct frame_extra_info *)
      xcalloc (1, sizeof (struct frame_extra_info));
+ 
+   /* Debugging flag.  */
+   add_show_from_set (add_set_cmd ("arm", class_maintenance, var_zinteger,
+                                 &arm_debug, "Set arm debugging.\n\
+ When non-zero, arm specific debugging is enabled.", &setdebuglist),
+                    &showdebuglist);
  }