[RFA] powerpc - extract a float return value

[RFA] powerpc - extract a float return value

[Jerome Guitton]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

Hello GDB folks,

The way GDB extracts float return values in rs6000-tdep.c
(rs6000_extract_return_value) seems dubious to me:

[...]
memcpy (&dd, &regbuf[DEPRECATED_REGISTER_BYTE (FP0_REGNUM + 1)], 8);
ff = (float) dd;
memcpy (valbuf, &ff, sizeof (float));
[...]

The cast will not work properly if the target and the host have not a similar
float representation.

I propose to fix that by a call to convert_typed_floating. See patch in
attachment.

I have not yet tested it against the testsuite, I will do that
tomorrow. In the meantime, if you have comments I would be happy to
address them!

--
Jerome

[-- Attachment #2: diff.2 --]
[-- Type: text/plain, Size: 1301 bytes --]

2004-04-20  Jerome Guitton  <guitton@gnat.fr>

	* rs6000-tdep.c (rs6000_extract_return_value): When extracting a float,
	use convert_typed_floating to get the appropriate format.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.191
diff -u -p -r1.191 rs6000-tdep.c
--- rs6000-tdep.c	1 Apr 2004 21:00:59 -0000	1.191
+++ rs6000-tdep.c	20 Apr 2004 17:45:24 -0000
@@ -1251,8 +1251,6 @@ rs6000_extract_return_value (struct type
   if (TYPE_CODE (valtype) == TYPE_CODE_FLT)
     {
 
-      double dd;
-      float ff;
       /* floats and doubles are returned in fpr1. fpr's have a size of 8 bytes.
          We need to truncate the return value into float size (4 byte) if
          necessary.  */
@@ -1263,9 +1261,11 @@ rs6000_extract_return_value (struct type
 		TYPE_LENGTH (valtype));
       else
 	{			/* float */
-	  memcpy (&dd, &regbuf[DEPRECATED_REGISTER_BYTE (FP0_REGNUM + 1)], 8);
-	  ff = (float) dd;
-	  memcpy (valbuf, &ff, sizeof (float));
+	  convert_typed_floating (&regbuf[DEPRECATED_REGISTER_BYTE
+                                          (FP0_REGNUM + 1)],
+				  builtin_type_double,
+				  valbuf,
+				  valtype);
 	}
     }
   else if (TYPE_CODE (valtype) == TYPE_CODE_ARRAY

Re: [RFA] powerpc - extract a float return value

[Kevin Buettner]

On Tue, 20 Apr 2004 20:03:48 +0200
Jerome Guitton <guitton@act-europe.fr> wrote:

> The way GDB extracts float return values in rs6000-tdep.c
> (rs6000_extract_return_value) seems dubious to me:
> 
> [...]
> memcpy (&dd, &regbuf[DEPRECATED_REGISTER_BYTE (FP0_REGNUM + 1)], 8);
> ff = (float) dd;
> memcpy (valbuf, &ff, sizeof (float));
> [...]
> 
> The cast will not work properly if the target and the host have not a similar
> float representation.

I agree.

> I propose to fix that by a call to convert_typed_floating. See patch in
> attachment.

This looks right to me.

The rs6000 return value code will need to be revamped to not refer to
DEPRECATED_REGISTER_BYTE.  And, in fact, it probably ought to be
restructured to use the mechanisms found in ppc-linux.c.  See
ppc_linux_return_value() and ppc_sysv_abi_return_value().  This
is not a barrier to getting your current patch in; I only mention
it in case you wish to tackle this bit of work too.

> I have not yet tested it against the testsuite, I will do that
> tomorrow. In the meantime, if you have comments I would be happy to
> address them!

Please let me know the results of your tests.  If they look good,
please commit your patch.

Thanks!

Kevin

Re: [RFA] powerpc - extract a float return value

[Andrew Cagney]

Kevin,

The function rs6000_extract_return_value needs to be replaced, and now 
is the time to do it.

Andrew

Re: [RFA] powerpc - extract a float return value

[Jerome Guitton]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

Kevin Buettner (kevinb@redhat.com):

> The rs6000 return value code will need to be revamped to not refer to
> DEPRECATED_REGISTER_BYTE.  And, in fact, it probably ought to be
> restructured to use the mechanisms found in ppc-linux.c.  See
> ppc_linux_return_value() and ppc_sysv_abi_return_value().  This
> is not a barrier to getting your current patch in; I only mention
> it in case you wish to tackle this bit of work too.

Sorry, I would not really easy for me, I do not have a convenient
setup for PPC (that is actually the reason why it took me 3 days
to be able to test against the testsuite :-)

> > I have not yet tested it against the testsuite, I will do that
> > tomorrow. In the meantime, if you have comments I would be happy to
> > address them!
> 
> Please let me know the results of your tests.  If they look good,
> please commit your patch.

OK, thanks. Tested on the ppc-elf simulator, on a x86-linux host.
No regression.

I have slightly modified the patch, I do not see any reason to have a special
case for doubles now. OK to apply with this change?

-- 
Jerome

[-- Attachment #2: diff.3 --]
[-- Type: text/plain, Size: 1489 bytes --]

2004-04-23  Jerome Guitton  <guitton@gnat.fr>

	* rs6000-tdep.c (rs6000_extract_return_value): When extracting a float,
	use convert_typed_floating to get the appropriate format.

Index: rs6000-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v
retrieving revision 1.191
diff -u -p -r1.191 rs6000-tdep.c
--- rs6000-tdep.c	1 Apr 2004 21:00:59 -0000	1.191
+++ rs6000-tdep.c	21 Apr 2004 10:14:03 -0000
@@ -1251,22 +1251,15 @@ rs6000_extract_return_value (struct type
   if (TYPE_CODE (valtype) == TYPE_CODE_FLT)
     {
 
-      double dd;
-      float ff;
       /* floats and doubles are returned in fpr1. fpr's have a size of 8 bytes.
          We need to truncate the return value into float size (4 byte) if
          necessary.  */
 
-      if (TYPE_LENGTH (valtype) > 4)	/* this is a double */
-	memcpy (valbuf,
-		&regbuf[DEPRECATED_REGISTER_BYTE (FP0_REGNUM + 1)],
-		TYPE_LENGTH (valtype));
-      else
-	{			/* float */
-	  memcpy (&dd, &regbuf[DEPRECATED_REGISTER_BYTE (FP0_REGNUM + 1)], 8);
-	  ff = (float) dd;
-	  memcpy (valbuf, &ff, sizeof (float));
-	}
+      convert_typed_floating (&regbuf[DEPRECATED_REGISTER_BYTE
+                                      (FP0_REGNUM + 1)],
+                              builtin_type_double,
+                              valbuf,
+                              valtype);
     }
   else if (TYPE_CODE (valtype) == TYPE_CODE_ARRAY
            && TYPE_LENGTH (valtype) == 16

Re: [RFA] powerpc - extract a float return value

[Kevin Buettner]

On Fri, 23 Apr 2004 16:38:21 +0200
Jerome Guitton <guitton@ACT-Europe.FR> wrote:

> I have slightly modified the patch, I do not see any reason to have a special
> case for doubles now. OK to apply with this change?

Yes.

Thanks again!

Kevin

Re: [RFA] powerpc - extract a float return value

[Jerome Guitton]

Kevin Buettner (kevinb@redhat.com):

> > case for doubles now. OK to apply with this change?
> 
> Yes.

Thanks for your approval.
Patch commited.

-- 
Jerome

Re: [RFA] powerpc - extract a float return value

[Jerome Guitton]

Andrew Cagney (cagney@gnu.org):

> FYI, ppc-elf doesn't use rs6000_extract_return_value:

Ouch! (It was not true for GDB 6.0, right?)

> so those test results are not relevant :-(

Joel Brobecker has a setup for running the testsuite on a AIX machine, I will
see with him. Sorry for the noice :-(

-- 
Jerome

Re: [RFA] powerpc - extract a float return value

[Joel Brobecker]

> Joel Brobecker has a setup for running the testsuite on a AIX machine,
> I will see with him.

I have exerciced Jerome's patch on AiX 5.1, and saw no regression.
:-)

-- 
Joel

Re: [RFA] powerpc - extract a float return value

[Andrew Cagney]

> Andrew Cagney (cagney@gnu.org):
> 
> 
>>> FYI, ppc-elf doesn't use rs6000_extract_return_value:
> 
> 
> Ouch! (It was not true for GDB 6.0, right?)

I don't know (I completly rewrote the 32- and 64-bit SVR4 code).  BTW, 
when fixing a bug always look for a FAIL/PASS transition.

>>> so those test results are not relevant :-(
> 
> 
> Joel Brobecker has a setup for running the testsuite on a AIX machine, I will
> see with him. Sorry for the noice :-(

You want to look at structs.exp, it slams GDB against a wall.  I see 
we're missing an equivalent scalars.exp, I'll write that.

Andrew

Re: [RFA] powerpc - extract a float return value

[Andrew Cagney]

> OK, thanks. Tested on the ppc-elf simulator, on a x86-linux host.
> No regression.

FYI, ppc-elf doesn't use rs6000_extract_return_value:

   if (sysv_abi && wordsize == 8)
     set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value);
   else if (sysv_abi && wordsize == 4)
     set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value);
   else
     {
       set_gdbarch_deprecated_extract_return_value (gdbarch, 
rs6000_extract_return_value);
       set_gdbarch_deprecated_store_return_value (gdbarch, 
rs6000_store_return_value);
     }

so those test results are not relevant :-(

Kevin,

This AIX code is long overdue for an update, now is the time to do it. 
Even if it means accepting just hand test results.

Andrew