[PATCH v6 0/7] target/riscv: Add XVentanaCondOps and supporting infrastructure changes

Philipp Tomsich posted 7 patches 2 years, 3 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220202005249.3566542-1-philipp.tomsich@vrull.eu
Maintainers: Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Palmer Dabbelt <palmer@dabbelt.com>
MAINTAINERS                                   |   7 ++
target/riscv/XVentanaCondOps.decode           |  25 +++++
target/riscv/cpu.c                            |   3 +
target/riscv/cpu.h                            |  81 +++++++-------
target/riscv/insn_trans/trans_rvb.c.inc       |   8 +-
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 +-
.../insn_trans/trans_xventanacondops.c.inc    |  39 +++++++
target/riscv/meson.build                      |   1 +
target/riscv/translate.c                      |  60 ++++++----
11 files changed, 219 insertions(+), 115 deletions(-)
create mode 100644 target/riscv/XVentanaCondOps.decode
create mode 100644 target/riscv/insn_trans/trans_xventanacondops.c.inc
[PATCH v6 0/7] target/riscv: Add XVentanaCondOps and supporting infrastructure changes
Posted by Philipp Tomsich 2 years, 3 months ago
In adding our first X-extension (i.e., vendor-defined) on RISC-V with
XVentanaCondOps, we need to add a few instructure improvements to make
it easier to add similar vendor-defined extensions in the future:
- refactor access to the cfg->ext_* fields by making a pointer to the
  cfg structure (as cfg_ptr) available via DisasContext
- add a table-based list of decoders to invoke, each being guarded by
  a guard/predicate-function, that can be used to either add vendor
  extensions, large extensions or override (by listing the decoder
  before the one for standard extensions) patterns to handle errata


Changes in v6:
- add the 'vt' prefix to gen_condmask, renaming it to gen_vt_condmask

Changes in v5:
- use the typedef in DisasContext instead of the nakes struct
  for RISCVCPUConfig
- manually picked up those Reviewed-by tags from Richard that patman
  missed

Changes in v4:
- use a typedef into 'RISCVCPUConfig' (instead of the explicit
  'struct RISCVCPUConfig') to comply with the coding standard
  (as suggested in Richard's review of v3)
- add braces to comply with coding standard (as suggested by Richard)
- merge the two if-statements to reduce clutter after (now that the
  braces have been added)

Changes in v3:
- (new patch) refactor 'struct RISCVCPUConfig'
- (new patch) copy pointer to element cfg into DisasContext
- (new patch) test extension-availability through cfg_ptr in
  DisasContext, removing the fields that have been copied into
  DisasContext directly
- (new patch) change Zb[abcs] implementation to use cfg_ptr (copied
  into DisasContext) instead of going throuhg RISCV_CPU
- expose only the DisasContext* to predicate functions
- mark the table of decoder functions as static
- drop the inline from always_true_p, until the need arises (i.e.,
  someone finds a use for it and calls it directly)
- rewrite to drop the 'handled' temporary in iterating over the
  decoder table, removing the assignment in the condition of the if
- rename to trans_xventanacondops.c.inc (i.e. with the '.c')
- (in MATERIALISE_EXT_PREDICATE) don't annotate the predicate function
  for testing the availability of individual extensions as 'inline'
  and don't make CPURISCVState* visible to these predicate functions
- add a MAINTAINERS entry for XVentanaCondOps

Changes in v2:
- (new patch) iterate over a table of guarded decoder functions
- Split off decode table into XVentanaCondOps.decode
- Wire up XVentanaCondOps in the decoder-table

Philipp Tomsich (7):
  target/riscv: refactor (anonymous struct) RISCVCPU.cfg into 'struct
    RISCVCPUConfig'
  target/riscv: riscv_tr_init_disas_context: copy pointer-to-cfg into
    cfg_ptr
  target/riscv: access configuration through cfg_ptr in DisasContext
  target/riscv: access cfg structure through DisasContext
  target/riscv: iterate over a table of decoders
  target/riscv: Add XVentanaCondOps custom extension
  target/riscv: add a MAINTAINERS entry for XVentanaCondOps

 MAINTAINERS                                   |   7 ++
 target/riscv/XVentanaCondOps.decode           |  25 +++++
 target/riscv/cpu.c                            |   3 +
 target/riscv/cpu.h                            |  81 +++++++-------
 target/riscv/insn_trans/trans_rvb.c.inc       |   8 +-
 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 +-
 .../insn_trans/trans_xventanacondops.c.inc    |  39 +++++++
 target/riscv/meson.build                      |   1 +
 target/riscv/translate.c                      |  60 ++++++----
 11 files changed, 219 insertions(+), 115 deletions(-)
 create mode 100644 target/riscv/XVentanaCondOps.decode
 create mode 100644 target/riscv/insn_trans/trans_xventanacondops.c.inc

-- 
2.33.1


Re: [PATCH v6 0/7] target/riscv: Add XVentanaCondOps and supporting infrastructure changes
Posted by Alistair Francis 2 years, 3 months ago
On Wed, Feb 2, 2022 at 2:03 PM Philipp Tomsich <philipp.tomsich@vrull.eu> wrote:
>
>
> In adding our first X-extension (i.e., vendor-defined) on RISC-V with
> XVentanaCondOps, we need to add a few instructure improvements to make
> it easier to add similar vendor-defined extensions in the future:
> - refactor access to the cfg->ext_* fields by making a pointer to the
>   cfg structure (as cfg_ptr) available via DisasContext
> - add a table-based list of decoders to invoke, each being guarded by
>   a guard/predicate-function, that can be used to either add vendor
>   extensions, large extensions or override (by listing the decoder
>   before the one for standard extensions) patterns to handle errata
>
>
> Changes in v6:
> - add the 'vt' prefix to gen_condmask, renaming it to gen_vt_condmask
>
> Changes in v5:
> - use the typedef in DisasContext instead of the nakes struct
>   for RISCVCPUConfig
> - manually picked up those Reviewed-by tags from Richard that patman
>   missed
>
> Changes in v4:
> - use a typedef into 'RISCVCPUConfig' (instead of the explicit
>   'struct RISCVCPUConfig') to comply with the coding standard
>   (as suggested in Richard's review of v3)
> - add braces to comply with coding standard (as suggested by Richard)
> - merge the two if-statements to reduce clutter after (now that the
>   braces have been added)
>
> Changes in v3:
> - (new patch) refactor 'struct RISCVCPUConfig'
> - (new patch) copy pointer to element cfg into DisasContext
> - (new patch) test extension-availability through cfg_ptr in
>   DisasContext, removing the fields that have been copied into
>   DisasContext directly
> - (new patch) change Zb[abcs] implementation to use cfg_ptr (copied
>   into DisasContext) instead of going throuhg RISCV_CPU
> - expose only the DisasContext* to predicate functions
> - mark the table of decoder functions as static
> - drop the inline from always_true_p, until the need arises (i.e.,
>   someone finds a use for it and calls it directly)
> - rewrite to drop the 'handled' temporary in iterating over the
>   decoder table, removing the assignment in the condition of the if
> - rename to trans_xventanacondops.c.inc (i.e. with the '.c')
> - (in MATERIALISE_EXT_PREDICATE) don't annotate the predicate function
>   for testing the availability of individual extensions as 'inline'
>   and don't make CPURISCVState* visible to these predicate functions
> - add a MAINTAINERS entry for XVentanaCondOps
>
> Changes in v2:
> - (new patch) iterate over a table of guarded decoder functions
> - Split off decode table into XVentanaCondOps.decode
> - Wire up XVentanaCondOps in the decoder-table
>
> Philipp Tomsich (7):
>   target/riscv: refactor (anonymous struct) RISCVCPU.cfg into 'struct
>     RISCVCPUConfig'
>   target/riscv: riscv_tr_init_disas_context: copy pointer-to-cfg into
>     cfg_ptr
>   target/riscv: access configuration through cfg_ptr in DisasContext
>   target/riscv: access cfg structure through DisasContext
>   target/riscv: iterate over a table of decoders
>   target/riscv: Add XVentanaCondOps custom extension
>   target/riscv: add a MAINTAINERS entry for XVentanaCondOps

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  MAINTAINERS                                   |   7 ++
>  target/riscv/XVentanaCondOps.decode           |  25 +++++
>  target/riscv/cpu.c                            |   3 +
>  target/riscv/cpu.h                            |  81 +++++++-------
>  target/riscv/insn_trans/trans_rvb.c.inc       |   8 +-
>  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 +-
>  .../insn_trans/trans_xventanacondops.c.inc    |  39 +++++++
>  target/riscv/meson.build                      |   1 +
>  target/riscv/translate.c                      |  60 ++++++----
>  11 files changed, 219 insertions(+), 115 deletions(-)
>  create mode 100644 target/riscv/XVentanaCondOps.decode
>  create mode 100644 target/riscv/insn_trans/trans_xventanacondops.c.inc
>
> --
> 2.33.1
>
>

Re: [PATCH v6 0/7] target/riscv: Add XVentanaCondOps and supporting infrastructure changes
Posted by Philipp Tomsich 2 years, 2 months ago
Alistair,

there's a fix for a regression (inverted condition) caused by the changes
to the REQUIRE_ZB[ABCS] heading your way.

Thanks,
Philipp.

On Wed, 2 Feb 2022 at 07:36, Alistair Francis <alistair23@gmail.com> wrote:

> On Wed, Feb 2, 2022 at 2:03 PM Philipp Tomsich <philipp.tomsich@vrull.eu>
> wrote:
> >
> >
> > In adding our first X-extension (i.e., vendor-defined) on RISC-V with
> > XVentanaCondOps, we need to add a few instructure improvements to make
> > it easier to add similar vendor-defined extensions in the future:
> > - refactor access to the cfg->ext_* fields by making a pointer to the
> >   cfg structure (as cfg_ptr) available via DisasContext
> > - add a table-based list of decoders to invoke, each being guarded by
> >   a guard/predicate-function, that can be used to either add vendor
> >   extensions, large extensions or override (by listing the decoder
> >   before the one for standard extensions) patterns to handle errata
> >
> >
> > Changes in v6:
> > - add the 'vt' prefix to gen_condmask, renaming it to gen_vt_condmask
> >
> > Changes in v5:
> > - use the typedef in DisasContext instead of the nakes struct
> >   for RISCVCPUConfig
> > - manually picked up those Reviewed-by tags from Richard that patman
> >   missed
> >
> > Changes in v4:
> > - use a typedef into 'RISCVCPUConfig' (instead of the explicit
> >   'struct RISCVCPUConfig') to comply with the coding standard
> >   (as suggested in Richard's review of v3)
> > - add braces to comply with coding standard (as suggested by Richard)
> > - merge the two if-statements to reduce clutter after (now that the
> >   braces have been added)
> >
> > Changes in v3:
> > - (new patch) refactor 'struct RISCVCPUConfig'
> > - (new patch) copy pointer to element cfg into DisasContext
> > - (new patch) test extension-availability through cfg_ptr in
> >   DisasContext, removing the fields that have been copied into
> >   DisasContext directly
> > - (new patch) change Zb[abcs] implementation to use cfg_ptr (copied
> >   into DisasContext) instead of going throuhg RISCV_CPU
> > - expose only the DisasContext* to predicate functions
> > - mark the table of decoder functions as static
> > - drop the inline from always_true_p, until the need arises (i.e.,
> >   someone finds a use for it and calls it directly)
> > - rewrite to drop the 'handled' temporary in iterating over the
> >   decoder table, removing the assignment in the condition of the if
> > - rename to trans_xventanacondops.c.inc (i.e. with the '.c')
> > - (in MATERIALISE_EXT_PREDICATE) don't annotate the predicate function
> >   for testing the availability of individual extensions as 'inline'
> >   and don't make CPURISCVState* visible to these predicate functions
> > - add a MAINTAINERS entry for XVentanaCondOps
> >
> > Changes in v2:
> > - (new patch) iterate over a table of guarded decoder functions
> > - Split off decode table into XVentanaCondOps.decode
> > - Wire up XVentanaCondOps in the decoder-table
> >
> > Philipp Tomsich (7):
> >   target/riscv: refactor (anonymous struct) RISCVCPU.cfg into 'struct
> >     RISCVCPUConfig'
> >   target/riscv: riscv_tr_init_disas_context: copy pointer-to-cfg into
> >     cfg_ptr
> >   target/riscv: access configuration through cfg_ptr in DisasContext
> >   target/riscv: access cfg structure through DisasContext
> >   target/riscv: iterate over a table of decoders
> >   target/riscv: Add XVentanaCondOps custom extension
> >   target/riscv: add a MAINTAINERS entry for XVentanaCondOps
>
> Thanks!
>
> Applied to riscv-to-apply.next
>
> Alistair
>
> >
> >  MAINTAINERS                                   |   7 ++
> >  target/riscv/XVentanaCondOps.decode           |  25 +++++
> >  target/riscv/cpu.c                            |   3 +
> >  target/riscv/cpu.h                            |  81 +++++++-------
> >  target/riscv/insn_trans/trans_rvb.c.inc       |   8 +-
> >  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 +-
> >  .../insn_trans/trans_xventanacondops.c.inc    |  39 +++++++
> >  target/riscv/meson.build                      |   1 +
> >  target/riscv/translate.c                      |  60 ++++++----
> >  11 files changed, 219 insertions(+), 115 deletions(-)
> >  create mode 100644 target/riscv/XVentanaCondOps.decode
> >  create mode 100644 target/riscv/insn_trans/trans_xventanacondops.c.inc
> >
> > --
> > 2.33.1
> >
> >
>