mbox series

[v4,00/10] RISCV: Implement ISA Manual Table A.6 Mappings

Message ID 20230414170942.1695672-1-patrick@rivosinc.com
Headers show
Series RISCV: Implement ISA Manual Table A.6 Mappings | expand

Message

Patrick O'Neill April 14, 2023, 5:09 p.m. UTC
This patchset aims to make the RISCV atomics implementation stronger
than the recommended mapping present in table A.6 of the ISA manual.
  https://github.com/riscv/riscv-isa-manual/blob/c7cf84547b3aefacab5463add1734c1602b67a49/src/memory.tex#L1083-L1157

The current mapping in GCC is not internally consistent. Andrea Parri
pointed this out here along with a litmus test:
  https://inbox.sourceware.org/gcc-patches/Y1GbJuhcBFpPGJQ0@andrea/

As a result, we have an opportunity to jump straight to the A.6
implementation (meaning we will be compatible with LLVM's mappings which
are A.6). In light of a proposal by Hans Boehm and to avoid an ABI break
in the future, the mapping implemented is strictly stronger than the one
in table A.6 in order to be compatible with table A.7.
  https://lists.riscv.org/g/tech-unprivileged/topic/risc_v_memory_model_topics/92916241

This change incurs a performance penalty on SEQ_CST stores due to the
added trailing fence. Systems implementing table A.7 will have
significant performance gains relative to table A.6 and are expected to
be the standard memory model mapping in the RISCV ecosystem. Incurring
this added cost now will make it significantly easier for compiled
RISC-V binaries to transition to the A.7 memory model mapping.

If Hans' proposal is accepted, it makes sense to migrate to the mapping
recommended by table A.7. Since the stronger mapping in this patchset
(provided by Hans Boehm) appears to be compatible with both A.6 and A.7,
this transition should not result in an ABI break for GCC.

Patch 1 simplifies the memmodel to ignore MEMMODEL_SYNC_* cases (legacy
cases that aren't handled differently for RISC-V).
Patches 2-5 make the mappings strictly stronger.
Patches 5-9 weaken the mappings to be in line with table A.6 of the ISA
manual.
Patch 10 adds some basic conformance tests to ensure the implemented
mapping matches table A.6 with stronger SEQ_CST stores.

Christoph Muellner also submitted a similar patchset here:
  https://gcc.gnu.org/pipermail/gcc-patches/2022-May/595712.html
I used my previous patchset as a starting point since it was easier for 
me.

LLVM mapping notes:
* LLVM emits corresponding fences for atomic_signal_fence instructions.
  This seems to be an oversight since AFAIK atomic_signal_fence acts as
  a compiler directive. GCC does not emit any fences for
  atomic_signal_fence instructions.

Patchset v1:
  https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592950.html

Patchset v2:
  https://gcc.gnu.org/pipermail/gcc-patches/2023-April/615264.html

Patchset v3:
  https://gcc.gnu.org/pipermail/gcc-patches/2023-April/615431.html

Changes for v2:
* Use memmodel_base rather than a custom simplify_memmodel function
  (Inspired by Christoph Muellner's patch 1/9)
* Move instruction styling change from [v1 5/7] to [v2 3/8] to reduce
  [v2 6/8]'s complexity
* Eliminated %K flag for atomic store introduced in v1 in favor of
  if/else
* Rebase/test

Changes for v3:
* Use a trailing fence for atomic stores to be compatible with table A.7
* Emit an optimized fence r,rw following a SEQ_CST load
* Consolidate tests in [PATCH v3 10/10]
* Add tests for basic A.6 conformance

Changes for v4:
* Update cover letter to cover more of the reasoning behind moving to a
  compatability mapping
* Improve conformance testcases patch assertions and add new
  compare-exchange testcases

Patrick O'Neill (10):
  RISCV: Eliminate SYNC memory models
  RISCV: Enforce Libatomic LR/SC SEQ_CST
  RISCV: Enforce atomic compare_exchange SEQ_CST
  RISCV: Add AMO release bits
  RISCV: Strengthen atomic stores
  RISCV: Eliminate AMO op fences
  RISCV: Weaken compare_exchange LR/SC pairs
  RISCV: Weaken mem_thread_fence
  RISCV: Weaken atomic loads
  RISCV: Table A.6 conformance tests

 gcc/config/riscv/riscv-protos.h               |  3 +
 gcc/config/riscv/riscv.cc                     | 66 +++++++++++---
 gcc/config/riscv/sync.md                      | 89 ++++++++++++++++---
 .../riscv/amo-table-a-6-amo-add-1.c           |  8 ++
 .../riscv/amo-table-a-6-amo-add-2.c           |  8 ++
 .../riscv/amo-table-a-6-amo-add-3.c           |  8 ++
 .../riscv/amo-table-a-6-amo-add-4.c           |  8 ++
 .../riscv/amo-table-a-6-amo-add-5.c           |  8 ++
 .../riscv/amo-table-a-6-compare-exchange-1.c  | 10 +++
 .../riscv/amo-table-a-6-compare-exchange-2.c  | 10 +++
 .../riscv/amo-table-a-6-compare-exchange-3.c  | 10 +++
 .../riscv/amo-table-a-6-compare-exchange-4.c  | 10 +++
 .../riscv/amo-table-a-6-compare-exchange-5.c  | 10 +++
 .../riscv/amo-table-a-6-compare-exchange-6.c  | 11 +++
 .../riscv/amo-table-a-6-compare-exchange-7.c  | 10 +++
 .../gcc.target/riscv/amo-table-a-6-fence-1.c  |  8 ++
 .../gcc.target/riscv/amo-table-a-6-fence-2.c  | 10 +++
 .../gcc.target/riscv/amo-table-a-6-fence-3.c  | 10 +++
 .../gcc.target/riscv/amo-table-a-6-fence-4.c  | 10 +++
 .../gcc.target/riscv/amo-table-a-6-fence-5.c  | 10 +++
 .../gcc.target/riscv/amo-table-a-6-load-1.c   |  9 ++
 .../gcc.target/riscv/amo-table-a-6-load-2.c   | 11 +++
 .../gcc.target/riscv/amo-table-a-6-load-3.c   | 11 +++
 .../gcc.target/riscv/amo-table-a-6-store-1.c  |  9 ++
 .../gcc.target/riscv/amo-table-a-6-store-2.c  | 11 +++
 .../riscv/amo-table-a-6-store-compat-3.c      | 11 +++
 gcc/testsuite/gcc.target/riscv/pr89835.c      |  9 ++
 libgcc/config/riscv/atomic.c                  |  4 +-
 28 files changed, 362 insertions(+), 30 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-amo-add-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-amo-add-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-amo-add-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-amo-add-4.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-amo-add-5.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-compare-exchange-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-compare-exchange-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-compare-exchange-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-compare-exchange-4.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-compare-exchange-5.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-compare-exchange-6.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-compare-exchange-7.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-fence-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-fence-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-fence-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-fence-4.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-fence-5.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-load-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-load-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-load-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-store-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-store-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/amo-table-a-6-store-compat-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/pr89835.c