Re: [Qemu-devel] [PATCH] target/riscv: Zero extend the inputs of divuw and remuw

[Kito Cheng]

Verified with gcc testsuite on rv64gc, no new regression introduced, and
get less fails.

Palmer Dabbelt <address@hidden>於 2019年3月21日 週四，22:59寫道：

> While running the GCC test suite against 4.0.0-rc0, Kito found a
> regression introduced by the decodetree conversion that caused divuw and
> remuw to sign-extend their inputs.  The ISA manual says they are
> supposed to be zero extended:
>
>     DIVW and DIVUW instructions are only valid for RV64, and divide the
>     lower 32 bits of rs1 by the lower 32 bits of rs2, treating them as
>     signed and unsigned integers respectively, placing the 32-bit
>     quotient in rd, sign-extended to 64 bits. REMW and REMUW
>     instructions are only valid for RV64, and provide the corresponding
>     signed and unsigned remainder operations respectively.  Both REMW
>     and REMUW always sign-extend the 32-bit result to 64 bits, including
>     on a divide by zero.
>
> Here's Kito's reduced test case from the GCC test suite
>
>     unsigned calc_mp(unsigned mod)
>     {
>          unsigned a,b,c;
>          c=-1;
>          a=c/mod;
>          b=0-a*mod;
>          if (b > mod) { a += 1; b-=mod; }
>          return b;
>     }
>
>     int main(int argc, char *argv[])
>     {
>          unsigned x = 1234;
>          unsigned y = calc_mp(x);
>
>          if ((sizeof (y) == 4 && y != 680)
>       || (sizeof (y) == 2 && y != 134))
>     abort ();
>          exit (0);
>     }
>
> I haven't done any other testing on this, but it does fix the test case.
>
> Signed-off-by: Palmer Dabbelt <address@hidden>
> ---
>  target/riscv/insn_trans/trans_rvm.inc.c |  4 ++--
>  target/riscv/translate.c                | 21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvm.inc.c
> b/target/riscv/insn_trans/trans_rvm.inc.c
> index 204af225f8f3..47cd6edc72a1 100644
> --- a/target/riscv/insn_trans/trans_rvm.inc.c
> +++ b/target/riscv/insn_trans/trans_rvm.inc.c
> @@ -103,7 +103,7 @@ static bool trans_divw(DisasContext *ctx, arg_divw *a)
>  static bool trans_divuw(DisasContext *ctx, arg_divuw *a)
>  {
>      REQUIRE_EXT(ctx, RVM);
> -    return gen_arith_div_w(ctx, a, &gen_divu);
> +    return gen_arith_div_uw(ctx, a, &gen_divu);
>  }
>
>  static bool trans_remw(DisasContext *ctx, arg_remw *a)
> @@ -115,6 +115,6 @@ static bool trans_remw(DisasContext *ctx, arg_remw *a)
>  static bool trans_remuw(DisasContext *ctx, arg_remuw *a)
>  {
>      REQUIRE_EXT(ctx, RVM);
> -    return gen_arith_div_w(ctx, a, &gen_remu);
> +    return gen_arith_div_uw(ctx, a, &gen_remu);
>  }
>  #endif
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 049fa65c6611..dd763647ea55 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -600,6 +600,27 @@ static bool gen_arith_div_w(DisasContext *ctx, arg_r
> *a,
>      return true;
>  }
>
> +static bool gen_arith_div_uw(DisasContext *ctx, arg_r *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_ext32u_tl(source1, source1);
> +    tcg_gen_ext32u_tl(source2, source2);
> +
> +    (*func)(source1, source1, source2);
> +
> +    tcg_gen_ext32s_tl(source1, source1);
> +    gen_set_gpr(a->rd, source1);
> +    tcg_temp_free(source1);
> +    tcg_temp_free(source2);
> +    return true;
> +}
> +
>  #endif
>
>  static bool gen_arith(DisasContext *ctx, arg_r *a,
> --
> 2.19.2
>
>