From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36476) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9wvx-0007tu-9i for qemu-devel@nongnu.org; Thu, 24 Nov 2016 11:31:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9wvt-0002qI-Eo for qemu-devel@nongnu.org; Thu, 24 Nov 2016 11:31:37 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:42819 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c9wvt-0002pH-AC for qemu-devel@nongnu.org; Thu, 24 Nov 2016 11:31:33 -0500 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uAOGTEJJ052909 for ; Thu, 24 Nov 2016 11:31:32 -0500 Received: from e24smtp05.br.ibm.com (e24smtp05.br.ibm.com [32.104.18.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 26wx5wam1h-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 24 Nov 2016 11:31:31 -0500 Received: from localhost by e24smtp05.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 24 Nov 2016 14:31:29 -0200 Date: Thu, 24 Nov 2016 14:31:21 -0200 From: joserz@linux.vnet.ibm.com References: <1479918105-15616-1-git-send-email-joserz@linux.vnet.ibm.com> <1479918105-15616-2-git-send-email-joserz@linux.vnet.ibm.com> <8988b8ba-85fb-3382-5191-440ed8405a52@twiddle.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <8988b8ba-85fb-3382-5191-440ed8405a52@twiddle.net> Message-Id: <20161124163121.GA5922@pacoca> Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/4] target-ppc: Implement bcdcfsq. instruction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-ppc@nongnu.org, david@gibson.dropbear.id.au, qemu-devel@nongnu.org, bharata@linux.vnet.ibm.com Hello Richard, Thank you for your review, please read my answer below. On Thu, Nov 24, 2016 at 01:43:18AM +0100, Richard Henderson wrote: > On 11/23/2016 05:21 PM, Jose Ricardo Ziviani wrote: > >bcdcfsq.: Decimal convert from signed quadword. It is not possible > >to convert values less than 10^31-1 or greater than -10^31-1 to be > >represented in packed decimal format. > > > >Signed-off-by: Jose Ricardo Ziviani > >--- > > target-ppc/helper.h | 1 + > > target-ppc/int_helper.c | 45 +++++++++++++++++++++++++++++++++++++ > > target-ppc/translate/vmx-impl.inc.c | 7 ++++++ > > 3 files changed, 53 insertions(+) > > > >diff --git a/target-ppc/helper.h b/target-ppc/helper.h > >index da00f0a..87f533c 100644 > >--- a/target-ppc/helper.h > >+++ b/target-ppc/helper.h > >@@ -382,6 +382,7 @@ DEF_HELPER_3(bcdcfn, i32, avr, avr, i32) > > DEF_HELPER_3(bcdctn, i32, avr, avr, i32) > > DEF_HELPER_3(bcdcfz, i32, avr, avr, i32) > > DEF_HELPER_3(bcdctz, i32, avr, avr, i32) > >+DEF_HELPER_3(bcdcfsq, i32, avr, avr, i32) > > > > DEF_HELPER_2(xsadddp, void, env, i32) > > DEF_HELPER_2(xssubdp, void, env, i32) > >diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c > >index 8886a72..751909c 100644 > >--- a/target-ppc/int_helper.c > >+++ b/target-ppc/int_helper.c > >@@ -2874,6 +2874,51 @@ uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > return cr; > > } > > > >+uint32_t helper_bcdcfsq(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > >+{ > >+ int cr; > >+ int i; > >+ int ox_flag = 0; > >+ uint64_t lo_value; > >+ uint64_t hi_value; > >+ uint64_t max = 0x38d7ea4c68000; > > This is at heart a decimal number, and should be written as such. > Also, you need ULL for a 32-bit host compile. > OK > >+ if (divu128(&lo_value, &hi_value, max)) { > >+ ox_flag = 1; > >+ } else if (lo_value >= max && hi_value == 0) { > >+ ox_flag = 1; > >+ } > > Dispense with ox_flag and set cr = CRF_SO now. > OK > >+ for (i = 1; hi_value; hi_value /= 10, i++) { > >+ bcd_put_digit(&ret, hi_value % 10, i); > >+ } > >+ > >+ for (; lo_value; lo_value /= 10, i++) { > >+ bcd_put_digit(&ret, lo_value % 10, i); > >+ } > > How can this possibly work? You know there are 15 digits between high and > low, but you continue with i++? > > If hi_value == 1 && lo_value == 1, this should not produce 11, but > 10000000000000001. > Suppose we have hi_value = lo_value = 1 after divu128 above we will have hi_value = 744073709551617 lo_value = 18446 then, after the first for loop: hi_value = 0 lo_value = 18446 i = 16 ret = u128 = 0x8376822234287792511 finally, after the second loop: hi_value = 0 lo_value = 0 i = 21 ret = u128 = 0x0000000000018446744073709551617f Which is correct, this function converted a signed quadword to bcd correctly, using two doubleword variables: 1 << 64 | 1 = 18446744073709551617 Am I missing anything? Thank you! > > r~ >