This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] mips_push_arguments gdbarch-ified


These changes fix 16 testsuite failures, based in the Irix native compiler running with -n32 (and produce no regressions for -o32).
Yes, with tweaks.

General comment. You may want to consider restructuring the function to read:

for (argument in arguments)
for (argument broken down in to 8 byte chunks)
if (fp argument && fnpreg <= num fp arg regs)
write argument chunk to fp register
else if (!fp argument && reg <= num arg regs)
write chunk (carefully aligned) to gp register
else
write chunk to (carefully aligned) memory



The comment below out of date. Both n32 and n64 require quad word (4x32 == 128 bit) alignment.

+   /* First ensure that the stack and structure return address (if any)
+      are properly aligned.  The stack has to be at least 64-bit
+      aligned even on 32-bit machines, because doubles must be 64-bit
+      aligned.  On at least one MIPS variant, stack frames need to be
+      128-bit aligned, so we round to this widest known alignment.  */


No need to refer to EABI.

+   /* Now make space on the stack for the args.  We allocate more
+      than necessary for EABI, because the first few arguments are
+      passed in registers, but that's OK.  */

+ for (argnum = 0; argnum < nargs; argnum++)
+ len += ROUND_UP (TYPE_LENGTH (VALUE_TYPE (args[argnum])), + MIPS_STACK_ARGSIZE);
+ sp -= ROUND_UP (len, 16);
+ + if (mips_debug)
+ fprintf_unfiltered (gdb_stdlog, + "mips_push_arguments: sp=0x%lx allocated %d\n",
mips_n32n64_... et.al.



+ 			(long) sp, ROUND_UP (len, 16));
Might as well use ``paddr_nz (s)''. et.al.



