mbox series

[v4,0/7] target/riscv: Add XVentanaCondOps and supporting infrastructure changes

Message ID 20220130235759.1378871-1-philipp.tomsich@vrull.eu (mailing list archive)
Headers show
Series target/riscv: Add XVentanaCondOps and supporting infrastructure changes | expand

Message

Philipp Tomsich Jan. 30, 2022, 11:57 p.m. UTC
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 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

Comments

Richard Henderson Jan. 31, 2022, 8:25 a.m. UTC | #1
On 1/31/22 10:57, Philipp Tomsich 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 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)


Pick up Reviewed-by tags where they're given.  Please go back and grab them from v3.


r~
Philipp Tomsich Jan. 31, 2022, 8:34 a.m. UTC | #2
On Mon, 31 Jan 2022 at 09:25, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/31/22 10:57, Philipp Tomsich 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 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)
>
>
> Pick up Reviewed-by tags where they're given.  Please go back and grab them from v3.


Thanks for spotting this.  Looks like patman picked those up only for
the first two patches.
I'll go back and add them by hand.

Philipp.