qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 5/7] target-ppc: Implement bcdsr. ins


From: David Gibson
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 5/7] target-ppc: Implement bcdsr. instruction
Date: Tue, 6 Dec 2016 10:01:25 +1100
User-agent: Mutt/1.7.1 (2016-10-04)

On Mon, Dec 05, 2016 at 04:52:57PM -0200, address@hidden wrote:
> On Mon, Dec 05, 2016 at 02:19:26PM +1100, David Gibson wrote:
> > On Sat, Dec 03, 2016 at 03:00:04AM -0200, Jose Ricardo Ziviani wrote:
> > > bcdsr.: Decimal shift and round. This instruction works like bcds.
> > > however, when performing right shift, 1 will be added to the
> > > result if the last digit was >= 5.
> > > 
> > > Signed-off-by: Jose Ricardo Ziviani <address@hidden>
> > > ---
> > >  target-ppc/helper.h                 |  1 +
> > >  target-ppc/int_helper.c             | 45 
> > > +++++++++++++++++++++++++++++++++++++
> > >  target-ppc/translate/vmx-impl.inc.c |  1 +
> > >  target-ppc/translate/vmx-ops.inc.c  |  2 ++
> > >  4 files changed, 49 insertions(+)
> > > 
> > > diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> > > index 386ea67..d9528eb 100644
> > > --- a/target-ppc/helper.h
> > > +++ b/target-ppc/helper.h
> > > @@ -394,6 +394,7 @@ DEF_HELPER_4(bcdcpsgn, i32, avr, avr, avr, i32)
> > >  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_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 4b5eea1..c9fcb1a 100644
> > > --- a/target-ppc/int_helper.c
> > > +++ b/target-ppc/int_helper.c
> > > @@ -3124,6 +3124,51 @@ uint32_t helper_bcdus(ppc_avr_t *r, ppc_avr_t *a, 
> > > ppc_avr_t *b, uint32_t ps)
> > >      return cr;
> > >  }
> > >  
> > > +uint32_t helper_bcdsr(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b, uint32_t 
> > > ps)
> > > +{
> > > +    int cr;
> > > +    int i;
> > > +    int unused = 0;
> > > +    int invalid = 0;
> > > +    bool ox_flag = false;
> > > +    int sgnb = bcd_get_sgn(b);
> > > +    ppc_avr_t ret = *b;
> > > +    ret.u64[LO_IDX] &= ~0xf;
> > > +
> > > +#if defined(HOST_WORDS_BIGENDIAN)
> > > +    ppc_avr_t bcd_one = { .u64 = { 0, 0x10 } };
> > > +    int upper = ARRAY_SIZE(a->s32) - 1;
> > 
> > Same comment as previous patches about the shift argument.
> > 
> > > +#else
> > > +    ppc_avr_t bcd_one = { .u64 = { 0x10, 0 } };
> > > +    int upper = 0;
> > > +#endif
> > > +
> > > +    if (bcd_is_valid(b) == false) {
> > > +        return CRF_SO;
> > > +    }
> > > +
> > > +    if (a->s32[upper] > 0) {
> > > +        i = (a->s32[upper] > 31) ? 31 : a->s32[upper];
> > > +        ulshift(&ret.u64[LO_IDX], &ret.u64[HI_IDX], i * 4, &ox_flag);
> > > +    } else {
> > > +        i = (a->s32[upper] < -31) ? 31 : -a->s32[upper];
> > > +        urshift(&ret.u64[LO_IDX], &ret.u64[HI_IDX], i * 4);
> > > +
> > > +        if (bcd_get_digit(&ret, 0, &invalid) >= 5) {
> > 
> > So, the ISA actually says you increment only if the last digit is >
> > 5.  That doesn't seem like correct rounding, so it might be an error
> > in the ISA document - best check this with the hardware people.
> > 
> 
> Just checked with hw team here and they will have this updated to
> "greater than or equal to 5" in the next version (v3.0B).
> 
> I was told this operation is used more as arithmetic (divide by power of
> 10) than logical.

Right, that makes sense, but we needed to check.  After all if it was
the actual hardware that got this wrong (not just the documentation),
then qemu would need to match that, regardless of how useless the
instruction would be as a result.

> 
> > > +            bcd_add_mag(&ret, &ret, &bcd_one, &invalid, &unused);
> > > +        }
> > > +    }
> > > +    bcd_put_digit(&ret, bcd_preferred_sgn(sgnb, ps), 0);
> > > +
> > > +    cr = bcd_cmp_zero(&ret);
> > > +    if (unlikely(ox_flag)) {
> > > +        cr |= 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 fc54881..451abb5 100644
> > > --- a/target-ppc/translate/vmx-impl.inc.c
> > > +++ b/target-ppc/translate/vmx-impl.inc.c
> > > @@ -1018,6 +1018,7 @@ GEN_BCD2(bcdsetsgn)
> > >  GEN_BCD(bcdcpsgn);
> > >  GEN_BCD(bcds);
> > >  GEN_BCD(bcdus);
> > > +GEN_BCD(bcdsr);
> > >  
> > >  static void gen_xpnd04_1(DisasContext *ctx)
> > >  {
> > > diff --git a/target-ppc/translate/vmx-ops.inc.c 
> > > b/target-ppc/translate/vmx-ops.inc.c
> > > index cdd3abe..fa9c996 100644
> > > --- a/target-ppc/translate/vmx-ops.inc.c
> > > +++ b/target-ppc/translate/vmx-ops.inc.c
> > > @@ -132,6 +132,8 @@ GEN_HANDLER_E_2(vprtybd, 0x4, 0x1, 0x18, 9, 0, 
> > > PPC_NONE, PPC2_ISA300),
> > >  GEN_HANDLER_E_2(vprtybq, 0x4, 0x1, 0x18, 10, 0, PPC_NONE, PPC2_ISA300),
> > >  
> > >  GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE),
> > > +GEN_VXFORM_300(bcdsr, 0, 23),
> > > +GEN_VXFORM_300(bcdsr, 0, 31),
> > >  GEN_VXFORM_DUAL(vaddubs, vmul10uq, 0, 8, PPC_ALTIVEC, PPC_NONE),
> > >  GEN_VXFORM_DUAL(vadduhs, vmul10euq, 0, 9, PPC_ALTIVEC, PPC_NONE),
> > >  GEN_VXFORM(vadduws, 0, 10),
> > 
> 
> 

-- 
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: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]