qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v6 08/13] target-mips-ase-dsp: Add bit/manipulat


From: Aurelien Jarno
Subject: Re: [Qemu-devel] [PATCH v6 08/13] target-mips-ase-dsp: Add bit/manipulation instructions
Date: Thu, 23 Aug 2012 16:48:02 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Tue, Aug 21, 2012 at 02:53:14PM +0800, Jia Liu wrote:
> Add MIPS ASE DSP Bit/Manipulation instructions.
> 
> Signed-off-by: Jia Liu <address@hidden>
> ---
>  target-mips/dsp_helper.c |   79 +++++++++++++
>  target-mips/helper.h     |    7 ++
>  target-mips/translate.c  |  289 
> +++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 374 insertions(+), 1 deletion(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index ff51d2f..4564d9c 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -6211,6 +6211,85 @@ void helper_dmsubu(CPUMIPSState *env,
>  }
>  #endif
>  
> +/** DSP Bit/Manipulation Sub-class insns **/
> +target_ulong helper_bitrev(target_ulong rt)
> +{
> +    int32_t temp;
> +    uint32_t rd;
> +    int i, last;
> +
> +    temp = rt & MIPSDSP_LO;
> +    rd = 0;
> +    for (i = 0; i < 16; i++) {
> +        last = temp % 2;
> +        temp = temp >> 1;
> +        rd = rd | (last << (15 - i));
> +    }
> +
> +    return (target_ulong)rd;
> +}
> +
> +target_ulong helper_insv(CPUMIPSState *env, target_ulong rs, target_ulong rt)
> +{
> +    uint32_t pos, size, msb, lsb, rs_f, rt_f;
> +    uint32_t temp, temprs, temprt;
> +    uint32_t rs_notword, rt_notword;
> +    target_ulong dspc;
> +
> +    dspc = env->active_tc.DSPControl;
> +    pos  = dspc & 0x1F;
> +    size = (dspc >> 7) & 0x1F;
> +    msb  = pos + size - 1;
> +    lsb  = pos;
> +
> +    rs_notword = not_word_value(rs);
> +    rt_notword = not_word_value(rt);
> +
> +    if ((lsb > msb) || (rs_notword == 1) || (rt_notword == 1)) {
> +        return rt;
> +    }
> +
> +    rs_f = (((int32_t)0x01 << (msb - lsb + 1 + 1)) - 1) << lsb;
> +    rt_f = rs_f ^ 0xFFFFFFFF;
> +    temprs = rs & rs_f;
> +    temprt = rt & rt_f;
> +    temp = temprs | temprt;
> +
> +    return (target_long)(int32_t)temp;
> +}
> +
> +#if defined(TARGET_MIPS64)
> +target_ulong helper_dinsv(CPUMIPSState *env, target_ulong rs, target_ulong 
> rt)
> +{
> +    target_ulong dspctrl;
> +    target_ulong filter;
> +    uint8_t pos, size;
> +    uint8_t msb, lsb;
> +    uint64_t temp;
> +
> +    temp = rt;
> +    dspctrl = env->active_tc.DSPControl;
> +    pos = dspctrl & 0x7F;
> +    size = (dspctrl >> 7) & 0x3F;
> +
> +    msb = pos + size - 1;
> +    lsb = pos;
> +
> +    if ((lsb > msb) || (msb > 63)) {
> +        return temp;
> +    }
> +
> +    temp = 0;
> +    filter = ((target_ulong)0x01 << size) - 1;
> +    filter = filter << pos;
> +
> +    temp |= rs & filter;
> +    temp |= rt & (~filter);
> +
> +    return temp;
> +}
> +#endif
> +
>  #undef MIPSDSP_LHI
>  #undef MIPSDSP_LLO
>  #undef MIPSDSP_HI
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index 69bcb6c..df86ef9 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -582,4 +582,11 @@ DEF_HELPER_FLAGS_4(dmsub, 0, void, env, tl, tl, i32)
>  DEF_HELPER_FLAGS_4(dmsubu, 0, void, env, tl, tl, i32)
>  #endif
>  
> +/* DSP Bit/Manipulation Sub-class insns */
> +DEF_HELPER_FLAGS_1(bitrev, TCG_CALL_CONST | TCG_CALL_PURE, tl, tl)
> +DEF_HELPER_FLAGS_3(insv, 0, tl, env, tl, tl)
> +#if defined(TARGET_MIPS64)
> +DEF_HELPER_FLAGS_3(dinsv, 0, tl, env, tl, tl);
> +#endif
> +
>  #include "def-helper.h"
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index d8f83e0..1a9df0f 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -343,6 +343,11 @@ enum {
>  #if defined(TARGET_MIPS64)
>      OPC_DPAQ_W_QH_DSP  = 0x34 | OPC_SPECIAL3,
>  #endif
> +    /* DSP Bit/Manipulation Sub-class */
> +    OPC_INSV_DSP       = 0x0C | OPC_SPECIAL3,
> +#if defined(TARGET_MIPS64)
> +    OPC_DINSV_DSP      = 0x0D | OPC_SPECIAL3,
> +#endif
>  };
>  
>  /* BSHFL opcodes */
> @@ -450,6 +455,12 @@ enum {
>      OPC_PRECEU_PH_QBR   = (0x1D << 6) | OPC_ABSQ_S_PH_DSP,
>      OPC_PRECEU_PH_QBLA  = (0x1E << 6) | OPC_ABSQ_S_PH_DSP,
>      OPC_PRECEU_PH_QBRA  = (0x1F << 6) | OPC_ABSQ_S_PH_DSP,
> +    /* DSP Bit/Manipulation Sub-class */
> +    OPC_BITREV          = (0x1B << 6) | OPC_ABSQ_S_PH_DSP,
> +    OPC_REPL_QB         = (0x02 << 6) | OPC_ABSQ_S_PH_DSP,
> +    OPC_REPLV_QB        = (0x03 << 6) | OPC_ABSQ_S_PH_DSP,
> +    OPC_REPL_PH         = (0x0A << 6) | OPC_ABSQ_S_PH_DSP,
> +    OPC_REPLV_PH        = (0x0B << 6) | OPC_ABSQ_S_PH_DSP,
>  };
>  
>  #define MASK_CMPU_EQ_QB(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6)))
> @@ -518,6 +529,12 @@ enum {
>      OPC_MULSA_W_PH    = (0x02 << 6) | OPC_DPA_W_PH_DSP,
>  };
>  
> +#define MASK_INSV(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6)))
> +enum {
> +    /* DSP Bit/Manipulation Sub-class */
> +    OPC_INSV = (0x00 << 6) | OPC_INSV_DSP,
> +};
> +
>  #if defined(TARGET_MIPS64)
>  #define MASK_ABSQ_S_QH(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6)))
>  enum {
> @@ -539,6 +556,13 @@ enum {
>      OPC_ABSQ_S_OB       = (0x01 << 6) | OPC_ABSQ_S_QH_DSP,
>      OPC_ABSQ_S_PW       = (0x11 << 6) | OPC_ABSQ_S_QH_DSP,
>      OPC_ABSQ_S_QH       = (0x09 << 6) | OPC_ABSQ_S_QH_DSP,
> +    /* DSP Bit/Manipulation Sub-class */
> +    OPC_REPL_OB         = (0x02 << 6) | OPC_ABSQ_S_QH_DSP,
> +    OPC_REPL_PW         = (0x12 << 6) | OPC_ABSQ_S_QH_DSP,
> +    OPC_REPL_QH         = (0x0A << 6) | OPC_ABSQ_S_QH_DSP,
> +    OPC_REPLV_OB        = (0x03 << 6) | OPC_ABSQ_S_QH_DSP,
> +    OPC_REPLV_PW        = (0x13 << 6) | OPC_ABSQ_S_QH_DSP,
> +    OPC_REPLV_QH        = (0x0B << 6) | OPC_ABSQ_S_QH_DSP,
>  };
>  #endif
>  
> @@ -592,6 +616,14 @@ enum {
>  #endif
>  
>  #if defined(TARGET_MIPS64)
> +#define MASK_DINSV(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6)))
> +enum {
> +    /* DSP Bit/Manipulation Sub-class */
> +    OPC_DINSV = (0x00 << 6) | OPC_DINSV_DSP,
> +};
> +#endif
> +
> +#if defined(TARGET_MIPS64)
>  #define MASK_DPAQ_W_QH(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6)))
>  enum {
>      /* MIPS DSP Multiply Sub-class insns */
> @@ -12633,6 +12665,116 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx, int *is_branch)
>                  check_insn(env, ctx, ASE_DSP);
>                  gen_helper_preceu_ph_qbra(cpu_gpr[rd], cpu_gpr[rt]);
>                  break;
> +            case OPC_BITREV:
> +                check_insn(env, ctx, ASE_DSP);
> +                gen_helper_bitrev(cpu_gpr[rd], cpu_gpr[rt]);
> +                break;
> +            case OPC_REPL_QB:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv imm3, imm2, imm1, imm0, temp_rd;
> +                    temp_rd = tcg_const_tl(0);
> +
> +                    imm = (ctx->opcode >> 16) & 0xFF;
> +                    imm3 = tcg_const_tl(imm);
> +                    imm2 = tcg_const_tl(imm);
> +                    imm1 = tcg_const_tl(imm);
> +                    imm0 = tcg_const_tl(imm);
> +                    tcg_gen_shli_tl(imm3, imm3, 24);
> +                    tcg_gen_shli_tl(imm2, imm2, 16);
> +                    tcg_gen_shli_tl(imm1, imm1, 8);
> +                    tcg_gen_or_tl(temp_rd, temp_rd, imm3);
> +                    tcg_gen_or_tl(temp_rd, temp_rd, imm2);
> +                    tcg_gen_or_tl(temp_rd, temp_rd, imm1);
> +                    tcg_gen_or_tl(temp_rd, temp_rd, imm0);

What's the point of loading 4 times the same value in a TCG register,
and do the shifts in TCG code? Either load it once, and shift it after
each or TCG instruction, or directly load the shifted value. In both
case you can just use a single TCG variable.

> +#if defined(TARGET_MIPS64)
> +                    tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif
> +                    gen_store_gpr(temp_rd, rd);
> +                    tcg_temp_free(temp_rd);
> +                    tcg_temp_free(imm3);
> +                    tcg_temp_free(imm2);
> +                    tcg_temp_free(imm1);
> +                    tcg_temp_free(imm0);
> +                    break;
> +                }
> +            case OPC_REPLV_QB:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv rt3, rt2, rt1, rt0, temp_rd;
> +                    temp_rd = tcg_const_tl(0);
> +
> +                    rt3 = tcg_const_tl(0);
> +                    rt2 = tcg_const_tl(0);
> +                    rt1 = tcg_const_tl(0);
> +                    rt0 = tcg_const_tl(0);

Why loading 0 here...

> +                    tcg_gen_andi_tl(rt3, cpu_gpr[rt], 0xFF);
> +                    tcg_gen_andi_tl(rt2, cpu_gpr[rt], 0xFF);
> +                    tcg_gen_andi_tl(rt1, cpu_gpr[rt], 0xFF);
> +                    tcg_gen_andi_tl(rt0, cpu_gpr[rt], 0xFF);

... and override the value just below?

> +                    tcg_gen_shli_tl(rt3, rt3, 24);
> +                    tcg_gen_shli_tl(rt2, rt2, 16);
> +                    tcg_gen_shli_tl(rt1, rt1, 8);
> +
> +                    tcg_gen_or_tl(temp_rd, temp_rd, rt3);
> +                    tcg_gen_or_tl(temp_rd, temp_rd, rt2);
> +                    tcg_gen_or_tl(temp_rd, temp_rd, rt1);
> +                    tcg_gen_or_tl(temp_rd, temp_rd, rt0);

All of that seems quite complicated. It would be easier to do something like 
that:
    t = tcg_temp_new_tl();
    tcg_gen_ext8u(temp_rd, cpu_gpr[rt]);
    tcg_gen_shli(t, temp_rd, 8);
    tcg_gen_or(temp_rd, temp_rd, t);
    tcg_gen_shli(t, temp_rd, 16);
    tcg_gen_or(temp_rd, temp_rd, t);
    tcg_temp_free(t);

> +#if defined(TARGET_MIPS64)
> +                    tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif
> +                    gen_store_gpr(temp_rd, rd);
> +
> +                    tcg_temp_free(temp_rd);
> +                    tcg_temp_free(rt3);
> +                    tcg_temp_free(rt2);
> +                    tcg_temp_free(rt1);
> +                    tcg_temp_free(rt0);
> +                     break;
> +                }
> +            case OPC_REPL_PH:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv immhi, immlo, temp_rd;
> +                    temp_rd = tcg_const_tl(0);
> +
> +                    imm = (ctx->opcode >> 16) & 0x03FF;
> +                    immhi = tcg_const_tl(imm);
> +                    immlo = tcg_const_tl(imm);
> +                    tcg_gen_shli_tl(immhi, immhi, 16);

Again, don't load immhi for overriding just after.

> +                    tcg_gen_or_tl(temp_rd, immhi, immlo);

What about:

tcg_gen_mov_tl(temp_rd, imm << 16 | imm);

> +#if defined(TARGET_MIPS64)
> +                    tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif

With the proper cast you can even fold that in the mov_tl.

> +                    gen_store_gpr(temp_rd, rd);
> +                    tcg_temp_free(temp_rd);
> +                    tcg_temp_free(immhi);
> +                    tcg_temp_free(immlo);
> +                    break;
> +                }
> +            case OPC_REPLV_PH:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv rt0, rt1, temp_rd;
> +                    temp_rd = tcg_const_tl(0);
> +
> +                    rt0 = tcg_const_tl(0);
> +                    rt1 = tcg_const_tl(0);
> +                    tcg_gen_andi_tl(rt0, cpu_gpr[rt], 0xFFFF);
> +                    tcg_gen_andi_tl(rt1, cpu_gpr[rt], 0xFFFF);
> +                    tcg_gen_shli_tl(rt1, rt1, 16);
> +                    tcg_gen_or_tl(temp_rd, rt1, rt0);
> +#if defined(TARGET_MIPS64)
> +                    tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif

