From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34678) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bznJ8-0006KE-Vn for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:13:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bznJ6-0002S6-Ay for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:13:34 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:53176 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 1bznJ6-0002S1-6M for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:13:32 -0400 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 u9RG8Xdc014083 for ; Thu, 27 Oct 2016 12:13:31 -0400 Received: from e24smtp05.br.ibm.com (e24smtp05.br.ibm.com [32.104.18.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 26bmu30ej5-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 27 Oct 2016 12:13:31 -0400 Received: from localhost by e24smtp05.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 27 Oct 2016 14:13:28 -0200 Date: Thu, 27 Oct 2016 14:13:13 -0200 From: joserz@linux.vnet.ibm.com References: <1477487938-23921-1-git-send-email-joserz@linux.vnet.ibm.com> <1477487938-23921-3-git-send-email-joserz@linux.vnet.ibm.com> <20161027012017.GB19918@umbus.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161027012017.GB19918@umbus.fritz.box> Message-Id: <20161027161313.GB19890@pacoca> Subject: Re: [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, nikunj@linux.vnet.ibm.com, bharata@linux.vnet.ibm.com On Thu, Oct 27, 2016 at 12:20:17PM +1100, David Gibson wrote: > On Wed, Oct 26, 2016 at 11:18:56AM -0200, Jose Ricardo Ziviani wrote: > > bcdctn. converts from BCD to National numeric format. National format > > uses a byte to represent a digit where the most significant nibble is > > always 0x3 and the least sign. nibbles is the digit itself. > > > > Signed-off-by: Jose Ricardo Ziviani > > --- > > target-ppc/helper.h | 1 + > > target-ppc/int_helper.c | 46 +++++++++++++++++++++++++++++++++++++ > > target-ppc/translate/vmx-impl.inc.c | 24 ++++++++++++++++++- > > 3 files changed, 70 insertions(+), 1 deletion(-) > > > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > > index d30ec60..92eaaf0 100644 > > --- a/target-ppc/helper.h > > +++ b/target-ppc/helper.h > > @@ -370,6 +370,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr) > > DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32) > > DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32) > > DEF_HELPER_3(bcdcfn, i32, avr, avr, i32) > > +DEF_HELPER_2(bcdctn, i32, avr, avr) > > > > 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 494c74e..cffe82c 100644 > > --- a/target-ppc/int_helper.c > > +++ b/target-ppc/int_helper.c > > @@ -2494,6 +2494,17 @@ static uint8_t get_national_digit(ppc_avr_t *reg, int n) > > #endif > > } > > > > +static void set_national_digit(ppc_avr_t *reg, uint8_t val, int n) > > +{ > > +#if defined(HOST_WORDS_BIGENDIAN) > > + reg->u16[8 - n] &= 0; > > + reg->u16[8 - n] |= val; > > The &= always sets the value to 0, so you might as well just use a > plain assignment in place of the &=, |=. OK! I'll fix it in v2. > > > +#else > > + reg->u16[n] &= 0; > > + reg->u16[n] |= val; > > +#endif > > +} > > + > > static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b) > > { > > int i; > > @@ -2667,6 +2678,41 @@ uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > return cr; > > } > > > > +uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b) > > +{ > > + int i; > > + int cr = 0; > > + int invalid = 0; > > + int sgnb = bcd_get_sgn(b); > > + ppc_avr_t ret = { .u64 = { 0, 0 } }; > > + > > + int eq_flag = (b->u64[HI_IDX] == 0) && ((b->u64[LO_IDX] >> 4) == 0); > > + int ox_flag = (b->u64[HI_IDX] != 0) || ((b->u64[LO_IDX] >> 8) != 0); > > This looks wrong. You're shifing the low half right 8 bits == 2 > nybbles == 1 digit + sign. So this will set the overflow flag if your > input is a number of >1 digit. I think you want >>32, so it only sets > overflow if the input exceeds 7 decimal digits + sign. OK! I'll fix it in v2. > > > + for (i = 1; i < 8; i++) { > > + set_national_digit(&ret, 0x30 + bcd_get_digit(b, i, &invalid), i); > > + } > > + set_national_digit(&ret, (sgnb == -1) ? NATIONAL_NEG : NATIONAL_PLUS, 0); > > + > > + if (!eq_flag) { > > + cr = (sgnb == -1) ? 1 << CRF_LT : 1 << CRF_GT; > > + } else { > > + cr = 1 << CRF_EQ; > > + } > > + > > + if (ox_flag) { > > + cr |= 1 << CRF_SO; > > + } > > + > > + if (unlikely(invalid)) { > > + cr = 1 << CRF_SO; > > + } > > + > > + *r = ret; > > + > > + return cr; > > +} > > + > > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a) > > { > > int i; > > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/vmx-impl.inc.c > > index 2abdcac..4364881 100644 > > --- a/target-ppc/translate/vmx-impl.inc.c > > +++ b/target-ppc/translate/vmx-impl.inc.c > > @@ -894,9 +894,29 @@ static void gen_##op(DisasContext *ctx) \ > > tcg_temp_free_i32(ps); \ > > } > > > > +#define GEN_BCD3(op) \ > > +static void gen_##op(DisasContext *ctx) \ > > +{ \ > > + TCGv_ptr rb, rd; \ > > + \ > > + if (unlikely(!ctx->altivec_enabled)) { \ > > + gen_exception(ctx, POWERPC_EXCP_VPU); \ > > + return; \ > > + } \ > > + \ > > + rb = gen_avr_ptr(rB(ctx->opcode)); \ > > + rd = gen_avr_ptr(rD(ctx->opcode)); \ > > + \ > > + gen_helper_##op(cpu_crf[6], rd, rb); \ > > + \ > > + tcg_temp_free_ptr(rb); \ > > + tcg_temp_free_ptr(rd); \ > > +} > > GEN_BCD(bcdadd) > > GEN_BCD(bcdsub) > > GEN_BCD2(bcdcfn) > > +GEN_BCD3(bcdctn) > > > > static void gen_xpnd04_1(DisasContext *ctx) > > { > > @@ -908,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx) > > case 4: > > break; /* bcdctz. */ > > case 5: > > - break; /* bcdctn. */ > > + gen_bcdctn(ctx); > > + break; > > case 6: > > break; /* bcdcfz. */ > > case 7: > > Uh.. doesn't adding bcdctn to this make it identical to gen_xpnd04_2? Actually this opcode is invalid for bcdctn as per ISA3.0. Do you think that I should throw something like a SIGILL for this particular case? > > > @@ -1024,3 +1045,4 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE, > > #undef GEN_VAFORM_PAIRED > > > > #undef GEN_BCD2 > > +#undef GEN_BCD3 > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson