This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: mips-tdep.c: FP varargs fixes
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