[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 09/14] target-mips-ase-dsp: Add bit/manipulat
|
From: |
Aurelien Jarno |
|
Subject: |
Re: [Qemu-devel] [PATCH v7 09/14] target-mips-ase-dsp: Add bit/manipulation instructions |
|
Date: |
Thu, 6 Sep 2012 11:11:30 +0200 |
|
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Aug 28, 2012 at 02:36:20PM +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 | 268
> +++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 353 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;
> + }
The manual says that in case the values are not sign extended, the
result is unpredictable. It doesn't mean you have to catch this case, it
means that as long as QEMU doesn't crash whatever result it returns is
fine. The lsb > msb test should be kept though, as otherwise negative
shift values might cause problem.
> + 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;
> +
I don't think this is correct, though I haven't checked in details.
> + 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 88c20cb..b452e3b 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 */
> @@ -12674,6 +12706,95 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
> check_dsp(ctx);
> gen_helper_preceu_ph_qbra(cpu_gpr[rd], cpu_gpr[rt]);
> break;
> + case OPC_BITREV:
> + check_dsp(ctx);
> + gen_helper_bitrev(cpu_gpr[rd], cpu_gpr[rt]);
> + break;
> + case OPC_REPL_QB:
> + check_dsp(ctx);
> + {
> + TCGv t, temp_rd;
> +
> + t = tcg_temp_new();
> + temp_rd = tcg_temp_new();
> +
> + imm = (ctx->opcode >> 16) & 0xFF;
> + t = tcg_const_tl(imm);
> + tcg_gen_mov_tl(temp_rd, t);
> + tcg_gen_shli_tl(t, temp_rd, 8);
> + tcg_gen_or_tl(temp_rd, temp_rd, t);
> + tcg_gen_shli_tl(t, temp_rd, 16);
> + tcg_gen_or_tl(temp_rd, temp_rd, t);
> + tcg_gen_shli_tl(t, temp_rd, 24);
> + tcg_gen_or_tl(temp_rd, temp_rd, t);
> +#if defined(TARGET_MIPS64)
> + tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif
> + tcg_gen_mov_tl(cpu_gpr[rd], temp_rd);
> + tcg_temp_free(t);
> + tcg_temp_free(temp_rd);
> + break;
> + }
The value to load from registers can be computed directly from imm. You
don't need TCG for that. The only TCG instruction which is need should
be this one:
tcg_gen_movi_tl(cpu_gpr[rd], value);
> + case OPC_REPLV_QB:
> + check_dsp(ctx);
> + {
> + TCGv t, temp_rd;
> +
> + t = tcg_temp_new();
> + temp_rd = tcg_temp_new();
> +
> + tcg_gen_ext8u_tl(temp_rd, cpu_gpr[rt]);
> + tcg_gen_shli_tl(t, temp_rd, 8);
> + tcg_gen_or_tl(temp_rd, temp_rd, t);
> + tcg_gen_shli_tl(t, temp_rd, 16);
> + tcg_gen_or_tl(temp_rd, temp_rd, t);
> + tcg_gen_shli_tl(t, temp_rd, 24);
> + tcg_gen_or_tl(temp_rd, temp_rd, t);
> +#if defined(TARGET_MIPS64)
> + tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif
> + tcg_gen_mov_tl(cpu_gpr[rd], temp_rd);
> +
> + tcg_temp_free(t);
> + tcg_temp_free(temp_rd);
> + break;
> + }
Ditto.
> + case OPC_REPL_PH:
> + check_dsp(ctx);
> + {
> + TCGv temp_rd;
> +
> + temp_rd = tcg_temp_new();
> +
> + imm = (ctx->opcode >> 16) & 0x03FF;
> + tcg_gen_movi_tl(temp_rd, imm << 16 | imm);
> +#if defined(TARGET_MIPS64)
> + tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif
> + tcg_gen_mov_tl(cpu_gpr[rd], temp_rd);
> + tcg_temp_free(temp_rd);
> + break;
> + }
This is better than above, still you should no need the ext32s.
> + case OPC_REPLV_PH:
> + check_dsp(ctx);
> + {
> + TCGv t, temp_rd;
> +
> + t = tcg_temp_new();
> + temp_rd = tcg_temp_new();
> +
> + tcg_gen_ext16u_tl(temp_rd, cpu_gpr[rt]);
> + tcg_gen_shli_tl(t, temp_rd, 16);
> + tcg_gen_or_tl(temp_rd, temp_rd, t);
> +#if defined(TARGET_MIPS64)
> + tcg_gen_ext32s_i64(temp_rd, temp_rd);
> +#endif
> + tcg_gen_mov_tl(cpu_gpr[rd], temp_rd);
> +
> + tcg_temp_free(t);
> + tcg_temp_free(temp_rd);
> + break;
> + }
Same.
> default: /* Invalid */
> MIPS_INVAL("MASK ABSQ_S.PH");
> generate_exception(ctx, EXCP_RI);
> @@ -13174,6 +13295,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_dsp(ctx);
> + {
> + 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:
> @@ -13254,6 +13391,119 @@ static void decode_opc (CPUMIPSState *env,
> DisasContext *ctx, int *is_branch)
> check_dsp(ctx);
> gen_helper_preceu_qh_obra(cpu_gpr[rd], cpu_gpr[rt]);
> break;
> + case OPC_REPL_OB:
> + check_dsp(ctx);
> + {
> + 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;
> + }
Same, the value can be computed without TCG.
> + case OPC_REPL_PW:
> + check_dsp(ctx);
> + {
> + 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;
Same.
> + }
> + case OPC_REPL_QH:
> + check_dsp(ctx);
> + {
> + 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;
> + }
Same
> + case OPC_REPLV_OB:
> + check_dsp(ctx);
> + {
> + 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);
I doubt you want an ori here. What you probably need is an ext8u.
> + 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);
> + }
There are better way to do that, you can create a 16-bit value from 2
8-bit values, and the 32-bit values from the 2 16-bit values.
> + gen_store_gpr(temp_rd, rd);
> + tcg_temp_free_i64(immv);
> + tcg_temp_free_i64(temp_rd);
> + break;
> + }
Also even if it is MIPS64 specific, please use _tl instead of _i64
> + 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);
> +
This looks wrong.
> + 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);
This can be optimized as explained above.
> +
> + 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_dspr2(ctx);
> gen_helper_absq_s_ob(cpu_gpr[rd], cpu_env, cpu_gpr[rt]);
> @@ -13615,6 +13865,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_dsp(ctx);
> + 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) {
> @@ -13789,7 +14055,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
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v7 09/14] target-mips-ase-dsp: Add bit/manipulation instructions,
Aurelien Jarno <=