From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28306 invoked by alias); 26 Mar 2007 16:00:29 -0000 Received: (qmail 28293 invoked by uid 22791); 26 Mar 2007 16:00:28 -0000 X-Spam-Check-By: sourceware.org Received: from dmz.mips-uk.com (HELO dmz.mips-uk.com) (194.74.144.194) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 26 Mar 2007 17:00:22 +0100 Received: from internal-mx1 ([192.168.192.240] helo=ukservices1.mips.com) by dmz.mips-uk.com with esmtp (Exim 3.35 #1 (Debian)) id 1HVrcB-0005wh-00; Mon, 26 Mar 2007 17:00:11 +0100 Received: from perivale.mips.com ([192.168.192.200]) by ukservices1.mips.com with esmtp (Exim 3.36 #1 (Debian)) id 1HVrc1-00068i-00; Mon, 26 Mar 2007 17:00:01 +0100 Received: from macro (helo=localhost) by perivale.mips.com with local-esmtp (Exim 4.63) (envelope-from ) id 1HVrc1-00047j-4d; Mon, 26 Mar 2007 17:00:01 +0100 Date: Mon, 26 Mar 2007 16:00:00 -0000 From: "Maciej W. Rozycki" To: Mark Kettenis cc: gdb-patches@sourceware.org, Nigel Stephens , "Maciej W. Rozycki" Subject: Re: mips-tdep.c: FP varargs fixes In-Reply-To: <200703231449.l2NEnQSb031165@brahms.sibelius.xs4all.nl> Message-ID: References: <200703231449.l2NEnQSb031165@brahms.sibelius.xs4all.nl> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-MIPS-Technologies-UK-MailScanner: Found to be clean X-MIPS-Technologies-UK-MailScanner-From: macro@mips.com Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-03/txt/msg00231.txt.bz2 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 Nigel Stephens * 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