+ + /* Initialize the integer and float register pointers. */
+ argreg = A0_REGNUM;
+ float_argreg = FPA0_REGNUM;
+ + /* the struct_return pointer occupies the first parameter-passing reg */
+ if (struct_return)
+ {
+ if (mips_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "mips_push_arguments: struct_return reg=%d 0x%lx\n",
+ argreg, (long) struct_addr);
+ write_register (argreg++, struct_addr);


This can go.  n32n64 registers do not have homes.

+       if (MIPS_REGS_HAVE_HOME_P)
+ 	stack_offset += MIPS_STACK_ARGSIZE;

+ }
+ + /* Now load as many as possible of the first arguments into
+ registers, and push the rest onto the stack. Loop thru args
+ from first to last. */
+ for (argnum = 0; argnum < nargs; argnum++)
+ {
+ char *val;
+ char valbuf[MAX_REGISTER_RAW_SIZE];
... = alloca (MAX_REGISTER_RAW_SIZE);



+ struct value *arg = args[argnum];
+ struct type *arg_type = check_typedef (VALUE_TYPE (arg));
+ int len = TYPE_LENGTH (arg_type);
+ enum type_code typecode = TYPE_CODE (arg_type);
+ + if (mips_debug)
+ fprintf_unfiltered (gdb_stdlog,
+ "mips_push_arguments: %d len=%d type=%d",
+ argnum + 1, len, (int) typecode);
+ + val = (char *) VALUE_CONTENTS (arg);


The below can go.  It only applies to o32 ...

+       /* 32-bit ABIs always start floating point arguments in an
+          even-numbered floating point register.  Round the FP register
+          up before the check to see if there are any FP registers
+          left.  Non MIPS_EABI targets also pass the FP in the integer
+          registers so also round up normal registers.  */
+       if (!FP_REGISTER_DOUBLE
+ 	  && fp_register_arg_p (typecode, arg_type))
+ 	{
+ 	  if ((float_argreg & 1))
+ 	    float_argreg++;
+ 	}



The below can go, it doesn't apply to n32/n64.

+       /* Floating point arguments passed in registers have to be
+          treated specially.  On 32-bit architectures, doubles
+          are passed in register pairs; the even register gets
+          the low word, and the odd register gets the high word.
+          On non-EABI processors, the first two floating point arguments are
+          also copied to general registers, because MIPS16 functions
+          don't use float registers for arguments.  This duplication of
+          arguments in general registers can't hurt non-MIPS16 functions
+          because those registers are normally skipped.  */

+       if (fp_register_arg_p (typecode, arg_type)
+ 	  && float_argreg <= MIPS_LAST_FP_ARG_REGNUM)
+ 	{


The entire if() below can be zapped.  FP_REGISTER_DOUBLE is true.
(if someone is stupid to try to use n32/n64 on a MIPS1 then they loose :-)

+ 	  if (!FP_REGISTER_DOUBLE && len == 8)
+ 	    {
+ 	      int low_offset = TARGET_BYTE_ORDER == BFD_ENDIAN_BIG ? 4 : 0;
+ 	      unsigned long regval;


That will leave just this else clause ...

+ 	  else
+ 	    {
+ 	      /* This is a floating point value that fits entirely
+ 	         in a single register.  */
+ 	      /* On 32 bit ABI's the float_argreg is further adjusted
+                  above to ensure that it is even register aligned.  */
+ 	      LONGEST regval = extract_unsigned_integer (val, len);
+ 	      if (mips_debug)
+ 		fprintf_unfiltered (gdb_stdlog, " - fpreg=%d val=%s",
+ 				    float_argreg, phex (regval, len));
+ 	      write_register (float_argreg++, regval);


My comment is wrong and should be deleted.  n32 uses every register.

+ 	      /* CAGNEY: 32 bit MIPS ABI's always reserve two FP
+ 		 registers for each argument.  The below is (my
+ 		 guess) to ensure that the corresponding integer
+ 		 register has reserved the same space.  */

+ 	      if (mips_debug)
+ 		fprintf_unfiltered (gdb_stdlog, " - reg=%d val=%s",
+ 				    argreg, phex (regval, len));
+ 	      write_register (argreg, regval);


Since FP_REGISTER_DOUBLE is true, this can be simplified.

+ 	      argreg += FP_REGISTER_DOUBLE ? 1 : 2;
+ 	    }


The below can go. n32n64 do not have a home for the registers used as parameters.

+ 	  /* Reserve space for the FP register.  */
+ 	  if (MIPS_REGS_HAVE_HOME_P)
+ 	    stack_offset += ROUND_UP (len, MIPS_STACK_ARGSIZE);
+ 	}


+ else
+ {
+ /* Copy the argument to general registers or the stack in
+ register-sized pieces. Large arguments are split between
+ registers and stack. */
+ /* Note: structs whose size is not a multiple of MIPS_REGSIZE
+ are treated specially: Irix cc passes them in registers
+ where gcc sometimes puts them on the stack. For maximum
+ compatibility, we will put them in both places. */
+ int odd_sized_struct = ((len > MIPS_SAVED_REGSIZE) &&
+ (len % MIPS_SAVED_REGSIZE != 0));
+ /* Note: Floating-point values that didn't fit into an FP
+ register are only written to memory. */
+ while (len > 0)
+ {
+ /* Rememer if the argument was written to the stack. */
+ int stack_used_p = 0;
+ int partial_len = len < MIPS_SAVED_REGSIZE ? + len : MIPS_SAVED_REGSIZE;

+ + if (mips_debug)
+ fprintf_unfiltered (gdb_stdlog, " -- partial=%d",
+ partial_len);

I believe the below if is wrong. It should only write to the stack when something like:

!fp_register_arg && argreg > MIPS_LAST_ARG_REGNUM
|| fpregister_arg && fpreg > LAST_FPREGNUM

Odd sized_structs are put in registers (gcc might not though :-()

+ 	      /* Write this portion of the argument to the stack.  */
+ 	      if (argreg > MIPS_LAST_ARG_REGNUM
+ 		  || odd_sized_struct
+ 		  || fp_register_arg_p (typecode, arg_type))

+ 		  write_memory (addr, val, partial_len);
+ 		}


See suggestion at start about re-structuring the code.


+ 	      /* Note!!! This is NOT an else clause.  Odd sized
+ 	         structs may go thru BOTH paths.  Floating point
+ 	         arguments will not.  */
+ 	      /* Write this portion of the argument to a general
+                  purpose register.  */
+ 	      if (argreg <= MIPS_LAST_ARG_REGNUM
+ 		  && !fp_register_arg_p (typecode, arg_type))

+ {
+ LONGEST regval = extract_unsigned_integer (val, partial_len);
+ + /* A non-floating-point argument being passed in a
+ general register. If a struct or union, and if
+ the remaining length is smaller than the register
+ size, we have to adjust the register value on
+ big endian targets.
+ + It does not seem to be necessary to do the
+ same for integral types.
+ + cagney/2001-07-23: gdb/179: Also, GCC, when
+ outputting LE O32 with sizeof (struct) <
+ MIPS_SAVED_REGSIZE, generates a left shift as
+ part of storing the argument in a register a
+ register (the left shift isn't generated when
+ sizeof (struct) >= MIPS_SAVED_REGSIZE). Since it
+ is quite possible that this is GCC contradicting
+ the LE/O32 ABI, GDB has not been adjusted to
+ accommodate this. Either someone needs to
+ demonstrate that the LE/O32 ABI specifies such a
+ left shift OR this new ABI gets identified as
+ such and GDB gets tweaked accordingly. */
+ + if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
+ && partial_len < MIPS_SAVED_REGSIZE
+ && (typecode == TYPE_CODE_STRUCT ||
+ typecode == TYPE_CODE_UNION))
+ regval <<= ((MIPS_SAVED_REGSIZE - partial_len) *
+ TARGET_CHAR_BIT);
+ + if (mips_debug)
+ fprintf_filtered (gdb_stdlog, " - reg=%d val=%s",
+ argreg,
+ phex (regval, MIPS_SAVED_REGSIZE));
+ write_register (argreg, regval);
+ argreg++;

The below is wrong for n32/n64. Even when the general registers have run out, FP registers continue to be filled in.

+ /* Prevent subsequent floating point arguments from
+ being passed in floating point registers. */
+ float_argreg = MIPS_LAST_FP_ARG_REGNUM + 1;
+ }
+ + len -= partial_len;
+ val += partial_len;


+ /* Compute the the offset into the stack at which we
+ will copy the next parameter.
+ + In older ABIs, the caller reserved space for
+ registers that contained arguments. This was loosely
+ refered to as their "home". Consequently, space is
+ always allocated.
+ + In N32, the stack_offset only needs to be adjusted
+ when it has been used. FIXME is this true? */
+ + if (MIPS_REGS_HAVE_HOME_P || stack_used_p)
+ stack_offset += ROUND_UP (partial_len, MIPS_STACK_ARGSIZE);
+ }
+ }
+ if (mips_debug)
+ fprintf_unfiltered (gdb_stdlog, "\n");
+ }
+ + /* Return adjusted stack pointer. */
+ return sp;
+ }
Andrew


PS: Not sure what happened here:

*************** return_value_location (struct type *valt
*** 3612,3618 ****
  	  lo->reg_offset = MIPS_SAVED_REGSIZE - len;
  	  lo->len = len;
  	  hi->reg_offset = 0;
! 	  hi->len = 0;
  	}
        else if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
  	       && len > MIPS_SAVED_REGSIZE	/* odd-size structs */
--- 3910,3916 ----
  	  lo->reg_offset = MIPS_SAVED_REGSIZE - len;
  	  lo->len = len;
  	  hi->reg_offset = 0;
!   	  hi->len = 0;
  	}
        else if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG
  	       && len > MIPS_SAVED_REGSIZE	/* odd-size structs */

Andrew



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]