| Message ID | 20220604231004.49990-4-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Fix issue 1060 | expand |
On Sun, Jun 5, 2022 at 9:12 AM Richard Henderson <richard.henderson@linaro.org> wrote: > > The set of instructions that require decode_save_opc for > unwinding is really fairly small -- only insns that can > raise ILLEGAL_INSN at runtime. This includes CSR, anything > that uses a *new* fp rounding mode, and many privileged insns. > > Since unwind info is stored as the difference from the > previous insn, storing a 0 for most insns minimizes the > size of the unwind info. > > Booting a debian kernel image to the missing rootfs panic yields > > - gen code size 22226819/1026886656 > + gen code size 21601907/1026886656 > > on 41k TranslationBlocks, a savings of 610kB or a bit less than 3%. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/translate.c | 18 +++++++++--------- > target/riscv/insn_trans/trans_privileged.c.inc | 4 ++++ > target/riscv/insn_trans/trans_rvh.c.inc | 2 ++ > target/riscv/insn_trans/trans_rvi.c.inc | 2 ++ > 4 files changed, 17 insertions(+), 9 deletions(-) > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index 6e4bbea1cd..b328a7b2ff 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -204,6 +204,13 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in) > tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan); > } > > +static void decode_save_opc(DisasContext *ctx) > +{ > + assert(ctx->insn_start != NULL); > + tcg_set_insn_start_param(ctx->insn_start, 1, ctx->opcode); > + ctx->insn_start = NULL; > +} > + > static void gen_set_pc_imm(DisasContext *ctx, target_ulong dest) > { > if (get_xl(ctx) == MXL_RV32) { > @@ -633,6 +640,8 @@ static void gen_set_rm(DisasContext *ctx, int rm) > return; > } > > + /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ > + decode_save_opc(ctx); > gen_helper_set_rounding_mode(cpu_env, tcg_constant_i32(rm)); > } > > @@ -1011,13 +1020,6 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc) > /* Include decoders for factored-out extensions */ > #include "decode-XVentanaCondOps.c.inc" > > -static inline void decode_save_opc(DisasContext *ctx, target_ulong opc) > -{ > - assert(ctx->insn_start != NULL); > - tcg_set_insn_start_param(ctx->insn_start, 1, opc); > - ctx->insn_start = NULL; > -} > - > static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) > { > /* > @@ -1034,7 +1036,6 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) > > /* Check for compressed insn */ > if (extract16(opcode, 0, 2) != 3) { > - decode_save_opc(ctx, opcode); > if (!has_ext(ctx, RVC)) { > gen_exception_illegal(ctx); > } else { > @@ -1049,7 +1050,6 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) > opcode32 = deposit32(opcode32, 16, 16, > translator_lduw(env, &ctx->base, > ctx->base.pc_next + 2)); > - decode_save_opc(ctx, opcode32); > ctx->opcode = opcode32; > ctx->pc_succ_insn = ctx->base.pc_next + 4; > > diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc > index 53613682e8..46f96ad74d 100644 > --- a/target/riscv/insn_trans/trans_privileged.c.inc > +++ b/target/riscv/insn_trans/trans_privileged.c.inc > @@ -75,6 +75,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) > { > #ifndef CONFIG_USER_ONLY > if (has_ext(ctx, RVS)) { > + decode_save_opc(ctx); > gen_helper_sret(cpu_pc, cpu_env); > tcg_gen_exit_tb(NULL, 0); /* no chaining */ > ctx->base.is_jmp = DISAS_NORETURN; > @@ -90,6 +91,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) > static bool trans_mret(DisasContext *ctx, arg_mret *a) > { > #ifndef CONFIG_USER_ONLY > + decode_save_opc(ctx); > gen_helper_mret(cpu_pc, cpu_env); > tcg_gen_exit_tb(NULL, 0); /* no chaining */ > ctx->base.is_jmp = DISAS_NORETURN; > @@ -102,6 +104,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a) > static bool trans_wfi(DisasContext *ctx, arg_wfi *a) > { > #ifndef CONFIG_USER_ONLY > + decode_save_opc(ctx); > gen_set_pc_imm(ctx, ctx->pc_succ_insn); > gen_helper_wfi(cpu_env); > return true; > @@ -113,6 +116,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a) > static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a) > { > #ifndef CONFIG_USER_ONLY > + decode_save_opc(ctx); > gen_helper_tlb_flush(cpu_env); > return true; > #endif > diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc > index cebcb3f8f6..4f8aecddc7 100644 > --- a/target/riscv/insn_trans/trans_rvh.c.inc > +++ b/target/riscv/insn_trans/trans_rvh.c.inc > @@ -169,6 +169,7 @@ static bool trans_hfence_gvma(DisasContext *ctx, arg_sfence_vma *a) > { > REQUIRE_EXT(ctx, RVH); > #ifndef CONFIG_USER_ONLY > + decode_save_opc(ctx); > gen_helper_hyp_gvma_tlb_flush(cpu_env); > return true; > #endif > @@ -179,6 +180,7 @@ static bool trans_hfence_vvma(DisasContext *ctx, arg_sfence_vma *a) > { > REQUIRE_EXT(ctx, RVH); > #ifndef CONFIG_USER_ONLY > + decode_save_opc(ctx); > gen_helper_hyp_tlb_flush(cpu_env); > return true; > #endif > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > index f1342f30f8..cf17458022 100644 > --- a/target/riscv/insn_trans/trans_rvi.c.inc > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > @@ -822,6 +822,8 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a) > > static bool do_csr_post(DisasContext *ctx) > { > + /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ > + decode_save_opc(ctx); > /* We may have changed important cpu state -- exit to main loop. */ > gen_set_pc_imm(ctx, ctx->pc_succ_insn); > tcg_gen_exit_tb(NULL, 0); > -- > 2.34.1 > >
diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 6e4bbea1cd..b328a7b2ff 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -204,6 +204,13 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in) tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan); } +static void decode_save_opc(DisasContext *ctx) +{ + assert(ctx->insn_start != NULL); + tcg_set_insn_start_param(ctx->insn_start, 1, ctx->opcode); + ctx->insn_start = NULL; +} + static void gen_set_pc_imm(DisasContext *ctx, target_ulong dest) { if (get_xl(ctx) == MXL_RV32) { @@ -633,6 +640,8 @@ static void gen_set_rm(DisasContext *ctx, int rm) return; } + /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ + decode_save_opc(ctx); gen_helper_set_rounding_mode(cpu_env, tcg_constant_i32(rm)); } @@ -1011,13 +1020,6 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc) /* Include decoders for factored-out extensions */ #include "decode-XVentanaCondOps.c.inc" -static inline void decode_save_opc(DisasContext *ctx, target_ulong opc) -{ - assert(ctx->insn_start != NULL); - tcg_set_insn_start_param(ctx->insn_start, 1, opc); - ctx->insn_start = NULL; -} - static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) { /* @@ -1034,7 +1036,6 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) /* Check for compressed insn */ if (extract16(opcode, 0, 2) != 3) { - decode_save_opc(ctx, opcode); if (!has_ext(ctx, RVC)) { gen_exception_illegal(ctx); } else { @@ -1049,7 +1050,6 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) opcode32 = deposit32(opcode32, 16, 16, translator_lduw(env, &ctx->base, ctx->base.pc_next + 2)); - decode_save_opc(ctx, opcode32); ctx->opcode = opcode32; ctx->pc_succ_insn = ctx->base.pc_next + 4; diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc index 53613682e8..46f96ad74d 100644 --- a/target/riscv/insn_trans/trans_privileged.c.inc +++ b/target/riscv/insn_trans/trans_privileged.c.inc @@ -75,6 +75,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) { #ifndef CONFIG_USER_ONLY if (has_ext(ctx, RVS)) { + decode_save_opc(ctx); gen_helper_sret(cpu_pc, cpu_env); tcg_gen_exit_tb(NULL, 0); /* no chaining */ ctx->base.is_jmp = DISAS_NORETURN; @@ -90,6 +91,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a) static bool trans_mret(DisasContext *ctx, arg_mret *a) { #ifndef CONFIG_USER_ONLY + decode_save_opc(ctx); gen_helper_mret(cpu_pc, cpu_env); tcg_gen_exit_tb(NULL, 0); /* no chaining */ ctx->base.is_jmp = DISAS_NORETURN; @@ -102,6 +104,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a) static bool trans_wfi(DisasContext *ctx, arg_wfi *a) { #ifndef CONFIG_USER_ONLY + decode_save_opc(ctx); gen_set_pc_imm(ctx, ctx->pc_succ_insn); gen_helper_wfi(cpu_env); return true; @@ -113,6 +116,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a) static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a) { #ifndef CONFIG_USER_ONLY + decode_save_opc(ctx); gen_helper_tlb_flush(cpu_env); return true; #endif diff --git a/target/riscv/insn_trans/trans_rvh.c.inc b/target/riscv/insn_trans/trans_rvh.c.inc index cebcb3f8f6..4f8aecddc7 100644 --- a/target/riscv/insn_trans/trans_rvh.c.inc +++ b/target/riscv/insn_trans/trans_rvh.c.inc @@ -169,6 +169,7 @@ static bool trans_hfence_gvma(DisasContext *ctx, arg_sfence_vma *a) { REQUIRE_EXT(ctx, RVH); #ifndef CONFIG_USER_ONLY + decode_save_opc(ctx); gen_helper_hyp_gvma_tlb_flush(cpu_env); return true; #endif @@ -179,6 +180,7 @@ static bool trans_hfence_vvma(DisasContext *ctx, arg_sfence_vma *a) { REQUIRE_EXT(ctx, RVH); #ifndef CONFIG_USER_ONLY + decode_save_opc(ctx); gen_helper_hyp_tlb_flush(cpu_env); return true; #endif diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index f1342f30f8..cf17458022 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -822,6 +822,8 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a) static bool do_csr_post(DisasContext *ctx) { + /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ + decode_save_opc(ctx); /* We may have changed important cpu state -- exit to main loop. */ gen_set_pc_imm(ctx, ctx->pc_succ_insn); tcg_gen_exit_tb(NULL, 0);
The set of instructions that require decode_save_opc for unwinding is really fairly small -- only insns that can raise ILLEGAL_INSN at runtime. This includes CSR, anything that uses a *new* fp rounding mode, and many privileged insns. Since unwind info is stored as the difference from the previous insn, storing a 0 for most insns minimizes the size of the unwind info. Booting a debian kernel image to the missing rootfs panic yields - gen code size 22226819/1026886656 + gen code size 21601907/1026886656 on 41k TranslationBlocks, a savings of 610kB or a bit less than 3%. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/riscv/translate.c | 18 +++++++++--------- target/riscv/insn_trans/trans_privileged.c.inc | 4 ++++ target/riscv/insn_trans/trans_rvh.c.inc | 2 ++ target/riscv/insn_trans/trans_rvi.c.inc | 2 ++ 4 files changed, 17 insertions(+), 9 deletions(-)