From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1K9GBu-0003EE-It for qemu-devel@nongnu.org; Thu, 19 Jun 2008 05:12:26 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1K9GBs-0003DO-Iy for qemu-devel@nongnu.org; Thu, 19 Jun 2008 05:12:25 -0400 Received: from [199.232.76.173] (port=35418 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1K9GBs-0003DL-7Y for qemu-devel@nongnu.org; Thu, 19 Jun 2008 05:12:24 -0400 Received: from honiara.magic.fr ([195.154.193.36]:35532) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1K9GBr-0004WO-RG for qemu-devel@nongnu.org; Thu, 19 Jun 2008 05:12:24 -0400 Received: from [192.168.0.2] (ppp-36.net-123.static.magiconline.fr [80.118.184.36]) by honiara.magic.fr (8.13.1/8.13.1) with ESMTP id m5J9CGco032496 for ; Thu, 19 Jun 2008 11:12:18 +0200 Subject: Re: [Qemu-devel] [PATCH] ppc32 guests: fix computation of XER.{CA, OV} in addme, subfme, mullwo From: "J. Mayer" In-Reply-To: References: <200805110204.47184.jseward@acm.org> <200806180006.51954.jseward@acm.org> <2CD863BB-B874-483E-8FF3-63952C6FAD5D@adacore.com> <200806190936.58201.jseward@acm.org> Content-Type: multipart/mixed; boundary="=-trsJj4MdGuFeoHG8jOaW" Date: Thu, 19 Jun 2008 11:12:12 +0200 Message-Id: <1213866733.19143.58.camel@localhost> Mime-Version: 1.0 Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org --=-trsJj4MdGuFeoHG8jOaW Content-Type: text/plain Content-Transfer-Encoding: 7bit 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 Never organized --=-trsJj4MdGuFeoHG8jOaW Content-Disposition: attachment; filename=addme_subfme.diff Content-Type: text/x-patch; name=addme_subfme.diff; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit 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 --=-trsJj4MdGuFeoHG8jOaW--