This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] sh-tdep.c: Fix little endian problem with doubles
Corinna Vinschen writes:
> On Fri, Oct 10, 2003 at 05:47:20PM -0400, Elena Zannoni wrote:
> > Corinna Vinschen writes:
> > > --- sh-tdep.c.INTERIM 2003-10-04 13:22:01.000000000 +0200
> > > +++ sh-tdep.c 2003-10-07 18:42:13.000000000 +0200
> > > @@ -846,6 +846,17 @@ sh_push_dummy_call_fpu (struct gdbarch *
> > > /* Argument goes in a float argument register. */
> > > reg_size = register_size (gdbarch, flt_argreg);
> > > regval = extract_unsigned_integer (val, reg_size);
> > > + /* A float type taking two registers must be handled
> > > + differently in LE mode. */
> > > + if (TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE
> > > + && len == 2 * reg_size)
> > > + {
> > > + regcache_cooked_write_unsigned (regcache, flt_argreg + 1,
> > > + regval);
> > > + val += reg_size;
> > > + len -= reg_size;
> > > + regval = extract_unsigned_integer (val, reg_size);
> > > + }
> >
> > I'd prefer if there is an 'else if' clause just for the
> > doubles. I.e. don't use len in the test, but TYPE_LENGTH(type). This is
> > too confusing.
>
> Erm... sorry, I don't quite understand. An `else if' in conjuction
> with what `if'? Actually, the double case is handled normally in
> BE mode, it's only slightly different in LE mode in that the registers
> are swapped. The above code just makes the swap so I really don't see
> what the problem is.
I mean: the function is structured so that there is pretty much a
clause for each possible type. Just add another one. I don't care if
there is a bit of code duplication. Something like that, or similar.
else if (TYPE_CODE (type) == TYPE_CODE_FLT
&& flt_argreg <= FLOAT_ARGLAST_REGNUM
&& TYPE_LENGTH(type) == reg_size)
{ do old stuff}
else if (TYPE_CODE (type) == TYPE_CODE_FLT
&& TYPE_LENGTH(type) == 2 * reg_size
&& flt_argreg <= FLOAT_ARGLAST_REGNUM)
{
if (TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE)
{do something}
else
{do something else}
}
else if (blah)
Actually, the test that (flt_argreg <= FLOAT_ARGLAST_REGNUM) may not
be sufficient anymore, because you are going to be using 2 registers,
and you could end up beyond FLOAT_ARGLAST_REGNUM. Or is it fine to
have the argument using the last float register, and the stack? Hmm,
is sh_next_flt_argreg taking care that doesn't happen? Seems so.
> I see, however, that a TYPE_LENGTH(type) might be more readable than the
> `len' and even more correct, since len is modified in the loop. Yes,
> that makes sense.
>
> Back to the `else if'. Wouldn't it be better just to pump up the comment
> to explain what happens? Instead of
>
> A float type taking two registers must be handled differently in LE mode.
>
> better something along the lines
>
> In little endian mode, float types taking two registers (doubles on sh4,
> long doubles on sh2e, sh3e and sh4) must be stored swapped in the argument
> registers. The below code first writes the first 32 bits in the next but
> one register, increments the val and len values accordingly and then
> proceeds as normal by writing the second 32 bits in the next register.
This would be good to add anyway.
elena
>
> ?
>
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Developer
> Red Hat, Inc.