Re: [PATCH v7 17/18] target/riscv: modification of the trans_csrxx for 128-bit support

[Alistair Francis]

On Tue, Dec 14, 2021 at 2:48 AM Frédéric Pétrot
 wrote:
>
> As opposed to the gen_arith and gen_shift generation helpers, the csr insns
> do not have a common prototype, so the choice to generate 32/64 or 128-bit
> helper calls is done in the trans_csrxx functions.
>
> Signed-off-by: Frédéric Pétrot 
> Co-authored-by: Fabien Portas 
> Reviewed-by: Richard Henderson 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvi.c.inc | 205 ++--
>  1 file changed, 160 insertions(+), 45 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index ca354130ec..3a0ae28fef 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -881,20 +881,78 @@ static bool do_csrrw(DisasContext *ctx, int rd, int rc, 
> TCGv src, TCGv mask)
>  return do_csr_post(ctx);
>  }
>
> +static bool do_csrr_i128(DisasContext *ctx, int rd, int rc)
> +{
> +TCGv destl = dest_gpr(ctx, rd);
> +TCGv desth = dest_gprh(ctx, rd);
> +TCGv_i32 csr = tcg_constant_i32(rc);
> +
> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +gen_io_start();
> +}
> +gen_helper_csrr_i128(destl, cpu_env, csr);
> +tcg_gen_ld_tl(desth, cpu_env, offsetof(CPURISCVState, retxh));
> +gen_set_gpr128(ctx, rd, destl, desth);
> +return do_csr_post(ctx);
> +}
> +
> +static bool do_csrw_i128(DisasContext *ctx, int rc, TCGv srcl, TCGv srch)
> +{
> +TCGv_i32 csr = tcg_constant_i32(rc);
> +
> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +gen_io_start();
> +}
> +gen_helper_csrw_i128(cpu_env, csr, srcl, srch);
> +return do_csr_post(ctx);
> +}
> +
> +static bool do_csrrw_i128(DisasContext *ctx, int rd, int rc,
> +  TCGv srcl, TCGv srch, TCGv maskl, TCGv maskh)
> +{
> +TCGv destl = dest_gpr(ctx, rd);
> +TCGv desth = dest_gprh(ctx, rd);
> +TCGv_i32 csr = tcg_constant_i32(rc);
> +
> +if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> +gen_io_start();
> +}
> +gen_helper_csrrw_i128(destl, cpu_env, csr, srcl, srch, maskl, maskh);
> +tcg_gen_ld_tl(desth, cpu_env, offsetof(CPURISCVState, retxh));
> +gen_set_gpr128(ctx, rd, destl, desth);
> +return do_csr_post(ctx);
> +}
> +
>  static bool trans_csrrw(DisasContext *ctx, arg_csrrw *a)
>  {
> -TCGv src = get_gpr(ctx, a->rs1, EXT_NONE);
> -
> -/*
> - * If rd == 0, the insn shall not read the csr, nor cause any of the
> - * side effects that might occur on a csr read.
> - */
> -if (a->rd == 0) {
> -return do_csrw(ctx, a->csr, src);
> +if (get_xl(ctx) < MXL_RV128) {
> +TCGv src = get_gpr(ctx, a->rs1, EXT_NONE);
> +
> +/*
> + * If rd == 0, the insn shall not read the csr, nor cause any of the
> + * side effects that might occur on a csr read.
> + */
> +if (a->rd == 0) {
> +return do_csrw(ctx, a->csr, src);
> +}
> +
> +TCGv mask = tcg_constant_tl(-1);
> +return do_csrrw(ctx, a->rd, a->csr, src, mask);
> +} else {
> +TCGv srcl = get_gpr(ctx, a->rs1, EXT_NONE);
> +TCGv srch = get_gprh(ctx, a->rs1);
> +
> +/*
> + * If rd == 0, the insn shall not read the csr, nor cause any of the
> + * side effects that might occur on a csr read.
> + */
> +if (a->rd == 0) {
> +return do_csrw_i128(ctx, a->csr, srcl, srch);
> +}
> +
> +TCGv mask = tcg_constant_tl(-1);
> +return do_csrrw_i128(ctx, a->rd, a->csr, srcl, srch, mask, mask);
>  }
> -
> -TCGv mask = tcg_constant_tl(-1);
> -return do_csrrw(ctx, a->rd, a->csr, src, mask);
>  }
>
>  static bool trans_csrrs(DisasContext *ctx, arg_csrrs *a)
> @@ -906,13 +964,24 @@ static bool trans_csrrs(DisasContext *ctx, arg_csrrs *a)
>   * a zero value, the instruction will still attempt to write the
>   * unmodified value back to the csr and will cause side effects.
>   */
> -if (a->rs1 == 0) {
> -return do_csrr(ctx, a->rd, a->csr);
> +if (get_xl(ctx) < MXL_RV128) {
> +if (a->rs1 == 0) {
> +return do_csrr(ctx, a->rd, a->csr);
> +}
> +
> +TCGv ones = tcg_constant_tl(-1);
> +TCGv mask = get_gpr(ctx, a->rs1, EXT_ZERO);
> +return do_csrrw(ctx, a->rd, a->csr, ones, mask);
> +} else {
> +if (a->rs1 == 0) {
> +return do_csrr_i128(ctx, a->rd, a->csr);
> +}
> +
> +TCGv ones = tcg_constant_tl(-1);
> +TCGv maskl = get_gpr(ctx, a->rs1, EXT_ZERO);
> +TCGv maskh = get_gprh(ctx, a->rs1);
> +return do_csrrw_i128(ctx, a->rd, a->csr, ones, ones, maskl, maskh);
>  }
> -
> -TCGv ones = tcg_constant_tl(-1);
> -TCGv mask = get_gpr(ctx, a->rs1, EXT_ZERO);
> -return do_csrrw(ctx, a->rd, a->csr, ones, mask);
>  }
>
>  static bool trans_csrrc(DisasContext *ctx, arg_csrrc *a)

[PATCH v7 17/18] target/riscv: modification of the trans_csrxx for 128-bit support

[Frédéric Pétrot]

As opposed to the gen_arith and gen_shift generation helpers, the csr insns
do not have a common prototype, so the choice to generate 32/64 or 128-bit
helper calls is done in the trans_csrxx functions.

Signed-off-by: Frédéric Pétrot 
Co-authored-by: Fabien Portas 
Reviewed-by: Richard Henderson 
---
 target/riscv/insn_trans/trans_rvi.c.inc | 205 ++--
 1 file changed, 160 insertions(+), 45 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
b/target/riscv/insn_trans/trans_rvi.c.inc
index ca354130ec..3a0ae28fef 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -881,20 +881,78 @@ static bool do_csrrw(DisasContext *ctx, int rd, int rc, 
TCGv src, TCGv mask)
 return do_csr_post(ctx);
 }
 
