[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 09/17] target/riscv: Reorg csr instructions
From: |
Alistair Francis |
Subject: |
Re: [PATCH 09/17] target/riscv: Reorg csr instructions |
Date: |
Fri, 23 Jul 2021 15:00:31 +1000 |
On Fri, Jul 9, 2021 at 2:46 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Introduce csrr and csrw helpers, for read-only and write-only insns.
>
> Note that we do not properly implement this in riscv_csrrw, in that
> we cannot distinguish true read-only (rs1 == 0) from any other zero
> write_mask another source register -- this should still raise an
> exception for read-only registers.
>
> Only issue gen_io_start for CF_USE_ICOUNT.
> Use ctx->zero for csrrc.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/helper.h | 6 +-
> target/riscv/op_helper.c | 18 +--
> target/riscv/insn_trans/trans_rvi.c.inc | 170 +++++++++++++++++-------
> 3 files changed, 129 insertions(+), 65 deletions(-)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 415e37bc37..460eee9988 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -65,9 +65,9 @@ DEF_HELPER_FLAGS_2(gorc, TCG_CALL_NO_RWG_SE, tl, tl, tl)
> DEF_HELPER_FLAGS_2(gorcw, TCG_CALL_NO_RWG_SE, tl, tl, tl)
>
> /* Special functions */
> -DEF_HELPER_3(csrrw, tl, env, tl, tl)
> -DEF_HELPER_4(csrrs, tl, env, tl, tl, tl)
> -DEF_HELPER_4(csrrc, tl, env, tl, tl, tl)
> +DEF_HELPER_2(csrr, tl, env, int)
> +DEF_HELPER_3(csrw, void, env, int, tl)
> +DEF_HELPER_4(csrrw, tl, env, int, tl, tl)
> #ifndef CONFIG_USER_ONLY
> DEF_HELPER_2(sret, tl, env, tl)
> DEF_HELPER_2(mret, tl, env, tl)
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 3c48e739ac..ee7c24efe7 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -37,11 +37,10 @@ void helper_raise_exception(CPURISCVState *env, uint32_t
> exception)
> riscv_raise_exception(env, exception, 0);
> }
>
> -target_ulong helper_csrrw(CPURISCVState *env, target_ulong src,
> - target_ulong csr)
> +target_ulong helper_csrr(CPURISCVState *env, int csr)
> {
> target_ulong val = 0;
> - RISCVException ret = riscv_csrrw(env, csr, &val, src, -1);
> + RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0);
>
> if (ret != RISCV_EXCP_NONE) {
> riscv_raise_exception(env, ret, GETPC());
> @@ -49,23 +48,20 @@ target_ulong helper_csrrw(CPURISCVState *env,
> target_ulong src,
> return val;
> }
>
> -target_ulong helper_csrrs(CPURISCVState *env, target_ulong src,
> - target_ulong csr, target_ulong rs1_pass)
> +void helper_csrw(CPURISCVState *env, int csr, target_ulong src)
> {
> - target_ulong val = 0;
> - RISCVException ret = riscv_csrrw(env, csr, &val, -1, rs1_pass ? src : 0);
> + RISCVException ret = riscv_csrrw(env, csr, NULL, src, -1);
>
> if (ret != RISCV_EXCP_NONE) {
> riscv_raise_exception(env, ret, GETPC());
> }
> - return val;
> }
>
> -target_ulong helper_csrrc(CPURISCVState *env, target_ulong src,
> - target_ulong csr, target_ulong rs1_pass)
> +target_ulong helper_csrrw(CPURISCVState *env, int csr,
> + target_ulong src, target_ulong write_mask)
> {
> target_ulong val = 0;
> - RISCVException ret = riscv_csrrw(env, csr, &val, 0, rs1_pass ? src : 0);
> + RISCVException ret = riscv_csrrw(env, csr, &val, src, write_mask);
>
> if (ret != RISCV_EXCP_NONE) {
> riscv_raise_exception(env, ret, GETPC());
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index 840187a4d6..3705aad380 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -452,80 +452,148 @@ static bool trans_fence_i(DisasContext *ctx,
> arg_fence_i *a)
> return true;
> }
>
> -#define RISCV_OP_CSR_PRE do {\
> - source1 = tcg_temp_new(); \
> - csr_store = tcg_temp_new(); \
> - dest = tcg_temp_new(); \
> - rs1_pass = tcg_temp_new(); \
> - gen_get_gpr(source1, a->rs1); \
> - tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next); \
> - tcg_gen_movi_tl(rs1_pass, a->rs1); \
> - tcg_gen_movi_tl(csr_store, a->csr); \
> - gen_io_start();\
> -} while (0)
> +static bool do_csr_post(DisasContext *ctx)
> +{
> + /* We may have changed important cpu state -- exit to main loop. */
> + tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
> + exit_tb(ctx);
> + ctx->base.is_jmp = DISAS_NORETURN;
> + return true;
> +}
>
> -#define RISCV_OP_CSR_POST do {\
> - gen_set_gpr(a->rd, dest); \
> - tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); \
> - exit_tb(ctx); \
> - ctx->base.is_jmp = DISAS_NORETURN; \
> - tcg_temp_free(source1); \
> - tcg_temp_free(csr_store); \
> - tcg_temp_free(dest); \
> - tcg_temp_free(rs1_pass); \
> -} while (0)
> +static bool do_csrr(DisasContext *ctx, int rd, int rc)
> +{
> + TCGv dest = gpr_dst(ctx, rd);
> + TCGv_i32 csr = tcg_constant_i32(rc);
>
> + if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> + gen_io_start();
> + }
> + gen_helper_csrr(dest, cpu_env, csr);
> + return do_csr_post(ctx);
> +}
> +
> +static bool do_csrw(DisasContext *ctx, int rc, TCGv src)
> +{
> + TCGv_i32 csr = tcg_constant_i32(rc);
> +
> + if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> + gen_io_start();
> + }
> + gen_helper_csrw(cpu_env, csr, src);
> + return do_csr_post(ctx);
> +}
> +
> +static bool do_csrrw(DisasContext *ctx, int rd, int rc, TCGv src, TCGv mask)
> +{
> + TCGv dest = gpr_dst(ctx, rd);
> + TCGv_i32 csr = tcg_constant_i32(rc);
> +
> + if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> + gen_io_start();
> + }
> + gen_helper_csrrw(dest, cpu_env, csr, src, mask);
> + return do_csr_post(ctx);
> +}
>
> static bool trans_csrrw(DisasContext *ctx, arg_csrrw *a)
> {
> - TCGv source1, csr_store, dest, rs1_pass;
> - RISCV_OP_CSR_PRE;
> - gen_helper_csrrw(dest, cpu_env, source1, csr_store);
> - RISCV_OP_CSR_POST;
> - return true;
> + TCGv src = gpr_src(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(ctx, a->csr, src);
> + }
> +
> + 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)
> {
> - TCGv source1, csr_store, dest, rs1_pass;
> - RISCV_OP_CSR_PRE;
> - gen_helper_csrrs(dest, cpu_env, source1, csr_store, rs1_pass);
> - RISCV_OP_CSR_POST;
> - return true;
> + /*
> + * If rs1 == 0, the insn shall not write to the csr at all, nor
> + * cause any of the side effects that might occur on a csr write.
> + * Note that if rs1 specifies a register other than x0, holding
> + * 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);
> + }
> +
> + TCGv ones = tcg_constant_tl(-1);
> + TCGv mask = gpr_src(ctx, a->rs1);
> + return do_csrrw(ctx, a->rd, a->csr, ones, mask);
> }
>
> static bool trans_csrrc(DisasContext *ctx, arg_csrrc *a)
> {
> - TCGv source1, csr_store, dest, rs1_pass;
> - RISCV_OP_CSR_PRE;
> - gen_helper_csrrc(dest, cpu_env, source1, csr_store, rs1_pass);
> - RISCV_OP_CSR_POST;
> - return true;
> + /*
> + * If rs1 == 0, the insn shall not write to the csr at all, nor
> + * cause any of the side effects that might occur on a csr write.
> + * Note that if rs1 specifies a register other than x0, holding
> + * 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);
> + }
> +
> + TCGv mask = gpr_src(ctx, a->rs1);
> + return do_csrrw(ctx, a->rd, a->csr, ctx->zero, mask);
> }
>
> static bool trans_csrrwi(DisasContext *ctx, arg_csrrwi *a)
> {
> - TCGv source1, csr_store, dest, rs1_pass;
> - RISCV_OP_CSR_PRE;
> - gen_helper_csrrw(dest, cpu_env, rs1_pass, csr_store);
> - RISCV_OP_CSR_POST;
> - return true;
> + 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);
> }
>
> static bool trans_csrrsi(DisasContext *ctx, arg_csrrsi *a)
> {
> - TCGv source1, csr_store, dest, rs1_pass;
> - RISCV_OP_CSR_PRE;
> - gen_helper_csrrs(dest, cpu_env, rs1_pass, csr_store, rs1_pass);
> - RISCV_OP_CSR_POST;
> - return true;
> + /*
> + * If rs1 == 0, the insn shall not write to the csr at all, nor
> + * cause any of the side effects that might occur on a csr write.
> + * Note that if rs1 specifies a register other than x0, holding
> + * 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);
> + }
> +
> + 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)
> {
> - TCGv source1, csr_store, dest, rs1_pass;
> - RISCV_OP_CSR_PRE;
> - gen_helper_csrrc(dest, cpu_env, rs1_pass, csr_store, rs1_pass);
> - RISCV_OP_CSR_POST;
> - return true;
> + /*
> + * If rs1 == 0, the insn shall not write to the csr at all, nor
> + * cause any of the side effects that might occur on a csr write.
> + * Note that if rs1 specifies a register other than x0, holding
> + * 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);
> + }
> +
> + TCGv mask = tcg_constant_tl(a->rs1);
> + return do_csrrw(ctx, a->rd, a->csr, ctx->zero, mask);
> }
> --
> 2.25.1
>
>
- Re: [PATCH 14/17] target/riscv: Tidy trans_rvh.c.inc, (continued)
- [PATCH 06/17] target/riscv: Use gpr_src in branches, Richard Henderson, 2021/07/09
- [PATCH 16/17] target/riscv: Use gpr_{src,dst} for RVV, Richard Henderson, 2021/07/09
- [PATCH 07/17] target/riscv: Use gpr_{src,dst} for integer load/store, Richard Henderson, 2021/07/09
- [PATCH 08/17] target/riscv: Use gpr_{src, dst} for word shift operations, Richard Henderson, 2021/07/09
- [PATCH 09/17] target/riscv: Reorg csr instructions, Richard Henderson, 2021/07/09
- Re: [PATCH 09/17] target/riscv: Reorg csr instructions,
Alistair Francis <=
- [PATCH 11/17] target/riscv: Use gpr_{src,dst} for RVB, Richard Henderson, 2021/07/09
- [PATCH 12/17] target/riscv: Use gpr_{src,dst} for RVF, Richard Henderson, 2021/07/09
- [PATCH 13/17] target/riscv: Use gpr_{src,dst} for RVD, Richard Henderson, 2021/07/09
- [PATCH 15/17] target/riscv: Use gen_arith for mulh and mulhu, Richard Henderson, 2021/07/09
- [PATCH 17/17] target/riscv: Remove gen_get_gpr, Richard Henderson, 2021/07/09