mbox series

[v2,0/2] riscv: Adding support for XTHead(F)MemIdx

Message ID 20231020095348.2455729-1-christoph.muellner@vrull.eu
Headers
Series riscv: Adding support for XTHead(F)MemIdx |

Message

Christoph Müllner Oct. 20, 2023, 9:53 a.m. UTC 
  From: Christoph Müllner <christoph.muellner@vrull.eu>

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&peak) 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

Comments

Jeff Law Oct. 20, 2023, 2:33 p.m. UTC | #1 
On 10/20/23 03:53, Christoph Muellner wrote:
> From: Christoph Müllner <christoph.muellner@vrull.eu>
> 
> 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
Christoph Müllner Oct. 20, 2023, 6:08 p.m. UTC | #2 
On Fri, Oct 20, 2023 at 4:33 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 10/20/23 03:53, Christoph Muellner wrote:
> > From: Christoph Müllner <christoph.muellner@vrull.eu>
> >
> > 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