Re: mips-tdep.c: FP varargs fixes

["Maciej W. Rozycki" <macro@mips.com>]

On Fri, 23 Mar 2007, Mark Kettenis wrote:

> > @@ -3272,7 +3283,7 @@
> >  	      /* Write this portion of the argument to a general
> >  	         purpose register.  */
> >  	      if (argreg <= MIPS_LAST_ARG_REGNUM
> > -		  && !fp_register_arg_p (typecode, arg_type))
> > +		  /*&& !fp_register_arg_p (typecode, arg_type)*/)
> >  		{
> >  		  LONGEST regval = extract_signed_integer (val, partial_len);
> >  		  /* Value may need to be sign extended, because
> > 
> 
> What's up with this bit?  Your ChangeLog message says this:
> 
> > 	boundary, align stack_offset too.  Write floating-point
> > 	arguments to the appropriate integer register, if we're not
> > 	passing in a floating point register.
> 
> But that matches the old code, not the new code.

 That's unfortunate wording, but I took this chance as an opportunity to 
further review the patch and here is the result.  The current code is 
incorrect as is, because for the O32 ABI once two FP arguments have been 
passed in floating point registers or any non-FP arguments have been 
passed in general registers, but the amount of data passed so far has not 
reached four 32-bit words (which may be the case if single floats are 
involved) further FP arguments are passed in general registers.  Only once 
four 32-bit words have been used, further arguments are passed on the 
stack.

 I have now adjusted mips_o64_push_dummy_call() accordingly and rephrased 
the text in the ChangeLog entry for clarity.

 This new patch has been tested natively for mips-unknown-linux-gnu and 
remotely for mipsisa32-sde-elf, using mips-sim-sde32/-EB and 
mips-sim-sde32/-EL as the targets.

2007-03-26  Maciej W. Rozycki  <macro@mips.com>
            Nigel Stephens  <nigel@mips.com>

	* mips-tdep.c (mips_o32_push_dummy_call): Take account of
	argument alignment requirements when calculating stack space
	required.  When aligning an arg register to eight bytes
	boundary, align stack_offset too.  Write floating-point
	arguments to the appropriate integer register if need go there.
	(mips_o64_push_dummy_call): Likewise.

 It looks like the calls to mips_abi_regsize() within are now redundant in 
these functions, because the result is implied by the very fact of calling 
either of these functions, but this is functionally independent from this 
change so I think it should be dealt with separately.

 OK to apply?

  Maciej

12100-0.diff
Index: binutils-quilt/src/gdb/mips-tdep.c
===================================================================
--- binutils-quilt.orig/src/gdb/mips-tdep.c	2007-03-26 14:16:14.000000000 +0100
+++ binutils-quilt/src/gdb/mips-tdep.c	2007-03-26 15:46:29.000000000 +0100
@@ -3071,8 +3071,17 @@
 
   /* Now make space on the stack for the args.  */
   for (argnum = 0; argnum < nargs; argnum++)
-    len += align_up (TYPE_LENGTH (value_type (args[argnum])),
-		     mips_stack_argsize (gdbarch));
+    {
+      struct type *arg_type = check_typedef (value_type (args[argnum]));
+      int arglen = TYPE_LENGTH (arg_type);
+
+      /* Align to double-word if necessary.  */
+      if (mips_abi_regsize (gdbarch) < 8
+	  && mips_type_needs_double_align (arg_type))
+	len = align_up (len, mips_stack_argsize (gdbarch) * 2);
+      /* Allocate space on the stack.  */
+      len += align_up (arglen, mips_stack_argsize (gdbarch));
+    }
   sp -= align_up (len, 16);
 
   if (mips_debug)
@@ -3208,10 +3217,11 @@
 	      && mips_type_needs_double_align (arg_type))
 	    {
 	      if ((argreg & 1))
-		argreg++;
+		{
+		  argreg++;
+		  stack_offset += mips_abi_regsize (gdbarch);
+		}
 	    }
-	  /* Note: Floating-point values that didn't fit into an FP
-	     register are only written to memory.  */
 	  while (len > 0)
 	    {
 	      /* Remember if the argument was written to the stack.  */
@@ -3225,8 +3235,7 @@
 
 	      /* 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))
+		  || odd_sized_struct)
 		{
 		  /* Should shorter than int integer values be
 		     promoted to int before being stored? */
@@ -3267,12 +3276,10 @@
 		}
 
 	      /* Note!!! This is NOT an else clause.  Odd sized
-	         structs may go thru BOTH paths.  Floating point
-	         arguments will not.  */
+	         structs may go thru BOTH paths.  */
 	      /* Write this portion of the argument to a general
 	         purpose register.  */
-	      if (argreg <= MIPS_LAST_ARG_REGNUM
-		  && !fp_register_arg_p (typecode, arg_type))
+	      if (argreg <= MIPS_LAST_ARG_REGNUM)
 		{
 		  LONGEST regval = extract_signed_integer (val, partial_len);
 		  /* Value may need to be sign extended, because
@@ -3525,8 +3532,17 @@
 
   /* Now make space on the stack for the args.  */
   for (argnum = 0; argnum < nargs; argnum++)
-    len += align_up (TYPE_LENGTH (value_type (args[argnum])),
-		     mips_stack_argsize (gdbarch));
+    {
+      struct type *arg_type = check_typedef (value_type (args[argnum]));
+      int arglen = TYPE_LENGTH (arg_type);
+
+      /* Align to double-word if necessary.  */
+      if (mips_abi_regsize (gdbarch) < 8
+	  && mips_type_needs_double_align (arg_type))
+	len = align_up (len, mips_stack_argsize (gdbarch) * 2);
+      /* Allocate space on the stack.  */
+      len += align_up (arglen, mips_stack_argsize (gdbarch));
+    }
   sp -= align_up (len, 16);
 
   if (mips_debug)
@@ -3662,10 +3678,11 @@
 	      && mips_type_needs_double_align (arg_type))
 	    {
 	      if ((argreg & 1))
-		argreg++;
+		{
+		  argreg++;
+		  stack_offset += mips_abi_regsize (gdbarch);
+		}
 	    }
-	  /* Note: Floating-point values that didn't fit into an FP
-	     register are only written to memory.  */
 	  while (len > 0)
 	    {
 	      /* Remember if the argument was written to the stack.  */
@@ -3679,8 +3696,7 @@
 
 	      /* 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))
+		  || odd_sized_struct)
 		{
 		  /* Should shorter than int integer values be
 		     promoted to int before being stored? */
@@ -3721,12 +3737,10 @@
 		}
 
 	      /* Note!!! This is NOT an else clause.  Odd sized
-	         structs may go thru BOTH paths.  Floating point
-	         arguments will not.  */
+	         structs may go thru BOTH paths.  */
 	      /* Write this portion of the argument to a general
 	         purpose register.  */
-	      if (argreg <= MIPS_LAST_ARG_REGNUM
-		  && !fp_register_arg_p (typecode, arg_type))
+	      if (argreg <= MIPS_LAST_ARG_REGNUM)
 		{
 		  LONGEST regval = extract_signed_integer (val, partial_len);
 		  /* Value may need to be sign extended, because