Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Add experimental support of Zaamo and Zalrsc #77424

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wangpc-pp
Copy link
Contributor

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.

`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.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" mc Machine (object) code llvm:support labels Jan 9, 2024
@wangpc-pp wangpc-pp requested review from topperc and asb and removed request for topperc January 9, 2024 08:44
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 9, 2024

@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-llvm-support
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-mc

Author: Wang Pengcheng (wangpc-pp)

Changes

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.


Patch is 47.75 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77424.diff

26 Files Affected:

  • (modified) clang/lib/Basic/Targets/RISCV.cpp (+1-1)
  • (modified) clang/test/Preprocessor/riscv-target-features.c (+19)
  • (modified) llvm/docs/RISCVUsage.rst (+2)
  • (modified) llvm/lib/Support/RISCVISAInfo.cpp (+2)
  • (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+25-1)
  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+4-3)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoA.td (+20-12)
  • (modified) llvm/test/CodeGen/RISCV/atomic-cmpxchg-branch-on-result.ll (+4)
  • (modified) llvm/test/CodeGen/RISCV/atomic-cmpxchg-flag.ll (+2)
  • (modified) llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll (+8)
  • (modified) llvm/test/CodeGen/RISCV/atomic-rmw-discard.ll (+4)
  • (modified) llvm/test/CodeGen/RISCV/atomic-rmw-sub.ll (+4)
  • (modified) llvm/test/CodeGen/RISCV/atomic-rmw.ll (+8)
  • (modified) llvm/test/CodeGen/RISCV/atomic-signext.ll (+4)
  • (modified) llvm/test/CodeGen/RISCV/atomicrmw-uinc-udec-wrap.ll (+4)
  • (modified) llvm/test/CodeGen/RISCV/attributes.ll (+8)
  • (modified) llvm/test/MC/RISCV/rv32i-invalid.s (+1-1)
  • (added) llvm/test/MC/RISCV/rv32zaamo-invalid.s (+11)
  • (added) llvm/test/MC/RISCV/rv32zaamo-valid.s (+122)
  • (added) llvm/test/MC/RISCV/rv32zalrsc-invalid.s (+7)
  • (added) llvm/test/MC/RISCV/rv32zalrsc-valid.s (+36)
  • (added) llvm/test/MC/RISCV/rv64zaamo-invalid.s (+11)
  • (added) llvm/test/MC/RISCV/rv64zaamo-valid.s (+157)
  • (added) llvm/test/MC/RISCV/rv64zalrsc-invalid.s (+7)
  • (added) llvm/test/MC/RISCV/rv64zalrsc-valid.s (+42)
  • (modified) llvm/unittests/Support/RISCVISAInfoTest.cpp (+2)
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  1000000{{$}}
 
+// 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 1000000{{$}}
 
+// RUN: %clang --target=riscv32 -menable-experimental-extensions \
+// RUN: -march=rv32i_zalrsc0p1 -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-ZALRSC-EXT %s
+// RUN: %clang --target=riscv64 -menable-experimental-extensions \
+// RUN: -march=rv64i_zalrsc0p1 -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-ZALRSC-EXT %s
+// CHECK-ZALRSC-EXT: __riscv_zalrsc 1000{{$}}
+
 // RUN: %clang --target=riscv32-unknown-linux-gnu \
 // RUN: -march=rv32izfa -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZFA-EXT %s
diff --git a/llvm/docs/RISCVUsage.rst b/llvm/docs/RISCVUsage.rst
index 99c7146825f5ee..1eb1823faebac1 100644
--- a/llvm/docs/RISCVUsage.rst
+++ b/llvm/docs/RISCVUsage.rst
@@ -96,6 +96,8 @@ on support follow.
      ``Svnapot``      Assembly Support
      ``Svpbmt``       Supported
      ``V``            Supported
+     ``Zaamo``        Supported
+     ``Zalrsc``       Supported
      ``Zawrs``        Assembly Support
      ``Zba``          Supported
      ``Zbb``          Supported
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 70f531e40b90e6..51bef21c324ce5 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -188,7 +188,9 @@ static const RISCVSupportedExtension SupportedExtensions[] = {
 
 // NOTE: This table should be sorted alphabetically by extension name.
 static const RISCVSupportedExtension SupportedExperimentalExtensions[] = {
+    {"zaamo", RISCVExtensionVersion{0, 1}},
     {"zacas", RISCVExtensionVersion{1, 0}},
+    {"zalrsc", RISCVExtensionVersion{0, 1}},
 
     {"zcmop", RISCVExtensionVersion{0, 2}},
 
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index bb7a3291085d43..69ca7161254408 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -41,6 +41,30 @@ def HasStdExtA : Predicate<"Subtarget->hasStdExtA()">,
                            AssemblerPredicate<(all_of FeatureStdExtA),
                            "'A' (Atomic Instructions)">;
 
+def FeatureStdExtZaamo
+    : SubtargetFeature<"experimental-zaamo", "HasStdExtZaamo", "true",
+                       "'Zaamo' (Atomic Memory Operations)">;
+def HasStdExtZaamo : Predicate<"Subtarget->hasStdExtZaamo()">,
+                               AssemblerPredicate<(all_of FeatureStdExtZaamo),
+                               "'Zaamo' (Atomic Memory Operations)">;
+def HasStdExtAOrZaamo
+    : Predicate<"Subtarget->hasStdExtA() || Subtarget->hasStdExtZaamo()">,
+                AssemblerPredicate<(any_of FeatureStdExtA, FeatureStdExtZaamo),
+                                   "'A' (Atomic Instructions) or "
+                                   "'Zaamo' (Atomic Memory Operations)">;
+
+def FeatureStdExtZalrsc
+    : SubtargetFeature<"experimental-zalrsc", "HasStdExtZalrsc", "true",
+                       "'Zalrsc' (Load-Reserved/Store-Conditional)">;
+def HasStdExtZalrsc : Predicate<"Subtarget->hasStdExtZalrsc()">,
+                                AssemblerPredicate<(all_of FeatureStdExtZalrsc),
+                                "'Zalrsc' (Load-Reserved/Store-Conditional)">;
+def HasStdExtAOrZalrsc
+    : Predicate<"Subtarget->hasStdExtA() || Subtarget->hasStdExtZalrsc()">,
+                AssemblerPredicate<(any_of FeatureStdExtA, FeatureStdExtZalrsc),
+                                   "'A' (Atomic Instructions) or "
+                                   "'Zalrsc' (Load-Reserved/Store-Conditional)">;
+
 def FeatureStdExtF
     : SubtargetFeature<"f", "HasStdExtF", "true",
                        "'F' (Single-Precision Floating-Point)",
@@ -1044,7 +1068,7 @@ def FeatureForcedAtomics : SubtargetFeature<
     "forced-atomics", "HasForcedAtomics", "true",
     "Assume that lock-free native-width atomics are available">;
 def HasAtomicLdSt
-    : Predicate<"Subtarget->hasStdExtA() || Subtarget->hasForcedAtomics()">;
+    : Predicate<"Subtarget->hasStdExtA() || Subtarget->hasStdExtZalrsc() || Subtarget->hasForcedAtomics()">;
 
 def FeatureTaggedGlobals : SubtargetFeature<"tagged-globals",
     "AllowTaggedGlobals",
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index a5b33e8e293a17..3f3ad7ba496381 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -628,7 +628,8 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
     setOperationAction(ISD::PREFETCH, MVT::Other, Legal);
   }
 
-  if (Subtarget.hasStdExtA()) {
+  if (Subtarget.hasStdExtA() || Subtarget.hasStdExtZaamo() ||
+      Subtarget.hasStdExtZalrsc()) {
     setMaxAtomicSizeInBitsSupported(Subtarget.getXLen());
     setMinCmpXchgSizeInBits(32);
   } else if (Subtarget.hasForcedAtomics()) {
@@ -1334,7 +1335,7 @@ RISCVTargetLowering::RISCVTargetLowering(const TargetMachine &TM,
     }
   }
 
-  if (Subtarget.hasStdExtA()) {
+  if (Subtarget.hasStdExtA() || Subtarget.hasStdExtZaamo()) {
     setOperationAction(ISD::ATOMIC_LOAD_SUB, XLenVT, Expand);
     if (RV64LegalI32 && Subtarget.is64Bit())
       setOperationAction(ISD::ATOMIC_LOAD_SUB, MVT::i32, Expand);
@@ -16215,7 +16216,7 @@ unsigned RISCVTargetLowering::ComputeNumSignBitsForTargetNode(
       // 32 for both 64 and 32.
       assert(Subtarget.getXLen() == 64);
       assert(getMinCmpXchgSizeInBits() == 32);
-      assert(Subtarget.hasStdExtA());
+      assert(Subtarget.hasStdExtA() || Subtarget.hasStdExtZalrsc());
       return 33;
     }
     break;
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoA.td b/llvm/lib/Target/RISCV/RISCVInstrInfoA.td
index 4d0567e41abcb7..8d2283d2a306b4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoA.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoA.td
@@ -47,10 +47,14 @@ multiclass AMO_rr_aq_rl<bits<5> funct5, bits<3> funct3, string opcodestr> {
 // Instructions
 //===----------------------------------------------------------------------===//
 
-let Predicates = [HasStdExtA], IsSignExtendingOpW = 1 in {
+let IsSignExtendingOpW = 1 in {
+let Predicates = [HasStdExtAOrZalrsc] in {
 defm LR_W       : LR_r_aq_rl<0b010, "lr.w">, Sched<[WriteAtomicLDW, ReadAtomicLDW]>;
 defm SC_W       : AMO_rr_aq_rl<0b00011, 0b010, "sc.w">,
                   Sched<[WriteAtomicSTW, ReadAtomicSTW, ReadAtomicSTW]>;
+} // Predicates = [HasStdExtAOrZalrsc]
+
+let Predicates = [HasStdExtAOrZaamo] in {
 defm AMOSWAP_W  : AMO_rr_aq_rl<0b00001, 0b010, "amoswap.w">,
                   Sched<[WriteAtomicW, ReadAtomicWA, ReadAtomicWD]>;
 defm AMOADD_W   : AMO_rr_aq_rl<0b00000, 0b010, "amoadd.w">,
@@ -69,12 +73,16 @@ defm AMOMINU_W  : AMO_rr_aq_rl<0b11000, 0b010, "amominu.w">,
                   Sched<[WriteAtomicW, ReadAtomicWA, ReadAtomicWD]>;
 defm AMOMAXU_W  : AMO_rr_aq_rl<0b11100, 0b010, "amomaxu.w">,
                   Sched<[WriteAtomicW, ReadAtomicWA, ReadAtomicWD]>;
-} // Predicates = [HasStdExtA]
+} // Predicates = [HasStdExtAOrZaamo]
+} // IsSignExtendingOpW = 1
 
-let Predicates = [HasStdExtA, IsRV64] in {
+let Predicates = [HasStdExtAOrZalrsc, IsRV64] in {
 defm LR_D       : LR_r_aq_rl<0b011, "lr.d">, Sched<[WriteAtomicLDD, ReadAtomicLDD]>;
 defm SC_D       : AMO_rr_aq_rl<0b00011, 0b011, "sc.d">,
                   Sched<[WriteAtomicSTD, ReadAtomicSTD, ReadAtomicSTD]>;
+} // Predicates = [HasStdExtAOrZalrsc, IsRV64]
+
+let Predicates = [HasStdExtAOrZaamo, IsRV64] in {
 defm AMOSWAP_D  : AMO_rr_aq_rl<0b00001, 0b011, "amoswap.d">,
                   Sched<[WriteAtomicD, ReadAtomicDA, ReadAtomicDD]>;
 defm AMOADD_D   : AMO_rr_aq_rl<0b00000, 0b011, "amoadd.d">,
@@ -93,7 +101,7 @@ defm AMOMINU_D  : AMO_rr_aq_rl<0b11000, 0b011, "amominu.d">,
                   Sched<[WriteAtomicD, ReadAtomicDA, ReadAtomicDD]>;
 defm AMOMAXU_D  : AMO_rr_aq_rl<0b11100, 0b011, "amomaxu.d">,
                   Sched<[WriteAtomicD, ReadAtomicDA, ReadAtomicDD]>;
-} // Predicates = [HasStdExtA, IsRV64]
+} // Predicates = [HasStdExtAOrZaamo, IsRV64]
 
 //===----------------------------------------------------------------------===//
 // Pseudo-instructions and codegen patterns
@@ -121,7 +129,7 @@ let Predicates = [HasAtomicLdSt, IsRV64] in {
 
 multiclass AMOPat<string AtomicOp, string BaseInst, ValueType vt = XLenVT,
                   list<Predicate> ExtraPreds = []> {
-let Predicates = !listconcat([HasStdExtA, NotHasStdExtZtso], ExtraPreds) in {
+let Predicates = !listconcat([HasStdExtAOrZaamo, NotHasStdExtZtso], ExtraPreds) in {
   def : PatGprGpr<!cast<PatFrag>(AtomicOp#"_monotonic"),
                   !cast<RVInst>(BaseInst), vt>;
   def : PatGprGpr<!cast<PatFrag>(AtomicOp#"_acquire"),
@@ -133,7 +141,7 @@ let Predicates = !listconcat([HasStdExtA, NotHasStdExtZtso], ExtraPreds) in {
   def : PatGprGpr<!cast<PatFrag>(AtomicOp#"_seq_cst"),
                   !cast<RVInst>(BaseInst#"_AQ_RL"), vt>;
 }
-let Predicates = !listconcat([HasStdExtA, HasStdExtZtso], ExtraPreds) in {
+let Predicates = !listconcat([HasStdExtAOrZaamo, HasStdExtZtso], ExtraPreds) in {
   def : PatGprGpr<!cast<PatFrag>(AtomicOp#"_monotonic"),
                   !cast<RVInst>(BaseInst), vt>;
   def : PatGprGpr<!cast<PatFrag>(AtomicOp#"_acquire"),
@@ -157,7 +165,7 @@ defm : AMOPat<"atomic_load_min_32", "AMOMIN_W">;
 defm : AMOPat<"atomic_load_umax_32", "AMOMAXU_W">;
 defm : AMOPat<"atomic_load_umin_32", "AMOMINU_W">;
 
-let Predicates = [HasStdExtA] in {
+let Predicates = [HasStdExtAOrZalrsc] in {
 
 /// Pseudo AMOs
 
@@ -304,7 +312,7 @@ def : Pat<(int_riscv_masked_cmpxchg_i32
           (PseudoMaskedCmpXchg32
             GPR:$addr, GPR:$cmpval, GPR:$newval, GPR:$mask, timm:$ordering)>;
 
-} // Predicates = [HasStdExtA]
+} // Predicates = [HasStdExtAOrZalrsc]
 
 defm : AMOPat<"atomic_swap_64", "AMOSWAP_D", i64, [IsRV64]>;
 defm : AMOPat<"atomic_load_add_64", "AMOADD_D", i64, [IsRV64]>;
@@ -316,7 +324,7 @@ defm : AMOPat<"atomic_load_min_64", "AMOMIN_D", i64, [IsRV64]>;
 defm : AMOPat<"atomic_load_umax_64", "AMOMAXU_D", i64, [IsRV64]>;
 defm : AMOPat<"atomic_load_umin_64", "AMOMINU_D", i64, [IsRV64]>;
 
-let Predicates = [HasStdExtA, IsRV64] in {
+let Predicates = [HasStdExtAOrZalrsc, IsRV64] in {
 
 /// 64-bit pseudo AMOs
 
@@ -361,7 +369,7 @@ def : Pat<(int_riscv_masked_cmpxchg_i64
             GPR:$addr, GPR:$cmpval, GPR:$newval, GPR:$mask, timm:$ordering),
           (PseudoMaskedCmpXchg32
             GPR:$addr, GPR:$cmpval, GPR:$newval, GPR:$mask, timm:$ordering)>;
-} // Predicates = [HasStdExtA, IsRV64]
+} // Predicates = [HasStdExtAOrZalrsc, IsRV64]
 
 //===----------------------------------------------------------------------===//
 // Experimental RV64 i32 legalization patterns.
@@ -372,7 +380,7 @@ class PatGprGprA<SDPatternOperator OpNode, RVInst Inst, ValueType vt>
 
 multiclass AMOPat2<string AtomicOp, string BaseInst, ValueType vt = XLenVT,
                    list<Predicate> ExtraPreds = []> {
-let Predicates = !listconcat([HasStdExtA, NotHasStdExtZtso], ExtraPreds) in {
+let Predicates = !listconcat([HasStdExtAOrZaamo, NotHasStdExtZtso], ExtraPreds) in {
   def : PatGprGprA<!cast<PatFrag>(AtomicOp#"_monotonic"),
                    !cast<RVInst>(BaseInst), vt>;
   def : PatGprGprA<!cast<PatFrag>(AtomicOp#"_acquire"),
@@ -384,7 +392,7 @@ let Predicates = !listconcat([HasStdExtA, NotHasStdExtZtso], ExtraPreds) in {
   def : PatGprGprA<!cast<PatFrag>(AtomicOp#"_seq_cst"),
                    !cast<RVInst>(BaseInst#"_AQ_RL"), vt>;
 }
-let Predicates = !listconcat([HasStdExtA, HasStdExtZtso], ExtraPreds) in {
+let Predicates = !listconcat([HasStdExtAOrZaamo, HasStdExtZtso], ExtraPreds) in {
   def : PatGprGprA<!cast<PatFrag>(AtomicOp#"_monotonic"),
                    !cast<RVInst>(BaseInst), vt>;
   def : PatGprGprA<!cast<PatFrag>(AtomicOp#"_acquire"),
diff --git a/llvm/test/CodeGen/RISCV/atomic-cmpxchg-branch-on-result.ll b/llvm/test/CodeGen/RISCV/atomic-cmpxchg-branch-on-result.ll
index 651f58d324422f..86a6dd5df5e316 100644
--- a/llvm/test/CodeGen/RISCV/atomic-cmpxchg-branch-on-result.ll
+++ b/llvm/test/CodeGen/RISCV/atomic-cmpxchg-branch-on-result.ll
@@ -3,6 +3,10 @@
 ; RUN:   | FileCheck -check-prefixes=CHECK,RV32IA %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=CHECK,RV64IA %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zalrsc -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=CHECK,RV32IA %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zalrsc -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=CHECK,RV64IA %s
 
 ; Test cmpxchg followed by a branch on the cmpxchg success value to see if the
 ; branch is folded into the cmpxchg expansion.
diff --git a/llvm/test/CodeGen/RISCV/atomic-cmpxchg-flag.ll b/llvm/test/CodeGen/RISCV/atomic-cmpxchg-flag.ll
index f25571b5cf2531..9934b1ed0cdc62 100644
--- a/llvm/test/CodeGen/RISCV/atomic-cmpxchg-flag.ll
+++ b/llvm/test/CodeGen/RISCV/atomic-cmpxchg-flag.ll
@@ -1,6 +1,8 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefix=RV64IA %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zalrsc -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IA %s
 
 ; This test ensures that the output of the 'lr.w' instruction is sign-extended.
 ; Previously, the default zero-extension was being used and 'cmp' parameter
diff --git a/llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll b/llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll
index 46ed01b11584f9..b56f956cb22c81 100644
--- a/llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll
+++ b/llvm/test/CodeGen/RISCV/atomic-cmpxchg.ll
@@ -5,12 +5,20 @@
 ; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-WMO %s
 ; RUN: llc -mtriple=riscv32 -mattr=+a,+experimental-ztso -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-TSO %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zalrsc -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-WMO %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zalrsc,+experimental-ztso -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-TSO %s
 ; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefix=RV64I %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-WMO %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a,+experimental-ztso -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-TSO %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zalrsc -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-WMO %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zalrsc,+experimental-ztso -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-TSO %s
 
 define void @cmpxchg_i8_monotonic_monotonic(ptr %ptr, i8 %cmp, i8 %val) nounwind {
 ; RV32I-LABEL: cmpxchg_i8_monotonic_monotonic:
diff --git a/llvm/test/CodeGen/RISCV/atomic-rmw-discard.ll b/llvm/test/CodeGen/RISCV/atomic-rmw-discard.ll
index 8d3fc96109262e..d3741c53c5818b 100644
--- a/llvm/test/CodeGen/RISCV/atomic-rmw-discard.ll
+++ b/llvm/test/CodeGen/RISCV/atomic-rmw-discard.ll
@@ -3,6 +3,10 @@
 ; RUN:   | FileCheck -check-prefixes=RV32 %s
 ; RUN: llc -O3 -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV64 %s
+; RUN: llc -O3 -mtriple=riscv32 -mattr=+experimental-zaamo -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV32 %s
+; RUN: llc -O3 -mtriple=riscv64 -mattr=+experimental-zaamo -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64 %s
 
 define void @amoswap_w_discard(ptr %a, i32 %b) nounwind {
 ; RV32-LABEL: amoswap_w_discard:
diff --git a/llvm/test/CodeGen/RISCV/atomic-rmw-sub.ll b/llvm/test/CodeGen/RISCV/atomic-rmw-sub.ll
index 4dafd6a08d973b..016d5fd21ffa6f 100644
--- a/llvm/test/CodeGen/RISCV/atomic-rmw-sub.ll
+++ b/llvm/test/CodeGen/RISCV/atomic-rmw-sub.ll
@@ -3,10 +3,14 @@
 ; RUN:   | FileCheck -check-prefix=RV32I %s
 ; RUN: llc -mtriple=riscv32 -mattr=+a -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV32IA %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zaamo -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV32IA %s
 ; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefix=RV64I %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV64IA %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zaamo -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IA %s
 
 define i32 @atomicrmw_sub_i32_constant(ptr %a) nounwind {
 ; RV32I-LABEL: atomicrmw_sub_i32_constant:
diff --git a/llvm/test/CodeGen/RISCV/atomic-rmw.ll b/llvm/test/CodeGen/RISCV/atomic-rmw.ll
index d4c067b7b8a40c..2e850e6f0bb43d 100644
--- a/llvm/test/CodeGen/RISCV/atomic-rmw.ll
+++ b/llvm/test/CodeGen/RISCV/atomic-rmw.ll
@@ -5,12 +5,20 @@
 ; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-WMO %s
 ; RUN: llc -mtriple=riscv32 -mattr=+a,+experimental-ztso -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-TSO %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zaamo,+experimental-zalrsc -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-WMO %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zaamo,+experimental-zalrsc,+experimental-ztso -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV32IA,RV32IA-TSO %s
 ; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefix=RV64I %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-WMO %s
 ; RUN: llc -mtriple=riscv64 -mattr=+a,+experimental-ztso -verify-machineinstrs < %s \
 ; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-TSO %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zaamo,+experimental-zalrsc -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-WMO %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zaamo,+experimental-zalrsc,+experimental-ztso -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IA,RV64IA-TSO %s
 
 define i8 @atomicrmw_xchg_i8_monotonic(ptr %a, i8 %b) nounwind...
[truncated]

@wangpc-pp wangpc-pp requested review from dtcxzyw, topperc, asb and kito-cheng and removed request for asb January 9, 2024 08:45
@@ -0,0 +1,11 @@
# RUN: not llvm-mc -triple riscv32 -mattr=+experimental-zaamo < %s 2>&1 | FileCheck %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we split rv32a-invalid.s into two files? I think it is better than duplicating tests for new extensions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@asb
Copy link
Contributor

asb commented Jan 15, 2024

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 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, similar to when +forced-atomics is enabled, if there isn't an alternate lowering (the AMOs should be expressible in terms of lr/sc).

@asb
Copy link
Contributor

asb commented Jan 17, 2024

CC @jyknight who can hopefully confirm if my interpretation is correct. Based on your comment on #77814, perhaps the expectation is that libatomic would have a lock-free implementation for any __sync calls used up to the max atomic width (e.g. lr/sc based if there's no AMO instructions)?

@jyknight
Copy link
Member

There's two sets of atomic functions:
__atomic_* are provided by libatomic, and might use locking, or not.
__sync_* should always be lock-free. These are only used on certain architectures where it's guaranteed that the operation can be implemented lock-free, but it's desirable for whatever reason to not do so inline.

For this patch, I think the correct behavior is:

  • If Zalrsc is present, but Zaamo is not, you may either emit an LR/SC loop (what I'd recommend), or emit an out-of-line call to __sync_* (which needs to be implemented with that LR/SC loop.)
  • If Zaamo is present, but neither Zalrsc nor Zacas are present, I think there's no way to implement a cmpxchg operation. This means lock-free atomics cannot be supported, so it should setMaxAtomicSizeInBitsSupported(0).

@asb
Copy link
Contributor

asb commented Jan 18, 2024

Thanks James, that matches what I'd understood. Just one comment on this though:

If Zaamo is present, but neither Zalrsc nor Zacas are present, I think there's no way to implement a cmpxchg operation. This means lock-free atomics cannot be supported, so it should setMaxAtomicSizeInBitsSupported(0).

Especially given that Zaamo is documented as targeting embedded systems, emitting __sync calls for the non-AMO atomics (as would be done with +forced-atomics, presumably they'd be implemented by turning off interrupts) seems defensible doesn't it? Or is there a correctness issue with that I'm missing?

@jyknight
Copy link
Member

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...

@asb
Copy link
Contributor

asb commented Jan 18, 2024

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.

@jrtc27
Copy link
Collaborator

jrtc27 commented Jan 18, 2024

I guess Zaamo + Zacas is technically a way one could implement atomics without LR/SC?

@dtcxzyw
Copy link
Member

dtcxzyw commented Jan 18, 2024

I guess Zaamo + Zacas is technically a way one could implement atomics without LR/SC?

The Zacas extension depends upon the A extension.

@topperc
Copy link
Collaborator

topperc commented Jan 18, 2024

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

@topperc
Copy link
Collaborator

topperc commented Jan 18, 2024

Can we split the CodeGen part out of this patch?

@wangpc-pp
Copy link
Contributor Author

Can we split the CodeGen part out of this patch?

MC part is #78970.
This PR will be the CodeGen part and I will update this PR later.

@ved-rivos
Copy link

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

The spec now clarifies that with Zaamo, Zacas depends on Zaamo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category llvm:support mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants