[llvm] [clang] [RISCV] Add experimental support of Zaamo and Zalrsc (PR #77424)
https://github.com/wangpc-pp created https://github.com/llvm/llvm-project/pull/77424 `A` extension has been split into two parts: Zaamo (Atomic Memory Operations) and Zalrsc (Load-Reserved/Store-Conditional). See also https://github.com/riscv/riscv-zaamo-zalrsc. This patch adds the basic compiler support. Tests for `A` extension are reused. >From a16ff19b7666060db74bebd4cb7e7132776285a7 Mon Sep 17 00:00:00 2001 From: wangpc Date: Tue, 9 Jan 2024 16:12:40 +0800 Subject: [PATCH] [RISCV] Add experimental support of Zaamo and Zalrsc `A` extension has been split into two parts: Zaamo (Atomic Memory Operations) and Zalrsc (Load-Reserved/Store-Conditional). See also https://github.com/riscv/riscv-zaamo-zalrsc. This patch adds the basic compiler support. Tests for `A` extension are reused. --- clang/lib/Basic/Targets/RISCV.cpp | 2 +- .../test/Preprocessor/riscv-target-features.c | 19 +++ llvm/docs/RISCVUsage.rst | 2 + llvm/lib/Support/RISCVISAInfo.cpp | 2 + llvm/lib/Target/RISCV/RISCVFeatures.td| 26 ++- llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 7 +- llvm/lib/Target/RISCV/RISCVInstrInfoA.td | 32 ++-- .../RISCV/atomic-cmpxchg-branch-on-result.ll | 4 + .../test/CodeGen/RISCV/atomic-cmpxchg-flag.ll | 2 + llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll | 8 + llvm/test/CodeGen/RISCV/atomic-rmw-discard.ll | 4 + llvm/test/CodeGen/RISCV/atomic-rmw-sub.ll | 4 + llvm/test/CodeGen/RISCV/atomic-rmw.ll | 8 + llvm/test/CodeGen/RISCV/atomic-signext.ll | 4 + .../CodeGen/RISCV/atomicrmw-uinc-udec-wrap.ll | 4 + llvm/test/CodeGen/RISCV/attributes.ll | 8 + llvm/test/MC/RISCV/rv32i-invalid.s| 2 +- llvm/test/MC/RISCV/rv32zaamo-invalid.s| 11 ++ llvm/test/MC/RISCV/rv32zaamo-valid.s | 122 ++ llvm/test/MC/RISCV/rv32zalrsc-invalid.s | 7 + llvm/test/MC/RISCV/rv32zalrsc-valid.s | 36 llvm/test/MC/RISCV/rv64zaamo-invalid.s| 11 ++ llvm/test/MC/RISCV/rv64zaamo-valid.s | 157 ++ llvm/test/MC/RISCV/rv64zalrsc-invalid.s | 7 + llvm/test/MC/RISCV/rv64zalrsc-valid.s | 42 + llvm/unittests/Support/RISCVISAInfoTest.cpp | 2 + 26 files changed, 515 insertions(+), 18 deletions(-) create mode 100644 llvm/test/MC/RISCV/rv32zaamo-invalid.s create mode 100644 llvm/test/MC/RISCV/rv32zaamo-valid.s create mode 100644 llvm/test/MC/RISCV/rv32zalrsc-invalid.s create mode 100644 llvm/test/MC/RISCV/rv32zalrsc-valid.s create mode 100644 llvm/test/MC/RISCV/rv64zaamo-invalid.s create mode 100644 llvm/test/MC/RISCV/rv64zaamo-valid.s create mode 100644 llvm/test/MC/RISCV/rv64zalrsc-invalid.s create mode 100644 llvm/test/MC/RISCV/rv64zalrsc-valid.s diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp index daaa8639ae8358..7aff435b715ca1 100644 --- a/clang/lib/Basic/Targets/RISCV.cpp +++ b/clang/lib/Basic/Targets/RISCV.cpp @@ -176,7 +176,7 @@ void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts, Builder.defineMacro("__riscv_muldiv"); } - if (ISAInfo->hasExtension("a")) { + if (ISAInfo->hasExtension("a") || ISAInfo->hasExtension("zaamo")) { Builder.defineMacro("__riscv_atomic"); Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1"); Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2"); diff --git a/clang/test/Preprocessor/riscv-target-features.c b/clang/test/Preprocessor/riscv-target-features.c index 02d8d34116f804..69ba912880f800 100644 --- a/clang/test/Preprocessor/riscv-target-features.c +++ b/clang/test/Preprocessor/riscv-target-features.c @@ -114,7 +114,9 @@ // CHECK-NOT: __riscv_smaia {{.*$}} // CHECK-NOT: __riscv_ssaia {{.*$}} +// CHECK-NOT: __riscv_zaamo {{.*$}} // CHECK-NOT: __riscv_zacas {{.*$}} +// CHECK-NOT: __riscv_zalrsc {{.*$}} // CHECK-NOT: __riscv_zfa {{.*$}} // CHECK-NOT: __riscv_zfbfmin {{.*$}} // CHECK-NOT: __riscv_zicfilp {{.*$}} @@ -1025,6 +1027,15 @@ // RUN: -o - | FileCheck --check-prefix=CHECK-SSAIA-EXT %s // CHECK-SSAIA-EXT: __riscv_ssaia 100{{$}} +// RUN: %clang --target=riscv32 -menable-experimental-extensions \ +// RUN: -march=rv32i_zaamo0p1 -x c -E -dM %s \ +// RUN: -o - | FileCheck --check-prefix=CHECK-ZAAMO-EXT %s +// RUN: %clang --target=riscv64 -menable-experimental-extensions \ +// RUN: -march=rv64i_zaamo0p1 -x c -E -dM %s \ +// RUN: -o - | FileCheck --check-prefix=CHECK-ZAAMO-EXT %s +// CHECK-ZAAMO-EXT: __riscv_atomic 1 +// CHECK-ZAAMO-EXT: __riscv_zaamo 1000{{$}} + // RUN: %clang --target=riscv32 -menable-experimental-extensions \ // RUN: -march=rv32i_zacas1p0 -x c -E -dM %s \ // RUN: -o - | FileCheck --check-prefix=CHECK-ZACAS-EXT %s @@ -1033,6 +1044,14 @@ // RUN: -o - | FileCheck --check-prefix=CHECK-ZACAS-EXT %s // CHECK-ZACAS-EXT: __riscv_zacas 100{{$}} +// RUN: %clang --target=riscv32 -menable-experimental-extensions \ +// RUN: -march=rv32i_zalrsc0p1
[llvm] [clang] [RISCV] Add experimental support of Zaamo and Zalrsc (PR #77424)
@@ -0,0 +1,11 @@ +# RUN: not llvm-mc -triple riscv32 -mattr=+experimental-zaamo < %s 2>&1 | FileCheck %s wangpc-pp wrote: This can be done in the future, I think. Current implementation refers to the `Zmmul` (which is a sub-extension of M extension). The case is the same for `Zaamo` and `Zalrsc` I think. https://github.com/llvm/llvm-project/pull/77424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [RISCV] Add experimental support of Zaamo and Zalrsc (PR #77424)
ved-rivos wrote: > > > I guess Zaamo + Zacas is technically a way one could implement atomics > > > without LR/SC? > > > > > > The Zacas extension depends upon the A extension. > > I filed an issue asking about that > [riscv/riscv-zaamo-zalrsc#5](https://github.com/riscv/riscv-zaamo-zalrsc/issues/5) The spec now clarifies that with Zaamo, Zacas depends on Zaamo. https://github.com/llvm/llvm-project/pull/77424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [RISCV] Add experimental support of Zaamo and Zalrsc (PR #77424)
asb wrote: There should be a release note for this as well as the RISCVUsage update. I have concerns that the codegen part of this isn't correct. Specifically, the [requirement](https://llvm.org/docs/Atomics.html#atomics-and-codegen) that "One very important property of the atomic operations is that if your backend supports any inline lock-free atomic operations of a given size, you should support ALL operations of that size in a lock-free manner." It would be good if the codegen tests were updated so we see AMO codegen when only zalrsc is present, and cmpxchg when there's only zaamo. I _think_ that for the atomics not natively supported at widths up to and including xlen, the tests should show lowering to the `__sync_*` instructions [documented here](https://llvm.org/docs/Atomics.html#libcalls-sync), similar to when +forced-atomics is enabled. https://github.com/llvm/llvm-project/pull/77424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [RISCV] Add experimental support of Zaamo and Zalrsc (PR #77424)
jyknight wrote: Yes, that's an acceptable/correct solution in that circumstance. Given we already have a forced-atomics option, IMO it probably makes sense to still require users to specify that explicitly, rather than effectively defaulting it to on with Zaamo. However, I must say, I cannot understand why this is even a thing that anyone would want. Why would anyone design a single-processor RISCV system that doesn't implement LR/SC? If you don't have the issue of coherent memory across multiple CPUs, LR/SC is utterly trivial to implement -- it's 1 bit of hidden state, indicating whether there is an active reservation. You set the bit in LR. In SC, you check if it's set; if so, execute the store, clear the bit, and return success, otherwise return failure. So, like...why... https://github.com/llvm/llvm-project/pull/77424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [RISCV] Add experimental support of Zaamo and Zalrsc (PR #77424)
asb wrote: > However, I must say, I cannot understand why this is even a thing that anyone > would want. Why would anyone design a single-processor RISCV system that > doesn't implement LR/SC? If you don't have the issue of coherent memory > across multiple CPUs, LR/SC is utterly trivial to implement -- it's 1 bit of > hidden state, indicating whether there is an active reservation. You set the > bit in LR. In SC, you check if it's set; if so, execute the store, clear the > bit, and return success, otherwise return failure. So, like...why... That's a fair question. I know that some vendors have talked about using them for manipulating device registers (this is mentioned in the RISC-V spec too "Another use of AMOs is to provide atomic updates to memory-mapped device registers (e..g, setting, clearing, or toggling bits) in the I/O space"). So one use case is having a cheap (cycle count and code size) read-modify-write for device registers on a very simple core. But as you say, having lr/sc as well isn't expensive on a single core system. If you're doing something like only supporting AMOs on the peripheral memory space then you probably wouldn't want the compiler generating atomic operations for you anyway. I'm mentioning it in the RISC-V sync-up call shortly to see if anyone has insight on how they would intend to use the extension in practice. https://github.com/llvm/llvm-project/pull/77424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [RISCV] Add experimental support of Zaamo and Zalrsc (PR #77424)
topperc wrote: Can we split the CodeGen part out of this patch? https://github.com/llvm/llvm-project/pull/77424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits