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

2022-11-28 Thread Wilfred Mallawa
On Tue, 2022-11-29 at 09:38 +0800, weiwei wrote:
> 
> 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 
> > > Signed-off-by: Junqiang Wang 
> > > Reviewed-by: Richard Henderson 
> > > Reviewed-by: Alistair Francis 
> > > ---
> > >  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."
Yea, that sounds good!
> By the way, why do you think it should be 'C extensions' ?
Ah crap, sorry! I misread it, it looks correct.

Regards,
Wilfred

> Regards,
> Weiwei Li
>  
> >   
> > > +    if (!ctx->cfg_ptr->ext_zca) {
> > >  gen_exception_illegal(ctx);
> > >  } else {
> > >  ctx->opcode = opcode;
> > otherwise,
> > Reviewed-by: Wilfred Mallawa 
> > 
> > Wilfred
>  



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

2022-11-28 Thread 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
Signed-off-by: Junqiang Wang
Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
---
  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

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

2022-11-28 Thread Wilfred Mallawa
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 
> Signed-off-by: Junqiang Wang 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Alistair Francis 
> ---
>  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`
> +    if (!ctx->cfg_ptr->ext_zca) {
>  gen_exception_illegal(ctx);
>  } else {
>  ctx->opcode = opcode;

otherwise,
Reviewed-by: Wilfred Mallawa 

Wilfred


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

2022-11-28 Thread Weiwei Li
Modify the check for C extension to Zca (C implies Zca)

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 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
+ */
+if (!ctx->cfg_ptr->ext_zca) {
 gen_exception_illegal(ctx);
 } else {
 ctx->opcode = opcode;
-- 
2.25.1