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] Zabha/Zacas implies Zaamo #115694

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Conversation

tclin914
Copy link
Contributor

@llvmbot llvmbot added backend:RISC-V mc Machine (object) code labels Nov 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2024

@llvm/pr-subscribers-mc

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

Author: Jim Lin (tclin914)

Changes

The Zabha/Zacas extension depends upon the Zaamo extension.
Ref: https://github.com/riscv/riscv-isa-manual/blob/main/src/zacas.adoc
https://github.com/riscv/riscv-isa-manual/blob/main/src/zabha.adoc.


Full diff: https://github.com/llvm/llvm-project/pull/115694.diff

5 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+4-2)
  • (modified) llvm/lib/TargetParser/RISCVISAInfo.cpp (-5)
  • (modified) llvm/test/CodeGen/RISCV/attributes.ll (+4-4)
  • (modified) llvm/test/MC/RISCV/attribute-arch.s (+1-1)
  • (modified) llvm/unittests/TargetParser/RISCVISAInfoTest.cpp (-10)
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index ad733e1488b5ca..6ec7f9ab78c0d4 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -237,14 +237,16 @@ def HasStdExtAOrZaamo
 
 def FeatureStdExtZabha
     : RISCVExtension<"zabha", 1, 0,
-                     "'Zabha' (Byte and Halfword Atomic Memory Operations)">;
+                     "'Zabha' (Byte and Halfword Atomic Memory Operations)",
+                     [FeatureStdExtZaamo]>;
 def HasStdExtZabha : Predicate<"Subtarget->hasStdExtZabha()">,
                      AssemblerPredicate<(all_of FeatureStdExtZabha),
                          "'Zabha' (Byte and Halfword Atomic Memory Operations)">;
 
 def FeatureStdExtZacas
     : RISCVExtension<"zacas", 1, 0,
