Re: [Qemu-devel] [PATCH v2 26/29] target/riscv: Remove manual decoding of RV32/64M insn

[Richard Henderson]

On 10/20/18 8:14 AM, Bastian Koppelmann wrote:
>  static bool trans_mulh(DisasContext *ctx, arg_mulh *a, uint32_t insn)
>  {
> -    gen_arith(ctx, OPC_RISC_MULH, 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_muls2_tl(source2, source1, source1, source2);

If you wanted, you could probably put this line into a function to keep using
trans_arith.  But this is ok too.

>  static bool trans_mulw(DisasContext *ctx, arg_mulw *a, uint32_t insn)
>  {
> -    gen_arith(ctx, OPC_RISC_MULW, a->rd, a->rs1, a->rs2);
> -    return true;
> +#ifdef TARGET_RISCV64
> +    return trans_arith(ctx, a, &tcg_gen_mul_tl);
> +#else
> +    return false;
> +#endif

This is wrong; you need a final sign-extend.

> +#ifdef TARGET_RISCV64
> +static bool gen_arith_w(DisasContext *ctx, arg_arith *a,
> +                        void(*func)(TCGv, TCGv, TCGv))
> +{
> +    TCGv source1, source2;
> +    source1 = tcg_temp_new();
> +    source2 = tcg_temp_new();
> +
> +    gen_get_gpr(source1, a->rs1);
> +    gen_get_gpr(source2, a->rs2);
> +    tcg_gen_ext32s_tl(source1, source1);
> +    tcg_gen_ext32s_tl(source2, source2);
> +
> +    (*func)(source1, source1, source2);
> +
> +    gen_set_gpr(a->rd, source1);
> +    tcg_temp_free(source1);
> +    tcg_temp_free(source2);
> +    return true;
> +}
> +#endif

This appears to have been extracted only from, and is only applicable to, the
word division routines.  It probably should be renamed lest someone think that
it could be used for addw and mulw, which require the output be extended.


r~