* [PATCH v2 14/19] target/riscv: progressively load the instruction during decode [not found] <20200213225109.13120-1-alex.bennee@linaro.org> @ 2020-02-13 22:51 ` Alex Bennée 2020-02-14 1:23 ` Alistair Francis 2020-02-14 19:34 ` Robert Foley 0 siblings, 2 replies; 3+ messages in thread From: Alex Bennée @ 2020-02-13 22:51 UTC (permalink / raw) To: qemu-devel Cc: cota, aaron, peter.puhov, robert.foley, kuhn.chenqun, robhenry, fam, berrange, f4bug, richard.henderson, balrogg, aurelien, pbonzini, stefanha, stefanb, marcandre.lureau, Alex Bennée, Palmer Dabbelt, Alistair Francis, Sagar Karandikar, Bastian Koppelmann, open list:RISC-V TCG CPUs The plugin system would throw up a harmless warning when it detected that a disassembly of an instruction didn't use all it's bytes. Fix the riscv decoder to only load the instruction bytes it needs as it needs them. This drops opcode from the ctx in favour if passing the appropriately sized opcode down a few levels of the decode. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- v2 - use extract16 for uint16_t opcodes squash! target/riscv: progressively load the instruction during decode --- target/riscv/instmap.h | 8 ++++---- target/riscv/translate.c | 40 +++++++++++++++++++++------------------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/target/riscv/instmap.h b/target/riscv/instmap.h index f8ad7d60fd5..40b6d2b64de 100644 --- a/target/riscv/instmap.h +++ b/target/riscv/instmap.h @@ -344,8 +344,8 @@ enum { #define GET_C_LW_IMM(inst) ((extract32(inst, 6, 1) << 2) \ | (extract32(inst, 10, 3) << 3) \ | (extract32(inst, 5, 1) << 6)) -#define GET_C_LD_IMM(inst) ((extract32(inst, 10, 3) << 3) \ - | (extract32(inst, 5, 2) << 6)) +#define GET_C_LD_IMM(inst) ((extract16(inst, 10, 3) << 3) \ + | (extract16(inst, 5, 2) << 6)) #define GET_C_J_IMM(inst) ((extract32(inst, 3, 3) << 1) \ | (extract32(inst, 11, 1) << 4) \ | (extract32(inst, 2, 1) << 5) \ @@ -363,7 +363,7 @@ enum { #define GET_C_RD(inst) GET_RD(inst) #define GET_C_RS1(inst) GET_RD(inst) #define GET_C_RS2(inst) extract32(inst, 2, 5) -#define GET_C_RS1S(inst) (8 + extract32(inst, 7, 3)) -#define GET_C_RS2S(inst) (8 + extract32(inst, 2, 3)) +#define GET_C_RS1S(inst) (8 + extract16(inst, 7, 3)) +#define GET_C_RS2S(inst) (8 + extract16(inst, 2, 3)) #endif diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 14dc71156be..d5de7f468a7 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -44,7 +44,6 @@ typedef struct DisasContext { /* pc_succ_insn points to the instruction following base.pc_next */ target_ulong pc_succ_insn; target_ulong priv_ver; - uint32_t opcode; uint32_t mstatus_fs; uint32_t misa; uint32_t mem_idx; @@ -492,45 +491,45 @@ static void gen_set_rm(DisasContext *ctx, int rm) tcg_temp_free_i32(t0); } -static void decode_RV32_64C0(DisasContext *ctx) +static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode) { - uint8_t funct3 = extract32(ctx->opcode, 13, 3); - uint8_t rd_rs2 = GET_C_RS2S(ctx->opcode); - uint8_t rs1s = GET_C_RS1S(ctx->opcode); + uint8_t funct3 = extract16(opcode, 13, 3); + uint8_t rd_rs2 = GET_C_RS2S(opcode); + uint8_t rs1s = GET_C_RS1S(opcode); switch (funct3) { case 3: #if defined(TARGET_RISCV64) /* C.LD(RV64/128) -> ld rd', offset[7:3](rs1')*/ gen_load_c(ctx, OPC_RISC_LD, rd_rs2, rs1s, - GET_C_LD_IMM(ctx->opcode)); + GET_C_LD_IMM(opcode)); #else /* C.FLW (RV32) -> flw rd', offset[6:2](rs1')*/ gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s, - GET_C_LW_IMM(ctx->opcode)); + GET_C_LW_IMM(opcode)); #endif break; case 7: #if defined(TARGET_RISCV64) /* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/ gen_store_c(ctx, OPC_RISC_SD, rs1s, rd_rs2, - GET_C_LD_IMM(ctx->opcode)); + GET_C_LD_IMM(opcode)); #else /* C.FSW (RV32) -> fsw rs2', offset[6:2](rs1')*/ gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2, - GET_C_LW_IMM(ctx->opcode)); + GET_C_LW_IMM(opcode)); #endif break; } } -static void decode_RV32_64C(DisasContext *ctx) +static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode) { - uint8_t op = extract32(ctx->opcode, 0, 2); + uint8_t op = extract16(opcode, 0, 2); switch (op) { case 0: - decode_RV32_64C0(ctx); + decode_RV32_64C0(ctx, opcode); break; } } @@ -709,22 +708,25 @@ static bool gen_shift(DisasContext *ctx, arg_r *a, /* Include the auto-generated decoder for 16 bit insn */ #include "decode_insn16.inc.c" -static void decode_opc(DisasContext *ctx) +static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) { /* check for compressed insn */ - if (extract32(ctx->opcode, 0, 2) != 3) { + if (extract16(opcode, 0, 2) != 3) { if (!has_ext(ctx, RVC)) { gen_exception_illegal(ctx); } else { ctx->pc_succ_insn = ctx->base.pc_next + 2; - if (!decode_insn16(ctx, ctx->opcode)) { + if (!decode_insn16(ctx, opcode)) { /* fall back to old decoder */ - decode_RV32_64C(ctx); + decode_RV32_64C(ctx, opcode); } } } else { + uint32_t opcode32 = opcode; + opcode32 = deposit32(opcode32, 16, 16, + translator_lduw(env, ctx->base.pc_next + 2)); ctx->pc_succ_insn = ctx->base.pc_next + 4; - if (!decode_insn32(ctx, ctx->opcode)) { + if (!decode_insn32(ctx, opcode32)) { gen_exception_illegal(ctx); } } @@ -776,9 +778,9 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *ctx = container_of(dcbase, DisasContext, base); CPURISCVState *env = cpu->env_ptr; + uint16_t opcode16 = translator_lduw(env, ctx->base.pc_next); - ctx->opcode = translator_ldl(env, ctx->base.pc_next); - decode_opc(ctx); + decode_opc(env, ctx, opcode16); ctx->base.pc_next = ctx->pc_succ_insn; if (ctx->base.is_jmp == DISAS_NEXT) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 14/19] target/riscv: progressively load the instruction during decode 2020-02-13 22:51 ` [PATCH v2 14/19] target/riscv: progressively load the instruction during decode Alex Bennée @ 2020-02-14 1:23 ` Alistair Francis 2020-02-14 19:34 ` Robert Foley 1 sibling, 0 replies; 3+ messages in thread From: Alistair Francis @ 2020-02-14 1:23 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel@nongnu.org Developers, Fam Zheng, Sagar Karandikar, Alistair Francis, open list:RISC-V TCG CPUs, stefanb, aaron, Emilio G. Cota, Marc-André Lureau, robert.foley, Richard Henderson, Stefan Hajnoczi, peter.puhov, kuhn.chenqun, Daniel P. Berrange, Bastian Koppelmann, Philippe Mathieu-Daudé, robhenry, Palmer Dabbelt, Paolo Bonzini, Aurelien Jarno On Thu, Feb 13, 2020 at 3:08 PM Alex Bennée <alex.bennee@linaro.org> wrote: > > The plugin system would throw up a harmless warning when it detected > that a disassembly of an instruction didn't use all it's bytes. Fix > the riscv decoder to only load the instruction bytes it needs as it > needs them. > > This drops opcode from the ctx in favour if passing the appropriately > sized opcode down a few levels of the decode. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > > --- > v2 > - use extract16 for uint16_t opcodes > > squash! target/riscv: progressively load the instruction during decode > --- > target/riscv/instmap.h | 8 ++++---- > target/riscv/translate.c | 40 +++++++++++++++++++++------------------- > 2 files changed, 25 insertions(+), 23 deletions(-) > > diff --git a/target/riscv/instmap.h b/target/riscv/instmap.h > index f8ad7d60fd5..40b6d2b64de 100644 > --- a/target/riscv/instmap.h > +++ b/target/riscv/instmap.h > @@ -344,8 +344,8 @@ enum { > #define GET_C_LW_IMM(inst) ((extract32(inst, 6, 1) << 2) \ > | (extract32(inst, 10, 3) << 3) \ > | (extract32(inst, 5, 1) << 6)) > -#define GET_C_LD_IMM(inst) ((extract32(inst, 10, 3) << 3) \ > - | (extract32(inst, 5, 2) << 6)) > +#define GET_C_LD_IMM(inst) ((extract16(inst, 10, 3) << 3) \ > + | (extract16(inst, 5, 2) << 6)) > #define GET_C_J_IMM(inst) ((extract32(inst, 3, 3) << 1) \ > | (extract32(inst, 11, 1) << 4) \ > | (extract32(inst, 2, 1) << 5) \ > @@ -363,7 +363,7 @@ enum { > #define GET_C_RD(inst) GET_RD(inst) > #define GET_C_RS1(inst) GET_RD(inst) > #define GET_C_RS2(inst) extract32(inst, 2, 5) > -#define GET_C_RS1S(inst) (8 + extract32(inst, 7, 3)) > -#define GET_C_RS2S(inst) (8 + extract32(inst, 2, 3)) > +#define GET_C_RS1S(inst) (8 + extract16(inst, 7, 3)) > +#define GET_C_RS2S(inst) (8 + extract16(inst, 2, 3)) > > #endif > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index 14dc71156be..d5de7f468a7 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -44,7 +44,6 @@ typedef struct DisasContext { > /* pc_succ_insn points to the instruction following base.pc_next */ > target_ulong pc_succ_insn; > target_ulong priv_ver; > - uint32_t opcode; > uint32_t mstatus_fs; > uint32_t misa; > uint32_t mem_idx; > @@ -492,45 +491,45 @@ static void gen_set_rm(DisasContext *ctx, int rm) > tcg_temp_free_i32(t0); > } > > -static void decode_RV32_64C0(DisasContext *ctx) > +static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode) > { > - uint8_t funct3 = extract32(ctx->opcode, 13, 3); > - uint8_t rd_rs2 = GET_C_RS2S(ctx->opcode); > - uint8_t rs1s = GET_C_RS1S(ctx->opcode); > + uint8_t funct3 = extract16(opcode, 13, 3); > + uint8_t rd_rs2 = GET_C_RS2S(opcode); > + uint8_t rs1s = GET_C_RS1S(opcode); > > switch (funct3) { > case 3: > #if defined(TARGET_RISCV64) > /* C.LD(RV64/128) -> ld rd', offset[7:3](rs1')*/ > gen_load_c(ctx, OPC_RISC_LD, rd_rs2, rs1s, > - GET_C_LD_IMM(ctx->opcode)); > + GET_C_LD_IMM(opcode)); > #else > /* C.FLW (RV32) -> flw rd', offset[6:2](rs1')*/ > gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s, > - GET_C_LW_IMM(ctx->opcode)); > + GET_C_LW_IMM(opcode)); > #endif > break; > case 7: > #if defined(TARGET_RISCV64) > /* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/ > gen_store_c(ctx, OPC_RISC_SD, rs1s, rd_rs2, > - GET_C_LD_IMM(ctx->opcode)); > + GET_C_LD_IMM(opcode)); > #else > /* C.FSW (RV32) -> fsw rs2', offset[6:2](rs1')*/ > gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2, > - GET_C_LW_IMM(ctx->opcode)); > + GET_C_LW_IMM(opcode)); > #endif > break; > } > } > > -static void decode_RV32_64C(DisasContext *ctx) > +static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode) > { > - uint8_t op = extract32(ctx->opcode, 0, 2); > + uint8_t op = extract16(opcode, 0, 2); > > switch (op) { > case 0: > - decode_RV32_64C0(ctx); > + decode_RV32_64C0(ctx, opcode); > break; > } > } > @@ -709,22 +708,25 @@ static bool gen_shift(DisasContext *ctx, arg_r *a, > /* Include the auto-generated decoder for 16 bit insn */ > #include "decode_insn16.inc.c" > > -static void decode_opc(DisasContext *ctx) > +static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) > { > /* check for compressed insn */ > - if (extract32(ctx->opcode, 0, 2) != 3) { > + if (extract16(opcode, 0, 2) != 3) { > if (!has_ext(ctx, RVC)) { > gen_exception_illegal(ctx); > } else { > ctx->pc_succ_insn = ctx->base.pc_next + 2; > - if (!decode_insn16(ctx, ctx->opcode)) { > + if (!decode_insn16(ctx, opcode)) { > /* fall back to old decoder */ > - decode_RV32_64C(ctx); > + decode_RV32_64C(ctx, opcode); > } > } > } else { > + uint32_t opcode32 = opcode; > + opcode32 = deposit32(opcode32, 16, 16, > + translator_lduw(env, ctx->base.pc_next + 2)); > ctx->pc_succ_insn = ctx->base.pc_next + 4; > - if (!decode_insn32(ctx, ctx->opcode)) { > + if (!decode_insn32(ctx, opcode32)) { > gen_exception_illegal(ctx); > } > } > @@ -776,9 +778,9 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) > { > DisasContext *ctx = container_of(dcbase, DisasContext, base); > CPURISCVState *env = cpu->env_ptr; > + uint16_t opcode16 = translator_lduw(env, ctx->base.pc_next); > > - ctx->opcode = translator_ldl(env, ctx->base.pc_next); > - decode_opc(ctx); > + decode_opc(env, ctx, opcode16); > ctx->base.pc_next = ctx->pc_succ_insn; > > if (ctx->base.is_jmp == DISAS_NEXT) { > -- > 2.20.1 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 14/19] target/riscv: progressively load the instruction during decode 2020-02-13 22:51 ` [PATCH v2 14/19] target/riscv: progressively load the instruction during decode Alex Bennée 2020-02-14 1:23 ` Alistair Francis @ 2020-02-14 19:34 ` Robert Foley 1 sibling, 0 replies; 3+ messages in thread From: Robert Foley @ 2020-02-14 19:34 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel, cota, aaron, Peter Puhov, kuhn.chenqun, robhenry, fam, berrange, f4bug, Richard Henderson, balrogg, aurelien, pbonzini, stefanha, stefanb, marcandre.lureau, Palmer Dabbelt, Alistair Francis, Sagar Karandikar, Bastian Koppelmann, open list:RISC-V TCG CPUs On Thu, 13 Feb 2020 at 18:00, Alex Bennée <alex.bennee@linaro.org> wrote: > > The plugin system would throw up a harmless warning when it detected > that a disassembly of an instruction didn't use all it's bytes. Fix > the riscv decoder to only load the instruction bytes it needs as it > needs them. > > This drops opcode from the ctx in favour if passing the appropriately > sized opcode down a few levels of the decode. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Robert Foley <robert.foley@linaro.org> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-14 19:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200213225109.13120-1-alex.bennee@linaro.org>
2020-02-13 22:51 ` [PATCH v2 14/19] target/riscv: progressively load the instruction during decode Alex Bennée
2020-02-14 1:23 ` Alistair Francis
2020-02-14 19:34 ` Robert Foley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).