qemu-riscv
[Top][All Lists]
Advanced

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

Re: [Qemu-riscv] [PATCH v3 26/35] target/riscv: Remove shift and slt ins


From: Richard Henderson
Subject: Re: [Qemu-riscv] [PATCH v3 26/35] target/riscv: Remove shift and slt insn manual decoding
Date: Wed, 31 Oct 2018 22:38:08 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1

On 10/31/18 1:20 PM, Bastian Koppelmann wrote:
>  static bool trans_slt(DisasContext *ctx, arg_slt *a)
>  {
> -    gen_arith(ctx, OPC_RISC_SLT, a->rd, a->rs1, a->rs2);
> +    TCGv source1 = tcg_temp_new();
> +    TCGv source2 = tcg_temp_new();
> +
> +    gen_get_gpr(source1, a->rs1);
> +    gen_get_gpr(source2, a->rs2);
> +
> +    tcg_gen_setcond_tl(TCG_COND_LT, source1, source1, source2);

I do wonder about extracting this one line to gen_slt so that you can re-use
gen_arith and gen_arithi.

> +    tcg_gen_setcond_tl(TCG_COND_LTU, source1, source1, source2);

Similarly.

>  static bool trans_sllw(DisasContext *ctx, arg_sllw *a)
>  {
> -    gen_arith(ctx, OPC_RISC_SLLW, a->rd, a->rs1, a->rs2);
> +    TCGv source1 = tcg_temp_new();
> +    TCGv source2 = tcg_temp_new();
> +
> +    gen_get_gpr(source1, a->rs1);
> +    gen_get_gpr(source2, a->rs2);
> +
> +    tcg_gen_andi_tl(source2, source2, 0x1F);
> +    tcg_gen_shl_tl(source1, source1, source2);
> +
> +    gen_set_gpr(a->rd, source1);

Missing the ext32s after the shift.

>  static bool trans_srlw(DisasContext *ctx, arg_srlw *a)
>  {
> -    gen_arith(ctx, OPC_RISC_SRLW, a->rd, a->rs1, a->rs2);
> +    TCGv source1 = tcg_temp_new();
> +    TCGv source2 = tcg_temp_new();
> +
> +    gen_get_gpr(source1, a->rs1);
> +    gen_get_gpr(source2, a->rs2);
> +
> +    /* clear upper 32 */
> +    tcg_gen_ext32u_tl(source1, source1);
> +    tcg_gen_andi_tl(source2, source2, 0x1F);
> +    tcg_gen_shr_tl(source1, source1, source2);

Likewise.  (Consider source2 == 0.)

> -    case OPC_RISC_SRL:
> -        tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
> -        tcg_gen_shr_tl(source1, source1, source2);
> -        break;
...
> -    case OPC_RISC_SRA:
> -        tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
> -        tcg_gen_sar_tl(source1, source1, source2);
> -        break;

I see that the bugs are in the original though, so fixing them in a separate
patch is certainly ok.


r~



reply via email to

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