Message ID | 20201118083044.13992-9-frank.chang@sifive.com |
---|---|
State | New |
Headers | show |
Series | support subsets of bitmanip extension | expand |
On 11/18/20 12:29 AM, frank.chang@sifive.com wrote: > +static void gen_sbop_shamt(TCGv ret, TCGv shamt) > +{ > + tcg_gen_andi_tl(ret, shamt, TARGET_LONG_BITS - 1); > +} > + > +static void gen_sbop_common(TCGv ret, TCGv shamt) > +{ > + TCGv t; > + t = tcg_temp_new(); All of the places where you declare then initialize on the next line, please merge them: TCGv t = tcg_temp_new(); It would be nice to share more code between the normal and *w versions. As it is, there's a *lot* of repetition with only TARGET_LONG_BITS vs 32 separating them. > + tcg_gen_not_tl(mask, mask); > + tcg_gen_and_tl(ret, arg1, mask); andc. r~
On 11/18/20 12:29 AM, frank.chang@sifive.com wrote: > +static bool trans_sbset(DisasContext *ctx, arg_sbset *a) > +{ > + REQUIRE_EXT(ctx, RVB); > + return gen_arith(ctx, a, &gen_sbset); > +} > + > +static bool trans_sbseti(DisasContext *ctx, arg_sbseti *a) > +{ > + REQUIRE_EXT(ctx, RVB); > + return gen_arith_shamt_tl(ctx, a, &gen_sbset); > +} > + > +static bool trans_sbclr(DisasContext *ctx, arg_sbclr *a) > +{ > + REQUIRE_EXT(ctx, RVB); > + return gen_arith(ctx, a, &gen_sbclr); > +} Coming back to my re-use of code thing, these should use gen_shift. That handles the truncate of source2 to the shift amount. > +static bool trans_sbclri(DisasContext *ctx, arg_sbclri *a) > +{ > + REQUIRE_EXT(ctx, RVB); > + return gen_arith_shamt_tl(ctx, a, &gen_sbclr); > +} > + > +static bool trans_sbinv(DisasContext *ctx, arg_sbinv *a) > +{ > + REQUIRE_EXT(ctx, RVB); > + return gen_arith(ctx, a, &gen_sbinv); > +} > + > +static bool trans_sbinvi(DisasContext *ctx, arg_sbinvi *a) > +{ > + REQUIRE_EXT(ctx, RVB); > + return gen_arith_shamt_tl(ctx, a, &gen_sbinv); > +} I think there ought to be a gen_shifti for these. Similarly gen_shiftiw and gen_shiftw, which would truncate and sign-extend the result. Existing code in trans_rvi.c.inc should be converted to use the new functions. r~
On 11/19/20 12:35 PM, Richard Henderson wrote: > On 11/18/20 12:29 AM, frank.chang@sifive.com wrote: >> +static bool trans_sbset(DisasContext *ctx, arg_sbset *a) >> +{ >> + REQUIRE_EXT(ctx, RVB); >> + return gen_arith(ctx, a, &gen_sbset); >> +} >> + >> +static bool trans_sbseti(DisasContext *ctx, arg_sbseti *a) >> +{ >> + REQUIRE_EXT(ctx, RVB); >> + return gen_arith_shamt_tl(ctx, a, &gen_sbset); >> +} >> + >> +static bool trans_sbclr(DisasContext *ctx, arg_sbclr *a) >> +{ >> + REQUIRE_EXT(ctx, RVB); >> + return gen_arith(ctx, a, &gen_sbclr); >> +} > > Coming back to my re-use of code thing, these should use gen_shift. That > handles the truncate of source2 to the shift amount. > >> +static bool trans_sbclri(DisasContext *ctx, arg_sbclri *a) >> +{ >> + REQUIRE_EXT(ctx, RVB); >> + return gen_arith_shamt_tl(ctx, a, &gen_sbclr); >> +} >> + >> +static bool trans_sbinv(DisasContext *ctx, arg_sbinv *a) >> +{ >> + REQUIRE_EXT(ctx, RVB); >> + return gen_arith(ctx, a, &gen_sbinv); >> +} >> + >> +static bool trans_sbinvi(DisasContext *ctx, arg_sbinvi *a) >> +{ >> + REQUIRE_EXT(ctx, RVB); >> + return gen_arith_shamt_tl(ctx, a, &gen_sbinv); >> +} > > I think there ought to be a gen_shifti for these. Hmm. I just realized that gen_shifti would have a generator callback with a constant argument, a-la tcg_gen_shli_tl. I don't know if it's worth duplicating gen_sbclr et al for a constant argument. And the sloi/sroi insns besides. Perhaps a gen_shifti_var helper instead? Let me know what you think, but at the moment we're left with an incoherent set of helpers that seem split along lines that are less than ideal. r~
On Fri, Nov 20, 2020 at 5:04 AM Richard Henderson < richard.henderson@linaro.org> wrote: > On 11/19/20 12:35 PM, Richard Henderson wrote: > > On 11/18/20 12:29 AM, frank.chang@sifive.com wrote: > >> +static bool trans_sbset(DisasContext *ctx, arg_sbset *a) > >> +{ > >> + REQUIRE_EXT(ctx, RVB); > >> + return gen_arith(ctx, a, &gen_sbset); > >> +} > >> + > >> +static bool trans_sbseti(DisasContext *ctx, arg_sbseti *a) > >> +{ > >> + REQUIRE_EXT(ctx, RVB); > >> + return gen_arith_shamt_tl(ctx, a, &gen_sbset); > >> +} > >> + > >> +static bool trans_sbclr(DisasContext *ctx, arg_sbclr *a) > >> +{ > >> + REQUIRE_EXT(ctx, RVB); > >> + return gen_arith(ctx, a, &gen_sbclr); > >> +} > > > > Coming back to my re-use of code thing, these should use gen_shift. That > > handles the truncate of source2 to the shift amount. > > > >> +static bool trans_sbclri(DisasContext *ctx, arg_sbclri *a) > >> +{ > >> + REQUIRE_EXT(ctx, RVB); > >> + return gen_arith_shamt_tl(ctx, a, &gen_sbclr); > >> +} > >> + > >> +static bool trans_sbinv(DisasContext *ctx, arg_sbinv *a) > >> +{ > >> + REQUIRE_EXT(ctx, RVB); > >> + return gen_arith(ctx, a, &gen_sbinv); > >> +} > >> + > >> +static bool trans_sbinvi(DisasContext *ctx, arg_sbinvi *a) > >> +{ > >> + REQUIRE_EXT(ctx, RVB); > >> + return gen_arith_shamt_tl(ctx, a, &gen_sbinv); > >> +} > > > > I think there ought to be a gen_shifti for these. > > Hmm. I just realized that gen_shifti would have a generator callback with > a > constant argument, a-la tcg_gen_shli_tl. > > I don't know if it's worth duplicating gen_sbclr et al for a constant > argument. > And the sloi/sroi insns besides. Perhaps a gen_shifti_var helper instead? > > Let me know what you think, but at the moment we're left with an > incoherent set > of helpers that seem split along lines that are less than ideal. > > > r~ > Thanks Richard and sorry for the late reply..... If we can have gen_shift(), gen_shifti(), gen_shiftw() and gen_shiftiw(), then we can eliminate the needs of: gen_arith_shamt_tl(), gen_sbop_shamt(), gen_sbopw_shamt() and gen_sbopw_common() and most of the *w version generators can be removed, too. For *w version, we just need to call gen_shiftw() or gen_shiftiw() with the reused non-*w version generator. For example: static bool trans_sbclrw(DisasContext *ctx, arg_sbclrw *a) { REQUIRE_EXT(ctx, RVB); return gen_shiftw(ctx, a, &gen_sbclr); } static bool trans_sbclriw(DisasContext *ctx, arg_sbclriw *a) { REQUIRE_EXT(ctx, RVB); return gen_shiftiw(ctx, a, &gen_sbclr); } both of which can reuse gen_sbclr() generator: static void gen_sbclr(TCGv ret, TCGv arg1, TCGv shamt) { TCGv t = tcg_temp_new(); tcg_gen_movi_tl(t, 1); tcg_gen_shl_tl(t, t, shamt); tcg_gen_andc_tl(ret, arg1, t); tcg_temp_free(t); } The gen_shift*() I have now are as follow: static bool gen_shift(DisasContext *ctx, arg_r *a, void(*func)(TCGv, TCGv, TCGv)) { 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, TARGET_LONG_BITS - 1); (*func)(source1, source1, source2); gen_set_gpr(a->rd, source1); tcg_temp_free(source1); tcg_temp_free(source2); return true; } static bool gen_shifti(DisasContext *ctx, arg_shift *a, void(*func)(TCGv, TCGv, TCGv)) { TCGv source1 = tcg_temp_new(); TCGv source2 = tcg_temp_new(); gen_get_gpr(source1, a->rs1); tcg_gen_movi_tl(source2, a->shamt); tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1); (*func)(source1, source1, source2); gen_set_gpr(a->rd, source1); tcg_temp_free(source1); tcg_temp_free(source2); return true; } static bool gen_shiftw(DisasContext *ctx, arg_r *a, void(*func)(TCGv, TCGv, TCGv)) { 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, 31); (*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; } static bool gen_shiftiw(DisasContext *ctx, arg_shift *a, void(*func)(TCGv, TCGv, TCGv)) { TCGv source1 = tcg_temp_new(); TCGv source2 = tcg_temp_new(); gen_get_gpr(source1, a->rs1); tcg_gen_movi_tl(source2, a->shamt); tcg_gen_andi_tl(source2, source2, 31); (*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; } They may be further merged as most of them are duplicate with only the differences of: gen_get_gpr(source2, a->rs2); vs. tcg_gen_movi_tl(source2, a->shamt); TARGET_LONG_BITS - 1 vs. 31, and tcg_gen_ext32s_tl(); to sign-extend the 32-bit return value for *w instructions Any thoughts? Frank Chang
diff --git a/target/riscv/insn32-64.decode b/target/riscv/insn32-64.decode index 2f00f96e36b..92f3aaac3b6 100644 --- a/target/riscv/insn32-64.decode +++ b/target/riscv/insn32-64.decode @@ -94,3 +94,11 @@ pcntw 011000000010 ..... 001 ..... 0011011 @r2 packw 0000100 .......... 100 ..... 0111011 @r packuw 0100100 .......... 100 ..... 0111011 @r +sbsetw 0010100 .......... 001 ..... 0111011 @r +sbclrw 0100100 .......... 001 ..... 0111011 @r +sbinvw 0110100 .......... 001 ..... 0111011 @r +sbextw 0100100 .......... 101 ..... 0111011 @r + +sbsetiw 0010100 .......... 001 ..... 0011011 @sh5 +sbclriw 0100100 .......... 001 ..... 0011011 @sh5 +sbinviw 0110100 .......... 001 ..... 0011011 @sh5 diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode index 626641333c6..69e542da19c 100644 --- a/target/riscv/insn32.decode +++ b/target/riscv/insn32.decode @@ -611,3 +611,12 @@ min 0000101 .......... 100 ..... 0110011 @r minu 0000101 .......... 101 ..... 0110011 @r max 0000101 .......... 110 ..... 0110011 @r maxu 0000101 .......... 111 ..... 0110011 @r +sbset 0010100 .......... 001 ..... 0110011 @r +sbclr 0100100 .......... 001 ..... 0110011 @r +sbinv 0110100 .......... 001 ..... 0110011 @r +sbext 0100100 .......... 101 ..... 0110011 @r + +sbseti 001010 ........... 001 ..... 0010011 @sh +sbclri 010010 ........... 001 ..... 0010011 @sh +sbinvi 011010 ........... 001 ..... 0010011 @sh +sbexti 010010 ........... 101 ..... 0010011 @sh diff --git a/target/riscv/insn_trans/trans_rvb.c.inc b/target/riscv/insn_trans/trans_rvb.c.inc index bf15611f85a..dabf8e09c3d 100644 --- a/target/riscv/insn_trans/trans_rvb.c.inc +++ b/target/riscv/insn_trans/trans_rvb.c.inc @@ -107,6 +107,54 @@ static bool trans_sext_h(DisasContext *ctx, arg_sext_h *a) return gen_unary(ctx, a, &tcg_gen_ext16s_tl); } +static bool trans_sbset(DisasContext *ctx, arg_sbset *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbset); +} + +static bool trans_sbseti(DisasContext *ctx, arg_sbseti *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith_shamt_tl(ctx, a, &gen_sbset); +} + +static bool trans_sbclr(DisasContext *ctx, arg_sbclr *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbclr); +} + +static bool trans_sbclri(DisasContext *ctx, arg_sbclri *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith_shamt_tl(ctx, a, &gen_sbclr); +} + +static bool trans_sbinv(DisasContext *ctx, arg_sbinv *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbinv); +} + +static bool trans_sbinvi(DisasContext *ctx, arg_sbinvi *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith_shamt_tl(ctx, a, &gen_sbinv); +} + +static bool trans_sbext(DisasContext *ctx, arg_sbext *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbext); +} + +static bool trans_sbexti(DisasContext *ctx, arg_sbexti *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith_shamt_tl(ctx, a, &gen_sbext); +} + { /* RV64-only instructions */ #ifdef TARGET_RISCV64 @@ -141,4 +189,46 @@ static bool trans_packuw(DisasContext *ctx, arg_packuw *a) return gen_arith(ctx, a, &gen_packuw); } +static bool trans_sbsetw(DisasContext *ctx, arg_sbsetw *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbsetw); +} + +static bool trans_sbsetiw(DisasContext *ctx, arg_sbsetiw *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith_shamt_tl(ctx, a, &gen_sbsetw); +} + +static bool trans_sbclrw(DisasContext *ctx, arg_sbclrw *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbclrw); +} + +static bool trans_sbclriw(DisasContext *ctx, arg_sbclriw *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith_shamt_tl(ctx, a, &gen_sbclrw); +} + +static bool trans_sbinvw(DisasContext *ctx, arg_sbinvw *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbinvw); +} + +static bool trans_sbinviw(DisasContext *ctx, arg_sbinviw *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith_shamt_tl(ctx, a, &gen_sbinvw); +} + +static bool trans_sbextw(DisasContext *ctx, arg_sbextw *a) +{ + REQUIRE_EXT(ctx, RVB); + return gen_arith(ctx, a, &gen_sbextw); +} + #endif diff --git a/target/riscv/translate.c b/target/riscv/translate.c index fb30ee83aa8..e7d9e4a1abf 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -647,6 +647,24 @@ static bool gen_arith_imm_tl(DisasContext *ctx, arg_i *a, return true; } +static bool gen_arith_shamt_tl(DisasContext *ctx, arg_shift *a, + void (*func)(TCGv, TCGv, TCGv)) +{ + TCGv source1, source2; + source1 = tcg_temp_new(); + source2 = tcg_temp_new(); + + gen_get_gpr(source1, a->rs1); + tcg_gen_movi_tl(source2, a->shamt); + + (*func)(source1, source1, source2); + + gen_set_gpr(a->rd, source1); + tcg_temp_free(source1); + tcg_temp_free(source2); + return true; +} + #ifdef TARGET_RISCV64 static void gen_addw(TCGv ret, TCGv arg1, TCGv arg2) { @@ -801,6 +819,74 @@ static void gen_packh(TCGv ret, TCGv arg1, TCGv arg2) tcg_temp_free(higher); } +static void gen_sbop_shamt(TCGv ret, TCGv shamt) +{ + tcg_gen_andi_tl(ret, shamt, TARGET_LONG_BITS - 1); +} + +static void gen_sbop_common(TCGv ret, TCGv shamt) +{ + TCGv t; + t = tcg_temp_new(); + + gen_sbop_shamt(ret, shamt); + + tcg_gen_movi_tl(t, 1); + tcg_gen_shl_tl(ret, t, ret); + + tcg_temp_free(t); +} + +static void gen_sbset(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv mask; + mask = tcg_temp_new(); + + gen_sbop_common(mask, arg2); + + tcg_gen_or_tl(ret, arg1, mask); + + tcg_temp_free(mask); +} + +static void gen_sbclr(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv mask; + mask = tcg_temp_new(); + + gen_sbop_common(mask, arg2); + + tcg_gen_not_tl(mask, mask); + tcg_gen_and_tl(ret, arg1, mask); + + tcg_temp_free(mask); +} + +static void gen_sbinv(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv mask; + mask = tcg_temp_new(); + + gen_sbop_common(mask, arg2); + + tcg_gen_xor_tl(ret, arg1, mask); + + tcg_temp_free(mask); +} + +static void gen_sbext(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv shamt; + shamt = tcg_temp_new(); + + gen_sbop_shamt(shamt, arg2); + tcg_gen_shr_tl(ret, arg1, shamt); + + tcg_gen_andi_tl(ret, ret, 1); + + tcg_temp_free(shamt); +} + #ifdef TARGET_RISCV64 @@ -867,6 +953,75 @@ static void gen_packuw(TCGv ret, TCGv arg1, TCGv arg2) tcg_temp_free(higher); } +static void gen_sbopw_shamt(TCGv ret, TCGv shamt) +{ + tcg_gen_andi_tl(ret, shamt, 31); +} + +static void gen_sbopw_common(TCGv ret, TCGv shamt) +{ + TCGv t; + t = tcg_temp_new(); + + gen_sbopw_shamt(ret, shamt); + tcg_gen_movi_tl(t, 1); + tcg_gen_shl_tl(ret, t, ret); + + tcg_temp_free(t); +} + +static void gen_sbsetw(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv mask; + mask = tcg_temp_new(); + + gen_sbopw_common(mask, arg2); + tcg_gen_or_tl(ret, arg1, mask); + + tcg_gen_ext32s_tl(ret, ret); + + tcg_temp_free(mask); +} + +static void gen_sbclrw(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv mask; + mask = tcg_temp_new(); + + gen_sbopw_common(mask, arg2); + tcg_gen_not_tl(mask, mask); + tcg_gen_and_tl(ret, arg1, mask); + + tcg_gen_ext32s_tl(ret, ret); + + tcg_temp_free(mask); +} + +static void gen_sbinvw(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv mask; + mask = tcg_temp_new(); + + gen_sbopw_common(mask, arg2); + tcg_gen_xor_tl(ret, arg1, mask); + + tcg_gen_ext32s_tl(ret, ret); + + tcg_temp_free(mask); +} + +static void gen_sbextw(TCGv ret, TCGv arg1, TCGv arg2) +{ + TCGv shamt; + shamt = tcg_temp_new(); + + gen_sbopw_shamt(shamt, arg2); + tcg_gen_shr_tl(ret, arg1, shamt); + tcg_gen_andi_tl(ret, ret, 1); + + tcg_temp_free(shamt); +} + #endif static bool gen_arith(DisasContext *ctx, arg_r *a,