| Message ID | 20220407183351.295188-1-patrick@rivosinc.com |
|---|---|
| Headers | show |
| Series | RISCV: Implement ISA Manual Table A.6 Mappings | expand |
The litmus test in this RFC is flawed since it does not assert that the
LR/SC pair succeeds. The condition in the RFC is permitted iff the LR/SC
pair fails. After correcting this flaw [1][2], the litmus test condition
is correctly forbidden.
This correction does not mean that the A.6 mapping is guaranteed to be
fully compatible with the current GCC/LLVM mapping. This just means that
this particular case does not appear to be an issue.
Thanks,
Patrick
[1] Corrected herd7 litmus test:
RISCV W-RMW
{
0:x7=A; 0:x8=B; 0:x1=1;
1:x7=A; 1:x8=B; 1:x1=1;
}
P0 | P1 ;
fence rw,w | fence rw,rw ;
sw x1,0(x8) | sw x1,0(x7) ;
lr.w.aq x3,0(x7) | fence rw,rw ;
sc.w.rl x2,x1,0(x7) | lw x2,0(x8) ;
~exists (0:x2=0 /\ 0:x3=0 /\ 1:x2=0)
[2] Corrected herd7 litmus test (with retry-loop):
RISCV W-RMW
{
0:x7=A; 0:x8=B; 0:x1=1;
1:x7=A; 1:x8=B; 1:x1=1;
}
P0 | P1 ;
fence rw,w | sw x1,0(x7) ;
sw x1,0(x8) | fence rw,rw ;
LC00: | lw x2,0(x8) ;
lr.w.aq x3,0(x7) | ;
sc.w.rl x2,x1,0(x7) | ;
bne x2,x0,LC00 | ;
~exists (0:x2=0 /\ 0:x3=0 /\ 1:x2=0)
This series should not be applied as it causes an ABI break. This RFC aims to bring the RISCV atomics implementation in line with 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 This mapping being implemented would result in an ABI break due to libatomic's LR.aq/SC.rl mapping and the A.6's SEQ_CST store mapping not enforcing SEQ_CST when placed next to eachother. This can be seen using the following Herd7 litmus test: RISCV W-RMW (* Seq_cst store along with LR.aq/SC.rl is insufficient for a seq_cst store, seq_cst RMW mapping. *) { 0:x7=A; 0:x8=B; 1:x7=A; 1:x8=B; } P0 | P1 ; ori x1,x0,1 | ori x1,x0,1 ; fence rw,w | fence rw,rw ; sw x1,0(x8) | sw x1,0(x7) ; lr.w.aq x3,0(x7) | fence rw,rw ; sc.w.rl x1,x1,0(x7) | lw x2,0(x8) ; exists (0:x3=0 /\ 1:x2=0) In GCC for SEQ_CST store, we currently emit fence iorw,ow + amoswap.aq, which successfully enforces ordering for the given litmus test. This will only be a problem in GCC if we move the SEQ_CST store to the A.6 mapping. Note: LLVM implements fence rw,w + sw https://godbolt.org/z/n68P7ne1W That means that LLVM isn't compatible with libatomic's LR.aq/SC.rl. * PR target/89835: The RISC-V target uses amoswap.w for relaxed stores Patrick O'Neill (7): RISCV: Enforce Libatomic LR/SC SEQ_CST RISCV: Enforce Atomic Compare Exchange SEQ_CST RISCV: Add AMO release bits RISCV: Optimize AMO Ops RISCV: Optimize LR/SC Pairs RISCV: Optimize Atomic Stores RISCV: Relax mem_thread_fence gcc/config/riscv/riscv-protos.h | 6 ++ gcc/config/riscv/riscv.cc | 93 +++++++++++++++++-- gcc/config/riscv/sync.md | 46 ++++++--- .../gcc.target/riscv/amo-thread-fence-1.c | 6 ++ .../gcc.target/riscv/amo-thread-fence-2.c | 6 ++ .../gcc.target/riscv/amo-thread-fence-3.c | 6 ++ .../gcc.target/riscv/amo-thread-fence-4.c | 6 ++ .../gcc.target/riscv/amo-thread-fence-5.c | 6 ++ .../gcc.target/riscv/inline-atomics-model-1.c | 12 +++ .../gcc.target/riscv/inline-atomics-model-2.c | 12 +++ .../gcc.target/riscv/inline-atomics-model-3.c | 12 +++ .../gcc.target/riscv/inline-atomics-model-4.c | 12 +++ .../gcc.target/riscv/inline-atomics-model-5.c | 12 +++ gcc/testsuite/gcc.target/riscv/pr89835.c | 9 ++ libgcc/config/riscv/atomic.c | 4 +- 15 files changed, 223 insertions(+), 25 deletions(-) create mode 100644 gcc/testsuite/gcc.target/riscv/amo-thread-fence-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/amo-thread-fence-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/amo-thread-fence-3.c create mode 100644 gcc/testsuite/gcc.target/riscv/amo-thread-fence-4.c create mode 100644 gcc/testsuite/gcc.target/riscv/amo-thread-fence-5.c create mode 100644 gcc/testsuite/gcc.target/riscv/inline-atomics-model-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/inline-atomics-model-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/inline-atomics-model-3.c create mode 100644 gcc/testsuite/gcc.target/riscv/inline-atomics-model-4.c create mode 100644 gcc/testsuite/gcc.target/riscv/inline-atomics-model-5.c create mode 100644 gcc/testsuite/gcc.target/riscv/pr89835.c