Re: [PATCH v2 0/2] riscv: Adding support for XTHead(F)MemIdx
On Fri, Oct 20, 2023 at 4:33 PM Jeff Law wrote: > > > > On 10/20/23 03:53, Christoph Muellner wrote: > > From: Christoph Müllner > > > > This two patches add support for the XTheadMemIdx > > and XTheadFMemIdx ISA extensions, that support additional > > addressing modes. The extensions are implemented in a range > > of T-Head cores (e.g. C906, C910, C920) and are available > > on the market for quite some time. > > > > The ISA spec can be found here: > >https://github.com/T-head-Semi/thead-extension-spec > > > > An initial version of these patches has been sent a while ago. > > Jeff Law suggested to use INSNs instead of peepholes to let > > the combiner do the optimization. This is the major change > > that this patches have seen. > Did you happen to do any before/after testing? And if so, did using the > combiner help with discovery of these cases? I would expect it to have > done so, but it's always nice to have a confirmation. I had no doubt this would be equal or better, therefore I did not plan to do that. However, measuring this is not that hard, so I just did the exercise of forward-porting the peephole-based patchset (and all tiny fixes that the v2 has). I then built xalancbmk_r/peak (randomly selected) with both compilers and compared the number of indexed loads and indexed stores in the binary: v1: 3982 indexed loads / 2447 indexed stores v2: 4110 indexed loads (+3.2%) / 2476 indexed stores (+1.2%) So your suggestion indeed helps to discover additional cases. Thanks again for that! BR Christoph
Re: [PATCH v2 0/2] riscv: Adding support for XTHead(F)MemIdx
On 10/20/23 03:53, Christoph Muellner wrote: From: Christoph Müllner This two patches add support for the XTheadMemIdx and XTheadFMemIdx ISA extensions, that support additional addressing modes. The extensions are implemented in a range of T-Head cores (e.g. C906, C910, C920) and are available on the market for quite some time. The ISA spec can be found here: https://github.com/T-head-Semi/thead-extension-spec An initial version of these patches has been sent a while ago. Jeff Law suggested to use INSNs instead of peepholes to let the combiner do the optimization. This is the major change that this patches have seen. Did you happen to do any before/after testing? And if so, did using the combiner help with discovery of these cases? I would expect it to have done so, but it's always nice to have a confirmation. If not, no big deal from a review standpoint. Given I looked at these before, I'll take this small kit again. Jeff
[PATCH v2 0/2] riscv: Adding support for XTHead(F)MemIdx
From: Christoph Müllner This two patches add support for the XTheadMemIdx and XTheadFMemIdx ISA extensions, that support additional addressing modes. The extensions are implemented in a range of T-Head cores (e.g. C906, C910, C920) and are available on the market for quite some time. The ISA spec can be found here: https://github.com/T-head-Semi/thead-extension-spec An initial version of these patches has been sent a while ago. Jeff Law suggested to use INSNs instead of peepholes to let the combiner do the optimization. This is the major change that this patches have seen. Both patches come with their own tests and don't introduce any regressions for RV32 or RV64. Further the patches did not show any issues with SPEC CPU 2017 (base) using multiple combinations of XThead* extensions. The patches have been tested on QEMU and a C920-based machine. Changes in v2: * Convert peepholes to INSNs (let the combiner do the work) * Enable XTheadFMemIdx optimizations only if XTheadMemIdx is available (to address the case when GP regs are used in FP mode (e.g. (reg:DF a2)) * Add a insn_and_split (th_memidx_operand) to address the case when reload splits off the index calculation. Christoph Müllner (2): riscv: thead: Add support for the XTheadMemIdx ISA extension riscv: thead: Add support for the XTheadFMemIdx ISA extension gcc/config/riscv/constraints.md | 26 + gcc/config/riscv/riscv-protos.h | 29 + gcc/config/riscv/riscv.cc | 24 +- gcc/config/riscv/riscv.h | 6 +- gcc/config/riscv/riscv.md | 26 +- gcc/config/riscv/thead.cc | 485 ++ gcc/config/riscv/thead.md | 594 +- .../riscv/xtheadfmemidx-index-update.c| 20 + .../xtheadfmemidx-index-xtheadbb-update.c | 20 + .../riscv/xtheadfmemidx-index-xtheadbb.c | 22 + .../gcc.target/riscv/xtheadfmemidx-index.c| 22 + .../riscv/xtheadfmemidx-uindex-update.c | 20 + .../xtheadfmemidx-uindex-xtheadbb-update.c| 20 + .../riscv/xtheadfmemidx-uindex-xtheadbb.c | 24 + .../gcc.target/riscv/xtheadfmemidx-uindex.c | 25 + .../gcc.target/riscv/xtheadmemidx-helpers.h | 152 + .../riscv/xtheadmemidx-index-update.c | 27 + .../xtheadmemidx-index-xtheadbb-update.c | 27 + .../riscv/xtheadmemidx-index-xtheadbb.c | 36 ++ .../gcc.target/riscv/xtheadmemidx-index.c | 36 ++ .../riscv/xtheadmemidx-modify-xtheadbb.c | 74 +++ .../gcc.target/riscv/xtheadmemidx-modify.c| 74 +++ .../riscv/xtheadmemidx-uindex-update.c| 27 + .../xtheadmemidx-uindex-xtheadbb-update.c | 27 + .../riscv/xtheadmemidx-uindex-xtheadbb.c | 44 ++ .../gcc.target/riscv/xtheadmemidx-uindex.c| 44 ++ 26 files changed, 1917 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-index-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-index-xtheadbb-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-index-xtheadbb.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-index.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-uindex-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-uindex-xtheadbb-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-uindex-xtheadbb.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadfmemidx-uindex.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-helpers.h create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-index-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-index-xtheadbb-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-index-xtheadbb.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-index.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-modify-xtheadbb.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-modify.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-uindex-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-uindex-xtheadbb-update.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-uindex-xtheadbb.c create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmemidx-uindex.c -- 2.41.0
