Re: [PATCH v6 3/7] target/riscv: access configuration through cfg_ptr in DisasContext

2022-02-07 Thread Alistair Francis
On Tue, Feb 8, 2022 at 4:07 PM Alistair Francis  wrote:
>
> On Wed, Feb 2, 2022 at 11:26 AM Philipp Tomsich
>  wrote:
> >
> > The implementation in trans_{rvi,rvv,rvzfh}.c.inc accesses the shallow
> > copies (in DisasContext) of some of the elements available in the
> > RISCVCPUConfig structure.  This commit redirects accesses to use the
> > cfg_ptr copied into DisasContext and removes the shallow copies.
> >
> > Signed-off-by: Philipp Tomsich 
> > Reviewed-by: Alistair Francis 
> > Suggested-by: Richard Henderson 
> > Reviewed-by: Richard Henderson 
> >
> > ---
> >
> > (no changes since v3)
> >
> > Changes in v3:
> > - (new patch) test extension-availability through cfg_ptr in
> >   DisasContext, removing the fields that have been copied into
> >   DisasContext directly
> >
> >  target/riscv/insn_trans/trans_rvi.c.inc   |   2 +-
> >  target/riscv/insn_trans/trans_rvv.c.inc   | 104 +++---
> >  target/riscv/insn_trans/trans_rvzfh.c.inc |   4 +-
> >  target/riscv/translate.c  |  14 ---
> >  4 files changed, 55 insertions(+), 69 deletions(-)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> > b/target/riscv/insn_trans/trans_rvi.c.inc
> > index 3cd1b3f877..f1342f30f8 100644
> > --- a/target/riscv/insn_trans/trans_rvi.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> > @@ -806,7 +806,7 @@ static bool trans_fence(DisasContext *ctx, arg_fence *a)
> >
> >  static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
> >  {
> > -if (!ctx->ext_ifencei) {
> > +if (!ctx->cfg_ptr->ext_ifencei) {
> >  return false;
> >  }
> >
> > diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> > b/target/riscv/insn_trans/trans_rvv.c.inc
> > index f85a9e83b4..ff09e345ad 100644
> > --- a/target/riscv/insn_trans/trans_rvv.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> > @@ -74,7 +74,7 @@ static bool require_zve32f(DisasContext *s)
> >  }
> >
> >  /* Zve32f doesn't support FP64. (Section 18.2) */
> > -return s->ext_zve32f ? s->sew <= MO_32 : true;
> > +return s->cfg_ptr->ext_zve32f ? s->sew <= MO_32 : true;
> >  }
> >
> >  static bool require_scale_zve32f(DisasContext *s)
> > @@ -85,7 +85,7 @@ static bool require_scale_zve32f(DisasContext *s)
> >  }
> >
> >  /* Zve32f doesn't support FP64. (Section 18.2) */
> > -return s->ext_zve64f ? s->sew <= MO_16 : true;
> > +return s->cfg_ptr->ext_zve64f ? s->sew <= MO_16 : true;
> >  }
> >
> >  static bool require_zve64f(DisasContext *s)
> > @@ -96,7 +96,7 @@ static bool require_zve64f(DisasContext *s)
> >  }
> >
> >  /* Zve64f doesn't support FP64. (Section 18.2) */
> > -return s->ext_zve64f ? s->sew <= MO_32 : true;
> > +return s->cfg_ptr->ext_zve64f ? s->sew <= MO_32 : true;
> >  }
> >
> >  static bool require_scale_zve64f(DisasContext *s)
> > @@ -107,7 +107,7 @@ static bool require_scale_zve64f(DisasContext *s)
> >  }
> >
> >  /* Zve64f doesn't support FP64. (Section 18.2) */
> > -return s->ext_zve64f ? s->sew <= MO_16 : true;
> > +return s->cfg_ptr->ext_zve64f ? s->sew <= MO_16 : true;
> >  }
> >
> >  /* Destination vector register group cannot overlap source mask register. 
> > */
> > @@ -174,7 +174,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, 
> > TCGv s2)
> >  TCGv s1, dst;
> >
> >  if (!require_rvv(s) ||
> > -!(has_ext(s, RVV) || s->ext_zve32f || s->ext_zve64f)) {
> > +!(has_ext(s, RVV) || s->cfg_ptr->ext_zve32f || 
> > s->cfg_ptr->ext_zve64f)) {
>
> This fails checkpatch as the line is too long
>
> Can you run checkpatch on the series and re-send it?

Argh, there are too many patches depending on this!

Don't worry about resending it, I'll fixup the failures (assuming that's ok).

Alistair

>
> Alistair



Re: [PATCH v6 3/7] target/riscv: access configuration through cfg_ptr in DisasContext

2022-02-07 Thread Alistair Francis
On Wed, Feb 2, 2022 at 11:26 AM Philipp Tomsich
 wrote:
>
> The implementation in trans_{rvi,rvv,rvzfh}.c.inc accesses the shallow
> copies (in DisasContext) of some of the elements available in the
> RISCVCPUConfig structure.  This commit redirects accesses to use the
> cfg_ptr copied into DisasContext and removes the shallow copies.
>
> Signed-off-by: Philipp Tomsich 
> Reviewed-by: Alistair Francis 
> Suggested-by: Richard Henderson 
> Reviewed-by: Richard Henderson 
>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - (new patch) test extension-availability through cfg_ptr in
>   DisasContext, removing the fields that have been copied into
>   DisasContext directly
>
>  target/riscv/insn_trans/trans_rvi.c.inc   |   2 +-
>  target/riscv/insn_trans/trans_rvv.c.inc   | 104 +++---
>  target/riscv/insn_trans/trans_rvzfh.c.inc |   4 +-
>  target/riscv/translate.c  |  14 ---
>  4 files changed, 55 insertions(+), 69 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index 3cd1b3f877..f1342f30f8 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -806,7 +806,7 @@ static bool trans_fence(DisasContext *ctx, arg_fence *a)
>
>  static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
>  {
> -if (!ctx->ext_ifencei) {
> +if (!ctx->cfg_ptr->ext_ifencei) {
>  return false;
>  }
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index f85a9e83b4..ff09e345ad 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -74,7 +74,7 @@ static bool require_zve32f(DisasContext *s)
>  }
>
>  /* Zve32f doesn't support FP64. (Section 18.2) */
> -return s->ext_zve32f ? s->sew <= MO_32 : true;
> +return s->cfg_ptr->ext_zve32f ? s->sew <= MO_32 : true;
>  }
>
>  static bool require_scale_zve32f(DisasContext *s)
> @@ -85,7 +85,7 @@ static bool require_scale_zve32f(DisasContext *s)
>  }
>
>  /* Zve32f doesn't support FP64. (Section 18.2) */
> -return s->ext_zve64f ? s->sew <= MO_16 : true;
> +return s->cfg_ptr->ext_zve64f ? s->sew <= MO_16 : true;
>  }
>
>  static bool require_zve64f(DisasContext *s)
> @@ -96,7 +96,7 @@ static bool require_zve64f(DisasContext *s)
>  }
>
>  /* Zve64f doesn't support FP64. (Section 18.2) */
> -return s->ext_zve64f ? s->sew <= MO_32 : true;
> +return s->cfg_ptr->ext_zve64f ? s->sew <= MO_32 : true;
>  }
>
>  static bool require_scale_zve64f(DisasContext *s)
> @@ -107,7 +107,7 @@ static bool require_scale_zve64f(DisasContext *s)
>  }
>
>  /* Zve64f doesn't support FP64. (Section 18.2) */
> -return s->ext_zve64f ? s->sew <= MO_16 : true;
> +return s->cfg_ptr->ext_zve64f ? s->sew <= MO_16 : true;
>  }
>
>  /* Destination vector register group cannot overlap source mask register. */
> @@ -174,7 +174,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, 
> TCGv s2)
>  TCGv s1, dst;
>
>  if (!require_rvv(s) ||
> -!(has_ext(s, RVV) || s->ext_zve32f || s->ext_zve64f)) {
> +!(has_ext(s, RVV) || s->cfg_ptr->ext_zve32f || 
> s->cfg_ptr->ext_zve64f)) {

This fails checkpatch as the line is too long

Can you run checkpatch on the series and re-send it?

Alistair