diff mbox

fix for aarch64 sim adds64 bug

Message ID CABXYE2VZaNYKD3Oz5NCBNzpR2pVekkT-_hppiN2UAcUHfy1acA@mail.gmail.com
State New, archived
Headers

Commit Message

Jim Wilson Dec. 12, 2016, 6:47 p.m. UTC 
  The set_flags_for_add64 function is computing carry out wrong.  It may
also be computing overflow wrong.  I replaced it with a copy of the
set_flags_for_sub64 function which is OK, just negating the tests for
value2.  I added a testcase to verify the change.  The testcase fails
without the patch, and works with the patch.

I also verified with a gcc C testsuite run.  The failures drop from
2710 to 2627.

The new testcase requires my previous patch to fix the
aarch64/testutils.inc file.

Jim

Comments

Nick Clifton Dec. 13, 2016, 10:46 a.m. UTC | #1 
Hi Jim,

> I also verified with a gcc C testsuite run.  The failures drop from
> 2710 to 2627.
> 
> The new testcase requires my previous patch to fix the
> aarch64/testutils.inc file.

Patch approved - please apply.

Cheers
  Nick
diff mbox

Patch

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

	* sim/aarch64/simulator.c (NEG, POS): Move before set_flags_for_add64.
	(set_flags_for_add64): Replace with a modified copy of
	set_flags_for_sub64.

diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c
index 34fd17d..e6406dc 100644
--- a/sim/aarch64/simulator.c
+++ b/sim/aarch64/simulator.c
@@ -1659,55 +1659,34 @@  set_flags_for_add32 (sim_cpu *cpu, int32_t value1, int32_t value2)
   aarch64_set_CPSR (cpu, flags);
 }
 
+#define NEG(a) (((a) & signbit) == signbit)
+#define POS(a) (((a) & signbit) == 0)
+
 static void
 set_flags_for_add64 (sim_cpu *cpu, uint64_t value1, uint64_t value2)
 {
-  int64_t   sval1 = value1;
-  int64_t   sval2 = value2;
-  uint64_t  result = value1 + value2;
-  int64_t   sresult = sval1 + sval2;
-  uint32_t  flags = 0;
+  uint64_t result = value1 + value2;
+  uint32_t flags = 0;
+  uint64_t signbit = 1ULL << 63;
 
   if (result == 0)
     flags |= Z;
 
-  if (result & (1ULL << 63))
+  if (NEG (result))
     flags |= N;
 
-  if (sval1 < 0)
-    {
-      if (sval2 < 0)
-	{
-	  /* Negative plus a negative.  Overflow happens if
-	     the result is greater than either of the operands.  */
-	  if (sresult > sval1 || sresult > sval2)
-	    flags |= V;
-	}
-      /* else Negative plus a positive.  Overflow cannot happen.  */
-    }
-  else /* value1 is +ve.  */
-    {
-      if (sval2 < 0)
-	{
-	  /* Overflow can only occur if we computed "0 - MININT".  */
-	  if (sval1 == 0 && sval2 == (1LL << 63))
-	    flags |= V;
-	}
-      else
-	{
-	  /* Postive plus positive - overflow has happened if the
-	     result is smaller than either of the operands.  */
-	  if (result < value1 || result < value2)
-	    flags |= V | C;
-	}
-    }
+  if (   (NEG (value1) && NEG (value2))
+      || (NEG (value1) && POS (result))
+      || (NEG (value2) && POS (result)))
+    flags |= C;
+
+  if (   (NEG (value1) && NEG (value2) && POS (result))
+      || (POS (value1) && POS (value2) && NEG (result)))
+    flags |= V;
 
   aarch64_set_CPSR (cpu, flags);
 }
 
-#define NEG(a) (((a) & signbit) == signbit)
-#define POS(a) (((a) & signbit) == 0)
-
 static void
 set_flags_for_sub32 (sim_cpu *cpu, uint32_t value1, uint32_t value2)
 {