+static bool do_csrr_i128(DisasContext *ctx, int rd, int rc)
+{
+TCGv destl = dest_gpr(ctx, rd);
+TCGv desth = dest_gprh(ctx, rd);
+TCGv_i32 csr = tcg_constant_i32(rc);
+
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+gen_helper_csrr_i128(destl, cpu_env, csr);
+tcg_gen_ld_tl(desth, cpu_env, offsetof(CPURISCVState, retxh));
+gen_set_gpr128(ctx, rd, destl, desth);
+return do_csr_post(ctx);
+}
+
+static bool do_csrw_i128(DisasContext *ctx, int rc, TCGv srcl, TCGv srch)
+{
+TCGv_i32 csr = tcg_constant_i32(rc);
+
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+gen_helper_csrw_i128(cpu_env, csr, srcl, srch);
+return do_csr_post(ctx);
+}
+
+static bool do_csrrw_i128(DisasContext *ctx, int rd, int rc,
+  TCGv srcl, TCGv srch, TCGv maskl, TCGv maskh)
+{
+TCGv destl = dest_gpr(ctx, rd);
+TCGv desth = dest_gprh(ctx, rd);
+TCGv_i32 csr = tcg_constant_i32(rc);
+
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+gen_helper_csrrw_i128(destl, cpu_env, csr, srcl, srch, maskl, maskh);
+tcg_gen_ld_tl(desth, cpu_env, offsetof(CPURISCVState, retxh));
+gen_set_gpr128(ctx, rd, destl, desth);
+return do_csr_post(ctx);
+}
+
 static bool trans_csrrw(DisasContext *ctx, arg_csrrw *a)
 {
-TCGv src = get_gpr(ctx, a->rs1, EXT_NONE);
-
-/*
- * If rd == 0, the insn shall not read the csr, nor cause any of the
- * side effects that might occur on a csr read.
- */
-if (a->rd == 0) {
-return do_csrw(ctx, a->csr, src);
+if (get_xl(ctx) < MXL_RV128) {
+TCGv src = get_gpr(ctx, a->rs1, EXT_NONE);
+
+/*
+ * If rd == 0, the insn shall not read the csr, nor cause any of the
+ * side effects that might occur on a csr read.
+ */
+if (a->rd == 0) {
+return do_csrw(ctx, a->csr, src);
+}
+
+TCGv mask = tcg_constant_tl(-1);
+return do_csrrw(ctx, a->rd, a->csr, src, mask);
+} else {
+TCGv srcl = get_gpr(ctx, a->rs1, EXT_NONE);
+TCGv srch = get_gprh(ctx, a->rs1);
+
+/*
+ * If rd == 0, the insn shall not read the csr, nor cause any of the
+ * side effects that might occur on a csr read.
+ */
+if (a->rd == 0) {
+return do_csrw_i128(ctx, a->csr, srcl, srch);
+}
+
+TCGv mask = tcg_constant_tl(-1);
+return do_csrrw_i128(ctx, a->rd, a->csr, srcl, srch, mask, mask);
 }
-
-TCGv mask = tcg_constant_tl(-1);
-return do_csrrw(ctx, a->rd, a->csr, src, mask);
 }
 
 static bool trans_csrrs(DisasContext *ctx, arg_csrrs *a)
@@ -906,13 +964,24 @@ static bool trans_csrrs(DisasContext *ctx, arg_csrrs *a)
  * a zero value, the instruction will still attempt to write the
  * unmodified value back to the csr and will cause side effects.
  */
-if (a->rs1 == 0) {
-return do_csrr(ctx, a->rd, a->csr);
+if (get_xl(ctx) < MXL_RV128) {
+if (a->rs1 == 0) {
+return do_csrr(ctx, a->rd, a->csr);
+}
+
+TCGv ones = tcg_constant_tl(-1);
+TCGv mask = get_gpr(ctx, a->rs1, EXT_ZERO);
+return do_csrrw(ctx, a->rd, a->csr, ones, mask);
+} else {
+if (a->rs1 == 0) {
+return do_csrr_i128(ctx, a->rd, a->csr);
+}
+
+TCGv ones = tcg_constant_tl(-1);
+TCGv maskl = get_gpr(ctx, a->rs1, EXT_ZERO);
+TCGv maskh = get_gprh(ctx, a->rs1);
+return do_csrrw_i128(ctx, a->rd, a->csr, ones, ones, maskl, maskh);
 }
-
-TCGv ones = tcg_constant_tl(-1);
-TCGv mask = get_gpr(ctx, a->rs1, EXT_ZERO);
-return do_csrrw(ctx, a->rd, a->csr, ones, mask);
 }
 
 static bool trans_csrrc(DisasContext *ctx, arg_csrrc *a)
@@ -924,28 +993,54 @@ static bool trans_csrrc(DisasContext *ctx, arg_csrrc *a)
  * a zero value, the instruction will still attempt to write the
  * unmodified value back to the csr and will cause side effects.
  */
-if (a->rs1 == 0) {
-return do_csrr(ctx, a->rd, a->csr);
+if (get_xl(ctx) < MXL_RV128) {
+if (a->rs1 == 0) {
+