Introduce get_gpr, dest_gpr, temp_new -- new helpers that do not force
tcg globals into temps, returning a constant 0 for $zero as source and
a new temp for $zero as destination.
Introduce ctx->w for simplifying word operations, such as addw.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/riscv/translate.c | 102 +++++++++++++++++++++++++++++++--------
1 file changed, 82 insertions(+), 20 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index d540c85a1a..d5cf5e5826 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -39,15 +39,25 @@ static TCGv load_val;
#include "exec/gen-icount.h"
+/*
+ * If an operation is being performed on less than TARGET_LONG_BITS,
+ * it may require the inputs to be sign- or zero-extended; which will
+ * depend on the exact operation being performed.
+ */
+typedef enum {
+ EXT_NONE,
+ EXT_SIGN,
+ EXT_ZERO,
+} DisasExtend;
+
typedef struct DisasContext {
DisasContextBase base;
/* pc_succ_insn points to the instruction following base.pc_next */
target_ulong pc_succ_insn;
target_ulong priv_ver;
- bool virt_enabled;
+ target_ulong misa;
uint32_t opcode;
uint32_t mstatus_fs;
- target_ulong misa;
uint32_t mem_idx;
/* Remember the rounding mode encoded in the previous fp instruction,
which we have already installed into env->fp_status. Or -1 for
@@ -55,6 +65,8 @@ typedef struct DisasContext {
to any system register, which includes CSR_FRM, so we do not have
to reset this known value. */
int frm;
+ bool w;
+ bool virt_enabled;
bool ext_ifencei;
bool hlsx;
/* vector extension */
@@ -64,7 +76,10 @@ typedef struct DisasContext {
uint16_t vlen;
uint16_t mlen;
bool vl_eq_vlmax;
+ uint8_t ntemp;
CPUState *cs;
+ TCGv zero;
+ TCGv temp[4];
} DisasContext;
static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -172,27 +187,64 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
}
}
-/* Wrapper for getting reg values - need to check of reg is zero since
- * cpu_gpr[0] is not actually allocated
+/*
+ * Wrappers for getting reg values.
+ *
+ * The $zero register does not have cpu_gpr[0] allocated -- we supply the
+ * constant zero as a source, and an uninitialized sink as destination.
+ *
+ * Further, we may provide an extension for word operations.
*/
-static void gen_get_gpr(DisasContext *ctx, TCGv t, int reg_num)
+static TCGv temp_new(DisasContext *ctx)
{
- if (reg_num == 0) {
- tcg_gen_movi_tl(t, 0);
- } else {
- tcg_gen_mov_tl(t, cpu_gpr[reg_num]);
- }
+ assert(ctx->ntemp < ARRAY_SIZE(ctx->temp));
+ return ctx->temp[ctx->ntemp++] = tcg_temp_new();
}
-/* Wrapper for setting reg values - need to check of reg is zero since
- * cpu_gpr[0] is not actually allocated. this is more for safety purposes,
- * since we usually avoid calling the OP_TYPE_gen function if we see a write to
- * $zero
- */
-static void gen_set_gpr(DisasContext *ctx, int reg_num_dst, TCGv t)
+static TCGv get_gpr(DisasContext *ctx, int reg_num, DisasExtend ext)
{
- if (reg_num_dst != 0) {
- tcg_gen_mov_tl(cpu_gpr[reg_num_dst], t);
+ TCGv t;
+
+ if (reg_num == 0) {
+ return ctx->zero;
+ }
+
+ switch (ctx->w ? ext : EXT_NONE) {
+ case EXT_NONE:
+ return cpu_gpr[reg_num];
+ case EXT_SIGN:
+ t = temp_new(ctx);
+ tcg_gen_ext32s_tl(t, cpu_gpr[reg_num]);
+ return t;
+ case EXT_ZERO:
+ t = temp_new(ctx);
+ tcg_gen_ext32u_tl(t, cpu_gpr[reg_num]);
+ return t;
+ }
+ g_assert_not_reached();
+}
+
+static void gen_get_gpr(DisasContext *ctx, TCGv t, int reg_num)
+{
+ tcg_gen_mov_tl(t, get_gpr(ctx, reg_num, EXT_NONE));
+}
+
+static TCGv __attribute__((unused)) dest_gpr(DisasContext *ctx, int reg_num)
+{
+ if (reg_num == 0 || ctx->w) {
+ return temp_new(ctx);
+ }
+ return cpu_gpr[reg_num];
+}
+
+static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
+{
+ if (reg_num != 0) {
+ if (ctx->w) {
+ tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
+ } else {
+ tcg_gen_mov_tl(cpu_gpr[reg_num], t);
+ }
}
}
@@ -927,8 +979,11 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->cs = cs;
}
-static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
+static void riscv_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)
{
+ DisasContext *ctx = container_of(dcbase, DisasContext, base);
+
+ ctx->zero = tcg_constant_tl(0);
}
static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
@@ -946,6 +1001,13 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
decode_opc(env, ctx, opcode16);
ctx->base.pc_next = ctx->pc_succ_insn;
+ ctx->w = false;
+
+ for (int i = ctx->ntemp - 1; i >= 0; --i) {
+ tcg_temp_free(ctx->temp[i]);
+ ctx->temp[i] = NULL;
+ }
+ ctx->ntemp = 0;
if (ctx->base.is_jmp == DISAS_NEXT) {
target_ulong page_start;
@@ -997,7 +1059,7 @@ static const TranslatorOps riscv_tr_ops = {
void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
{
- DisasContext ctx;
+ DisasContext ctx = { };
translator_loop(&riscv_tr_ops, &ctx.base, cs, tb, max_insns);
}
--
2.25.1
On Wed, Aug 18, 2021 at 5:22 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Introduce get_gpr, dest_gpr, temp_new -- new helpers that do not force
> tcg globals into temps, returning a constant 0 for $zero as source and
> a new temp for $zero as destination.
>
> Introduce ctx->w for simplifying word operations, such as addw.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/riscv/translate.c | 102 +++++++++++++++++++++++++++++++--------
> 1 file changed, 82 insertions(+), 20 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index d540c85a1a..d5cf5e5826 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -39,15 +39,25 @@ static TCGv load_val;
>
> #include "exec/gen-icount.h"
>
> +/*
> + * If an operation is being performed on less than TARGET_LONG_BITS,
> + * it may require the inputs to be sign- or zero-extended; which will
> + * depend on the exact operation being performed.
> + */
> +typedef enum {
> + EXT_NONE,
> + EXT_SIGN,
> + EXT_ZERO,
> +} DisasExtend;
> +
> typedef struct DisasContext {
> DisasContextBase base;
> /* pc_succ_insn points to the instruction following base.pc_next */
> target_ulong pc_succ_insn;
> target_ulong priv_ver;
> - bool virt_enabled;
> + target_ulong misa;
> uint32_t opcode;
> uint32_t mstatus_fs;
> - target_ulong misa;
> uint32_t mem_idx;
> /* Remember the rounding mode encoded in the previous fp instruction,
> which we have already installed into env->fp_status. Or -1 for
> @@ -55,6 +65,8 @@ typedef struct DisasContext {
> to any system register, which includes CSR_FRM, so we do not have
> to reset this known value. */
> int frm;
> + bool w;
> + bool virt_enabled;
> bool ext_ifencei;
> bool hlsx;
> /* vector extension */
> @@ -64,7 +76,10 @@ typedef struct DisasContext {
> uint16_t vlen;
> uint16_t mlen;
> bool vl_eq_vlmax;
> + uint8_t ntemp;
> CPUState *cs;
> + TCGv zero;
> + TCGv temp[4];
Why is 4? Is it enough? Perhaps a comment here is needed here?
> } DisasContext;
>
> static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -172,27 +187,64 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
> }
> }
>
> -/* Wrapper for getting reg values - need to check of reg is zero since
> - * cpu_gpr[0] is not actually allocated
> +/*
> + * Wrappers for getting reg values.
> + *
> + * The $zero register does not have cpu_gpr[0] allocated -- we supply the
> + * constant zero as a source, and an uninitialized sink as destination.
> + *
> + * Further, we may provide an extension for word operations.
> */
> -static void gen_get_gpr(DisasContext *ctx, TCGv t, int reg_num)
> +static TCGv temp_new(DisasContext *ctx)
> {
> - if (reg_num == 0) {
> - tcg_gen_movi_tl(t, 0);
> - } else {
> - tcg_gen_mov_tl(t, cpu_gpr[reg_num]);
> - }
> + assert(ctx->ntemp < ARRAY_SIZE(ctx->temp));
> + return ctx->temp[ctx->ntemp++] = tcg_temp_new();
> }
>
> -/* Wrapper for setting reg values - need to check of reg is zero since
> - * cpu_gpr[0] is not actually allocated. this is more for safety purposes,
> - * since we usually avoid calling the OP_TYPE_gen function if we see a write to
> - * $zero
> - */
> -static void gen_set_gpr(DisasContext *ctx, int reg_num_dst, TCGv t)
> +static TCGv get_gpr(DisasContext *ctx, int reg_num, DisasExtend ext)
> {
> - if (reg_num_dst != 0) {
> - tcg_gen_mov_tl(cpu_gpr[reg_num_dst], t);
> + TCGv t;
> +
> + if (reg_num == 0) {
> + return ctx->zero;
> + }
> +
> + switch (ctx->w ? ext : EXT_NONE) {
> + case EXT_NONE:
> + return cpu_gpr[reg_num];
> + case EXT_SIGN:
> + t = temp_new(ctx);
> + tcg_gen_ext32s_tl(t, cpu_gpr[reg_num]);
> + return t;
> + case EXT_ZERO:
> + t = temp_new(ctx);
> + tcg_gen_ext32u_tl(t, cpu_gpr[reg_num]);
> + return t;
> + }
> + g_assert_not_reached();
> +}
> +
> +static void gen_get_gpr(DisasContext *ctx, TCGv t, int reg_num)
> +{
> + tcg_gen_mov_tl(t, get_gpr(ctx, reg_num, EXT_NONE));
> +}
> +
> +static TCGv __attribute__((unused)) dest_gpr(DisasContext *ctx, int reg_num)
> +{
> + if (reg_num == 0 || ctx->w) {
> + return temp_new(ctx);
> + }
> + return cpu_gpr[reg_num];
> +}
> +
> +static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> +{
> + if (reg_num != 0) {
> + if (ctx->w) {
> + tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
What about zero extension?
> + } else {
> + tcg_gen_mov_tl(cpu_gpr[reg_num], t);
> + }
> }
> }
>
> @@ -927,8 +979,11 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->cs = cs;
> }
>
> -static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
> +static void riscv_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)
> {
> + DisasContext *ctx = container_of(dcbase, DisasContext, base);
> +
> + ctx->zero = tcg_constant_tl(0);
This is better to be done in riscv_tr_init_disas_context() where ctx
members are initialized.
> }
>
> static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
> @@ -946,6 +1001,13 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
>
> decode_opc(env, ctx, opcode16);
> ctx->base.pc_next = ctx->pc_succ_insn;
> + ctx->w = false;
> +
> + for (int i = ctx->ntemp - 1; i >= 0; --i) {
> + tcg_temp_free(ctx->temp[i]);
> + ctx->temp[i] = NULL;
> + }
> + ctx->ntemp = 0;
>
> if (ctx->base.is_jmp == DISAS_NEXT) {
> target_ulong page_start;
> @@ -997,7 +1059,7 @@ static const TranslatorOps riscv_tr_ops = {
>
> void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> {
> - DisasContext ctx;
> + DisasContext ctx = { };
Why is this change? I believe we should explicitly initialize the ctx
in riscv_tr_init_disas_context()
>
> translator_loop(&riscv_tr_ops, &ctx.base, cs, tb, max_insns);
> }
> --
Regards,
Bin
On 8/18/21 12:58 AM, Bin Meng wrote:
>> +static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
>> +{
>> + if (reg_num != 0) {
>> + if (ctx->w) {
>> + tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
>
> What about zero extension?
All of the RV64 word instructions sign-extend the result.
>> void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
>> {
>> - DisasContext ctx;
>> + DisasContext ctx = { };
>
> Why is this change? I believe we should explicitly initialize the ctx
> in riscv_tr_init_disas_context()
I considered it easier to zero-init the whole thing here.
r~
On 8/18/21 12:58 AM, Bin Meng wrote: >> + TCGv temp[4]; > > Why is 4? Is it enough? Perhaps a comment here is needed here? It's a round number that will cover three operands plus an extra for address computation. r~
On Wed, Aug 18, 2021 at 7:23 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Introduce get_gpr, dest_gpr, temp_new -- new helpers that do not force
> tcg globals into temps, returning a constant 0 for $zero as source and
> a new temp for $zero as destination.
>
> Introduce ctx->w for simplifying word operations, such as addw.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/translate.c | 102 +++++++++++++++++++++++++++++++--------
> 1 file changed, 82 insertions(+), 20 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index d540c85a1a..d5cf5e5826 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -39,15 +39,25 @@ static TCGv load_val;
>
> #include "exec/gen-icount.h"
>
> +/*
> + * If an operation is being performed on less than TARGET_LONG_BITS,
> + * it may require the inputs to be sign- or zero-extended; which will
> + * depend on the exact operation being performed.
> + */
> +typedef enum {
> + EXT_NONE,
> + EXT_SIGN,
> + EXT_ZERO,
> +} DisasExtend;
> +
> typedef struct DisasContext {
> DisasContextBase base;
> /* pc_succ_insn points to the instruction following base.pc_next */
> target_ulong pc_succ_insn;
> target_ulong priv_ver;
> - bool virt_enabled;
> + target_ulong misa;
> uint32_t opcode;
> uint32_t mstatus_fs;
> - target_ulong misa;
> uint32_t mem_idx;
> /* Remember the rounding mode encoded in the previous fp instruction,
> which we have already installed into env->fp_status. Or -1 for
> @@ -55,6 +65,8 @@ typedef struct DisasContext {
> to any system register, which includes CSR_FRM, so we do not have
> to reset this known value. */
> int frm;
> + bool w;
> + bool virt_enabled;
> bool ext_ifencei;
> bool hlsx;
> /* vector extension */
> @@ -64,7 +76,10 @@ typedef struct DisasContext {
> uint16_t vlen;
> uint16_t mlen;
> bool vl_eq_vlmax;
> + uint8_t ntemp;
> CPUState *cs;
> + TCGv zero;
> + TCGv temp[4];
> } DisasContext;
>
> static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -172,27 +187,64 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
> }
> }
>
> -/* Wrapper for getting reg values - need to check of reg is zero since
> - * cpu_gpr[0] is not actually allocated
> +/*
> + * Wrappers for getting reg values.
> + *
> + * The $zero register does not have cpu_gpr[0] allocated -- we supply the
> + * constant zero as a source, and an uninitialized sink as destination.
> + *
> + * Further, we may provide an extension for word operations.
> */
> -static void gen_get_gpr(DisasContext *ctx, TCGv t, int reg_num)
> +static TCGv temp_new(DisasContext *ctx)
> {
> - if (reg_num == 0) {
> - tcg_gen_movi_tl(t, 0);
> - } else {
> - tcg_gen_mov_tl(t, cpu_gpr[reg_num]);
> - }
> + assert(ctx->ntemp < ARRAY_SIZE(ctx->temp));
> + return ctx->temp[ctx->ntemp++] = tcg_temp_new();
> }
>
> -/* Wrapper for setting reg values - need to check of reg is zero since
> - * cpu_gpr[0] is not actually allocated. this is more for safety purposes,
> - * since we usually avoid calling the OP_TYPE_gen function if we see a write to
> - * $zero
> - */
> -static void gen_set_gpr(DisasContext *ctx, int reg_num_dst, TCGv t)
> +static TCGv get_gpr(DisasContext *ctx, int reg_num, DisasExtend ext)
> {
> - if (reg_num_dst != 0) {
> - tcg_gen_mov_tl(cpu_gpr[reg_num_dst], t);
> + TCGv t;
> +
> + if (reg_num == 0) {
> + return ctx->zero;
> + }
> +
> + switch (ctx->w ? ext : EXT_NONE) {
> + case EXT_NONE:
> + return cpu_gpr[reg_num];
> + case EXT_SIGN:
> + t = temp_new(ctx);
> + tcg_gen_ext32s_tl(t, cpu_gpr[reg_num]);
> + return t;
> + case EXT_ZERO:
> + t = temp_new(ctx);
> + tcg_gen_ext32u_tl(t, cpu_gpr[reg_num]);
> + return t;
> + }
> + g_assert_not_reached();
> +}
> +
> +static void gen_get_gpr(DisasContext *ctx, TCGv t, int reg_num)
> +{
> + tcg_gen_mov_tl(t, get_gpr(ctx, reg_num, EXT_NONE));
> +}
> +
> +static TCGv __attribute__((unused)) dest_gpr(DisasContext *ctx, int reg_num)
> +{
> + if (reg_num == 0 || ctx->w) {
> + return temp_new(ctx);
> + }
> + return cpu_gpr[reg_num];
> +}
> +
> +static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> +{
> + if (reg_num != 0) {
> + if (ctx->w) {
> + tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
> + } else {
> + tcg_gen_mov_tl(cpu_gpr[reg_num], t);
> + }
> }
> }
>
> @@ -927,8 +979,11 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->cs = cs;
> }
>
> -static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
> +static void riscv_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)
> {
> + DisasContext *ctx = container_of(dcbase, DisasContext, base);
> +
> + ctx->zero = tcg_constant_tl(0);
> }
>
> static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
> @@ -946,6 +1001,13 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
>
> decode_opc(env, ctx, opcode16);
> ctx->base.pc_next = ctx->pc_succ_insn;
> + ctx->w = false;
> +
> + for (int i = ctx->ntemp - 1; i >= 0; --i) {
> + tcg_temp_free(ctx->temp[i]);
> + ctx->temp[i] = NULL;
> + }
> + ctx->ntemp = 0;
>
> if (ctx->base.is_jmp == DISAS_NEXT) {
> target_ulong page_start;
> @@ -997,7 +1059,7 @@ static const TranslatorOps riscv_tr_ops = {
>
> void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns)
> {
> - DisasContext ctx;
> + DisasContext ctx = { };
>
> translator_loop(&riscv_tr_ops, &ctx.base, cs, tb, max_insns);
> }
> --
> 2.25.1
>
>
© 2016 - 2026 Red Hat, Inc.