From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3823 invoked by alias); 25 Sep 2003 21:39:03 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 3808 invoked from network); 25 Sep 2003 21:39:00 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 25 Sep 2003 21:39:00 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h8PLd0108836 for ; Thu, 25 Sep 2003 17:39:00 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h8PLd0c03383 for ; Thu, 25 Sep 2003 17:39:00 -0400 Received: from localhost.redhat.com (devserv.devel.redhat.com [172.16.58.1]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id h8PLcxTX016779 for ; Thu, 25 Sep 2003 17:38:59 -0400 Received: by localhost.redhat.com (Postfix, from userid 469) id 4D7042CCA4; Thu, 25 Sep 2003 17:48:50 -0400 (EDT) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <16243.25282.58326.132364@localhost.redhat.com> Date: Thu, 25 Sep 2003 21:39:00 -0000 To: gdb-patches@sources.redhat.com Subject: Re: SH follow up, part 1 (was Re: [RFA] sh-tdep.c: Follow up patch to implement two different ABIs) In-Reply-To: <20030924095552.GA8414@cygbert.vinschen.de> References: <20030917161127.GM9981@cygbert.vinschen.de> <16240.45203.260059.116019@localhost.redhat.com> <20030924095552.GA8414@cygbert.vinschen.de> X-SW-Source: 2003-09/txt/msg00578.txt.bz2 Corinna Vinschen writes: > On Tue, Sep 23, 2003 at 04:44:03PM -0400, Elena Zannoni wrote: > > I think this patch has too much stuff in it. Can you change it to fix > > the gcc abi (the fpu stuff), and then add the rest for the renesas > > variant? > > Ok, step 1: > Seems ok, are there diffs in the test results before&after? Are these explanations also in the code? If not, can you please add them? > - To simplify the sh_push_dummy_call*() functions, I created two helper > functions which took the part of right-justifying values < 4 byte, > called sh_justify_value_in_reg(), and to count the number of bytes > to be allocated on stack for all arguments, called sh_stack_allocsize(). > > - Two new functions are now used to evaluate the next floating point > register to use for an argument: > > - gcc passes floats in little-endian mode using regsters criss-crossing, > fr5, fr4, fr7, fr6, fr9, fr8 ... In big-endian mode the order is simple > fr4, fr5, fr6, ... > > - Doubles are passed in even register pairs, fr4/fr5, fr6/fr7, ... > Example: > > void foo(float a, double b, float c); > > a is passed in fr4, b then skips the odd-numbered fr5 register, so it's > passed in fr6/fr7 and c is then passed in the next free register, fr8. > > - I renamed the flag `odd_sized_struct' to a more descriptive name, > `pass_on_stack' since the old name does in no way reflect how the > decision about passing on stack or in register is actually made. The > actual decision is as follows: > > - On FPU CPUs, pass in registers unless the datatype is bigger than 16 bytes. > > - On non-FPU CPUs, no special case exists. > > - On all CPUs, everything else is passed in registers until the argument > registers are filled up, the remaining arguments are passed on stack. > If an argument doesn't fit entirely in the remaining registers, it's > split between regs and stack as see fit. > > - The code and comment that some data is sometimes passed in registers > *and* stack simultaneously is dropped. That's not how it works. > > > Actually this is part of the original port as contributed by Steve > > Chamberlain. Is this true of gcc or of the original hitachi abi? > > Neither, nor. It must have been some sort of misunderstanding or a bug > in an ancient gcc. I talked about this with Alexandre Oliva and he has > no idea when this has ever been the case for gcc at all. > > - Also in sh_push_dummy_call*(), the code which adds 4 for every 4 bytes > managed, is changed from e.g. > > len -= register_size (gdbarch, argreg); > > to > > len -= 4; > > The reason is that the original line is not exactly correct. It adds the > register_size of the *next* register, not the register actually filled > with data. Since all data and registers in question are 4 byte regs (and > the target specific code should know that anyway), I've simplified the > affected code. > > > I don't like to have harcoded register sizes, if argreg is wrong, is > > there a way to get the correct parameter? > > All registers are always 4 byte. It's easy enough and an additional comment > would be sufficient. > > If you don't like that and want to use register_size correctly in that case, > it would have to look like this: > Yes please. When you are done with this rewrite, it would be good to pass the file through gdb_indent.sh. elena > --- sh-tdep.c 2003-09-24 11:29:00.000000000 +0200 > +++ sh-tdep.c.weia 2003-09-24 11:46:28.000000000 +0200 > @@ -1046,7 +1046,7@@ sh_push_dummy_call_fpu (struct gdbarch * > struct type *type; > CORE_ADDR regval; > char *val; > - int len; > + int len, reg_size; > int pass_on_stack; > > /* first force sp to a 4-byte alignment */ > @@ -1098,6 +1098,7 @@ sh_push_dummy_call_fpu (struct gdbarch * > && flt_argreg <= FLOAT_ARGLAST_REGNUM) > { > /* Argument goes in a float argument register. */ > + reg_size = register_size (gdbarch, flt_argreg); > regval = extract_unsigned_integer (val, register_size (gdbarch, > argreg)); > regcache_cooked_write_unsigned (regcache, flt_argreg++, regval); > @@ -1105,6 +1106,7 @@ sh_push_dummy_call_fpu (struct gdbarch * > else if (argreg <= ARGLAST_REGNUM) > { > /* there's room in a register */ > + reg_size = register_size (gdbarch, argreg); > regval = extract_unsigned_integer (val, register_size (gdbarch, > argreg)); > regcache_cooked_write_unsigned (regcache, argreg++, regval); > @@ -1112,8 +1114,8 @@ sh_push_dummy_call_fpu (struct gdbarch * > /* Store the value 4 bytes at a time. This means that things > larger than 4 bytes may go partly in registers and partly > on the stack. */ > - len -= 4; > - val += 4; > + len -= reg_size; > + val += reg_size; > } > } > > Corinna > > > ChangeLog: > ========== > > * sh-tdep.c (sh_justify_value_in_reg): New function. > (sh_stack_allocsize): Ditto. > (flt_argreg_array): New array used for floating point argument > passing. > (sh_init_flt_argreg): New function. > (sh_next_flt_argreg): Ditto. > (sh_push_dummy_call_fpu): Simplify. Rename "odd_sized_struct" to > "pass_on_stack". Use new helper functions. Accomodate Renesas ABI. > Fix argument passing strategy. > (sh_push_dummy_call_nofpu): Ditto. > > Index: sh-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/sh-tdep.c,v > retrieving revision 1.141 > diff -u -p -r1.141 sh-tdep.c > --- sh-tdep.c 16 Sep 2003 18:56:35 -0000 1.141 > +++ sh-tdep.c 24 Sep 2003 09:29:10 -0000 > @@ -938,6 +938,98 @@ sh_frame_align (struct gdbarch *ignore, > not displace any of the other arguments passed in via registers R4 > to R7. */ > > +/* Helper function to justify value in register according to endianess. */ > +static char * > +sh_justify_value_in_reg (struct value *val, int len) > +{ > + static char valbuf[4]; > + > + memset (valbuf, 0, sizeof (valbuf)); > + if (len < 4) > + { > + /* value gets right-justified in the register or stack word */ > + if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG) > + memcpy (valbuf + (4 - len), (char *) VALUE_CONTENTS (val), len); > + else > + memcpy (valbuf, (char *) VALUE_CONTENTS (val), len); > + return valbuf; > + } > + return (char *) VALUE_CONTENTS (val); > +} > + > +/* Helper function to eval number of bytes to allocate on stack. */ > +static CORE_ADDR > +sh_stack_allocsize (int nargs, struct value **args) > +{ > + int stack_alloc = 0; > + while (nargs-- > 0) > + stack_alloc += ((TYPE_LENGTH (VALUE_TYPE (args[nargs])) + 3) & ~3); > + return stack_alloc; > +} > + > +/* Helper functions for getting the float arguments right. Registers usage > + depends on the ABI and the endianess. The comments should enlighten how > + it's intended to work. */ > + > +/* This array stores which of the float arg registers are already in use. */ > +static int flt_argreg_array[FLOAT_ARGLAST_REGNUM - FLOAT_ARG0_REGNUM + 1]; > + > +/* This function just resets the above array to "no reg used so far". */ > +static void > +sh_init_flt_argreg (void) > +{ > + memset (flt_argreg_array, 0, sizeof flt_argreg_array); > +} > + > +/* This function returns the next register to use for float arg passing. > + It returns either a valid value between FLOAT_ARG0_REGNUM and > + FLOAT_ARGLAST_REGNUM if a register is available, otherwise it returns > + FLOAT_ARGLAST_REGNUM + 1 to indicate that no register is available. > + > + Note that register number 0 in flt_argreg_array corresponds with the > + real float register fr4. In contrast to FLOAT_ARG0_REGNUM (value is > + 29) the parity of the register number is preserved, which is important > + for the double register passing test (see the "argreg & 1" test below). */ > +static int > +sh_next_flt_argreg (int len) > +{ > + int argreg; > + > + /* First search for the next free register. */ > + for (argreg = 0; argreg <= FLOAT_ARGLAST_REGNUM - FLOAT_ARG0_REGNUM; ++argreg) > + if (!flt_argreg_array[argreg]) > + break; > + > + /* No register left? */ > + if (argreg > FLOAT_ARGLAST_REGNUM - FLOAT_ARG0_REGNUM) > + return FLOAT_ARGLAST_REGNUM + 1; > + > + if (len == 8) > + { > + /* Doubles are always starting in a even register number. */ > + if (argreg & 1) > + { > + flt_argreg_array[argreg] = 1; > + > + ++argreg; > + > + /* No register left? */ > + if (argreg > FLOAT_ARGLAST_REGNUM - FLOAT_ARG0_REGNUM) > + return FLOAT_ARGLAST_REGNUM + 1; > + } > + /* Also mark the next register as used. */ > + flt_argreg_array[argreg + 1] = 1; > + } > + else if (TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE) > + { > + /* In little endian, gcc passes floats like this: f5, f4, f7, f6, ... */ > + if (!flt_argreg_array[argreg + 1]) > + ++argreg; > + } > + flt_argreg_array[argreg] = 1; > + return FLOAT_ARG0_REGNUM + argreg; > +} > + > static CORE_ADDR > sh_push_dummy_call_fpu (struct gdbarch *gdbarch, > CORE_ADDR func_addr, > @@ -947,15 +1039,15 @@ sh_push_dummy_call_fpu (struct gdbarch * > CORE_ADDR sp, int struct_return, > CORE_ADDR struct_addr) > { > - int stack_offset, stack_alloc; > - int argreg, flt_argreg; > + int stack_offset = 0; > + int argreg = ARG0_REGNUM; > + int flt_argreg; > int argnum; > struct type *type; > CORE_ADDR regval; > char *val; > - char valbuf[4]; > int len; > - int odd_sized_struct; > + int pass_on_stack; > > /* first force sp to a 4-byte alignment */ > sp = sh_frame_align (gdbarch, sp); > @@ -967,65 +1059,51 @@ sh_push_dummy_call_fpu (struct gdbarch * > STRUCT_RETURN_REGNUM, > struct_addr); > > - /* Now make sure there's space on the stack */ > - for (argnum = 0, stack_alloc = 0; argnum < nargs; argnum++) > - stack_alloc += ((TYPE_LENGTH (VALUE_TYPE (args[argnum])) + 3) & ~3); > - sp -= stack_alloc; /* make room on stack for args */ > + /* make room on stack for args */ > + sp -= sh_stack_allocsize (nargs, args); > + > + /* Initialize float argument mechanism. */ > + sh_init_flt_argreg (); > > /* Now load as many as possible of the first arguments into > registers, and push the rest onto the stack. There are 16 bytes > in four registers available. Loop thru args from first to last. */ > - > - argreg = ARG0_REGNUM; > - flt_argreg = FLOAT_ARG0_REGNUM; > - for (argnum = 0, stack_offset = 0; argnum < nargs; argnum++) > + for (argnum = 0; argnum < nargs; argnum++) > { > type = VALUE_TYPE (args[argnum]); > len = TYPE_LENGTH (type); > - memset (valbuf, 0, sizeof (valbuf)); > - if (len < 4) > - { > - /* value gets right-justified in the register or stack word */ > - if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG) > - memcpy (valbuf + (4 - len), > - (char *) VALUE_CONTENTS (args[argnum]), len); > - else > - memcpy (valbuf, (char *) VALUE_CONTENTS (args[argnum]), len); > - val = valbuf; > - } > - else > - val = (char *) VALUE_CONTENTS (args[argnum]); > + val = sh_justify_value_in_reg (args[argnum], len); > + > + /* Some decisions have to be made how various types are handled. > + This also differs in different ABIs. */ > + pass_on_stack = 0; > + if (len > 16) > + pass_on_stack = 1; /* Types bigger than 16 bytes are passed on stack. */ > + > + /* Find out the next register to use for a floating point value. */ > + if (TYPE_CODE (type) == TYPE_CODE_FLT) > + flt_argreg = sh_next_flt_argreg (len); > > - if (len > 4 && (len & 3) != 0) > - odd_sized_struct = 1; /* Such structs go entirely on stack. */ > - else if (len > 16) > - odd_sized_struct = 1; /* So do aggregates bigger than 4 words. */ > - else > - odd_sized_struct = 0; > while (len > 0) > { > if ((TYPE_CODE (type) == TYPE_CODE_FLT > - && flt_argreg > FLOAT_ARGLAST_REGNUM) > + && flt_argreg > FLOAT_ARGLAST_REGNUM) > || argreg > ARGLAST_REGNUM > - || odd_sized_struct) > - { > - /* must go on the stack */ > + || pass_on_stack) > + { > write_memory (sp + stack_offset, val, 4); > stack_offset += 4; > } > - /* NOTE WELL!!!!! This is not an "else if" clause!!! > - That's because some *&^%$ things get passed on the stack > - AND in the registers! */ > - if (TYPE_CODE (type) == TYPE_CODE_FLT && > - flt_argreg > 0 && flt_argreg <= FLOAT_ARGLAST_REGNUM) > + else if (TYPE_CODE (type) == TYPE_CODE_FLT > + && flt_argreg <= FLOAT_ARGLAST_REGNUM) > { > - /* Argument goes in a single-precision fp reg. */ > + /* Argument goes in a float argument register. */ > regval = extract_unsigned_integer (val, register_size (gdbarch, > argreg)); > regcache_cooked_write_unsigned (regcache, flt_argreg++, regval); > } > else if (argreg <= ARGLAST_REGNUM) > - { > + { > /* there's room in a register */ > regval = extract_unsigned_integer (val, register_size (gdbarch, > argreg)); > @@ -1034,8 +1112,8 @@ sh_push_dummy_call_fpu (struct gdbarch * > /* Store the value 4 bytes at a time. This means that things > larger than 4 bytes may go partly in registers and partly > on the stack. */ > - len -= register_size (gdbarch, argreg); > - val += register_size (gdbarch, argreg); > + len -= 4; > + val += 4; > } > } > > @@ -1057,15 +1135,13 @@ sh_push_dummy_call_nofpu (struct gdbarch > CORE_ADDR sp, int struct_return, > CORE_ADDR struct_addr) > { > - int stack_offset, stack_alloc; > - int argreg; > + int stack_offset = 0; > + int argreg = ARG0_REGNUM; > int argnum; > struct type *type; > CORE_ADDR regval; > char *val; > - char valbuf[4]; > int len; > - int odd_sized_struct; > > /* first force sp to a 4-byte alignment */ > sp = sh_frame_align (gdbarch, sp); > @@ -1077,52 +1153,27 @@ sh_push_dummy_call_nofpu (struct gdbarch > STRUCT_RETURN_REGNUM, > struct_addr); > > - /* Now make sure there's space on the stack */ > - for (argnum = 0, stack_alloc = 0; argnum < nargs; argnum++) > - stack_alloc += ((TYPE_LENGTH (VALUE_TYPE (args[argnum])) + 3) & ~3); > - sp -= stack_alloc; /* make room on stack for args */ > + /* make room on stack for args */ > + sp -= sh_stack_allocsize (nargs, args); > > /* Now load as many as possible of the first arguments into > registers, and push the rest onto the stack. There are 16 bytes > in four registers available. Loop thru args from first to last. */ > - > - argreg = ARG0_REGNUM; > - for (argnum = 0, stack_offset = 0; argnum < nargs; argnum++) > - { > + for (argnum = 0; argnum < nargs; argnum++) > + { > type = VALUE_TYPE (args[argnum]); > len = TYPE_LENGTH (type); > - memset (valbuf, 0, sizeof (valbuf)); > - if (len < 4) > - { > - /* value gets right-justified in the register or stack word */ > - if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG) > - memcpy (valbuf + (4 - len), > - (char *) VALUE_CONTENTS (args[argnum]), len); > - else > - memcpy (valbuf, (char *) VALUE_CONTENTS (args[argnum]), len); > - val = valbuf; > - } > - else > - val = (char *) VALUE_CONTENTS (args[argnum]); > + val = sh_justify_value_in_reg (args[argnum], len); > > - if (len > 4 && (len & 3) != 0) > - odd_sized_struct = 1; /* such structs go entirely on stack */ > - else > - odd_sized_struct = 0; > while (len > 0) > { > - if (argreg > ARGLAST_REGNUM > - || odd_sized_struct) > - { > - /* must go on the stack */ > + if (argreg > ARGLAST_REGNUM) > + { > write_memory (sp + stack_offset, val, 4); > - stack_offset += 4; > + stack_offset += 4; > } > - /* NOTE WELL!!!!! This is not an "else if" clause!!! > - That's because some *&^%$ things get passed on the stack > - AND in the registers! */ > - if (argreg <= ARGLAST_REGNUM) > - { > + else if (argreg <= ARGLAST_REGNUM) > + { > /* there's room in a register */ > regval = extract_unsigned_integer (val, register_size (gdbarch, > argreg)); > @@ -1131,8 +1182,8 @@ sh_push_dummy_call_nofpu (struct gdbarch > /* Store the value 4 bytes at a time. This means that things > larger than 4 bytes may go partly in registers and partly > on the stack. */ > - len -= register_size (gdbarch, argreg); > - val += register_size (gdbarch, argreg); > + len -= 4; > + val += 4; > } > } > > -- > Corinna Vinschen > Cygwin Developer > Red Hat, Inc.