Re: [PATCH v6 2/9] target/riscv: add support for Zca extension

[weiwei]

On 2022/11/29 07:06, Wilfred Mallawa wrote:

> On Mon, 2022-11-28 at 20:29 +0800, Weiwei Li wrote:
> > Modify the check for C extension to Zca (C implies Zca)
> >
> > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> >  target/riscv/translate.c                | 8 ++++++--
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
> > b/target/riscv/insn_trans/trans_rvi.c.inc
> > index 4496f21266..ef7c3002b0 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 (!has_ext(ctx, RVC)) {
> > +    if (!ctx->cfg_ptr->ext_zca) {
> >          TCGv t0 = tcg_temp_new();
> >  
> >          misaligned = gen_new_label();
> > @@ -178,7 +178,7 @@ static bool gen_branch(DisasContext *ctx, arg_b
> > *a, TCGCond cond)
> >  
> >      gen_set_label(l); /* branch taken */
> >  
> > -    if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3))
> > {
> > +    if (!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 2ab8772ebe..ee24b451e3 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -557,7 +557,7 @@ static void gen_jal(DisasContext *ctx, int rd,
> > target_ulong imm)
> >  
> >      /* check misaligned: */
> >      next_pc = ctx->base.pc_next + imm;
> > -    if (!has_ext(ctx, RVC)) {
> > +    if (!ctx->cfg_ptr->ext_zca) {
> >          if ((next_pc & 0x3) != 0) {
> >              gen_exception_inst_addr_mis(ctx);
> >              return;
> > @@ -1097,7 +1097,11 @@ static void decode_opc(CPURISCVState *env,
> > DisasContext *ctx, uint16_t opcode)
> >      ctx->virt_inst_excp = false;
> >      /* Check for compressed insn */
> >      if (insn_len(opcode) == 2) {
> > -        if (!has_ext(ctx, RVC)) {
> > +        /*
> > +         * Zca support all of the existing C extension, excluding
> > all
> > +         * compressed floating point loads and stores
> > +         */
> >       
> Look like a typo: *`supports` and *`C extensions`
>

Thanks a lot! 

Yeah,  it should be 'supports' here (and it's 'is' here in original Zc* 0.70.1 spec).

Maybe we can use the new description from newest spec here:

"The Zca extension is added as way to refer to instructions in the C extension that do not i

nclude the floating-point loads and stores."

By the way, why do you think it should be 'C extensions' ?

Regards,

Weiwei Li

> > +        if (!ctx->cfg_ptr->ext_zca) {
> >              gen_exception_illegal(ctx);
> >          } else {
> >              ctx->opcode = opcode;
> >       
> otherwise,
> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>
> Wilfred
>