Re: [Qemu-devel] [PATCH] ppc32 guests: fix computation of XER.{CA, OV} in addme, subfme, mullwo

["J. Mayer" <l_indien@magic.fr>]

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

On Thu, 2008-06-19 at 04:36 -0400, Tristan Gingold wrote:
> On Jun 19, 2008, at 3:36 AM, Julian Seward wrote:
> 
> >
> >>> addme   00000000 XER.CA=1  = 00000000 (cr 00000000 xer 20000000)
> >>
> >> Did I miss the obvious, but why does 0 + 1 - 1 generate a carry ?
> >
> > Because (as per the docs) it's computing rA + XER.CA + 0xFFFF_FFFF,
> > which is 0 + 1 + 0xFFFF_FFFF == 0x1_0000_0000.  Which does indeed
> > carry.  I tested now on both a 7447 and a G5 and they both do that.
> 
> Ok thanks.  So addme set the carry unless both xer.ca and rA are 0.
> 
> Your expression is a little bit complex, especially the second part.
> If xer_ca = 1 then res == argL, right ?

OK, then I found the faulty change (from me) and reverting it should do
the trick and fix addme and subfme in 32 and 64 bits cases. I also added
some missing casts for addmeo. The fix is quite simple: the carry should
never be reseted by those instructions as xer_ca == 1 will always lead
to set the carry with the result (T0 + 1 -1 is always equal to T0,
whatever T0 value...)

For the mullw(o) issue, I guess the mullw seems faulty too, as it should
produce a 64 bits signed result when running in 64 bits mode. And I
guess this part of your patch is useless as res is already an int64_t :
-    if (likely((int32_t)res == res)) {
+    if (likely( ((int64_t)(int32_t)res) == ((int64_t)res) )) {
.
Can you confirm ?

-- 
J. Mayer <l_indien@magic.fr>
Never organized

[-- Attachment #2: addme_subfme.diff --]
[-- Type: text/x-patch, Size: 2301 bytes --]

Index: op.c
===================================================================
RCS file: /sources/qemu/qemu/target-ppc/op.c,v
retrieving revision 1.68
diff -u -d -d -p -r1.68 op.c
--- op.c	16 Nov 2007 14:11:28 -0000	1.68
+++ op.c	19 Jun 2008 08:57:01 -0000
@@ -920,8 +920,6 @@ void OPPROTO op_add_me (void)
     T0 += xer_ca + (-1);
     if (likely((uint32_t)T1 != 0))
         xer_ca = 1;
-    else
-        xer_ca = 0;
     RETURN();
 }
 
@@ -931,8 +929,6 @@ void OPPROTO op_add_me_64 (void)
     T0 += xer_ca + (-1);
     if (likely((uint64_t)T1 != 0))
         xer_ca = 1;
-    else
-        xer_ca = 0;
     RETURN();
 }
 #endif
@@ -1213,8 +1209,6 @@ void OPPROTO op_subfme (void)
     T0 = ~T0 + xer_ca - 1;
     if (likely((uint32_t)T0 != UINT32_MAX))
         xer_ca = 1;
-    else
-        xer_ca = 0;
     RETURN();
 }
 
@@ -1224,8 +1218,6 @@ void OPPROTO op_subfme_64 (void)
     T0 = ~T0 + xer_ca - 1;
     if (likely((uint64_t)T0 != UINT64_MAX))
         xer_ca = 1;
-    else
-        xer_ca = 0;
     RETURN();
 }
 #endif
Index: op_helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-ppc/op_helper.c,v
retrieving revision 1.73
diff -u -d -d -p -r1.73 op_helper.c
--- op_helper.c	24 Nov 2007 02:03:55 -0000	1.73
+++ op_helper.c	19 Jun 2008 08:57:01 -0000
@@ -151,10 +151,8 @@ void do_addmeo (void)
     T0 += xer_ca + (-1);
     xer_ov = ((uint32_t)T1 & ((uint32_t)T1 ^ (uint32_t)T0)) >> 31;
     xer_so |= xer_ov;
-    if (likely(T1 != 0))
+    if (likely((uint32_t)T1 != 0))
         xer_ca = 1;
-    else
-        xer_ca = 0;
 }
 
 #if defined(TARGET_PPC64)
@@ -164,10 +162,8 @@ void do_addmeo_64 (void)
     T0 += xer_ca + (-1);
     xer_ov = ((uint64_t)T1 & ((uint64_t)T1 ^ (uint64_t)T0)) >> 63;
     xer_so |= xer_ov;
-    if (likely(T1 != 0))
+    if (likely((uint64_t)T1 != 0))
         xer_ca = 1;
-    else
-        xer_ca = 0;
 }
 #endif
 
@@ -312,8 +308,6 @@ void do_subfmeo (void)
     xer_so |= xer_ov;
     if (likely((uint32_t)T1 != UINT32_MAX))
         xer_ca = 1;
-    else
-        xer_ca = 0;
 }
 
 #if defined(TARGET_PPC64)
@@ -325,8 +319,6 @@ void do_subfmeo_64 (void)
     xer_so |= xer_ov;
     if (likely((uint64_t)T1 != UINT64_MAX))
         xer_ca = 1;
-    else
-        xer_ca = 0;
 }
 #endif