-                     "'Zacas' (Atomic Compare-And-Swap Instructions)">,
+                     "'Zacas' (Atomic Compare-And-Swap Instructions)",
+                     [FeatureStdExtZaamo]>,
       RISCVExtensionBitmask<0, 26>;
 def HasStdExtZacas : Predicate<"Subtarget->hasStdExtZacas()">,
                      AssemblerPredicate<(all_of FeatureStdExtZacas),
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index de5b5c39c9ed27..c1bc441fc3f63d 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -760,11 +760,6 @@ Error RISCVISAInfo::checkDependency() {
   if (XLen != 32 && Exts.count("zcf"))
     return getError("'zcf' is only supported for 'rv32'");
 
-  if (!(Exts.count("a") || Exts.count("zaamo")))
-    for (auto Ext : {"zacas", "zabha"})
-      if (Exts.count(Ext))
-        return getExtensionRequiresError(Ext, "a' or 'zaamo");
-
   if (Exts.count("xwchc") != 0) {
     if (XLen != 32)
       return getError("'xwchc' is only supported for 'rv32'");
diff --git a/llvm/test/CodeGen/RISCV/attributes.ll b/llvm/test/CodeGen/RISCV/attributes.ll
index 7624071f4f93ec..f61b403cc7c539 100644
--- a/llvm/test/CodeGen/RISCV/attributes.ll
+++ b/llvm/test/CodeGen/RISCV/attributes.ll
@@ -428,11 +428,11 @@
 ; RV32ZFBFMIN: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zfbfmin1p0"
 ; RV32ZVFBFMIN: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zve32f1p0_zve32x1p0_zvfbfmin1p0_zvl32b1p0"
 ; RV32ZVFBFWMA: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zfbfmin1p0_zve32f1p0_zve32x1p0_zvfbfmin1p0_zvfbfwma1p0_zvl32b1p0"
-; RV32ZACAS: .attribute 5, "rv32i2p1_a2p1_zacas1p0"
+; RV32ZACAS: .attribute 5, "rv32i2p1_a2p1_zaamo1p0_zacas1p0"
 ; RV32ZALASR: .attribute 5, "rv32i2p1_zalasr0p1"
 ; RV32ZAMA16B: .attribute 5, "rv32i2p1_zama16b1p0"
 ; RV32ZICFILP: .attribute 5, "rv32i2p1_zicfilp1p0_zicsr2p0"
-; RV32ZABHA: .attribute 5, "rv32i2p1_a2p1_zabha1p0"
+; RV32ZABHA: .attribute 5, "rv32i2p1_a2p1_zaamo1p0_zabha1p0"
 ; RV32ZVBC32E: .attribute 5, "rv32i2p1_zicsr2p0_zvbc32e0p7_zve32x1p0_zvl32b1p0"
 ; RV32ZVKGS: .attribute 5, "rv32i2p1_zicsr2p0_zve32x1p0_zvkg1p0_zvkgs0p7_zvl32b1p0"
 ; RV32SSNPM: .attribute 5, "rv32i2p1_ssnpm1p0"
@@ -574,10 +574,10 @@
 ; RV64ZFBFMIN: .attribute 5, "rv64i2p1_f2p2_zicsr2p0_zfbfmin1p0"
 ; RV64ZVFBFMIN: .attribute 5, "rv64i2p1_f2p2_zicsr2p0_zve32f1p0_zve32x1p0_zvfbfmin1p0_zvl32b1p0"
 ; RV64ZVFBFWMA: .attribute 5, "rv64i2p1_f2p2_zicsr2p0_zfbfmin1p0_zve32f1p0_zve32x1p0_zvfbfmin1p0_zvfbfwma1p0_zvl32b1p0"
-; RV64ZACAS: .attribute 5, "rv64i2p1_a2p1_zacas1p0"
+; RV64ZACAS: .attribute 5, "rv64i2p1_a2p1_zaamo1p0_zacas1p0"
 ; RV64ZALASR: .attribute 5, "rv64i2p1_zalasr0p1"
 ; RV64ZICFILP: .attribute 5, "rv64i2p1_zicfilp1p0_zicsr2p0"
-; RV64ZABHA: .attribute 5, "rv64i2p1_a2p1_zabha1p0"
+; RV64ZABHA: .attribute 5, "rv64i2p1_a2p1_zaamo1p0_zabha1p0"
 ; RV64ZVBC32E: .attribute 5, "rv64i2p1_zicsr2p0_zvbc32e0p7_zve32x1p0_zvl32b1p0"
 ; RV64ZVKGS: .attribute 5, "rv64i2p1_zicsr2p0_zve32x1p0_zvkg1p0_zvkgs0p7_zvl32b1p0"
 ; RV64SSNPM: .attribute 5, "rv64i2p1_ssnpm1p0"
diff --git a/llvm/test/MC/RISCV/attribute-arch.s b/llvm/test/MC/RISCV/attribute-arch.s
index 72a1db865e025d..30cf037b1f70a4 100644
--- a/llvm/test/MC/RISCV/attribute-arch.s
+++ b/llvm/test/MC/RISCV/attribute-arch.s
@@ -394,7 +394,7 @@
 # CHECK: .attribute     5, "rv32i2p1_f2p2_zicsr2p0_zfbfmin1p0_zve32f1p0_zve32x1p0_zvfbfmin1p0_zvfbfwma1p0_zvl32b1p0"
 
 .attribute arch, "rv32ia_zacas1p0"
-# CHECK: attribute      5, "rv32i2p1_a2p1_zacas1p0"
+# CHECK: attribute      5, "rv32i2p1_a2p1_zaamo1p0_zacas1p0"
 
 .attribute arch, "rv32izalasr0p1"
 # CHECK: attribute      5, "rv32i2p1_zalasr0p1"
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index 09c0f4159cc7ee..08944659d78f46 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -660,16 +660,6 @@ TEST(ParseArchString, MissingDepency) {
     EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
               "");
   }
-
-  for (StringRef Input : {"rv32i_zacas1p0"}) {
-    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
-              "'zacas' requires 'a' or 'zaamo' extension to also be specified");
-  }
-
-  for (StringRef Input : {"rv32i_zabha"}) {
-    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
-              "'zabha' requires 'a' or 'zaamo' extension to also be specified");
-  }
 }
 
 TEST(ParseArchString, RejectsUnrecognizedProfileNames) {

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM.
IIUC, there should be no compatibility issue since both of these two extensions are new?

@michaelmaitland
Copy link
Contributor

There was some related discussion at rust-lang/rust#130877 (comment). Will this cause a problem with compatibility with older versions of binutils?

@topperc
Copy link
Collaborator

topperc commented Nov 11, 2024

LGTM. IIUC, there should be no compatibility issue since both of these two extensions are new?

Zabha was added to binutils 8 months ago. Zaamo was added 6 months ago. Zacas was added 5 months ago. So its safe to make Zacas depend on Zaamo. Not sure if there was a binutils release between Zaamo and Zabha.

@tclin914
Copy link
Contributor Author

Zabha and Zacas were supported from binutils 2.43 (the latest release) and there wasn't a binutils release between them.
In binutils 2.43, Zabha/Zacas imply A and A implies Zaamo and Zalrsc. So that we can say Zabha/Zacas also imply Zaamo.
Zabha/Zacas have been fixed to imply Zaamo instead of A by https://patchwork.sourceware.org/project/binutils/patch/[email protected]/ in master branch last month.

The experiment I made with binutils 2.43:
$ riscv64-unknown-elf-clang -march=rv64izabha main.c -S -o main.s
$ cat main.s
...
.attribute 5, "rv64i2p1_zaamo1p0_zabha1p0"
...
$ riscv64-unknown-elf-as main.s -o main.o
$ riscv64-unknown-elf-readelf -A main.o
...
Tag_RISCV_arch: "rv64i2p1_a2p1_zaamo1p0_zabha1p0_zalrsc1p0"

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@tclin914 tclin914 merged commit 956361c into llvm:main Nov 12, 2024
11 checks passed
@tclin914 tclin914 deleted the zabha-imply-dev branch November 12, 2024 07:49
tclin914 added a commit that referenced this pull request Nov 19, 2024
zacas and zabha don't require the 'a' or 'zaamo' extension after
#115694.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants