| Message ID | 20220202005249.3566542-1-philipp.tomsich@vrull.eu (mailing list archive) |
|---|---|
| Headers | show |
| Series | target/riscv: Add XVentanaCondOps and supporting infrastructure changes | expand |
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 > >
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 > > > > >
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