Skip to content

Commit

Permalink
target/riscv: Zero extend the inputs of divuw and remuw
Browse files Browse the repository at this point in the history
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.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
  • Loading branch information
palmer-dabbelt committed Mar 22, 2019
1 parent 62a172e commit f17e02c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
4 changes: 2 additions & 2 deletions target/riscv/insn_trans/trans_rvm.inc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
21 changes: 21 additions & 0 deletions target/riscv/translate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit f17e02c

Please sign in to comment.