[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 5/6] target-ppc: Implement bcdtrun
From: |
joserz |
Subject: |
Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 5/6] target-ppc: Implement bcdtrunc. instruction |
Date: |
Thu, 8 Dec 2016 21:29:24 -0200 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Dec 09, 2016 at 08:06:14AM +1100, David Gibson wrote:
> On Thu, Dec 08, 2016 at 12:07:07AM -0200, Jose Ricardo Ziviani wrote:
> > bcdtrunc.: Decimal integer truncate. Given a BCD number in vrb and the
> > number of bytes to truncate in vra, the return register will have vrb
> > with such bits truncated.
> >
> > Signed-off-by: Jose Ricardo Ziviani <address@hidden>
>
> Um.. this is wrong on multiple accounts. Have you actually tested
> this with any input at all?
yes I did, a lot but it's possible that I get the idea wrong.
>
> > ---
> > target-ppc/helper.h | 1 +
> > target-ppc/int_helper.c | 40
> > +++++++++++++++++++++++++++++++++++++
> > target-ppc/translate/vmx-impl.inc.c | 5 +++++
> > target-ppc/translate/vmx-ops.inc.c | 4 ++--
> > 4 files changed, 48 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > index d9528eb..49965b0 100644
> > --- a/target-ppc/helper.h
> > +++ b/target-ppc/helper.h
> > @@ -395,6 +395,7 @@ DEF_HELPER_3(bcdsetsgn, i32, avr, avr, i32)
> > DEF_HELPER_4(bcds, i32, avr, avr, avr, i32)
> > DEF_HELPER_4(bcdus, i32, avr, avr, avr, i32)
> > DEF_HELPER_4(bcdsr, i32, avr, avr, avr, i32)
> > +DEF_HELPER_4(bcdtrunc, i32, avr, 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 cfef25f..dea08e6 100644
> > --- a/target-ppc/int_helper.c
> > +++ b/target-ppc/int_helper.c
> > @@ -3169,6 +3169,46 @@ uint32_t helper_bcdsr(ppc_avr_t *r, ppc_avr_t *a,
> > ppc_avr_t *b, uint32_t ps)
> > return cr;
> > }
> >
> > +uint32_t helper_bcdtrunc(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b,
> > uint32_t ps)
> > +{
> > + uint64_t mask;
> > + uint32_t bits;
> > + uint32_t ox_flag = 0;
> > + ppc_avr_t ret = *b;
> > +
> > +#if defined(HOST_WORDS_BIGENDIAN)
> > + int upper = ARRAY_SIZE(a->u16) - 1;
> > +#else
> > + int upper = 0;
> > +#endif
> > +
> > + if (bcd_is_valid(b) == false) {
> > + return CRF_SO;
> > + }
> > +
> > + bits = (a->u16[upper] & 31) << 2;
>
> According to the ISA, the value here gives the number of digits to
> *retain*, you're treating it as the number of digits to clear. You're
> also not accounting for the sign marker, which should be retained in
> any case.
I understood it is the number of bits to clear, I'll review it.
>
> Finally, the ISA says the value in bits 48:63 are treated as length
> with no mention of being truncated to 5 bits. Which means it's not
> clear what should happen if length > 31. You'll need to check with
> the hardware people about this.
ok
>
> > + if (bits >= 64) {
> > + if (ret.u64[HI_IDX] || (ret.u64[LO_IDX] >> (128 - bits))) {
> > + ox_flag = CRF_SO;
> > + }
> > +
> > + mask = (uint64_t)-1 >> (bits - 64);
> > + ret.u64[HI_IDX] = 0;
> > + ret.u64[LO_IDX] &= mask;
> > + } else {
> > + if (ret.u64[HI_IDX] >> (64 - bits)) {
> > + ox_flag = CRF_SO;
> > + }
> > +
> > + mask = (uint64_t)-1 >> bits;
> > + ret.u64[HI_IDX] &= mask;
> > + }
> > + bcd_put_digit(&ret, bcd_preferred_sgn(bcd_get_sgn(b), ps), 0);
> > + *r = ret;
> > +
> > + return bcd_cmp_zero(&ret) | ox_flag;
> > +}
> > +
> > 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 451abb5..1683f42 100644
> > --- a/target-ppc/translate/vmx-impl.inc.c
> > +++ b/target-ppc/translate/vmx-impl.inc.c
> > @@ -1019,6 +1019,7 @@ GEN_BCD(bcdcpsgn);
> > GEN_BCD(bcds);
> > GEN_BCD(bcdus);
> > GEN_BCD(bcdsr);
> > +GEN_BCD(bcdtrunc);
> >
> > static void gen_xpnd04_1(DisasContext *ctx)
> > {
> > @@ -1097,6 +1098,10 @@ GEN_VXFORM_DUAL(vsubudm, PPC2_ALTIVEC_207, PPC_NONE,
> > \
> > bcds, PPC_NONE, PPC2_ISA300)
> > GEN_VXFORM_DUAL(vsubuwm, PPC_ALTIVEC, PPC_NONE, \
> > bcdus, PPC_NONE, PPC2_ISA300)
> > +GEN_VXFORM_DUAL(vsubsbs, PPC_ALTIVEC, PPC_NONE, \
> > + bcdtrunc, PPC_NONE, PPC2_ISA300)
> > +GEN_VXFORM_DUAL(vsubuqm, PPC2_ALTIVEC_207, PPC_NONE, \
> > + bcdtrunc, PPC_NONE, PPC2_ISA300)
> >
> > static void gen_vsbox(DisasContext *ctx)
> > {
> > diff --git a/target-ppc/translate/vmx-ops.inc.c
> > b/target-ppc/translate/vmx-ops.inc.c
> > index fa9c996..e6167a4 100644
> > --- a/target-ppc/translate/vmx-ops.inc.c
> > +++ b/target-ppc/translate/vmx-ops.inc.c
> > @@ -143,14 +143,14 @@ GEN_VXFORM(vaddsws, 0, 14),
> > GEN_VXFORM_DUAL(vsububs, bcdadd, 0, 24, PPC_ALTIVEC, PPC_NONE),
> > GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, PPC_NONE),
> > GEN_VXFORM(vsubuws, 0, 26),
> > -GEN_VXFORM(vsubsbs, 0, 28),
> > +GEN_VXFORM_DUAL(vsubsbs, bcdtrunc, 0, 28, PPC_NONE, PPC2_ISA300),
> > GEN_VXFORM(vsubshs, 0, 29),
> > 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),
> > -GEN_VXFORM_207(vsubuqm, 0, 20),
> > GEN_VXFORM_207(vsubcuq, 0, 21),
> > +GEN_VXFORM_DUAL(vsubuqm, bcdtrunc, 0, 20, PPC2_ALTIVEC_207, PPC2_ISA300),
> > GEN_VXFORM_DUAL(vsubeuqm, vsubecuq, 31, 0xFF, PPC_NONE, PPC2_ALTIVEC_207),
> > GEN_VXFORM(vrlb, 2, 0),
> > GEN_VXFORM(vrlh, 2, 1),
>
> --
> 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
- [Qemu-devel] [PATCH v3 0/6] POWER9 TCG enablements - BCD functions - final part, Jose Ricardo Ziviani, 2016/12/07
- [Qemu-devel] [PATCH v3 1/6] target-ppc: Implement unsigned quadword left/right shift and unit tests, Jose Ricardo Ziviani, 2016/12/07
- [Qemu-devel] [PATCH v3 2/6] target-ppc: Implement bcds. instruction, Jose Ricardo Ziviani, 2016/12/07
- [Qemu-devel] [PATCH v3 3/6] target-ppc: Implement bcdus. instruction, Jose Ricardo Ziviani, 2016/12/07
- [Qemu-devel] [PATCH v3 6/6] target-ppc: Implement bcdutrunc. instruction, Jose Ricardo Ziviani, 2016/12/07
- [Qemu-devel] [PATCH v3 4/6] target-ppc: Implement bcdsr. instruction, Jose Ricardo Ziviani, 2016/12/07
- [Qemu-devel] [PATCH v3 5/6] target-ppc: Implement bcdtrunc. instruction, Jose Ricardo Ziviani, 2016/12/07