[clang] [llvm] [RISCV] Gate unratified profiles behind -menable-experimental-extensions (PR #92167)
Alex Bradbury via llvm-commits
llvm-commits at lists.llvm.org
Tue May 14 12:50:02 PDT 2024
https://github.com/asb created https://github.com/llvm/llvm-project/pull/92167
As discussed in the last sync-up call, because these profiles are not yet finalised they shouldn't be exposed to users unless they opt-in to them (much like experimental extensions). We may later want to add a more specific flag, but reusing `-menable-experimental-extensions` solves the immediate problem.
This is implemented using the new support for marking profiles s experimental added in #91993 to move the unratified profiles to RISCVExperimentalProfile and making the necessary changes to logic in RISCVISAInfo to handle this.
---
This PR also includes a commit that adds baseline test coverage for profile strings in RISCVISAInfoTest's parseArch tests.
>From a806961a477e28d719907ac0b46241fd7b7c7990 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Tue, 14 May 2024 13:40:31 +0100
Subject: [PATCH 1/2] [RISCV][test] Add tests for profiles with
RISCVISAInfo::ParseArchString
---
.../TargetParser/RISCVISAInfoTest.cpp | 63 ++++++++++++++++++-
1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index 7f2d1eb8c0170..d04f21fa20063 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -21,8 +21,8 @@ bool operator==(const RISCVISAUtils::ExtensionVersion &A,
}
TEST(ParseNormalizedArchString, RejectsInvalidChars) {
- for (StringRef Input :
- {"RV32", "rV64", "rv32i2P0", "rv64i2p0_A2p0", "rv32e2.0"}) {
+ for (StringRef Input : {"RV32", "rV64", "rv32i2P0", "rv64i2p0_A2p0",
+ "rv32e2.0", "rva20u64+zbc"}) {
EXPECT_EQ(
toString(RISCVISAInfo::parseNormalizedArchString(Input).takeError()),
"string may only contain [a-z0-9_]");
@@ -667,6 +667,65 @@ TEST(ParseArchString, RejectsConflictingExtensions) {
}
}
+TEST(ParseArchString, RejectsUnrecognizedProfileNames) {
+ for (StringRef Input : {"rvi23u99", "rvz23u64", "rva99u32"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "string must begin with rv32{i,e,g}, rv64{i,e,g}, or a supported "
+ "profile name");
+ }
+}
+
+TEST(ParseArchString, RejectsProfilesWithUnseparatedExtraExtensions) {
+ for (StringRef Input : {"rvi20u32m", "rvi20u64c"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "additional extensions must be after separator '_'");
+ }
+}
+
+TEST(ParseArchString, AcceptsBareProfileNames) {
+ auto MaybeRVA20U64 = RISCVISAInfo::parseArchString("rva20u64", true);
+ ASSERT_THAT_EXPECTED(MaybeRVA20U64, Succeeded());
+ const auto &Exts = (*MaybeRVA20U64)->getExtensions();
+ EXPECT_EQ(Exts.size(), 13UL);
+ EXPECT_EQ(Exts.count("i"), 1U);
+ EXPECT_EQ(Exts.count("m"), 1U);
+ EXPECT_EQ(Exts.count("f"), 1U);
+ EXPECT_EQ(Exts.count("a"), 1U);
+ EXPECT_EQ(Exts.count("d"), 1U);
+ EXPECT_EQ(Exts.count("c"), 1U);
+ EXPECT_EQ(Exts.count("za128rs"), 1U);
+ EXPECT_EQ(Exts.count("zicntr"), 1U);
+ EXPECT_EQ(Exts.count("ziccif"), 1U);
+ EXPECT_EQ(Exts.count("zicsr"), 1U);
+ EXPECT_EQ(Exts.count("ziccrse"), 1U);
+ EXPECT_EQ(Exts.count("ziccamoa"), 1U);
+ EXPECT_EQ(Exts.count("zicclsm"), 1U);
+
+ auto MaybeRVA23U64 = RISCVISAInfo::parseArchString("rva23u64", true);
+ ASSERT_THAT_EXPECTED(MaybeRVA23U64, Succeeded());
+ EXPECT_GT((*MaybeRVA23U64)->getExtensions().size(), 13UL);
+}
+
+TEST(ParseArchSTring, AcceptsProfileNamesWithSeparatedAdditionalExtensions) {
+ auto MaybeRVI20U64 = RISCVISAInfo::parseArchString("rvi20u64_m_zba", true);
+ ASSERT_THAT_EXPECTED(MaybeRVI20U64, Succeeded());
+ const auto &Exts = (*MaybeRVI20U64)->getExtensions();
+ EXPECT_EQ(Exts.size(), 3UL);
+ EXPECT_EQ(Exts.count("i"), 1U);
+ EXPECT_EQ(Exts.count("m"), 1U);
+ EXPECT_EQ(Exts.count("zba"), 1U);
+}
+
+TEST(ParseArchString,
+ RejectsProfilesWithAdditionalExtensionsGivenAlreadyInProfile) {
+ // This test was added to document the current behaviour. Discussion isn't
+ // believed to have taken place about if this is desirable or not.
+ EXPECT_EQ(
+ toString(
+ RISCVISAInfo::parseArchString("rva20u64_zicntr", true).takeError()),
+ "duplicated standard user-level extension 'zicntr'");
+}
+
TEST(ToFeatures, IIsDroppedAndExperimentalExtensionsArePrefixed) {
auto MaybeISAInfo1 =
RISCVISAInfo::parseArchString("rv64im_ztso", true, false);
>From 6c97493b78b1cb097fdde8c850814956c69f1822 Mon Sep 17 00:00:00 2001
From: Alex Bradbury <asb at igalia.com>
Date: Tue, 14 May 2024 20:45:41 +0100
Subject: [PATCH 2/2] [RISCV] Gate unratified profiles behind
-menable-experimental-extensions
As discussed in the last sync-up call, because these profiles are not
yet finalised they shouldn't be exposed to users unless they opt-in to
them (much like experimental extensions). We may later want to add a
more specific flag, but reusing `-menable-experimental-extensions`
solves the immediate problem.
---
clang/test/Driver/riscv-profiles.c | 10 +++++--
llvm/lib/Target/RISCV/RISCVProfiles.td | 14 +++++----
llvm/lib/TargetParser/RISCVISAInfo.cpp | 29 +++++++++++++++----
llvm/test/CodeGen/RISCV/attributes.ll | 10 +++----
.../TargetParser/RISCVISAInfoTest.cpp | 13 +++++++--
5 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/clang/test/Driver/riscv-profiles.c b/clang/test/Driver/riscv-profiles.c
index 298f301de3feb..55aa5b398cee9 100644
--- a/clang/test/Driver/riscv-profiles.c
+++ b/clang/test/Driver/riscv-profiles.c
@@ -111,7 +111,7 @@
// RVA22S64: "-target-feature" "+svinval"
// RVA22S64: "-target-feature" "+svpbmt"
-// RUN: %clang --target=riscv64 -### -c %s 2>&1 -march=rva23u64 \
+// RUN: %clang --target=riscv64 -### -c %s 2>&1 -march=rva23u64 -menable-experimental-extensions \
// RUN: | FileCheck -check-prefix=RVA23U64 %s
// RVA23U64: "-target-feature" "+m"
// RVA23U64: "-target-feature" "+a"
@@ -207,7 +207,7 @@
// RVA23S64: "-target-feature" "+svnapot"
// RVA23S64: "-target-feature" "+svpbmt"
-// RUN: %clang --target=riscv64 -### -c %s 2>&1 -march=rvb23u64 \
+// RUN: %clang --target=riscv64 -### -c %s 2>&1 -march=rvb23u64 -menable-experimental-extensions \
// RUN: | FileCheck -check-prefix=RVB23U64 %s
// RVB23U64: "-target-feature" "+m"
// RVB23U64: "-target-feature" "+a"
@@ -284,7 +284,7 @@
// RVB23S64: "-target-feature" "+svnapot"
// RVB23S64: "-target-feature" "+svpbmt"
-// RUN: %clang --target=riscv32 -### -c %s 2>&1 -march=rvm23u32 \
+// RUN: %clang --target=riscv32 -### -c %s 2>&1 -march=rvm23u32 -menable-experimental-extensions \
// RUN: | FileCheck -check-prefix=RVM23U32 %s
// RVM23U32: "-target-feature" "+m"
// RVM23U32: "-target-feature" "+zicbop"
@@ -322,3 +322,7 @@
// RUN: not %clang --target=riscv64 -### -c %s 2>&1 -march=rva22u64zfa | FileCheck -check-prefix=INVALID-ADDITIONAL %s
// INVALID-ADDITIONAL: error: invalid arch name 'rva22u64zfa', additional extensions must be after separator '_'
+
+// RUN: not %clang --target=riscv64 -### -c %s 2>&1 -march=rva23u64 | FileCheck -check-prefix=EXPERIMENTAL-NOFLAG %s
+// EXPERIMENTAL-NOFLAG: error: invalid arch name 'rva23u64'
+// EXPERIMENTAL-NOFLAG: requires '-menable-experimental-extensions' for profile 'rva23u64'
diff --git a/llvm/lib/Target/RISCV/RISCVProfiles.td b/llvm/lib/Target/RISCV/RISCVProfiles.td
index e56df33bd8cb0..c4a64681f5f12 100644
--- a/llvm/lib/Target/RISCV/RISCVProfiles.td
+++ b/llvm/lib/Target/RISCV/RISCVProfiles.td
@@ -13,6 +13,10 @@ class RISCVProfile<string name, list<SubtargetFeature> features>
// experimental.
bit Experimental = false;
}
+class RISCVExperimentalProfile<string name, list<SubtargetFeature> features>
+ : RISCVProfile<"experimental-"#name, features> {
+ let Experimental = true;
+}
defvar RVI20U32Features = [Feature32Bit, FeatureStdExtI];
defvar RVI20U64Features = [Feature64Bit, FeatureStdExtI];
@@ -201,8 +205,8 @@ def RVA20U64 : RISCVProfile<"rva20u64", RVA20U64Features>;
def RVA20S64 : RISCVProfile<"rva20s64", RVA20S64Features>;
def RVA22U64 : RISCVProfile<"rva22u64", RVA22U64Features>;
def RVA22S64 : RISCVProfile<"rva22s64", RVA22S64Features>;
-def RVA23U64 : RISCVProfile<"rva23u64", RVA23U64Features>;
-def RVA23S64 : RISCVProfile<"rva23s64", RVA23S64Features>;
-def RVB23U64 : RISCVProfile<"rvb23u64", RVB23U64Features>;
-def RVB23S64 : RISCVProfile<"rvb23s64", RVB23S64Features>;
-def RVM23U32 : RISCVProfile<"rvm23u32", RVM23U32Features>;
+def RVA23U64 : RISCVExperimentalProfile<"rva23u64", RVA23U64Features>;
+def RVA23S64 : RISCVExperimentalProfile<"rva23s64", RVA23S64Features>;
+def RVB23U64 : RISCVExperimentalProfile<"rvb23u64", RVB23U64Features>;
+def RVB23S64 : RISCVExperimentalProfile<"rvb23s64", RVB23S64Features>;
+def RVM23U32 : RISCVExperimentalProfile<"rvm23u32", RVM23U32Features>;
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index e22dd6032cb0c..b15d0f0f0be32 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -102,6 +102,10 @@ void llvm::riscvExtensionsHelp(StringMap<StringRef> DescMap) {
for (const auto &P : SupportedProfiles)
outs().indent(4) << P.Name << "\n";
+ outs() << "\nExperimental Profiles\n";
+ for (const auto &P : SupportedExperimentalProfiles)
+ outs().indent(4) << P.Name << "\n";
+
outs() << "\nUse -march to specify the target's extension.\n"
"For example, clang -march=rv32i_v1p0\n";
}
@@ -608,12 +612,25 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
XLen = 64;
} else {
// Try parsing as a profile.
- auto I = llvm::upper_bound(SupportedProfiles, Arch,
- [](StringRef Arch, const RISCVProfile &Profile) {
- return Arch < Profile.Name;
- });
-
- if (I != std::begin(SupportedProfiles) && Arch.starts_with((--I)->Name)) {
+ auto ProfileCmp = [](StringRef Arch, const RISCVProfile &Profile) {
+ return Arch < Profile.Name;
+ };
+ auto I = llvm::upper_bound(SupportedProfiles, Arch, ProfileCmp);
+ bool FoundProfile = I != std::begin(SupportedProfiles) &&
+ Arch.starts_with(std::prev(I)->Name);
+ if (!FoundProfile) {
+ I = llvm::upper_bound(SupportedExperimentalProfiles, Arch, ProfileCmp);
+ FoundProfile = (I != std::begin(SupportedExperimentalProfiles) &&
+ Arch.starts_with(std::prev(I)->Name));
+ if (FoundProfile && !EnableExperimentalExtension) {
+ return createStringError(errc::invalid_argument,
+ "requires '-menable-experimental-extensions' "
+ "for profile '" +
+ std::prev(I)->Name + "'");
+ }
+ }
+ if (FoundProfile) {
+ --I;
std::string NewArch = I->MArch.str();
StringRef ArchWithoutProfile = Arch.drop_front(I->Name.size());
if (!ArchWithoutProfile.empty()) {
diff --git a/llvm/test/CodeGen/RISCV/attributes.ll b/llvm/test/CodeGen/RISCV/attributes.ll
index 8f49f6648ad28..953ed5ee3795b 100644
--- a/llvm/test/CodeGen/RISCV/attributes.ll
+++ b/llvm/test/CodeGen/RISCV/attributes.ll
@@ -265,11 +265,11 @@
; RUN: llc -mtriple=riscv64 -mattr=+rva20s64 %s -o - | FileCheck --check-prefix=RVA20S64 %s
; RUN: llc -mtriple=riscv64 -mattr=+rva22u64 %s -o - | FileCheck --check-prefix=RVA22U64 %s
; RUN: llc -mtriple=riscv64 -mattr=+rva22s64 %s -o - | FileCheck --check-prefix=RVA22S64 %s
-; RUN: llc -mtriple=riscv64 -mattr=+rva23u64 %s -o - | FileCheck --check-prefix=RVA23U64 %s
-; RUN: llc -mtriple=riscv64 -mattr=+rva23s64 %s -o - | FileCheck --check-prefix=RVA23S64 %s
-; RUN: llc -mtriple=riscv64 -mattr=+rvb23u64 %s -o - | FileCheck --check-prefix=RVB23U64 %s
-; RUN: llc -mtriple=riscv64 -mattr=+rvb23s64 %s -o - | FileCheck --check-prefix=RVB23S64 %s
-; RUN: llc -mtriple=riscv32 -mattr=+rvm23u32 %s -o - | FileCheck --check-prefix=RVM23U32 %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-rva23u64 %s -o - | FileCheck --check-prefix=RVA23U64 %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-rva23s64 %s -o - | FileCheck --check-prefix=RVA23S64 %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-rvb23u64 %s -o - | FileCheck --check-prefix=RVB23U64 %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-rvb23s64 %s -o - | FileCheck --check-prefix=RVB23S64 %s
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-rvm23u32 %s -o - | FileCheck --check-prefix=RVM23U32 %s
; CHECK: .attribute 4, 16
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index d04f21fa20063..22fe31809319f 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -726,6 +726,13 @@ TEST(ParseArchString,
"duplicated standard user-level extension 'zicntr'");
}
+TEST(ParseArchString,
+ RejectsExperimentalProfilesIfEnableExperimentalExtensionsNotSet) {
+ EXPECT_EQ(
+ toString(RISCVISAInfo::parseArchString("rva23u64", false).takeError()),
+ "requires '-menable-experimental-extensions' for profile 'rva23u64'");
+}
+
TEST(ToFeatures, IIsDroppedAndExperimentalExtensionsArePrefixed) {
auto MaybeISAInfo1 =
RISCVISAInfo::parseArchString("rv64im_ztso", true, false);
@@ -1073,12 +1080,14 @@ Supported Profiles
rva20u64
rva22s64
rva22u64
+ rvi20u32
+ rvi20u64
+
+Experimental Profiles
rva23s64
rva23u64
rvb23s64
rvb23u64
- rvi20u32
- rvi20u64
rvm23u32
Use -march to specify the target's extension.
More information about the llvm-commits
mailing list