* [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I
@ 2016-10-26 13:18 Jose Ricardo Ziviani
2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw)
To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata
This serie contains 4 new instructions for POWER9 ISA3.0
bcdcfn.: Decimal Convert From National
bcdctn.: Decimal Convert To National
bcdcfz.: Decimal Convert From Zoned
bcdctz.: Decimal Convert to Zoned
Jose Ricardo Ziviani (4):
target-ppc: Implement bcdcfn. instruction
target-ppc: Implement bcdctn. instruction
target-ppc: Implement bcdcfz. instruction
target-ppc: Implement bcdctz. instruction
target-ppc/helper.h | 4 +
target-ppc/int_helper.c | 209 ++++++++++++++++++++++++++++++++++++
target-ppc/translate/vmx-impl.inc.c | 103 ++++++++++++++++++
target-ppc/translate/vmx-ops.inc.c | 4 +-
4 files changed, 318 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 16+ messages in thread* [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction 2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani @ 2016-10-26 13:18 ` Jose Ricardo Ziviani 2016-10-27 1:05 ` David Gibson 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw) To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata bcdcfn. converts from National numeric format to BCD. 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 <joserz@linux.vnet.ibm.com> --- target-ppc/helper.h | 1 + target-ppc/int_helper.c | 54 ++++++++++++++++++++++++++ target-ppc/translate/vmx-impl.inc.c | 75 +++++++++++++++++++++++++++++++++++++ target-ppc/translate/vmx-ops.inc.c | 4 +- 4 files changed, 132 insertions(+), 2 deletions(-) diff --git a/target-ppc/helper.h b/target-ppc/helper.h index 04c6421..d30ec60 100644 --- a/target-ppc/helper.h +++ b/target-ppc/helper.h @@ -369,6 +369,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(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 5aee0a8..494c74e 100644 --- a/target-ppc/int_helper.c +++ b/target-ppc/int_helper.c @@ -2417,6 +2417,8 @@ void helper_vsubecuq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c) #define BCD_NEG_PREF 0xD #define BCD_NEG_ALT 0xB #define BCD_PLUS_ALT_2 0xE +#define NATIONAL_PLUS 0x2B +#define NATIONAL_NEG 0x2D #if defined(HOST_WORDS_BIGENDIAN) #define BCD_DIG_BYTE(n) (15 - (n/2)) @@ -2483,6 +2485,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t digit, int n) } } +static uint8_t get_national_digit(ppc_avr_t *reg, int n) +{ +#if defined(HOST_WORDS_BIGENDIAN) + return reg->u16[8 - n] & 0xFF; +#else + return reg->u16[n] & 0xFF; +#endif +} + static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b) { int i; @@ -2613,6 +2624,49 @@ uint32_t helper_bcdsub(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps) return helper_bcdadd(r, a, &bcopy, ps); } +uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) +{ + int i; + int is_zero = 0; + int cr = 0; + int national = 0; + ppc_avr_t ret = { .u64 = { 0, 0 } }; + uint16_t sgnb = get_national_digit(b, 0); + int invalid = (sgnb != NATIONAL_PLUS && sgnb != NATIONAL_NEG); + + for (i = 1; i < 8; i++) { + national = get_national_digit(b, i); + + is_zero += (national == 0x30); + if (unlikely(national < 0x30 || national > 0x39)) { + invalid = 1; + } + + bcd_put_digit(&ret, national & 0xf, i); + } + + if (sgnb == NATIONAL_PLUS || + (b->u64[0] == 0 && b->u64[1] == 0)) { + bcd_put_digit(&ret, (ps == 0) ? BCD_PLUS_PREF_1 : BCD_PLUS_PREF_2, 0); + } else { + bcd_put_digit(&ret, BCD_NEG_PREF, 0); + } + + if (!is_zero) { + cr = (sgnb == NATIONAL_PLUS) ? 1 << CRF_GT : 1 << CRF_LT; + } else { + cr = 1 << CRF_EQ; + } + + 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 c8998f3..2abdcac 100644 --- a/target-ppc/translate/vmx-impl.inc.c +++ b/target-ppc/translate/vmx-impl.inc.c @@ -871,8 +871,81 @@ static void gen_##op(DisasContext *ctx) \ tcg_temp_free_i32(ps); \ } +#define GEN_BCD2(op) \ +static void gen_##op(DisasContext *ctx) \ +{ \ + TCGv_ptr rd, rb; \ + TCGv_i32 ps; \ + \ + 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)); \ + \ + ps = tcg_const_i32((ctx->opcode & 0x200) != 0); \ + \ + gen_helper_##op(cpu_crf[6], rd, rb, ps); \ + \ + tcg_temp_free_ptr(rb); \ + tcg_temp_free_ptr(rd); \ + tcg_temp_free_i32(ps); \ +} + GEN_BCD(bcdadd) GEN_BCD(bcdsub) +GEN_BCD2(bcdcfn) + +static void gen_xpnd04_1(DisasContext *ctx) +{ + switch (opc4(ctx->opcode)) { + case 0: + break; /* bcdctsq. */ + case 2: + break; /* bcdcfsq. */ + case 4: + break; /* bcdctz. */ + case 5: + break; /* bcdctn. */ + case 6: + break; /* bcdcfz. */ + case 7: + gen_bcdcfn(ctx); + break; + case 31: + break; /* bcdsetsgn. */ + default: + break; + } +} + +static void gen_xpnd04_2(DisasContext *ctx) +{ + switch (opc4(ctx->opcode)) { + case 0: + break; /* bcdctsq. */ + case 2: + break; /* bcdcfsq. */ + case 4: + break; /* bcdctz. */ + case 6: + break; /* bcdcfz. */ + case 7: + gen_bcdcfn(ctx); + break; + case 31: + break; /* bcdsetsgn. */ + default: + break; + } +} + +GEN_VXFORM_DUAL(vsubcuw, PPC_ALTIVEC, PPC_NONE, \ + xpnd04_1, PPC_NONE, PPC2_ISA300) +GEN_VXFORM_DUAL(vsubsws, PPC_ALTIVEC, PPC_NONE, \ + xpnd04_2, PPC_NONE, PPC2_ISA300) GEN_VXFORM_DUAL(vsububm, PPC_ALTIVEC, PPC_NONE, \ bcdadd, PPC_NONE, PPC2_ALTIVEC_207) @@ -949,3 +1022,5 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE, #undef GEN_VXFORM_NOA #undef GEN_VXFORM_UIMM #undef GEN_VAFORM_PAIRED + +#undef GEN_BCD2 diff --git a/target-ppc/translate/vmx-ops.inc.c b/target-ppc/translate/vmx-ops.inc.c index 68cba3e..7a5fec6 100644 --- a/target-ppc/translate/vmx-ops.inc.c +++ b/target-ppc/translate/vmx-ops.inc.c @@ -122,7 +122,7 @@ GEN_VXFORM_300(vslv, 2, 29), GEN_VXFORM(vslo, 6, 16), GEN_VXFORM(vsro, 6, 17), GEN_VXFORM(vaddcuw, 0, 6), -GEN_VXFORM(vsubcuw, 0, 22), +GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE), GEN_VXFORM(vaddubs, 0, 8), GEN_VXFORM(vadduhs, 0, 9), GEN_VXFORM(vadduws, 0, 10), @@ -134,7 +134,7 @@ GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE), GEN_VXFORM(vsubuws, 0, 26), GEN_VXFORM(vsubsbs, 0, 28), GEN_VXFORM(vsubshs, 0, 29), -GEN_VXFORM(vsubsws, 0, 30), +GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE), GEN_VXFORM_207(vadduqm, 0, 4), GEN_VXFORM_207(vaddcuq, 0, 5), GEN_VXFORM_DUAL(vaddeuqm, vaddecuq, 30, 0xFF, PPC_NONE, PPC2_ALTIVEC_207), -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani @ 2016-10-27 1:05 ` David Gibson 2016-10-27 15:29 ` joserz 0 siblings, 1 reply; 16+ messages in thread From: David Gibson @ 2016-10-27 1:05 UTC (permalink / raw) To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata [-- Attachment #1: Type: text/plain, Size: 8474 bytes --] On Wed, Oct 26, 2016 at 11:18:55AM -0200, Jose Ricardo Ziviani wrote: > bcdcfn. converts from National numeric format to BCD. 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 <joserz@linux.vnet.ibm.com> > --- > target-ppc/helper.h | 1 + > target-ppc/int_helper.c | 54 ++++++++++++++++++++++++++ > target-ppc/translate/vmx-impl.inc.c | 75 +++++++++++++++++++++++++++++++++++++ > target-ppc/translate/vmx-ops.inc.c | 4 +- > 4 files changed, 132 insertions(+), 2 deletions(-) > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > index 04c6421..d30ec60 100644 > --- a/target-ppc/helper.h > +++ b/target-ppc/helper.h > @@ -369,6 +369,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(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 5aee0a8..494c74e 100644 > --- a/target-ppc/int_helper.c > +++ b/target-ppc/int_helper.c > @@ -2417,6 +2417,8 @@ void helper_vsubecuq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c) > #define BCD_NEG_PREF 0xD > #define BCD_NEG_ALT 0xB > #define BCD_PLUS_ALT_2 0xE > +#define NATIONAL_PLUS 0x2B > +#define NATIONAL_NEG 0x2D > > #if defined(HOST_WORDS_BIGENDIAN) > #define BCD_DIG_BYTE(n) (15 - (n/2)) > @@ -2483,6 +2485,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t digit, int n) > } > } > > +static uint8_t get_national_digit(ppc_avr_t *reg, int n) > +{ > +#if defined(HOST_WORDS_BIGENDIAN) > + return reg->u16[8 - n] & 0xFF; > +#else > + return reg->u16[n] & 0xFF; > +#endif You're discarding the high byte of each digit here, which means you won't detect badly encoded values that have correct low bytes. > +} > + > static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b) > { > int i; > @@ -2613,6 +2624,49 @@ uint32_t helper_bcdsub(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps) > return helper_bcdadd(r, a, &bcopy, ps); > } > > +uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > +{ > + int i; > + int is_zero = 0; > + int cr = 0; > + int national = 0; > + ppc_avr_t ret = { .u64 = { 0, 0 } }; > + uint16_t sgnb = get_national_digit(b, 0); > + int invalid = (sgnb != NATIONAL_PLUS && sgnb != NATIONAL_NEG); > + > + for (i = 1; i < 8; i++) { > + national = get_national_digit(b, i); > + > + is_zero += (national == 0x30); > + if (unlikely(national < 0x30 || national > 0x39)) { > + invalid = 1; > + } > + > + bcd_put_digit(&ret, national & 0xf, i); > + } > + > + if (sgnb == NATIONAL_PLUS || > + (b->u64[0] == 0 && b->u64[1] == 0)) { The second part of this test doesn't seem to make much sense. I think you're trying to always put a positive sign on a zero result. But you're testing the national encoded input, not the BCD encoded output, and zero will *not* be all zero bits in national encoding. > + bcd_put_digit(&ret, (ps == 0) ? BCD_PLUS_PREF_1 : BCD_PLUS_PREF_2, 0); > + } else { > + bcd_put_digit(&ret, BCD_NEG_PREF, 0); > + } > + > + if (!is_zero) { From the logic above, 'is_zero' is currently a count of how many 0 digits the value has, not whether the value as a whole is zero. That means you will get the wrong result here. > + cr = (sgnb == NATIONAL_PLUS) ? 1 << CRF_GT : 1 << CRF_LT; > + } else { > + cr = 1 << CRF_EQ; > + } > + > + 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 c8998f3..2abdcac 100644 > --- a/target-ppc/translate/vmx-impl.inc.c > +++ b/target-ppc/translate/vmx-impl.inc.c > @@ -871,8 +871,81 @@ static void gen_##op(DisasContext *ctx) \ > tcg_temp_free_i32(ps); \ > } > > +#define GEN_BCD2(op) \ > +static void gen_##op(DisasContext *ctx) \ > +{ \ > + TCGv_ptr rd, rb; \ > + TCGv_i32 ps; \ > + \ > + 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)); \ > + \ > + ps = tcg_const_i32((ctx->opcode & 0x200) != 0); \ > + \ > + gen_helper_##op(cpu_crf[6], rd, rb, ps); \ > + \ > + tcg_temp_free_ptr(rb); \ > + tcg_temp_free_ptr(rd); \ > + tcg_temp_free_i32(ps); \ > +} > + > GEN_BCD(bcdadd) > GEN_BCD(bcdsub) > +GEN_BCD2(bcdcfn) > + > +static void gen_xpnd04_1(DisasContext *ctx) > +{ > + switch (opc4(ctx->opcode)) { > + case 0: > + break; /* bcdctsq. */ > + case 2: > + break; /* bcdcfsq. */ > + case 4: > + break; /* bcdctz. */ > + case 5: > + break; /* bcdctn. */ > + case 6: > + break; /* bcdcfz. */ > + case 7: > + gen_bcdcfn(ctx); > + break; > + case 31: > + break; /* bcdsetsgn. */ > + default: > + break; > + } > +} > + > +static void gen_xpnd04_2(DisasContext *ctx) > +{ > + switch (opc4(ctx->opcode)) { > + case 0: > + break; /* bcdctsq. */ > + case 2: > + break; /* bcdcfsq. */ > + case 4: > + break; /* bcdctz. */ > + case 6: > + break; /* bcdcfz. */ > + case 7: > + gen_bcdcfn(ctx); > + break; > + case 31: > + break; /* bcdsetsgn. */ > + default: > + break; > + } > +} I thougt the main opcode table now had support for opc4, so you shouldn't need these special expanders. > +GEN_VXFORM_DUAL(vsubcuw, PPC_ALTIVEC, PPC_NONE, \ > + xpnd04_1, PPC_NONE, PPC2_ISA300) > +GEN_VXFORM_DUAL(vsubsws, PPC_ALTIVEC, PPC_NONE, \ > + xpnd04_2, PPC_NONE, PPC2_ISA300) > > GEN_VXFORM_DUAL(vsububm, PPC_ALTIVEC, PPC_NONE, \ > bcdadd, PPC_NONE, PPC2_ALTIVEC_207) > @@ -949,3 +1022,5 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE, > #undef GEN_VXFORM_NOA > #undef GEN_VXFORM_UIMM > #undef GEN_VAFORM_PAIRED > + > +#undef GEN_BCD2 > diff --git a/target-ppc/translate/vmx-ops.inc.c b/target-ppc/translate/vmx-ops.inc.c > index 68cba3e..7a5fec6 100644 > --- a/target-ppc/translate/vmx-ops.inc.c > +++ b/target-ppc/translate/vmx-ops.inc.c > @@ -122,7 +122,7 @@ GEN_VXFORM_300(vslv, 2, 29), > GEN_VXFORM(vslo, 6, 16), > GEN_VXFORM(vsro, 6, 17), > GEN_VXFORM(vaddcuw, 0, 6), > -GEN_VXFORM(vsubcuw, 0, 22), > +GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE), > GEN_VXFORM(vaddubs, 0, 8), > GEN_VXFORM(vadduhs, 0, 9), > GEN_VXFORM(vadduws, 0, 10), > @@ -134,7 +134,7 @@ GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE), > GEN_VXFORM(vsubuws, 0, 26), > GEN_VXFORM(vsubsbs, 0, 28), > GEN_VXFORM(vsubshs, 0, 29), > -GEN_VXFORM(vsubsws, 0, 30), > +GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE), > GEN_VXFORM_207(vadduqm, 0, 4), > GEN_VXFORM_207(vaddcuq, 0, 5), > GEN_VXFORM_DUAL(vaddeuqm, vaddecuq, 30, 0xFF, PPC_NONE, PPC2_ALTIVEC_207), -- 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction 2016-10-27 1:05 ` David Gibson @ 2016-10-27 15:29 ` joserz 0 siblings, 0 replies; 16+ messages in thread From: joserz @ 2016-10-27 15:29 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata Hello David, Thank you very much for your review. As you might have noticed this is my first patch so I hope you don't mind about my newbie questions/explanations below. On Thu, Oct 27, 2016 at 12:05:30PM +1100, David Gibson wrote: > On Wed, Oct 26, 2016 at 11:18:55AM -0200, Jose Ricardo Ziviani wrote: > > bcdcfn. converts from National numeric format to BCD. 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 <joserz@linux.vnet.ibm.com> > > --- > > target-ppc/helper.h | 1 + > > target-ppc/int_helper.c | 54 ++++++++++++++++++++++++++ > > target-ppc/translate/vmx-impl.inc.c | 75 +++++++++++++++++++++++++++++++++++++ > > target-ppc/translate/vmx-ops.inc.c | 4 +- > > 4 files changed, 132 insertions(+), 2 deletions(-) > > > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > > index 04c6421..d30ec60 100644 > > --- a/target-ppc/helper.h > > +++ b/target-ppc/helper.h > > @@ -369,6 +369,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(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 5aee0a8..494c74e 100644 > > --- a/target-ppc/int_helper.c > > +++ b/target-ppc/int_helper.c > > @@ -2417,6 +2417,8 @@ void helper_vsubecuq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, ppc_avr_t *c) > > #define BCD_NEG_PREF 0xD > > #define BCD_NEG_ALT 0xB > > #define BCD_PLUS_ALT_2 0xE > > +#define NATIONAL_PLUS 0x2B > > +#define NATIONAL_NEG 0x2D > > > > #if defined(HOST_WORDS_BIGENDIAN) > > #define BCD_DIG_BYTE(n) (15 - (n/2)) > > @@ -2483,6 +2485,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t digit, int n) > > } > > } > > > > +static uint8_t get_national_digit(ppc_avr_t *reg, int n) > > +{ > > +#if defined(HOST_WORDS_BIGENDIAN) > > + return reg->u16[8 - n] & 0xFF; > > +#else > > + return reg->u16[n] & 0xFF; > > +#endif > > You're discarding the high byte of each digit here, which means you > won't detect badly encoded values that have correct low bytes. OK! The high byte is expected to be 0, I'll check if it's != 0 and set a flag like: *invalid = (reg->u16[n] >> 8) != 0; makes sense? > > > +} > > + > > static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b) > > { > > int i; > > @@ -2613,6 +2624,49 @@ uint32_t helper_bcdsub(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t ps) > > return helper_bcdadd(r, a, &bcopy, ps); > > } > > > > +uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > +{ > > + int i; > > + int is_zero = 0; > > + int cr = 0; > > + int national = 0; > > + ppc_avr_t ret = { .u64 = { 0, 0 } }; > > + uint16_t sgnb = get_national_digit(b, 0); > > + int invalid = (sgnb != NATIONAL_PLUS && sgnb != NATIONAL_NEG); > > + > > + for (i = 1; i < 8; i++) { > > + national = get_national_digit(b, i); > > + > > + is_zero += (national == 0x30); > > + if (unlikely(national < 0x30 || national > 0x39)) { > > + invalid = 1; > > + } > > + > > + bcd_put_digit(&ret, national & 0xf, i); > > + } > > + > > + if (sgnb == NATIONAL_PLUS || > > + (b->u64[0] == 0 && b->u64[1] == 0)) { > > The second part of this test doesn't seem to make much sense. I think > you're trying to always put a positive sign on a zero result. But > you're testing the national encoded input, not the BCD encoded output, > and zero will *not* be all zero bits in national encoding. OK! ISA defines invalid encoding as undefined behavior. I forced a positive sign to vrt when vrb = 0 (invalid enc.) but it is not necessary. I'll remove it in v2. > > > + bcd_put_digit(&ret, (ps == 0) ? BCD_PLUS_PREF_1 : BCD_PLUS_PREF_2, 0); > > + } else { > > + bcd_put_digit(&ret, BCD_NEG_PREF, 0); > > + } > > + > > + if (!is_zero) { > > From the logic above, 'is_zero' is currently a count of how many 0 > digits the value has, not whether the value as a whole is zero. That > means you will get the wrong result here. > OK! I'll fix that logic. > > + cr = (sgnb == NATIONAL_PLUS) ? 1 << CRF_GT : 1 << CRF_LT; > > + } else { > > + cr = 1 << CRF_EQ; > > + } > > + > > + 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 c8998f3..2abdcac 100644 > > --- a/target-ppc/translate/vmx-impl.inc.c > > +++ b/target-ppc/translate/vmx-impl.inc.c > > @@ -871,8 +871,81 @@ static void gen_##op(DisasContext *ctx) \ > > tcg_temp_free_i32(ps); \ > > } > > > > +#define GEN_BCD2(op) \ > > +static void gen_##op(DisasContext *ctx) \ > > +{ \ > > + TCGv_ptr rd, rb; \ > > + TCGv_i32 ps; \ > > + \ > > + 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)); \ > > + \ > > + ps = tcg_const_i32((ctx->opcode & 0x200) != 0); \ > > + \ > > + gen_helper_##op(cpu_crf[6], rd, rb, ps); \ > > + \ > > + tcg_temp_free_ptr(rb); \ > > + tcg_temp_free_ptr(rd); \ > > + tcg_temp_free_i32(ps); \ > > +} > > + > > GEN_BCD(bcdadd) > > GEN_BCD(bcdsub) > > +GEN_BCD2(bcdcfn) > > + > > +static void gen_xpnd04_1(DisasContext *ctx) > > +{ > > + switch (opc4(ctx->opcode)) { > > + case 0: > > + break; /* bcdctsq. */ > > + case 2: > > + break; /* bcdcfsq. */ > > + case 4: > > + break; /* bcdctz. */ > > + case 5: > > + break; /* bcdctn. */ > > + case 6: > > + break; /* bcdcfz. */ > > + case 7: > > + gen_bcdcfn(ctx); > > + break; > > + case 31: > > + break; /* bcdsetsgn. */ > > + default: > > + break; > > + } > > +} > > + > > +static void gen_xpnd04_2(DisasContext *ctx) > > +{ > > + switch (opc4(ctx->opcode)) { > > + case 0: > > + break; /* bcdctsq. */ > > + case 2: > > + break; /* bcdcfsq. */ > > + case 4: > > + break; /* bcdctz. */ > > + case 6: > > + break; /* bcdcfz. */ > > + case 7: > > + gen_bcdcfn(ctx); > > + break; > > + case 31: > > + break; /* bcdsetsgn. */ > > + default: > > + break; > > + } > > +} > > I thougt the main opcode table now had support for opc4, so you > shouldn't need these special expanders. This is the hardest one. :) For all the functions above we have opcode collisions today. Let's take the bcdcfn case, its opcodes are: 000100 vrt:5 00111 vrb:5 11110 00000 1, when ps=1 000100 vrt:5 00111 vrb:5 10110 00000 1, when ps=0 Registering the opcode in the opcode table at translate_init.c, the following sequence is assumed: 0x4 -> 0x0 -> 0x1e (ps=1) or 0x16 (ps=0) -> 0x7 However, 0x4, 0x0, 0x1e already maps to a direct opcode: vsubsws 000100 vrt:5 vra:5 vrb:5 11110 00000 0 as well as 0x4, 0x0, 0x16, that maps to: vsubcuw 000100 vrt:5 vra:5 vrb:5 10110 00000 0 I this case, gen_xpnd04_1 handles ps=0 collisions and gen_xpnd04_2 does the same when ps=1. So Nikunj gave me this great idea to handle these exceptional cases. > > > +GEN_VXFORM_DUAL(vsubcuw, PPC_ALTIVEC, PPC_NONE, \ > > + xpnd04_1, PPC_NONE, PPC2_ISA300) > > +GEN_VXFORM_DUAL(vsubsws, PPC_ALTIVEC, PPC_NONE, \ > > + xpnd04_2, PPC_NONE, PPC2_ISA300) > > > > GEN_VXFORM_DUAL(vsububm, PPC_ALTIVEC, PPC_NONE, \ > > bcdadd, PPC_NONE, PPC2_ALTIVEC_207) > > @@ -949,3 +1022,5 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE, > > #undef GEN_VXFORM_NOA > > #undef GEN_VXFORM_UIMM > > #undef GEN_VAFORM_PAIRED > > + > > +#undef GEN_BCD2 > > diff --git a/target-ppc/translate/vmx-ops.inc.c b/target-ppc/translate/vmx-ops.inc.c > > index 68cba3e..7a5fec6 100644 > > --- a/target-ppc/translate/vmx-ops.inc.c > > +++ b/target-ppc/translate/vmx-ops.inc.c > > @@ -122,7 +122,7 @@ GEN_VXFORM_300(vslv, 2, 29), > > GEN_VXFORM(vslo, 6, 16), > > GEN_VXFORM(vsro, 6, 17), > > GEN_VXFORM(vaddcuw, 0, 6), > > -GEN_VXFORM(vsubcuw, 0, 22), > > +GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE), > > GEN_VXFORM(vaddubs, 0, 8), > > GEN_VXFORM(vadduhs, 0, 9), > > GEN_VXFORM(vadduws, 0, 10), > > @@ -134,7 +134,7 @@ GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE), > > GEN_VXFORM(vsubuws, 0, 26), > > GEN_VXFORM(vsubsbs, 0, 28), > > GEN_VXFORM(vsubshs, 0, 29), > > -GEN_VXFORM(vsubsws, 0, 30), > > +GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE), > > GEN_VXFORM_207(vadduqm, 0, 4), > > GEN_VXFORM_207(vaddcuq, 0, 5), > > GEN_VXFORM_DUAL(vaddeuqm, vaddecuq, 30, 0xFF, PPC_NONE, PPC2_ALTIVEC_207), > > -- > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction 2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani @ 2016-10-26 13:18 ` Jose Ricardo Ziviani 2016-10-27 1:20 ` David Gibson 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani 3 siblings, 1 reply; 16+ messages in thread From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw) To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata 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 <joserz@linux.vnet.ibm.com> --- 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; +#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); + + 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: @@ -1024,3 +1045,4 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE, #undef GEN_VAFORM_PAIRED #undef GEN_BCD2 +#undef GEN_BCD3 -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani @ 2016-10-27 1:20 ` David Gibson 2016-10-27 16:13 ` joserz 0 siblings, 1 reply; 16+ messages in thread From: David Gibson @ 2016-10-27 1:20 UTC (permalink / raw) To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata [-- Attachment #1: Type: text/plain, Size: 5393 bytes --] 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 <joserz@linux.vnet.ibm.com> > --- > 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 &=, |=. > +#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. > + 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? > @@ -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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction 2016-10-27 1:20 ` David Gibson @ 2016-10-27 16:13 ` joserz 2016-10-31 0:16 ` David Gibson 0 siblings, 1 reply; 16+ messages in thread From: joserz @ 2016-10-27 16:13 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata 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 <joserz@linux.vnet.ibm.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction 2016-10-27 16:13 ` joserz @ 2016-10-31 0:16 ` David Gibson 0 siblings, 0 replies; 16+ messages in thread From: David Gibson @ 2016-10-31 0:16 UTC (permalink / raw) To: joserz; +Cc: qemu-ppc, qemu-devel, nikunj, bharata [-- Attachment #1: Type: text/plain, Size: 6468 bytes --] On Thu, Oct 27, 2016 at 02:13:13PM -0200, joserz@linux.vnet.ibm.com wrote: > 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 <joserz@linux.vnet.ibm.com> > > > --- > > > 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? Yes, if the opcode is invalid, you absolutely should be throwing an invalid instruction exception. > > > > > @@ -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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction 2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani @ 2016-10-26 13:18 ` Jose Ricardo Ziviani 2016-10-27 1:35 ` David Gibson 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani 3 siblings, 1 reply; 16+ messages in thread From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw) To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata bcdcfz. converts from Zoned numeric format to BCD. Zoned format uses a byte to represent a digit where the most significant nibble is 0x3 or 0xf, depending on the preferred signal. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- target-ppc/helper.h | 1 + target-ppc/int_helper.c | 55 +++++++++++++++++++++++++++++++++++++ target-ppc/translate/vmx-impl.inc.c | 7 +++-- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/target-ppc/helper.h b/target-ppc/helper.h index 92eaaf0..f460635 100644 --- a/target-ppc/helper.h +++ b/target-ppc/helper.h @@ -371,6 +371,7 @@ 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_3(bcdcfz, 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 cffe82c..8cbbdfc 100644 --- a/target-ppc/int_helper.c +++ b/target-ppc/int_helper.c @@ -2459,6 +2459,17 @@ static int bcd_preferred_sgn(int sgn, int ps) } } +static uint8_t get_nibble(ppc_avr_t *bcd, int n) +{ + uint8_t result; + if (!(n & 1)) { + result = bcd->u8[BCD_DIG_BYTE(n)] & 0xF; + } else { + result = bcd->u8[BCD_DIG_BYTE(n)] >> 4; + } + return result; +} + static uint8_t bcd_get_digit(ppc_avr_t *bcd, int n, int *invalid) { uint8_t result; @@ -2713,6 +2724,50 @@ uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b) return cr; } +uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) +{ + int i; + int j = 0; + int cr = 0; + int invalid = 0; + int eq_flag = 0; + int zone_digit = 0; + ppc_avr_t ret = { .u64 = { 0, 0 } }; + int sgnb = get_nibble(b, 1); + + if (unlikely(((sgnb < 0xA) && ps) || + ((get_nibble(b, 0) > 0x9) && !ps))) { + invalid = 1; + } + + for (i = 31, j = 16; i > 1; i -= 2, j--) { + zone_digit = get_nibble(b, i); + if (unlikely((ps && zone_digit != 0xF) || + (!ps && zone_digit != 0x3))) { + invalid = 1; + } + + eq_flag += zone_digit; + bcd_put_digit(&ret, get_nibble(b, i - 1), j); + } + bcd_put_digit(&ret, get_nibble(b, 0), 1); + + if ((ps && (sgnb == 0xB || sgnb == 0xD)) || + (!ps && (sgnb & 0x4))) { + bcd_put_digit(&ret, BCD_NEG_PREF, 0); + cr = (!eq_flag) ? 1 << CRF_LT : 1 << CRF_EQ; + } else { + bcd_put_digit(&ret, BCD_PLUS_PREF_1, 0); + cr = (!eq_flag) ? 1 << CRF_GT : 1 << CRF_EQ; + } + + 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 4364881..9192f8f 100644 --- a/target-ppc/translate/vmx-impl.inc.c +++ b/target-ppc/translate/vmx-impl.inc.c @@ -917,6 +917,7 @@ GEN_BCD(bcdadd) GEN_BCD(bcdsub) GEN_BCD2(bcdcfn) GEN_BCD3(bcdctn) +GEN_BCD2(bcdcfz) static void gen_xpnd04_1(DisasContext *ctx) { @@ -931,7 +932,8 @@ static void gen_xpnd04_1(DisasContext *ctx) gen_bcdctn(ctx); break; case 6: - break; /* bcdcfz. */ + gen_bcdcfz(ctx); + break; case 7: gen_bcdcfn(ctx); break; @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx) case 4: break; /* bcdctz. */ case 6: - break; /* bcdcfz. */ + gen_bcdcfz(ctx); + break; case 7: gen_bcdcfn(ctx); break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani @ 2016-10-27 1:35 ` David Gibson 2016-10-27 18:02 ` joserz 0 siblings, 1 reply; 16+ messages in thread From: David Gibson @ 2016-10-27 1:35 UTC (permalink / raw) To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata [-- Attachment #1: Type: text/plain, Size: 4996 bytes --] On Wed, Oct 26, 2016 at 11:18:57AM -0200, Jose Ricardo Ziviani wrote: > bcdcfz. converts from Zoned numeric format to BCD. Zoned format uses > a byte to represent a digit where the most significant nibble is 0x3 > or 0xf, depending on the preferred signal. > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> > --- > target-ppc/helper.h | 1 + > target-ppc/int_helper.c | 55 +++++++++++++++++++++++++++++++++++++ > target-ppc/translate/vmx-impl.inc.c | 7 +++-- > 3 files changed, 61 insertions(+), 2 deletions(-) > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > index 92eaaf0..f460635 100644 > --- a/target-ppc/helper.h > +++ b/target-ppc/helper.h > @@ -371,6 +371,7 @@ 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_3(bcdcfz, 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 cffe82c..8cbbdfc 100644 > --- a/target-ppc/int_helper.c > +++ b/target-ppc/int_helper.c > @@ -2459,6 +2459,17 @@ static int bcd_preferred_sgn(int sgn, int ps) > } > } > > +static uint8_t get_nibble(ppc_avr_t *bcd, int n) > +{ > + uint8_t result; > + if (!(n & 1)) { > + result = bcd->u8[BCD_DIG_BYTE(n)] & 0xF; > + } else { > + result = bcd->u8[BCD_DIG_BYTE(n)] >> 4; > + } > + return result; > +} This can be merged with bcd_get_digit() without too much effort. > static uint8_t bcd_get_digit(ppc_avr_t *bcd, int n, int *invalid) > { > uint8_t result; > @@ -2713,6 +2724,50 @@ uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b) > return cr; > } > > +uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > +{ > + int i; > + int j = 0; > + int cr = 0; > + int invalid = 0; > + int eq_flag = 0; > + int zone_digit = 0; > + ppc_avr_t ret = { .u64 = { 0, 0 } }; > + int sgnb = get_nibble(b, 1); > + > + if (unlikely(((sgnb < 0xA) && ps) || > + ((get_nibble(b, 0) > 0x9) && !ps))) { The second half of this condition doesn't belong here. It's a requirement for both ps values, and would make more sense applied along with the same test on the other digits. > + invalid = 1; > + } > + > + for (i = 31, j = 16; i > 1; i -= 2, j--) { I don't see why you're doing this in reverse order. > + zone_digit = get_nibble(b, i); > + if (unlikely((ps && zone_digit != 0xF) || > + (!ps && zone_digit != 0x3))) { > + invalid = 1; In this and the other instructions it would make sense to break out of the function as soon as you detect an invalid encoding. > + } You never check for invalid encoding in the low input nibble (i.e. value > 0x9). > + eq_flag += zone_digit; What?? How on earth does adding up the zone digits help you. For a valid encoding the answer should always be either 0x2d (PS=0) or 0xe1 (PS=1). > + bcd_put_digit(&ret, get_nibble(b, i - 1), j); > + } > + bcd_put_digit(&ret, get_nibble(b, 0), 1); > + > + if ((ps && (sgnb == 0xB || sgnb == 0xD)) || > + (!ps && (sgnb & 0x4))) { > + bcd_put_digit(&ret, BCD_NEG_PREF, 0); > + cr = (!eq_flag) ? 1 << CRF_LT : 1 << CRF_EQ; > + } else { > + bcd_put_digit(&ret, BCD_PLUS_PREF_1, 0); > + cr = (!eq_flag) ? 1 << CRF_GT : 1 << CRF_EQ; > + } > + > + 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 4364881..9192f8f 100644 > --- a/target-ppc/translate/vmx-impl.inc.c > +++ b/target-ppc/translate/vmx-impl.inc.c > @@ -917,6 +917,7 @@ GEN_BCD(bcdadd) > GEN_BCD(bcdsub) > GEN_BCD2(bcdcfn) > GEN_BCD3(bcdctn) > +GEN_BCD2(bcdcfz) > > static void gen_xpnd04_1(DisasContext *ctx) > { > @@ -931,7 +932,8 @@ static void gen_xpnd04_1(DisasContext *ctx) > gen_bcdctn(ctx); > break; > case 6: > - break; /* bcdcfz. */ > + gen_bcdcfz(ctx); > + break; > case 7: > gen_bcdcfn(ctx); > break; > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx) > case 4: > break; /* bcdctz. */ > case 6: > - break; /* bcdcfz. */ > + gen_bcdcfz(ctx); > + break; > case 7: > gen_bcdcfn(ctx); > break; -- 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction 2016-10-27 1:35 ` David Gibson @ 2016-10-27 18:02 ` joserz 0 siblings, 0 replies; 16+ messages in thread From: joserz @ 2016-10-27 18:02 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata I'll send an improved version in v2. Thank you for reviewing it. On Thu, Oct 27, 2016 at 12:35:19PM +1100, David Gibson wrote: > On Wed, Oct 26, 2016 at 11:18:57AM -0200, Jose Ricardo Ziviani wrote: > > bcdcfz. converts from Zoned numeric format to BCD. Zoned format uses > > a byte to represent a digit where the most significant nibble is 0x3 > > or 0xf, depending on the preferred signal. > > > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> > > --- > > target-ppc/helper.h | 1 + > > target-ppc/int_helper.c | 55 +++++++++++++++++++++++++++++++++++++ > > target-ppc/translate/vmx-impl.inc.c | 7 +++-- > > 3 files changed, 61 insertions(+), 2 deletions(-) > > > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > > index 92eaaf0..f460635 100644 > > --- a/target-ppc/helper.h > > +++ b/target-ppc/helper.h > > @@ -371,6 +371,7 @@ 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_3(bcdcfz, 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 cffe82c..8cbbdfc 100644 > > --- a/target-ppc/int_helper.c > > +++ b/target-ppc/int_helper.c > > @@ -2459,6 +2459,17 @@ static int bcd_preferred_sgn(int sgn, int ps) > > } > > } > > > > +static uint8_t get_nibble(ppc_avr_t *bcd, int n) > > +{ > > + uint8_t result; > > + if (!(n & 1)) { > > + result = bcd->u8[BCD_DIG_BYTE(n)] & 0xF; > > + } else { > > + result = bcd->u8[BCD_DIG_BYTE(n)] >> 4; > > + } > > + return result; > > +} > > This can be merged with bcd_get_digit() without too much effort. > > > static uint8_t bcd_get_digit(ppc_avr_t *bcd, int n, int *invalid) > > { > > uint8_t result; > > @@ -2713,6 +2724,50 @@ uint32_t helper_bcdctn(ppc_avr_t *r, ppc_avr_t *b) > > return cr; > > } > > > > +uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > +{ > > + int i; > > + int j = 0; > > + int cr = 0; > > + int invalid = 0; > > + int eq_flag = 0; > > + int zone_digit = 0; > > + ppc_avr_t ret = { .u64 = { 0, 0 } }; > > + int sgnb = get_nibble(b, 1); > > + > > + if (unlikely(((sgnb < 0xA) && ps) || > > + ((get_nibble(b, 0) > 0x9) && !ps))) { > > The second half of this condition doesn't belong here. It's a > requirement for both ps values, and would make more sense applied > along with the same test on the other digits. > > > + invalid = 1; > > + } > > + > > + for (i = 31, j = 16; i > 1; i -= 2, j--) { > > I don't see why you're doing this in reverse order. > > > + zone_digit = get_nibble(b, i); > > + if (unlikely((ps && zone_digit != 0xF) || > > + (!ps && zone_digit != 0x3))) { > > + invalid = 1; > > In this and the other instructions it would make sense to break out of > the function as soon as you detect an invalid encoding. > > > + } > > You never check for invalid encoding in the low input nibble > (i.e. value > 0x9). > > > + eq_flag += zone_digit; > > What?? How on earth does adding up the zone digits help you. For a > valid encoding the answer should always be either 0x2d (PS=0) or 0xe1 > (PS=1). > > > + bcd_put_digit(&ret, get_nibble(b, i - 1), j); > > + } > > + bcd_put_digit(&ret, get_nibble(b, 0), 1); > > + > > + if ((ps && (sgnb == 0xB || sgnb == 0xD)) || > > + (!ps && (sgnb & 0x4))) { > > + bcd_put_digit(&ret, BCD_NEG_PREF, 0); > > + cr = (!eq_flag) ? 1 << CRF_LT : 1 << CRF_EQ; > > + } else { > > + bcd_put_digit(&ret, BCD_PLUS_PREF_1, 0); > > + cr = (!eq_flag) ? 1 << CRF_GT : 1 << CRF_EQ; > > + } > > + > > + 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 4364881..9192f8f 100644 > > --- a/target-ppc/translate/vmx-impl.inc.c > > +++ b/target-ppc/translate/vmx-impl.inc.c > > @@ -917,6 +917,7 @@ GEN_BCD(bcdadd) > > GEN_BCD(bcdsub) > > GEN_BCD2(bcdcfn) > > GEN_BCD3(bcdctn) > > +GEN_BCD2(bcdcfz) > > > > static void gen_xpnd04_1(DisasContext *ctx) > > { > > @@ -931,7 +932,8 @@ static void gen_xpnd04_1(DisasContext *ctx) > > gen_bcdctn(ctx); > > break; > > case 6: > > - break; /* bcdcfz. */ > > + gen_bcdcfz(ctx); > > + break; > > case 7: > > gen_bcdcfn(ctx); > > break; > > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx) > > case 4: > > break; /* bcdctz. */ > > case 6: > > - break; /* bcdcfz. */ > > + gen_bcdcfz(ctx); > > + break; > > case 7: > > gen_bcdcfn(ctx); > > break; > > -- > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction 2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani ` (2 preceding siblings ...) 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani @ 2016-10-26 13:18 ` Jose Ricardo Ziviani 2016-10-27 1:47 ` David Gibson 3 siblings, 1 reply; 16+ messages in thread From: Jose Ricardo Ziviani @ 2016-10-26 13:18 UTC (permalink / raw) To: qemu-ppc; +Cc: qemu-devel, david, nikunj, bharata bcdctz. converts from BCD to Zoned numeric format. Zoned format uses a byte to represent a digit where the most significant nibble is 0x3 or 0xf, depending on the preferred signal. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- target-ppc/helper.h | 1 + target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++ target-ppc/translate/vmx-impl.inc.c | 7 +++-- 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/target-ppc/helper.h b/target-ppc/helper.h index f460635..3b928b8 100644 --- a/target-ppc/helper.h +++ b/target-ppc/helper.h @@ -372,6 +372,7 @@ 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_3(bcdcfz, i32, avr, avr, i32) +DEF_HELPER_3(bcdctz, 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 8cbbdfc..e0a84bb 100644 --- a/target-ppc/int_helper.c +++ b/target-ppc/int_helper.c @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) *r = ret; return cr; } + +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) +{ + int i; + int j; + int cr = 0; + int invalid = 0; + uint8_t digit = 0; + int sgnb = bcd_get_sgn(b); + int zone_lead = (ps) ? 0xF0 : 0x30; + 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); + + for (i = 31, j = 16; i > 1; i -= 2, j--) { + digit = get_nibble(b, j); + ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit; + + if (unlikely(digit > 9)) { + invalid = 1; + } + } + + if (unlikely(!sgnb)) { + sgnb = (!get_nibble(b, 0)) ? 1 : -1; + } + + if (ps) { + bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1); + } else { + bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1); + } + bcd_put_digit(&ret, get_nibble(b, 1), 0); + + if (!eq_flag) { + cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT; + } 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 9192f8f..62c44f0 100644 --- a/target-ppc/translate/vmx-impl.inc.c +++ b/target-ppc/translate/vmx-impl.inc.c @@ -918,6 +918,7 @@ GEN_BCD(bcdsub) GEN_BCD2(bcdcfn) GEN_BCD3(bcdctn) GEN_BCD2(bcdcfz) +GEN_BCD2(bcdctz) static void gen_xpnd04_1(DisasContext *ctx) { @@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx) case 2: break; /* bcdcfsq. */ case 4: - break; /* bcdctz. */ + gen_bcdctz(ctx); + break; case 5: gen_bcdctn(ctx); break; @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx) case 2: break; /* bcdcfsq. */ case 4: - break; /* bcdctz. */ + gen_bcdctz(ctx); + break; case 6: gen_bcdcfz(ctx); break; -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani @ 2016-10-27 1:47 ` David Gibson 2016-10-27 18:29 ` joserz 2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz 0 siblings, 2 replies; 16+ messages in thread From: David Gibson @ 2016-10-27 1:47 UTC (permalink / raw) To: Jose Ricardo Ziviani; +Cc: qemu-ppc, qemu-devel, nikunj, bharata [-- Attachment #1: Type: text/plain, Size: 4409 bytes --] On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote: > bcdctz. converts from BCD to Zoned numeric format. Zoned format uses > a byte to represent a digit where the most significant nibble is 0x3 > or 0xf, depending on the preferred signal. > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> > --- > target-ppc/helper.h | 1 + > target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++ > target-ppc/translate/vmx-impl.inc.c | 7 +++-- > 3 files changed, 60 insertions(+), 2 deletions(-) > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > index f460635..3b928b8 100644 > --- a/target-ppc/helper.h > +++ b/target-ppc/helper.h > @@ -372,6 +372,7 @@ 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_3(bcdcfz, i32, avr, avr, i32) > +DEF_HELPER_3(bcdctz, 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 8cbbdfc..e0a84bb 100644 > --- a/target-ppc/int_helper.c > +++ b/target-ppc/int_helper.c > @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > *r = ret; > return cr; > } > + > +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > +{ > + int i; > + int j; > + int cr = 0; > + int invalid = 0; > + uint8_t digit = 0; > + int sgnb = bcd_get_sgn(b); > + int zone_lead = (ps) ? 0xF0 : 0x30; > + 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); This doesn't look right. The zoned format can accomodate 16 digits. The low word of the packed format can accomodate 15 digits + sign. So it should be possible to convert a packed for with a value in the low nibble of the upper word. > + for (i = 31, j = 16; i > 1; i -= 2, j--) { Again not seeing any point for the reverse order. > + digit = get_nibble(b, j); > + ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit; > + > + if (unlikely(digit > 9)) { > + invalid = 1; > + } > + } > + > + if (unlikely(!sgnb)) { > + sgnb = (!get_nibble(b, 0)) ? 1 : -1; > + } Doesn't bcd_get_sgn() returning 0 indicate a badly encoded sign point. In which case you should set invalid, rather than making up a sign according to this apparently arbitrary scheme. > + > + if (ps) { > + bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1); > + } else { > + bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1); > + } > + bcd_put_digit(&ret, get_nibble(b, 1), 0); > + > + if (!eq_flag) { > + cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT; > + } 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 9192f8f..62c44f0 100644 > --- a/target-ppc/translate/vmx-impl.inc.c > +++ b/target-ppc/translate/vmx-impl.inc.c > @@ -918,6 +918,7 @@ GEN_BCD(bcdsub) > GEN_BCD2(bcdcfn) > GEN_BCD3(bcdctn) > GEN_BCD2(bcdcfz) > +GEN_BCD2(bcdctz) > > static void gen_xpnd04_1(DisasContext *ctx) > { > @@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx) > case 2: > break; /* bcdcfsq. */ > case 4: > - break; /* bcdctz. */ > + gen_bcdctz(ctx); > + break; > case 5: > gen_bcdctn(ctx); > break; > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx) > case 2: > break; /* bcdcfsq. */ > case 4: > - break; /* bcdctz. */ > + gen_bcdctz(ctx); > + break; > case 6: > gen_bcdcfz(ctx); > break; -- 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction 2016-10-27 1:47 ` David Gibson @ 2016-10-27 18:29 ` joserz 2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz 1 sibling, 0 replies; 16+ messages in thread From: joserz @ 2016-10-27 18:29 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, nikunj, bharata Everything will be fixed in v2. Thanks for reviewing it! On Thu, Oct 27, 2016 at 12:47:34PM +1100, David Gibson wrote: > On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote: > > bcdctz. converts from BCD to Zoned numeric format. Zoned format uses > > a byte to represent a digit where the most significant nibble is 0x3 > > or 0xf, depending on the preferred signal. > > > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> > > --- > > target-ppc/helper.h | 1 + > > target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++ > > target-ppc/translate/vmx-impl.inc.c | 7 +++-- > > 3 files changed, 60 insertions(+), 2 deletions(-) > > > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > > index f460635..3b928b8 100644 > > --- a/target-ppc/helper.h > > +++ b/target-ppc/helper.h > > @@ -372,6 +372,7 @@ 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_3(bcdcfz, i32, avr, avr, i32) > > +DEF_HELPER_3(bcdctz, 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 8cbbdfc..e0a84bb 100644 > > --- a/target-ppc/int_helper.c > > +++ b/target-ppc/int_helper.c > > @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > *r = ret; > > return cr; > > } > > + > > +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > +{ > > + int i; > > + int j; > > + int cr = 0; > > + int invalid = 0; > > + uint8_t digit = 0; > > + int sgnb = bcd_get_sgn(b); > > + int zone_lead = (ps) ? 0xF0 : 0x30; > > + 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); > > This doesn't look right. The zoned format can accomodate 16 digits. > The low word of the packed format can accomodate 15 digits + sign. So > it should be possible to convert a packed for with a value in the low > nibble of the upper word. > > > + for (i = 31, j = 16; i > 1; i -= 2, j--) { > > Again not seeing any point for the reverse order. > > > + digit = get_nibble(b, j); > > + ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit; > > + > > + if (unlikely(digit > 9)) { > > + invalid = 1; > > + } > > + } > > + > > + if (unlikely(!sgnb)) { > > + sgnb = (!get_nibble(b, 0)) ? 1 : -1; > > + } > > Doesn't bcd_get_sgn() returning 0 indicate a badly encoded sign > point. In which case you should set invalid, rather than making up a > sign according to this apparently arbitrary scheme. > > > + > > + if (ps) { > > + bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1); > > + } else { > > + bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1); > > + } > > + bcd_put_digit(&ret, get_nibble(b, 1), 0); > > + > > + if (!eq_flag) { > > + cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT; > > + } 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 9192f8f..62c44f0 100644 > > --- a/target-ppc/translate/vmx-impl.inc.c > > +++ b/target-ppc/translate/vmx-impl.inc.c > > @@ -918,6 +918,7 @@ GEN_BCD(bcdsub) > > GEN_BCD2(bcdcfn) > > GEN_BCD3(bcdctn) > > GEN_BCD2(bcdcfz) > > +GEN_BCD2(bcdctz) > > > > static void gen_xpnd04_1(DisasContext *ctx) > > { > > @@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx) > > case 2: > > break; /* bcdcfsq. */ > > case 4: > > - break; /* bcdctz. */ > > + gen_bcdctz(ctx); > > + break; > > case 5: > > gen_bcdctn(ctx); > > break; > > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx) > > case 2: > > break; /* bcdcfsq. */ > > case 4: > > - break; /* bcdctz. */ > > + gen_bcdctz(ctx); > > + break; > > case 6: > > gen_bcdcfz(ctx); > > break; > > -- > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction 2016-10-27 1:47 ` David Gibson 2016-10-27 18:29 ` joserz @ 2016-10-27 20:08 ` joserz 2016-10-31 2:50 ` David Gibson 1 sibling, 1 reply; 16+ messages in thread From: joserz @ 2016-10-27 20:08 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, qemu-devel, bharata On Thu, Oct 27, 2016 at 12:47:34PM +1100, David Gibson wrote: > On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote: > > bcdctz. converts from BCD to Zoned numeric format. Zoned format uses > > a byte to represent a digit where the most significant nibble is 0x3 > > or 0xf, depending on the preferred signal. > > > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> > > --- > > target-ppc/helper.h | 1 + > > target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++ > > target-ppc/translate/vmx-impl.inc.c | 7 +++-- > > 3 files changed, 60 insertions(+), 2 deletions(-) > > > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > > index f460635..3b928b8 100644 > > --- a/target-ppc/helper.h > > +++ b/target-ppc/helper.h > > @@ -372,6 +372,7 @@ 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_3(bcdcfz, i32, avr, avr, i32) > > +DEF_HELPER_3(bcdctz, 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 8cbbdfc..e0a84bb 100644 > > --- a/target-ppc/int_helper.c > > +++ b/target-ppc/int_helper.c > > @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > *r = ret; > > return cr; > > } > > + > > +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > +{ > > + int i; > > + int j; > > + int cr = 0; > > + int invalid = 0; > > + uint8_t digit = 0; > > + int sgnb = bcd_get_sgn(b); > > + int zone_lead = (ps) ? 0xF0 : 0x30; > > + 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); > > This doesn't look right. The zoned format can accomodate 16 digits. > The low word of the packed format can accomodate 15 digits + sign. So > it should be possible to convert a packed for with a value in the low > nibble of the upper word. David, it seems that zoned format can accommodate 15 digits + sign at most. Suppose I have this 16 bytes zoned number: f9f9f9f9f9f9f9f9f9f9f9f9f9f9f9xx. Converting it to packed bcd will give 999999999999999x. Do you agree? can I keep that ox_flag logic? > > > + for (i = 31, j = 16; i > 1; i -= 2, j--) { > > Again not seeing any point for the reverse order. > > > + digit = get_nibble(b, j); > > + ret.u8[BCD_DIG_BYTE(i)] = zone_lead + digit; > > + > > + if (unlikely(digit > 9)) { > > + invalid = 1; > > + } > > + } > > + > > + if (unlikely(!sgnb)) { > > + sgnb = (!get_nibble(b, 0)) ? 1 : -1; > > + } > > Doesn't bcd_get_sgn() returning 0 indicate a badly encoded sign > point. In which case you should set invalid, rather than making up a > sign according to this apparently arbitrary scheme. > > > + > > + if (ps) { > > + bcd_put_digit(&ret, (sgnb == 1) ? 0xC : 0xD, 1); > > + } else { > > + bcd_put_digit(&ret, (sgnb == 1) ? 0x3 : 0x7, 1); > > + } > > + bcd_put_digit(&ret, get_nibble(b, 1), 0); > > + > > + if (!eq_flag) { > > + cr = (sgnb == 1) ? 1 << CRF_GT : 1 << CRF_LT; > > + } 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 9192f8f..62c44f0 100644 > > --- a/target-ppc/translate/vmx-impl.inc.c > > +++ b/target-ppc/translate/vmx-impl.inc.c > > @@ -918,6 +918,7 @@ GEN_BCD(bcdsub) > > GEN_BCD2(bcdcfn) > > GEN_BCD3(bcdctn) > > GEN_BCD2(bcdcfz) > > +GEN_BCD2(bcdctz) > > > > static void gen_xpnd04_1(DisasContext *ctx) > > { > > @@ -927,7 +928,8 @@ static void gen_xpnd04_1(DisasContext *ctx) > > case 2: > > break; /* bcdcfsq. */ > > case 4: > > - break; /* bcdctz. */ > > + gen_bcdctz(ctx); > > + break; > > case 5: > > gen_bcdctn(ctx); > > break; > > @@ -952,7 +954,8 @@ static void gen_xpnd04_2(DisasContext *ctx) > > case 2: > > break; /* bcdcfsq. */ > > case 4: > > - break; /* bcdctz. */ > > + gen_bcdctz(ctx); > > + break; > > case 6: > > gen_bcdcfz(ctx); > > break; > > -- > 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction 2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz @ 2016-10-31 2:50 ` David Gibson 0 siblings, 0 replies; 16+ messages in thread From: David Gibson @ 2016-10-31 2:50 UTC (permalink / raw) To: joserz; +Cc: qemu-ppc, qemu-devel, bharata [-- Attachment #1: Type: text/plain, Size: 3138 bytes --] On Thu, Oct 27, 2016 at 06:08:20PM -0200, joserz@linux.vnet.ibm.com wrote: > On Thu, Oct 27, 2016 at 12:47:34PM +1100, David Gibson wrote: > > On Wed, Oct 26, 2016 at 11:18:58AM -0200, Jose Ricardo Ziviani wrote: > > > bcdctz. converts from BCD to Zoned numeric format. Zoned format uses > > > a byte to represent a digit where the most significant nibble is 0x3 > > > or 0xf, depending on the preferred signal. > > > > > > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> > > > --- > > > target-ppc/helper.h | 1 + > > > target-ppc/int_helper.c | 54 +++++++++++++++++++++++++++++++++++++ > > > target-ppc/translate/vmx-impl.inc.c | 7 +++-- > > > 3 files changed, 60 insertions(+), 2 deletions(-) > > > > > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > > > index f460635..3b928b8 100644 > > > --- a/target-ppc/helper.h > > > +++ b/target-ppc/helper.h > > > @@ -372,6 +372,7 @@ 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_3(bcdcfz, i32, avr, avr, i32) > > > +DEF_HELPER_3(bcdctz, 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 8cbbdfc..e0a84bb 100644 > > > --- a/target-ppc/int_helper.c > > > +++ b/target-ppc/int_helper.c > > > @@ -2768,6 +2768,60 @@ uint32_t helper_bcdcfz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > > *r = ret; > > > return cr; > > > } > > > + > > > +uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > > > +{ > > > + int i; > > > + int j; > > > + int cr = 0; > > > + int invalid = 0; > > > + uint8_t digit = 0; > > > + int sgnb = bcd_get_sgn(b); > > > + int zone_lead = (ps) ? 0xF0 : 0x30; > > > + 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); > > > > This doesn't look right. The zoned format can accomodate 16 digits. > > The low word of the packed format can accomodate 15 digits + sign. So > > it should be possible to convert a packed for with a value in the low > > nibble of the upper word. > > David, it seems that zoned format can accommodate 15 digits + sign > at most. Suppose I have this 16 bytes zoned number: > f9f9f9f9f9f9f9f9f9f9f9f9f9f9f9xx. Converting it to packed bcd will > give 999999999999999x. Do you agree? can I keep that ox_flag logic? Uh.. no. AFAICT from the description of the instruction the last byte of a zoned number contains *both* a sign nibble *and* the least significant digit of the number. So zoned format can represent 16 digits + sign. But IIRC you changed this in v2, so maybe you realised that already. -- 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-10-31 2:53 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-26 13:18 [Qemu-devel] [PATCH] 0/4] POWER9 TCG enablements - BCD functions part I Jose Ricardo Ziviani 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 1/4] target-ppc: Implement bcdcfn. instruction Jose Ricardo Ziviani 2016-10-27 1:05 ` David Gibson 2016-10-27 15:29 ` joserz 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 2/4] target-ppc: Implement bcdctn. instruction Jose Ricardo Ziviani 2016-10-27 1:20 ` David Gibson 2016-10-27 16:13 ` joserz 2016-10-31 0:16 ` David Gibson 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 3/4] target-ppc: Implement bcdcfz. instruction Jose Ricardo Ziviani 2016-10-27 1:35 ` David Gibson 2016-10-27 18:02 ` joserz 2016-10-26 13:18 ` [Qemu-devel] [PATCH] 4/4] target-ppc: Implement bcdctz. instruction Jose Ricardo Ziviani 2016-10-27 1:47 ` David Gibson 2016-10-27 18:29 ` joserz 2016-10-27 20:08 ` [Qemu-devel] [Qemu-ppc] " joserz 2016-10-31 2:50 ` David Gibson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).