target/riscv/insn_trans/trans_rvm.inc.c | 4 ++-- target/riscv/translate.c | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-)
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 <palmer@sifive.com>
---
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
On 3/21/19 7:59 AM, Palmer Dabbelt wrote:
> 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 <palmer@sifive.com>
> ---
> target/riscv/insn_trans/trans_rvm.inc.c | 4 ++--
> target/riscv/translate.c | 21 +++++++++++++++++++++
> 2 files changed, 23 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
Verified with gcc testsuite on rv64gc, no new regression introduced, and
get less fails.
Palmer Dabbelt <palmer@sifive.com>於 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 <palmer@sifive.com>
> ---
> 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
>
>
© 2016 - 2024 Red Hat, Inc.