Same kind of issues as above.

> +                    gen_store_gpr(temp_rd, rd);
> +                    tcg_temp_free(temp_rd);
> +                    tcg_temp_free(rt0);
> +                    tcg_temp_free(rt1);
> +                    break;
> +                }
>              default:            /* Invalid */
>                  MIPS_INVAL("MASK ABSQ_S.PH");
>                  generate_exception(ctx, EXCP_RI);
> @@ -13133,6 +13275,22 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx, int *is_branch)
>                  break;
>              }
>              break;
> +        case OPC_INSV_DSP:
> +            op2 = MASK_INSV(ctx->opcode);
> +            switch (op2) {
> +            case OPC_INSV:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    gen_helper_insv(cpu_gpr[rt], cpu_env,
> +                                    cpu_gpr[rs], cpu_gpr[rt]);
> +                    break;
> +                }
> +            default:            /* Invalid */
> +                MIPS_INVAL("MASK INSV");
> +                generate_exception(ctx, EXCP_RI);
> +                break;
> +            }
> +            break;
>  #if defined(TARGET_MIPS64)
>          case OPC_DEXTM ... OPC_DEXT:
>          case OPC_DINSM ... OPC_DINS:
> @@ -13213,6 +13371,119 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx, int *is_branch)
>                  check_insn(env, ctx, ASE_DSP);
>                  gen_helper_preceu_qh_obra(cpu_gpr[rd], cpu_gpr[rt]);
>                  break;
> +            case OPC_REPL_OB:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    int i;
> +                    TCGv_i64 immv, temp_rd;
> +                    temp_rd = tcg_const_i64(0);
> +
> +                    imm = (ctx->opcode >> 16) & 0xFF;
> +                    immv = tcg_const_i64(imm);
> +
> +                    tcg_gen_or_i64(temp_rd, temp_rd, immv);
> +                    for (i = 0; i < 7; i++) {
> +                        tcg_gen_shli_i64(temp_rd, temp_rd, 8);
> +                        tcg_gen_or_i64(temp_rd, temp_rd, immv);
> +                    }
> +                    gen_store_gpr(temp_rd, rd);
> +                    break;
> +                }
> +            case OPC_REPL_PW:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv_i64 temp = tcg_temp_new_i64();
> +
> +                    imm = (ctx->opcode >> 16) & 0x03FF;
> +                    imm = (int16_t)(imm << 6) >> 6;
> +
> +                    tcg_gen_movi_i64(temp,
> +                                     (uint64_t)(uint32_t)(int32_t)imm);
> +                    tcg_gen_shli_i64(temp, temp, 32);
> +                    tcg_gen_ori_i64(temp, temp,
> +                                    (uint64_t)(uint32_t)(int32_t)imm);
> +
> +                    gen_store_gpr(temp, rd);
> +
> +                    tcg_temp_free_i64(temp);
> +                    break;
> +                }
> +            case OPC_REPL_QH:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv_i64 temp = tcg_temp_new_i64();
> +
> +                    imm = (ctx->opcode >> 16) & 0x03FF;
> +                    imm = (int16_t)(imm << 6) >> 6;
> +
> +                    tcg_gen_movi_i64(temp, (uint64_t)(uint16_t)imm);
> +                    tcg_gen_shli_i64(temp, temp, 16);
> +                    tcg_gen_ori_i64(temp, temp, (uint64_t)(uint16_t)imm);
> +                    tcg_gen_shli_i64(temp, temp, 16);
> +                    tcg_gen_ori_i64(temp, temp, (uint64_t)(uint16_t)imm);
> +                    tcg_gen_shli_i64(temp, temp, 16);
> +                    tcg_gen_ori_i64(temp, temp, (uint64_t)(uint16_t)imm);
> +
> +                    gen_store_gpr(temp, rd);
> +
> +                    tcg_temp_free_i64(temp);
> +                    break;
> +                }
> +            case OPC_REPLV_OB:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    int i;
> +                    TCGv_i64 immv, temp_rd;
> +                    immv = tcg_const_i64(0);
> +                    temp_rd = tcg_const_i64(0);
> +
> +                    tcg_gen_ori_i64(immv, cpu_gpr[rt], 0xFF);
> +                    tcg_gen_or_i64(temp_rd, temp_rd, immv);
> +                    for (i = 0; i < 7; i++) {
> +                        tcg_gen_shli_i64(temp_rd, temp_rd, 8);
> +                        tcg_gen_or_i64(temp_rd, temp_rd, immv);
> +                    }
> +                    gen_store_gpr(temp_rd, rd);
> +                    tcg_temp_free_i64(immv);
> +                    tcg_temp_free_i64(temp_rd);
> +                    break;
> +                }
> +            case OPC_REPLV_PW:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv_i64 imm_v;
> +                    imm_v = tcg_const_i64(0);
> +
> +                    tcg_gen_ori_i64(imm_v, cpu_gpr[rt], 0xFFFFFFFF);
> +                    tcg_gen_ext32s_i64(cpu_gpr[rd], imm_v);
> +
> +                    tcg_temp_free_i64(imm_v);
> +                    break;
> +                }
> +            case OPC_REPLV_QH:
> +                check_insn(env, ctx, ASE_DSP);
> +                {
> +                    TCGv_i64 imm_v;
> +                    TCGv_i64 temp_rd;
> +
> +                    imm_v = tcg_const_i64(0);
> +                    temp_rd = tcg_const_i64(0);
> +                    tcg_gen_ori_i64(imm_v, cpu_gpr[rt], 0xffff);
> +
> +                    tcg_gen_or_i64(temp_rd, temp_rd, imm_v);
> +                    tcg_gen_shli_i64(temp_rd, temp_rd, 16);
> +                    tcg_gen_or_i64(temp_rd, temp_rd, imm_v);
> +                    tcg_gen_shli_i64(temp_rd, temp_rd, 16);
> +                    tcg_gen_or_i64(temp_rd, temp_rd, imm_v);
> +                    tcg_gen_shli_i64(temp_rd, temp_rd, 16);
> +                    tcg_gen_or_i64(temp_rd, temp_rd, imm_v);
> +
> +                    gen_store_gpr(temp_rd, rd);
> +
> +                    tcg_temp_free_i64(imm_v);
> +                    tcg_temp_free_i64(temp_rd);
> +                    break;
> +                }
>              case OPC_ABSQ_S_OB:
>                  check_insn(env, ctx, ASE_DSPR2);
>                  gen_helper_absq_s_ob(cpu_gpr[rd], cpu_env, cpu_gpr[rt]);
> @@ -13574,6 +13845,22 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx, int *is_branch)
>              }
>  #endif
>  #if defined(TARGET_MIPS64)
> +        case OPC_DINSV_DSP:
> +            op2 = MASK_INSV(ctx->opcode);
> +            switch (op2) {
> +            case OPC_DINSV:
> +                check_insn(env, ctx, ASE_DSP);
> +                gen_helper_dinsv(cpu_gpr[rt], cpu_env,
> +                                 cpu_gpr[rs], cpu_gpr[rt]);
> +                break;
> +            default:            /* Invalid */
> +                MIPS_INVAL("MASK DINSV");
> +                generate_exception(ctx, EXCP_RI);
> +                break;
> +            }
> +            break;
> +#endif
> +#if defined(TARGET_MIPS64)
>          case OPC_SHLL_OB_DSP:
>              op2 = MASK_SHLL_OB(ctx->opcode);
>              switch (op2) {
> @@ -13748,7 +14035,7 @@ static void decode_opc (CPUMIPSState *env, 
> DisasContext *ctx, int *is_branch)
>                  generate_exception(ctx, EXCP_RI);
>                  break;
>              }
> -          break;
> +            break;
>  #endif
>          default:            /* Invalid */
>              MIPS_INVAL("special3");
> -- 
> 1.7.9.5
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net



reply via email to

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