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
<frederic.petrot@univ-grenoble-alpes.fr> 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 <frederic.petrot@univ-grenoble-alpes.fr>
> Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

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)
> @@ -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) {
> +            return do_csrr(ctx, a->rd, a->csr);
> +        }
> +
> +        TCGv mask = get_gpr(ctx, a->rs1, EXT_ZERO);
> +        return do_csrrw(ctx, a->rd, a->csr, ctx->zero, mask);
> +    } else {
> +        if (a->rs1 == 0) {
> +            return do_csrr_i128(ctx, a->rd, a->csr);
> +        }
> +
> +        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,
> +                             ctx->zero, ctx->zero, maskl, maskh);
>      }
> -
> -    TCGv mask = get_gpr(ctx, a->rs1, EXT_ZERO);
> -    return do_csrrw(ctx, a->rd, a->csr, ctx->zero, mask);
>  }
>
>  static bool trans_csrrwi(DisasContext *ctx, arg_csrrwi *a)
>  {
> -    TCGv src = tcg_constant_tl(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(ctx, a->csr, src);
> +    if (get_xl(ctx) < MXL_RV128) {
> +        TCGv src = tcg_constant_tl(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(ctx, a->csr, src);
> +        }
> +
> +        TCGv mask = tcg_constant_tl(-1);
> +        return do_csrrw(ctx, a->rd, a->csr, src, mask);
> +    } else {
> +        TCGv src = tcg_constant_tl(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, src, ctx->zero);
> +        }
> +
> +        TCGv mask = tcg_constant_tl(-1);
> +        return do_csrrw_i128(ctx, a->rd, a->csr, src, ctx->zero, mask, mask);
>      }
> -
> -    TCGv mask = tcg_constant_tl(-1);
> -    return do_csrrw(ctx, a->rd, a->csr, src, mask);
>  }
>
>  static bool trans_csrrsi(DisasContext *ctx, arg_csrrsi *a)
> @@ -957,16 +1052,26 @@ static bool trans_csrrsi(DisasContext *ctx, arg_csrrsi 
> *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 = tcg_constant_tl(a->rs1);
> +        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 mask = tcg_constant_tl(a->rs1);
> +        return do_csrrw_i128(ctx, a->rd, a->csr, ones, ones, mask, 
> ctx->zero);
>      }
> -
> -    TCGv ones = tcg_constant_tl(-1);
> -    TCGv mask = tcg_constant_tl(a->rs1);
> -    return do_csrrw(ctx, a->rd, a->csr, ones, mask);
>  }
>
> -static bool trans_csrrci(DisasContext *ctx, arg_csrrci *a)
> +static bool trans_csrrci(DisasContext *ctx, arg_csrrci * a)
>  {
>      /*
>       * If rs1 == 0, the insn shall not write to the csr at all, nor
> @@ -975,10 +1080,20 @@ static bool trans_csrrci(DisasContext *ctx, arg_csrrci 
> *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 mask = tcg_constant_tl(a->rs1);
> +        return do_csrrw(ctx, a->rd, a->csr, ctx->zero, mask);
> +    } else {
> +        if (a->rs1 == 0) {
> +            return do_csrr_i128(ctx, a->rd, a->csr);
> +        }
> +
> +        TCGv mask = tcg_constant_tl(a->rs1);
> +        return do_csrrw_i128(ctx, a->rd, a->csr,
> +                             ctx->zero, ctx->zero, mask, ctx->zero);
>      }
> -
> -    TCGv mask = tcg_constant_tl(a->rs1);
> -    return do_csrrw(ctx, a->rd, a->csr, ctx->zero, mask);
>  }
> --
> 2.34.1
>
>