| Message ID | 20230412030648.80470-1-liweiwei@iscas.ac.cn |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Update check for Zca/zcf/zcd | expand |
On 4/12/23 00:06, Weiwei Li wrote: > Even though Zca/Zcf/Zcd can be included by C/F/D, however, their priv > version is higher than the priv version of C/F/D. So if we use check > for them instead of check for C/F/D totally, it will trigger new > problem when we try to disable the extensions based on the configured > priv version. > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> And I'll fold it into the next version of "[PATCH v6 0/9] target/riscv: rework CPU extensions validation" to fix the sifive break I'm experiencing there. Thanks, Daniel > target/riscv/insn_trans/trans_rvd.c.inc | 12 +++++++----- > target/riscv/insn_trans/trans_rvf.c.inc | 14 ++++++++------ > target/riscv/insn_trans/trans_rvi.c.inc | 5 +++-- > target/riscv/translate.c | 5 +++-- > 4 files changed, 21 insertions(+), 15 deletions(-) > > diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc > index 2c51e01c40..f8d0ae48c7 100644 > --- a/target/riscv/insn_trans/trans_rvd.c.inc > +++ b/target/riscv/insn_trans/trans_rvd.c.inc > @@ -31,9 +31,11 @@ > } \ > } while (0) > > -#define REQUIRE_ZCD(ctx) do { \ > - if (!ctx->cfg_ptr->ext_zcd) { \ > - return false; \ > +#define REQUIRE_ZCD_OR_DC(ctx) do { \ > + if (!ctx->cfg_ptr->ext_zcd) { \ > + if(!has_ext(ctx, RVD) || !has_ext(ctx, RVC)) { \ > + return false; \ > + } \ > } \ > } while (0) > > @@ -67,13 +69,13 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a) > > static bool trans_c_fld(DisasContext *ctx, arg_fld *a) > { > - REQUIRE_ZCD(ctx); > + REQUIRE_ZCD_OR_DC(ctx); > return trans_fld(ctx, a); > } > > static bool trans_c_fsd(DisasContext *ctx, arg_fsd *a) > { > - REQUIRE_ZCD(ctx); > + REQUIRE_ZCD_OR_DC(ctx); > return trans_fsd(ctx, a); > } > > diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc > index 9e9fa2087a..58467eb409 100644 > --- a/target/riscv/insn_trans/trans_rvf.c.inc > +++ b/target/riscv/insn_trans/trans_rvf.c.inc > @@ -30,10 +30,12 @@ > } \ > } while (0) > > -#define REQUIRE_ZCF(ctx) do { \ > - if (!ctx->cfg_ptr->ext_zcf) { \ > - return false; \ > - } \ > +#define REQUIRE_ZCF_OR_FC(ctx) do { \ > + if (!ctx->cfg_ptr->ext_zcf) { \ > + if(!has_ext(ctx, RVF) || !has_ext(ctx, RVC)) { \ > + return false; \ > + } \ > + } \ > } while (0) > > static bool trans_flw(DisasContext *ctx, arg_flw *a) > @@ -69,13 +71,13 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a) > > static bool trans_c_flw(DisasContext *ctx, arg_flw *a) > { > - REQUIRE_ZCF(ctx); > + REQUIRE_ZCF_OR_FC(ctx); > return trans_flw(ctx, a); > } > > static bool trans_c_fsw(DisasContext *ctx, arg_fsw *a) > { > - REQUIRE_ZCF(ctx); > + REQUIRE_ZCF_OR_FC(ctx); > return trans_fsw(ctx, a); > } > > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > index c70c495fc5..e33f63bea1 100644 > --- a/target/riscv/insn_trans/trans_rvi.c.inc > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a) > tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2); > > gen_set_pc(ctx, cpu_pc); > - if (!ctx->cfg_ptr->ext_zca) { > + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { > TCGv t0 = tcg_temp_new(); > > misaligned = gen_new_label(); > @@ -169,7 +169,8 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond) > > gen_set_label(l); /* branch taken */ > > - if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) { > + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca && > + ((ctx->base.pc_next + a->imm) & 0x3)) { > /* misaligned */ > gen_exception_inst_addr_mis(ctx); > } else { > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index d0094922b6..661e29ab39 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -551,7 +551,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm) > > /* check misaligned: */ > next_pc = ctx->base.pc_next + imm; > - if (!ctx->cfg_ptr->ext_zca) { > + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { > if ((next_pc & 0x3) != 0) { > gen_exception_inst_addr_mis(ctx); > return; > @@ -1137,7 +1137,8 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) > * The Zca extension is added as way to refer to instructions in the C > * extension that do not include the floating-point loads and stores > */ > - if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) { > + if ((has_ext(ctx, RVC) || ctx->cfg_ptr->ext_zca) && > + decode_insn16(ctx, opcode)) { > return; > } > } else {
On 4/12/23 00:06, Weiwei Li wrote: > Even though Zca/Zcf/Zcd can be included by C/F/D, however, their priv > version is higher than the priv version of C/F/D. So if we use check > for them instead of check for C/F/D totally, it will trigger new > problem when we try to disable the extensions based on the configured > priv version. > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- Two things: - the patch fails checkpatch.pl. I fixed it in my tree, but in case the patch needs a new version entirely here's the error: v7-0005-target-riscv-Mask-the-implicitly-enabled-extensio.patch has no obvious style problems and is ready for submission. Checking v7-0006-target-riscv-Update-check-for-Zca-zcf-zcd.patch... ERROR: space required before the open parenthesis '(' #36: FILE: target/riscv/insn_trans/trans_rvd.c.inc:36: + if(!has_ext(ctx, RVD) || !has_ext(ctx, RVC)) { \ ERROR: space required before the open parenthesis '(' #72: FILE: target/riscv/insn_trans/trans_rvf.c.inc:35: + if(!has_ext(ctx, RVF) || !has_ext(ctx, RVC)) { \ - yesterday Richard sent the following review in the patch "[RFC PATCH 3/4] target/riscv: check smstateen fcsr flag": ==================================== > +#define REQUIRE_ZFINX_OR_F(ctx) do { \ > + if (!has_ext(ctx, RVF)) { \ > + if (!ctx->cfg_ptr->ext_zfinx) { \ > + return false; \ > + } \ > + smstateen_fcsr_check(ctx); \ > } \ > } while (0) As a matter of style, I strongly object to a *nested* macro returning from the calling function. These should all be changed to normal functions of the form if (!require_xyz(ctx) || !require_abc(ctx)) { return something; } etc. insn_trans/trans_rvv.c.inc is much much cleaner in this respect. ==================================== I believe his comment is also valid for this patch as well due to how REQUIRE_ZCD_OR_DC(ctx) and REQUIRE_ZCF_OR_FC(ctx) is implemented. Before re-sending this patch as is it's better to check with him now. Richard, does this patch use the nested macro style you strongly object? Thanks, Daniel > target/riscv/insn_trans/trans_rvd.c.inc | 12 +++++++----- > target/riscv/insn_trans/trans_rvf.c.inc | 14 ++++++++------ > target/riscv/insn_trans/trans_rvi.c.inc | 5 +++-- > target/riscv/translate.c | 5 +++-- > 4 files changed, 21 insertions(+), 15 deletions(-) > > diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc > index 2c51e01c40..f8d0ae48c7 100644 > --- a/target/riscv/insn_trans/trans_rvd.c.inc > +++ b/target/riscv/insn_trans/trans_rvd.c.inc > @@ -31,9 +31,11 @@ > } \ > } while (0) > > -#define REQUIRE_ZCD(ctx) do { \ > - if (!ctx->cfg_ptr->ext_zcd) { \ > - return false; \ > +#define REQUIRE_ZCD_OR_DC(ctx) do { \ > + if (!ctx->cfg_ptr->ext_zcd) { \ > + if(!has_ext(ctx, RVD) || !has_ext(ctx, RVC)) { \ > + return false; \ > + } \ > } \ > } while (0) > > @@ -67,13 +69,13 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a) > > static bool trans_c_fld(DisasContext *ctx, arg_fld *a) > { > - REQUIRE_ZCD(ctx); > + REQUIRE_ZCD_OR_DC(ctx); > return trans_fld(ctx, a); > } > > static bool trans_c_fsd(DisasContext *ctx, arg_fsd *a) > { > - REQUIRE_ZCD(ctx); > + REQUIRE_ZCD_OR_DC(ctx); > return trans_fsd(ctx, a); > } > > diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc > index 9e9fa2087a..58467eb409 100644 > --- a/target/riscv/insn_trans/trans_rvf.c.inc > +++ b/target/riscv/insn_trans/trans_rvf.c.inc > @@ -30,10 +30,12 @@ > } \ > } while (0) > > -#define REQUIRE_ZCF(ctx) do { \ > - if (!ctx->cfg_ptr->ext_zcf) { \ > - return false; \ > - } \ > +#define REQUIRE_ZCF_OR_FC(ctx) do { \ > + if (!ctx->cfg_ptr->ext_zcf) { \ > + if(!has_ext(ctx, RVF) || !has_ext(ctx, RVC)) { \ > + return false; \ > + } \ > + } \ > } while (0) > > static bool trans_flw(DisasContext *ctx, arg_flw *a) > @@ -69,13 +71,13 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a) > > static bool trans_c_flw(DisasContext *ctx, arg_flw *a) > { > - REQUIRE_ZCF(ctx); > + REQUIRE_ZCF_OR_FC(ctx); > return trans_flw(ctx, a); > } > > static bool trans_c_fsw(DisasContext *ctx, arg_fsw *a) > { > - REQUIRE_ZCF(ctx); > + REQUIRE_ZCF_OR_FC(ctx); > return trans_fsw(ctx, a); > } > > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc > index c70c495fc5..e33f63bea1 100644 > --- a/target/riscv/insn_trans/trans_rvi.c.inc > +++ b/target/riscv/insn_trans/trans_rvi.c.inc > @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a) > tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2); > > gen_set_pc(ctx, cpu_pc); > - if (!ctx->cfg_ptr->ext_zca) { > + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { > TCGv t0 = tcg_temp_new(); > > misaligned = gen_new_label(); > @@ -169,7 +169,8 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond) > > gen_set_label(l); /* branch taken */ > > - if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) { > + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca && > + ((ctx->base.pc_next + a->imm) & 0x3)) { > /* misaligned */ > gen_exception_inst_addr_mis(ctx); > } else { > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index d0094922b6..661e29ab39 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -551,7 +551,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm) > > /* check misaligned: */ > next_pc = ctx->base.pc_next + imm; > - if (!ctx->cfg_ptr->ext_zca) { > + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { > if ((next_pc & 0x3) != 0) { > gen_exception_inst_addr_mis(ctx); > return; > @@ -1137,7 +1137,8 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) > * The Zca extension is added as way to refer to instructions in the C > * extension that do not include the floating-point loads and stores > */ > - if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) { > + if ((has_ext(ctx, RVC) || ctx->cfg_ptr->ext_zca) && > + decode_insn16(ctx, opcode)) { > return; > } > } else {
On 2023/4/13 01:03, Daniel Henrique Barboza wrote: > > > On 4/12/23 00:06, Weiwei Li wrote: >> Even though Zca/Zcf/Zcd can be included by C/F/D, however, their priv >> version is higher than the priv version of C/F/D. So if we use check >> for them instead of check for C/F/D totally, it will trigger new >> problem when we try to disable the extensions based on the configured >> priv version. >> >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> >> --- > > Two things: > > - the patch fails checkpatch.pl. I fixed it in my tree, but in case > the patch > needs a new version entirely here's the error: > > v7-0005-target-riscv-Mask-the-implicitly-enabled-extensio.patch has no > obvious style problems and is ready for submission. > > Checking v7-0006-target-riscv-Update-check-for-Zca-zcf-zcd.patch... > ERROR: space required before the open parenthesis '(' > #36: FILE: target/riscv/insn_trans/trans_rvd.c.inc:36: > + if(!has_ext(ctx, RVD) || !has_ext(ctx, RVC)) { \ > > ERROR: space required before the open parenthesis '(' > #72: FILE: target/riscv/insn_trans/trans_rvf.c.inc:35: > + if(!has_ext(ctx, RVF) || !has_ext(ctx, RVC)) { \ > > Sorry. I forgot to run the checkpatch.pl. I'll fix it later. > > - yesterday Richard sent the following review in the patch "[RFC PATCH > 3/4] > target/riscv: check smstateen fcsr flag": > > > ==================================== > >> +#define REQUIRE_ZFINX_OR_F(ctx) do { \ >> + if (!has_ext(ctx, RVF)) { \ >> + if (!ctx->cfg_ptr->ext_zfinx) { \ >> + return false; \ >> + } \ >> + smstateen_fcsr_check(ctx); \ >> } \ >> } while (0) > > As a matter of style, I strongly object to a *nested* macro returning > from the calling function. These should all be changed to normal > functions of the form > > if (!require_xyz(ctx) || !require_abc(ctx)) { > return something; > } > > etc. insn_trans/trans_rvv.c.inc is much much cleaner in this respect. > > ==================================== > > I believe his comment is also valid for this patch as well due to how > REQUIRE_ZCD_OR_DC(ctx) and REQUIRE_ZCF_OR_FC(ctx) is implemented. Before > re-sending this patch as is it's better to check with him now. I think there is no nested macro in REQUIRE_ZCD_OR_DC(has_ext() is an inline function) Regards, Weiwei Li > > Richard, does this patch use the nested macro style you strongly object? > > > Thanks, > > > Daniel > > >> target/riscv/insn_trans/trans_rvd.c.inc | 12 +++++++----- >> target/riscv/insn_trans/trans_rvf.c.inc | 14 ++++++++------ >> target/riscv/insn_trans/trans_rvi.c.inc | 5 +++-- >> target/riscv/translate.c | 5 +++-- >> 4 files changed, 21 insertions(+), 15 deletions(-) >> >> diff --git a/target/riscv/insn_trans/trans_rvd.c.inc >> b/target/riscv/insn_trans/trans_rvd.c.inc >> index 2c51e01c40..f8d0ae48c7 100644 >> --- a/target/riscv/insn_trans/trans_rvd.c.inc >> +++ b/target/riscv/insn_trans/trans_rvd.c.inc >> @@ -31,9 +31,11 @@ >> } \ >> } while (0) >> -#define REQUIRE_ZCD(ctx) do { \ >> - if (!ctx->cfg_ptr->ext_zcd) { \ >> - return false; \ >> +#define REQUIRE_ZCD_OR_DC(ctx) do { \ >> + if (!ctx->cfg_ptr->ext_zcd) { \ >> + if(!has_ext(ctx, RVD) || !has_ext(ctx, RVC)) { \ >> + return false; \ >> + } \ >> } \ >> } while (0) >> @@ -67,13 +69,13 @@ static bool trans_fsd(DisasContext *ctx, >> arg_fsd *a) >> static bool trans_c_fld(DisasContext *ctx, arg_fld *a) >> { >> - REQUIRE_ZCD(ctx); >> + REQUIRE_ZCD_OR_DC(ctx); >> return trans_fld(ctx, a); >> } >> static bool trans_c_fsd(DisasContext *ctx, arg_fsd *a) >> { >> - REQUIRE_ZCD(ctx); >> + REQUIRE_ZCD_OR_DC(ctx); >> return trans_fsd(ctx, a); >> } >> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc >> b/target/riscv/insn_trans/trans_rvf.c.inc >> index 9e9fa2087a..58467eb409 100644 >> --- a/target/riscv/insn_trans/trans_rvf.c.inc >> +++ b/target/riscv/insn_trans/trans_rvf.c.inc >> @@ -30,10 +30,12 @@ >> } \ >> } while (0) >> -#define REQUIRE_ZCF(ctx) do { \ >> - if (!ctx->cfg_ptr->ext_zcf) { \ >> - return false; \ >> - } \ >> +#define REQUIRE_ZCF_OR_FC(ctx) do { \ >> + if (!ctx->cfg_ptr->ext_zcf) { \ >> + if(!has_ext(ctx, RVF) || !has_ext(ctx, RVC)) { \ >> + return false; \ >> + } \ >> + } \ >> } while (0) >> static bool trans_flw(DisasContext *ctx, arg_flw *a) >> @@ -69,13 +71,13 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a) >> static bool trans_c_flw(DisasContext *ctx, arg_flw *a) >> { >> - REQUIRE_ZCF(ctx); >> + REQUIRE_ZCF_OR_FC(ctx); >> return trans_flw(ctx, a); >> } >> static bool trans_c_fsw(DisasContext *ctx, arg_fsw *a) >> { >> - REQUIRE_ZCF(ctx); >> + REQUIRE_ZCF_OR_FC(ctx); >> return trans_fsw(ctx, a); >> } >> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc >> b/target/riscv/insn_trans/trans_rvi.c.inc >> index c70c495fc5..e33f63bea1 100644 >> --- a/target/riscv/insn_trans/trans_rvi.c.inc >> +++ b/target/riscv/insn_trans/trans_rvi.c.inc >> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a) >> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2); >> gen_set_pc(ctx, cpu_pc); >> - if (!ctx->cfg_ptr->ext_zca) { >> + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { >> TCGv t0 = tcg_temp_new(); >> misaligned = gen_new_label(); >> @@ -169,7 +169,8 @@ static bool gen_branch(DisasContext *ctx, arg_b >> *a, TCGCond cond) >> gen_set_label(l); /* branch taken */ >> - if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & >> 0x3)) { >> + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca && >> + ((ctx->base.pc_next + a->imm) & 0x3)) { >> /* misaligned */ >> gen_exception_inst_addr_mis(ctx); >> } else { >> diff --git a/target/riscv/translate.c b/target/riscv/translate.c >> index d0094922b6..661e29ab39 100644 >> --- a/target/riscv/translate.c >> +++ b/target/riscv/translate.c >> @@ -551,7 +551,7 @@ static void gen_jal(DisasContext *ctx, int rd, >> target_ulong imm) >> /* check misaligned: */ >> next_pc = ctx->base.pc_next + imm; >> - if (!ctx->cfg_ptr->ext_zca) { >> + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { >> if ((next_pc & 0x3) != 0) { >> gen_exception_inst_addr_mis(ctx); >> return; >> @@ -1137,7 +1137,8 @@ static void decode_opc(CPURISCVState *env, >> DisasContext *ctx, uint16_t opcode) >> * The Zca extension is added as way to refer to >> instructions in the C >> * extension that do not include the floating-point loads >> and stores >> */ >> - if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) { >> + if ((has_ext(ctx, RVC) || ctx->cfg_ptr->ext_zca) && >> + decode_insn16(ctx, opcode)) { >> return; >> } >> } else {
diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc index 2c51e01c40..f8d0ae48c7 100644 --- a/target/riscv/insn_trans/trans_rvd.c.inc +++ b/target/riscv/insn_trans/trans_rvd.c.inc @@ -31,9 +31,11 @@ } \ } while (0) -#define REQUIRE_ZCD(ctx) do { \ - if (!ctx->cfg_ptr->ext_zcd) { \ - return false; \ +#define REQUIRE_ZCD_OR_DC(ctx) do { \ + if (!ctx->cfg_ptr->ext_zcd) { \ + if(!has_ext(ctx, RVD) || !has_ext(ctx, RVC)) { \ + return false; \ + } \ } \ } while (0) @@ -67,13 +69,13 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a) static bool trans_c_fld(DisasContext *ctx, arg_fld *a) { - REQUIRE_ZCD(ctx); + REQUIRE_ZCD_OR_DC(ctx); return trans_fld(ctx, a); } static bool trans_c_fsd(DisasContext *ctx, arg_fsd *a) { - REQUIRE_ZCD(ctx); + REQUIRE_ZCD_OR_DC(ctx); return trans_fsd(ctx, a); } diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 9e9fa2087a..58467eb409 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -30,10 +30,12 @@ } \ } while (0) -#define REQUIRE_ZCF(ctx) do { \ - if (!ctx->cfg_ptr->ext_zcf) { \ - return false; \ - } \ +#define REQUIRE_ZCF_OR_FC(ctx) do { \ + if (!ctx->cfg_ptr->ext_zcf) { \ + if(!has_ext(ctx, RVF) || !has_ext(ctx, RVC)) { \ + return false; \ + } \ + } \ } while (0) static bool trans_flw(DisasContext *ctx, arg_flw *a) @@ -69,13 +71,13 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a) static bool trans_c_flw(DisasContext *ctx, arg_flw *a) { - REQUIRE_ZCF(ctx); + REQUIRE_ZCF_OR_FC(ctx); return trans_flw(ctx, a); } static bool trans_c_fsw(DisasContext *ctx, arg_fsw *a) { - REQUIRE_ZCF(ctx); + REQUIRE_ZCF_OR_FC(ctx); return trans_fsw(ctx, a); } diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc index c70c495fc5..e33f63bea1 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a) tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2); gen_set_pc(ctx, cpu_pc); - if (!ctx->cfg_ptr->ext_zca) { + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { TCGv t0 = tcg_temp_new(); misaligned = gen_new_label(); @@ -169,7 +169,8 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond cond) gen_set_label(l); /* branch taken */ - if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) { + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca && + ((ctx->base.pc_next + a->imm) & 0x3)) { /* misaligned */ gen_exception_inst_addr_mis(ctx); } else { diff --git a/target/riscv/translate.c b/target/riscv/translate.c index d0094922b6..661e29ab39 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -551,7 +551,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm) /* check misaligned: */ next_pc = ctx->base.pc_next + imm; - if (!ctx->cfg_ptr->ext_zca) { + if (!has_ext(ctx, RVC) && !ctx->cfg_ptr->ext_zca) { if ((next_pc & 0x3) != 0) { gen_exception_inst_addr_mis(ctx); return; @@ -1137,7 +1137,8 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) * The Zca extension is added as way to refer to instructions in the C * extension that do not include the floating-point loads and stores */ - if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) { + if ((has_ext(ctx, RVC) || ctx->cfg_ptr->ext_zca) && + decode_insn16(ctx, opcode)) { return; } } else {