diff mbox

aarch64 sim bug fix for fcmp and infinity

Message ID CABXYE2Wr-EQEoV2wCE1QEHB_2mP=uF-BnF-iuhs9UMfTLn6VGw@mail.gmail.com
State New
Headers show

Commit Message

Jim Wilson Dec. 19, 2016, 3:11 a.m. UTC 
Debugging another gcc testsuite failure, I see that fcmp can fail when
both operands are infinities.  The code tries to compute the compare
result by subtracting operands, but this can yield NaN when both
operands are inifinities.  This is fixed by explicitly checking for
and handling this case.

While writing the tescase, I noticed that qNan and sNan are not
handled correctly, but this appears to be broken in more than just the
compare instructions, so I just added comments to document the problem
and will leave it to a later patch to fix.

The new testcase fails without the patch, and works with the patch.
Also, the gcc C testsuite unexpected failures drop  from 2627 to 2568.

Jim

Comments

Nick Clifton Dec. 21, 2016, 11:34 a.m. UTC | #1 
Hi Jim,

> Debugging another gcc testsuite failure, I see that fcmp can fail when

> both operands are infinities.  The code tries to compute the compare

> result by subtracting operands, but this can yield NaN when both

> operands are inifinities.  This is fixed by explicitly checking for

> and handling this case.

> 

> While writing the tescase, I noticed that qNan and sNan are not

> handled correctly, but this appears to be broken in more than just the

> compare instructions, so I just added comments to document the problem

> and will leave it to a later patch to fix.

> 

> The new testcase fails without the patch, and works with the patch.

> Also, the gcc C testsuite unexpected failures drop  from 2627 to 2568.

 
Approved - please apply - and thanks for the continued testing.

Cheers
  Nick
diff mbox

Patch

2016-12-18  Jim Wilson  <jim.wilson@linaro.org>

	sim/aarch64/
	* simulator.c (set_flags_for_float_compare): Add code to handle Inf.
	Add comment to document NaN issue.
	(set_flags_for_double_compare): Likewise.

	sim/testsuite/sim/aarch64/
	* fcmp.s: New.

diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c
index e6406dc..be3d6c7 100644
--- a/sim/aarch64/simulator.c
+++ b/sim/aarch64/simulator.c
@@ -8468,8 +8468,22 @@  set_flags_for_float_compare (sim_cpu *cpu, float fvalue1, float fvalue2)
 {
   uint32_t flags;
 
+  /* FIXME: Add exception raising.  */
   if (isnan (fvalue1) || isnan (fvalue2))
     flags = C|V;
+  else if (isinf (fvalue1) && isinf (fvalue2))
+    {
+      /* Subtracting two infinities may give a NaN.  We only need to compare
+	 the signs, which we can get from isinf.  */
+      int result = isinf (fvalue1) - isinf (fvalue2);
+
+      if (result == 0)
+	flags = Z|C;
+      else if (result < 0)
+	flags = N;
+      else /* (result > 0).  */
+	flags = C;
+    }
   else
     {
       float result = fvalue1 - fvalue2;
@@ -8540,8 +8554,22 @@  set_flags_for_double_compare (sim_cpu *cpu, double dval1, double dval2)
 {
   uint32_t flags;
 
+  /* FIXME: Add exception raising.  */
   if (isnan (dval1) || isnan (dval2))
     flags = C|V;
+  else if (isinf (dval1) && isinf (dval2))
+    {
+      /* Subtracting two infinities may give a NaN.  We only need to compare
+	 the signs, which we can get from isinf.  */
+      int result = isinf (dval1) - isinf (dval2);
+
+      if (result == 0)
+	flags = Z|C;
+      else if (result < 0)
+	flags = N;
+      else /* (result > 0).  */
+	flags = C;
+    }
   else
     {
       double result = dval1 - dval2;
diff --git a/sim/testsuite/sim/aarch64/fcmp.s b/sim/testsuite/sim/aarch64/fcmp.s
new file mode 100644
index 0000000..fd826c4
--- /dev/null
+++ b/sim/testsuite/sim/aarch64/fcmp.s
@@ -0,0 +1,146 @@ 
+# mach: aarch64
+
+# Check the FP compare instructions: fcmps, fcmpzs, fcmpes, fcmpzes, fcmpd,
+# fcmpzd, fcmped, fcmpzed.
+# For 1 operand compares, check 0, 1, -1, +Inf, -Inf.
+# For 2 operand compares, check 1/1, 1/-2, -1/2, +Inf/+Inf, +Inf/-Inf.
+# FIXME: Check for qNaN and sNaN when exception raising support added.
+
+.include "testutils.inc"
+
+	start
+	fmov s0, wzr
+	fcmp s0, #0.0
+	bne .Lfailure
+	fcmpe s0, #0.0
+	bne .Lfailure
+	fmov d0, xzr
+	fcmp d0, #0.0
+	bne .Lfailure
+	fcmpe d0, #0.0
+	bne .Lfailure
+
+	fmov s0, #1.0
+	fcmp s0, #0.0
+	blo .Lfailure
+	fcmpe s0, #0.0
+	blo .Lfailure
+	fmov d0, #1.0
+	fcmp d0, #0.0
+	blo .Lfailure
+	fcmpe d0, #0.0
+	blo .Lfailure
+
+	fmov s0, #-1.0
+	fcmp s0, #0.0
+	bpl .Lfailure
+	fcmpe s0, #0.0
+	bpl .Lfailure
+	fmov d0, #-1.0
+	fcmp d0, #0.0
+	bpl .Lfailure
+	fcmpe d0, #0.0
+	bpl .Lfailure
+
+	fmov s0, #1.0
+	fmov s1, wzr
+	fdiv s0, s0, s1
+	fcmp s0, #0.0
+	blo .Lfailure
+	fcmpe s0, #0.0
+	blo .Lfailure
+	fmov d0, #1.0
+	fmov d1, xzr
+	fdiv d0, d0, d1
+	fcmp d0, #0.0
+	blo .Lfailure
+	fcmpe d0, #0.0
+	blo .Lfailure
+
+	fmov s0, #-1.0
+	fmov s1, wzr
+	fdiv s0, s0, s1
+	fcmp s0, #0.0
+	bpl .Lfailure
+	fcmpe s0, #0.0
+	bpl .Lfailure
+	fmov d0, #-1.0
+	fmov d1, xzr
+	fdiv d0, d0, d1
+	fcmp d0, #0.0
+	bpl .Lfailure
+	fcmpe d0, #0.0
+	bpl .Lfailure
+
+	fmov s0, #1.0
+	fmov s1, #1.0
+	fcmp s0, s1
+	bne .Lfailure
+	fcmpe s0, s1
+	bne .Lfailure
+	fmov d0, #1.0
+	fmov d1, #1.0
+	fcmp d0, d1
+	bne .Lfailure
+	fcmpe d0, d1
+	bne .Lfailure
+
+	fmov s0, #1.0
+	fmov s1, #-2.0
+	fcmp s0, s1
+	blo .Lfailure
+	fcmpe s0, s1
+	blo .Lfailure
+	fmov d0, #1.0
+	fmov d1, #-2.0
+	fcmp d0, d1
+	blo .Lfailure
+	fcmpe d0, d1
+	blo .Lfailure
+
+	fmov s0, #-1.0
+	fmov s1, #2.0
+	fcmp s0, s1
+	bpl .Lfailure
+	fcmpe s0, s1
+	bpl .Lfailure
+	fmov d0, #-1.0
+	fmov d1, #2.0
+	fcmp d0, d1
+	bpl .Lfailure
+	fcmpe d0, d1
+	bpl .Lfailure
+
+	fmov s0, #1.0
+	fmov s1, wzr
+	fdiv s0, s0, s1
+	fcmp s0, s0
+	bne .Lfailure
+	fcmpe s0, s0
+	bne .Lfailure
+	fmov s1, #-1.0
+	fmov s2, wzr
+	fdiv s1, s1, s2
+	fcmp s0, s1
+	blo .Lfailure
+	fcmpe s0, s1
+	blo .Lfailure
+
+	fmov d0, #1.0
+	fmov d1, xzr
+	fdiv d0, d0, d1
+	fcmp d0, d0
+	bne .Lfailure
+	fcmpe d0, d0
+	bne .Lfailure
+	fmov d1, #-1.0
+	fmov d2, xzr
+	fdiv d1, d1, d2
+	fcmp d0, d1
+	blo .Lfailure
+	fcmpe d0, d1
+	blo .Lfailure
+
+	pass
+.Lfailure:
+	fail