Re: [Qemu-devel] [PATCH 7/7] target-mips: implement DSP (d)append sub-class with TCG

[Johnson, Eric]

> -----Original Message-----
> From: address@hidden [mailto:qemu-devel-
> address@hidden On Behalf Of Aurelien Jarno
> Sent: Friday, November 16, 2012 3:04 AM
> To: address@hidden
> Cc: Aurelien Jarno
> Subject: [Qemu-devel] [PATCH 7/7] target-mips: implement DSP (d)append
> sub-class with TCG
> 
> DSP instruction from the (d)append sub-class can be implemented with
> TCG. Use a different function for these instructions are they are quite
> different from compare-pick sub-class.
> 
> Fix BALIGN instruction for negative value, where the value should be
> zero-extended before being shift to the right.
> 
> Signed-off-by: Aurelien Jarno <address@hidden>
> ---
>  target-mips/dsp_helper.c |   67 -----------------------
>  target-mips/helper.h     |   13 -----
>  target-mips/translate.c  |  133 ++++++++++++++++++++++++++++++-----------
> -----
>  3 files changed, 87 insertions(+), 126 deletions(-)
> 
> diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
> index 474c249..22bbfa1 100644
> --- a/target-mips/dsp_helper.c
> +++ b/target-mips/dsp_helper.c
> @@ -3140,73 +3140,6 @@ PICK_INSN(pick_pw, 2, MIPSDSP_LLO, 32, 0);
>  #endif
>  #undef PICK_INSN
> 
> -#define APPEND_INSN(name, ret_32) \
> -target_ulong helper_##name(target_ulong rt, target_ulong rs, uint32_t sa)
> \
> -{
> \
> -    target_ulong temp;
> \
> -
> \
> -    if (ret_32) {
> \
> -        temp = ((rt & MIPSDSP_LLO) << sa) |
> \
> -               ((rs & MIPSDSP_LLO) & ((0x01 << sa) - 1));
> \
> -        temp = (target_long)(int32_t)(temp & MIPSDSP_LLO);
> \
> -    } else {
> \
> -        temp = (rt << sa) | (rs & ((0x01 << sa) - 1));
> \
> -    }
> \
> -
> \
> -    return temp;
> \
> -}
> -
> -APPEND_INSN(append, 1);
> -#ifdef TARGET_MIPS64
> -APPEND_INSN(dappend, 0);
> -#endif
> -#undef APPEND_INSN
> -
> -#define PREPEND_INSN(name, or_val, ret_32)                    \
> -target_ulong helper_##name(target_ulong rs, target_ulong rt,  \
> -                           uint32_t sa)                       \
> -{                                                             \
> -    sa |= or_val;                                             \
> -                                                              \
> -    if (1) {                                                  \
> -        return (target_long)(int32_t)(uint32_t)               \
> -            (((rs & MIPSDSP_LLO) << (32 - sa)) |              \
> -             ((rt & MIPSDSP_LLO) >> sa));                     \
> -    } else {                                                  \
> -        return (rs << (64 - sa)) | (rt >> sa);                \
> -    }                                                         \
> -}
> -
> -PREPEND_INSN(prepend, 0, 1);
> -#ifdef TARGET_MIPS64
> -PREPEND_INSN(prependw, 0, 0);
> -PREPEND_INSN(prependd, 0x20, 0);
> -#endif
> -#undef PREPEND_INSN
> -
> -#define BALIGN_INSN(name, filter, ret32) \
> -target_ulong helper_##name(target_ulong rs, target_ulong rt, uint32_t bp)
> \
> -{
> \
> -    bp = bp & 0x03;
> \
> -
> \
> -    if ((bp & 1) == 0) {
> \
> -        return rt;
> \
> -    } else {
> \
> -        if (ret32) {
> \
> -            return (target_long)(int32_t)((rt << (8 * bp)) |
> \
> -                                          (rs >> (8 * (4 - bp))));
> \
> -        } else {
> \
> -            return (rt << (8 * bp)) | (rs >> (8 * (8 - bp)));
> \
> -        }
> \
> -    }
> \
> -}
> -
> -BALIGN_INSN(balign, 0x03, 1);
> -#if defined(TARGET_MIPS64)
> -BALIGN_INSN(dbalign, 0x07, 0);
> -#endif
> -#undef BALIGN_INSN
> -
>  target_ulong helper_packrl_ph(target_ulong rs, target_ulong rt)
>  {
>      uint32_t rsl, rth;
> diff --git a/target-mips/helper.h b/target-mips/helper.h
> index acf9ebd..4373ac5 100644
> --- a/target-mips/helper.h
> +++ b/target-mips/helper.h
> @@ -654,19 +654,6 @@ DEF_HELPER_FLAGS_3(pick_ob, 0, tl, tl, tl, env)
>  DEF_HELPER_FLAGS_3(pick_qh, 0, tl, tl, tl, env)
>  DEF_HELPER_FLAGS_3(pick_pw, 0, tl, tl, tl, env)
>  #endif
> -DEF_HELPER_FLAGS_3(append, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#if defined(TARGET_MIPS64)
> -DEF_HELPER_FLAGS_3(dappend, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#endif
> -DEF_HELPER_FLAGS_3(prepend, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#if defined(TARGET_MIPS64)
> -DEF_HELPER_FLAGS_3(prependd, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -DEF_HELPER_FLAGS_3(prependw, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#endif
> -DEF_HELPER_FLAGS_3(balign, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#if defined(TARGET_MIPS64)
> -DEF_HELPER_FLAGS_3(dbalign, TCG_CALL_NO_RWG_SE, tl, tl, tl, i32)
> -#endif
>  DEF_HELPER_FLAGS_2(packrl_ph, TCG_CALL_NO_RWG_SE, tl, tl, tl)
>  #if defined(TARGET_MIPS64)
>  DEF_HELPER_FLAGS_2(packrl_pw, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index 910dd16..624d5a5 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -336,7 +336,7 @@ enum {
>      /* DSP Bit/Manipulation Sub-class */
>      OPC_INSV_DSP       = 0x0C | OPC_SPECIAL3,
>      OPC_DINSV_DSP      = 0x0D | OPC_SPECIAL3,
> -    /* MIPS DSP Compare-Pick Sub-class */
> +    /* MIPS DSP Append Sub-class */
>      OPC_APPEND_DSP     = 0x31 | OPC_SPECIAL3,
>      OPC_DAPPEND_DSP    = 0x35 | OPC_SPECIAL3,
>      /* MIPS DSP Accumulator and DSPControl Access Sub-class */
> @@ -543,7 +543,7 @@ enum {
> 
>  #define MASK_APPEND(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6)))
>  enum {
> -    /* MIPS DSP Compare-Pick Sub-class */
> +    /* MIPS DSP Append Sub-class */
>      OPC_APPEND  = (0x00 << 6) | OPC_APPEND_DSP,
>      OPC_PREPEND = (0x01 << 6) | OPC_APPEND_DSP,
>      OPC_BALIGN  = (0x10 << 6) | OPC_APPEND_DSP,
> @@ -667,7 +667,7 @@ enum {
> 
>  #define MASK_DAPPEND(op) (MASK_SPECIAL3(op) | (op & (0x1F << 6)))
>  enum {
> -    /* DSP Compare-Pick Sub-class */
> +    /* DSP Append Sub-class */
>      OPC_DAPPEND  = (0x00 << 6) | OPC_DAPPEND_DSP,
>      OPC_PREPENDD = (0x03 << 6) | OPC_DAPPEND_DSP,
>      OPC_PREPENDW = (0x01 << 6) | OPC_DAPPEND_DSP,
> @@ -13844,7 +13844,6 @@ static void gen_mipsdsp_add_cmp_pick(CPUMIPSState
> *env, DisasContext *ctx,
>                                       int ret, int v1, int v2, int
> check_ret)
>  {
>      const char *opn = "mipsdsp add compare pick";
> -    TCGv_i32 t0;
>      TCGv t1;
>      TCGv v1_t;
>      TCGv v2_t;
> @@ -13855,7 +13854,6 @@ static void gen_mipsdsp_add_cmp_pick(CPUMIPSState
> *env, DisasContext *ctx,
>          return;
>      }
> 
> -    t0 = tcg_temp_new_i32();
>      t1 = tcg_temp_new();
>      v1_t = tcg_temp_new();
>      v2_t = tcg_temp_new();
> @@ -13864,26 +13862,6 @@ static void gen_mipsdsp_add_cmp_pick(CPUMIPSState
> *env, DisasContext *ctx,
>      gen_load_gpr(v2_t, v2);
> 
>      switch (op1) {
> -    case OPC_APPEND_DSP:
> -        switch (op2) {
> -        case OPC_APPEND:
> -            tcg_gen_movi_i32(t0, v2);
> -            gen_helper_append(cpu_gpr[ret], cpu_gpr[ret], v1_t, t0);
> -            break;
> -        case OPC_PREPEND:
> -            tcg_gen_movi_i32(t0, v2);
> -            gen_helper_prepend(cpu_gpr[ret], v1_t, cpu_gpr[ret], t0);
> -            break;
> -        case OPC_BALIGN:
> -            tcg_gen_movi_i32(t0, v2);
> -            gen_helper_balign(cpu_gpr[ret], v1_t, cpu_gpr[ret], t0);
> -            break;
> -        default:            /* Invid */
> -            MIPS_INVAL("MASK APPEND");
> -            generate_exception(ctx, EXCP_RI);
> -            break;
> -        }
> -        break;
>      case OPC_CMPU_EQ_QB_DSP:
>          switch (op2) {
>          case OPC_CMPU_EQ_QB:
> @@ -14041,23 +14019,95 @@ static void
> gen_mipsdsp_add_cmp_pick(CPUMIPSState *env, DisasContext *ctx,
>              break;
>          }
>          break;
> +#endif
> +    }
> +
> +    tcg_temp_free(t1);
> +    tcg_temp_free(v1_t);
> +    tcg_temp_free(v2_t);
> +
> +    (void)opn; /* avoid a compiler warning */
> +    MIPS_DEBUG("%s", opn);
> +}
> +
> +static void gen_mipsdsp_append(CPUMIPSState *env, DisasContext *ctx,
> +                               uint32_t op1, int rt, int rs, int sa)
> +{
> +    const char *opn = "mipsdsp append/dappend";
> +    TCGv t0;
> +
> +    check_dspr2(env, ctx);
> +
> +    if (rt == 0) {
> +        /* Treat as NOP. */
> +        MIPS_DEBUG("NOP");
> +        return;
> +    }
> +
> +    t0 = tcg_temp_new();
> +    gen_load_gpr(t0, rs);
> +
> +    switch (op1) {
> +    case OPC_APPEND_DSP:
> +        switch (MASK_APPEND(ctx->opcode)) {
> +        case OPC_APPEND:
> +            if (sa != 0) {
> +                tcg_gen_deposit_tl(cpu_gpr[rt], t0, cpu_gpr[rt], sa, 32 -
> sa);
> +            }
> +            tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]);
> +            break;
> +        case OPC_PREPEND:
> +            if (sa != 0) {
> +                tcg_gen_ext32u_tl(cpu_gpr[rt], cpu_gpr[rt]);
> +                tcg_gen_shri_tl(cpu_gpr[rt], cpu_gpr[rt], sa);
> +                tcg_gen_shli_tl(t0, t0, 32 - sa);
> +                tcg_gen_or_tl(cpu_gpr[rt], cpu_gpr[rt], t0);
> +            }
> +            tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]);
> +            break;
> +        case OPC_BALIGN:
> +            sa &= 3;
> +            if (sa != 0 && sa != 2) {
> +                tcg_gen_shli_tl(cpu_gpr[rt], cpu_gpr[rt], 8 * sa);
> +                tcg_gen_ext32u_tl(t0, t0);
> +                tcg_gen_shri_tl(t0, t0, 8 * (4 - sa));
> +                tcg_gen_or_tl(cpu_gpr[rt], cpu_gpr[rt], t0);
> +            }
> +            tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]);
> +            break;
> +        default:            /* Invalid */
> +            MIPS_INVAL("MASK APPEND");
> +            generate_exception(ctx, EXCP_RI);
> +            break;
> +        }
> +        break;
> +#ifdef TARGET_MIPS64
>      case OPC_DAPPEND_DSP:
> -        switch (op2) {
> +        switch (MASK_DAPPEND(ctx->opcode)) {
>          case OPC_DAPPEND:
> -            tcg_gen_movi_i32(t0, v2);
> -            gen_helper_dappend(cpu_gpr[ret], v1_t, cpu_gpr[ret], t0);
> +            if (sa != 0) {
> +                tcg_gen_deposit_tl(cpu_gpr[rt], t0, cpu_gpr[rt], sa, 64 -
> sa);
> +            }
>              break;
>          case OPC_PREPENDD:
> -            tcg_gen_movi_i32(t0, v2);
> -            gen_helper_prependd(cpu_gpr[ret], v1_t, cpu_gpr[ret], t0);
> +            tcg_gen_shri_tl(cpu_gpr[rt], cpu_gpr[rt], 0x20 | sa);
> +            tcg_gen_shli_tl(t0, t0, 64 - (0x20 | sa));
> +            tcg_gen_or_tl(cpu_gpr[rt], t0, t0);
>              break;
>          case OPC_PREPENDW:
> -            tcg_gen_movi_i32(t0, v2);
> -            gen_helper_prependw(cpu_gpr[ret], v1_t, cpu_gpr[ret], t0);
> +            if (sa != 0) {
> +                tcg_gen_shri_tl(cpu_gpr[rt], cpu_gpr[rt], sa);
> +                tcg_gen_shli_tl(t0, t0, 64 - sa);
> +                tcg_gen_or_tl(cpu_gpr[rt], cpu_gpr[rt], t0);
> +            }
>              break;
>          case OPC_DBALIGN:
> -            tcg_gen_movi_i32(t0, v2);
> -            gen_helper_dbalign(cpu_gpr[ret], v1_t, cpu_gpr[ret], t0);
> +            sa &= 7;
> +            if (sa != 0 && sa != 2 && sa != 4) {
> +                tcg_gen_shli_tl(cpu_gpr[rt], cpu_gpr[rt], 8 * sa);
> +                tcg_gen_shri_tl(t0, t0, 8 * (8 - sa));
> +                tcg_gen_or_tl(cpu_gpr[rt], cpu_gpr[rt], t0);
> +            }
>              break;
>          default:            /* Invalid */
>              MIPS_INVAL("MASK DAPPEND");
> @@ -14067,12 +14117,7 @@ static void gen_mipsdsp_add_cmp_pick(CPUMIPSState
> *env, DisasContext *ctx,
>          break;
>  #endif
>      }
> -
> -    tcg_temp_free_i32(t0);
> -    tcg_temp_free(t1);
> -    tcg_temp_free(v1_t);
> -    tcg_temp_free(v2_t);
> -
> +    tcg_temp_free(t0);
>      (void)opn; /* avoid a compiler warning */
>      MIPS_DEBUG("%s", opn);
>  }
> @@ -14892,9 +14937,7 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
>              }
>              break;
>          case OPC_APPEND_DSP:
> -            check_dspr2(env, ctx);
> -            op2 = MASK_APPEND(ctx->opcode);
> -            gen_mipsdsp_add_cmp_pick(env, ctx, op1, op2, rt, rs, rd, 1);
> +            gen_mipsdsp_append(env, ctx, op1, rt, rs, rd);
>              break;
>          case OPC_EXTR_W_DSP:
>              op2 = MASK_EXTR_W(ctx->opcode);
> @@ -15068,9 +15111,7 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
>              }
>              break;
>          case OPC_DAPPEND_DSP:
> -            check_dspr2(ctx);
> -            op2 = MASK_DAPPEND(ctx->opcode);
> -            gen_mipsdsp_add_cmp_pick(ctx, op1, op2, rt, rs, rd, 1);
> +            gen_mipsdsp_append(env, ctx, op1, rt, rs, rd);
>              break;
>          case OPC_DEXTR_W_DSP:
>              op2 = MASK_DEXTR_W(ctx->opcode);
> --
> 1.7.10.4
> 

Reviewed-by: Eric Johnson <address@hidden>