qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v3 1/4] target-ppc: Implement bcdcfn. instructio


From: David Gibson
Subject: Re: [Qemu-devel] [PATCH v3 1/4] target-ppc: Implement bcdcfn. instruction
Date: Tue, 1 Nov 2016 11:30:33 +1100
User-agent: Mutt/1.7.1 (2016-10-04)

On Mon, Oct 31, 2016 at 03:34:46PM -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 <address@hidden>

Sorry, couple more tweaks I've thought of.


> ---
>  target-ppc/helper.h                 |  1 +
>  target-ppc/int_helper.c             | 54 ++++++++++++++++++++++++++
>  target-ppc/translate/vmx-impl.inc.c | 77 
> +++++++++++++++++++++++++++++++++++++
>  target-ppc/translate/vmx-ops.inc.c  |  4 +-
>  4 files changed, 134 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 3916b2e..3b23eed 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -371,6 +371,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 dca4798..dc7a429 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -2429,6 +2429,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))
> @@ -2495,6 +2497,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t 
> digit, int n)
>      }
>  }
>  
> +static uint16_t get_national_digit(ppc_avr_t *reg, int n)
> +{
> +#if defined(HOST_WORDS_BIGENDIAN)
> +    return reg->u16[8 - n];
> +#else
> +    return reg->u16[n];
> +#endif
> +}
> +
>  static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
>  {
>      int i;
> @@ -2625,6 +2636,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 neq_flag = 0;
> +    int cr = 0;
> +    uint16_t national = 0;
> +    uint16_t sgnb = get_national_digit(b, 0);
> +    ppc_avr_t ret = { .u64 = { 0, 0 } };
> +    int invalid = (sgnb != NATIONAL_PLUS && sgnb != NATIONAL_NEG);
> +
> +    for (i = 1; i < 8; i++) {
> +        national = get_national_digit(b, i);
> +
> +        neq_flag += (national != 0x30);

Specifically adding up the non-zero digits is a bit clunky.  You're
already converting the value to BCD, so you could...

> +        if (unlikely(national < 0x30 || national > 0x39)) {
> +            invalid = 1;
> +            break;
> +        }
> +
> +        bcd_put_digit(&ret, national & 0xf, i);
> +    }
> +
> +    if (sgnb == NATIONAL_PLUS) {
> +        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 (neq_flag) {

... just check if the BCD output is zero here.  I think that will
amount to ((ret.u64[1] >> 4) != 0), but it's probably worth adding a
bcd_is_zero() helper for this.

> +        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 fc612d9..38fea3f 100644
> --- a/target-ppc/translate/vmx-impl.inc.c
> +++ b/target-ppc/translate/vmx-impl.inc.c
> @@ -945,8 +945,83 @@ 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:
> +        gen_invalid(ctx);
> +        break;

More important change here.  I think you need to leave out the cases
you haven't implemented yet, and have them all cause an invalid
instruction exception.  That's not correct to the ISA, obviously, but
neither is treating them as a no-op.  Better to have the guest get a
fault, so that you know something has gone wrong.

> +    }
> +}
> +
> +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:
> +        gen_invalid(ctx);
> +        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)
> @@ -1023,3 +1098,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 cc7ed7e..637b43c 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_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),
> @@ -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: signature.asc
Description: PGP signature


reply via email to

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