[SH] Correct fabs and fneg insns in simulator

[SH] Correct fabs and fneg insns in simulator

[Oleg Endo]

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

Hi,

It seems that the implementation of the SH fabs and fneg insns in the
simulator is not correct.  They use the FP_UNARY macro which checks the
FPSCR.PR setting and raises an exception if PR = 1 (double precision)
and the register number is not even (i.e. a valid DF reg number).
For normal unary FP insns this is fine.  However, fneg and fabs perform
the same (integer) operations regardless of the FPSCR.PR setting.

This issue initially popped up here
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63260

I've checked some of the failing tests mentioned in GCC PR 63260 above
with the patch applied and the failures go away.

Could somebody please apply it if it's OK?

I'm not subscribed to gdb-patches, please CC me when replying.

Cheers,
Oleg

sim/ChangeLog:

2014-10-14  Oleg Endo <olegendo@gcc.gnu.org>

	* sh/gencode.c (fabs, fneg): Implement as integer operation 
	instead of using the FP_UNARY macro.

[-- Attachment #2: sh_sim_fneg_fabs.patch --]
[-- Type: text/x-patch, Size: 805 bytes --]

diff --git a/sim/sh/gencode.c b/sim/sh/gencode.c
index 738b718..bc65604 100644
--- a/sim/sh/gencode.c
+++ b/sim/sh/gencode.c
@@ -429,8 +429,14 @@ op tab[] =
 
   /* sh2e */
   { "", "", "fabs <FREG_N>", "1111nnnn01011101",
-    "FP_UNARY (n, fabs);",
-    "/* FIXME: FR (n) &= 0x7fffffff; */",
+    "  union",
+    "  {",
+    "    unsigned int i;",
+    "    float f;",
+    "  } u;",
+    "  u.f = FR (n);",
+    "  u.i &= 0x7fffffff;",
+    "  SET_FR (n, u.f);",
   },
 
   /* sh2e */
@@ -662,7 +668,14 @@ op tab[] =
 
   /* sh2e */
   { "", "", "fneg <FREG_N>", "1111nnnn01001101",
-    "FP_UNARY (n, -);",
+    "  union",
+    "  {",
+    "    unsigned int i;",
+    "    float f;",
+    "  } u;",
+    "  u.f = FR (n);",
+    "  u.i ^= 0x80000000;",
+    "  SET_FR (n, u.f);",
   },
 
   /* sh4a */

Re: [SH] Correct fabs and fneg insns in simulator

[Joel Brobecker]

Hello Oleg,,

> It seems that the implementation of the SH fabs and fneg insns in the
> simulator is not correct.  They use the FP_UNARY macro which checks the
> FPSCR.PR setting and raises an exception if PR = 1 (double precision)
> and the register number is not even (i.e. a valid DF reg number).
> For normal unary FP insns this is fine.  However, fneg and fabs perform
> the same (integer) operations regardless of the FPSCR.PR setting.
> 
> This issue initially popped up here
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63260
> 
> I've checked some of the failing tests mentioned in GCC PR 63260 above
> with the patch applied and the failures go away.
> 
> Could somebody please apply it if it's OK?
> 
> I'm not subscribed to gdb-patches, please CC me when replying.
> 
> Cheers,
> Oleg
> 
> sim/ChangeLog:
> 
> 2014-10-14  Oleg Endo <olegendo@gcc.gnu.org>
> 
> 	* sh/gencode.c (fabs, fneg): Implement as integer operation 
> 	instead of using the FP_UNARY macro.

Thank you for the patch. We don't have an SH maintainer for GDB,
and in the absence of any feedback from other interested people,
we will just trust you on this. So I pushed this patch under the
"tiny patch" rule.

If you ever think of contributing other patches to GDB, we'll probably
have to start thinking about getting your copyright assignment to
cover GDB as well.

-- 
Joel

Re: [SH] Correct fabs and fneg insns in simulator

[Oleg Endo]

Hi,

On Fri, 2014-11-28 at 19:51 +0400, Joel Brobecker wrote:
> Hello Oleg,,
> 
> > It seems that the implementation of the SH fabs and fneg insns in the
> > simulator is not correct.  They use the FP_UNARY macro which checks the
> > FPSCR.PR setting and raises an exception if PR = 1 (double precision)
> > and the register number is not even (i.e. a valid DF reg number).
> > For normal unary FP insns this is fine.  However, fneg and fabs perform
> > the same (integer) operations regardless of the FPSCR.PR setting.
> > 
> > This issue initially popped up here
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63260
> > 
> > I've checked some of the failing tests mentioned in GCC PR 63260 above
> > with the patch applied and the failures go away.
> > 
> > Could somebody please apply it if it's OK?
> > 
> > I'm not subscribed to gdb-patches, please CC me when replying.
> > 
> > Cheers,
> > Oleg
> > 
> > sim/ChangeLog:
> > 
> > 2014-10-14  Oleg Endo <olegendo@gcc.gnu.org>
> > 
> > 	* sh/gencode.c (fabs, fneg): Implement as integer operation 
> > 	instead of using the FP_UNARY macro.
> 
> Thank you for the patch. We don't have an SH maintainer for GDB,
> and in the absence of any feedback from other interested people,
> we will just trust you on this. So I pushed this patch under the
> "tiny patch" rule.

Thanks.

> If you ever think of contributing other patches to GDB, we'll probably
> have to start thinking about getting your copyright assignment to
> cover GDB as well.

Yes, that might be a good idea.  Could you please send me the necessary
papers (or initiate the procedure)?

Cheers,
Oleg