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

[Alistair Francis]

On Tue, Feb 8, 2022 at 4:07 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Feb 2, 2022 at 11:26 AM Philipp Tomsich
> <philipp.tomsich@vrull.eu> 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 <philipp.tomsich@vrull.eu>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >
> > ---
> >
> > (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