[llvm] [RISCV] Remove IgnoreUnknown from RISCVISAInfo::parseArchString. (PR #97372)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 1 18:39:42 PDT 2024
https://github.com/topperc created https://github.com/llvm/llvm-project/pull/97372
This isn't used in tree, and thus I don't know what the expectations for its behavior really are.
>From 8860a00d7fdbb6c8fea79be1c5d3e1acadee4a6c Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 1 Jul 2024 18:28:16 -0700
Subject: [PATCH] [RISCV] Remove IgnoreUnknown from
RISCVISAInfo::parseArchString.
This isn't used in tree, and thus I don't know what the expectations
for its behavior really are.
---
llvm/include/llvm/TargetParser/RISCVISAInfo.h | 3 +-
llvm/lib/TargetParser/RISCVISAInfo.cpp | 36 ++++----------
.../TargetParser/RISCVISAInfoTest.cpp | 48 -------------------
3 files changed, 9 insertions(+), 78 deletions(-)
diff --git a/llvm/include/llvm/TargetParser/RISCVISAInfo.h b/llvm/include/llvm/TargetParser/RISCVISAInfo.h
index 5e9cf67eddcfd..ba2965600decd 100644
--- a/llvm/include/llvm/TargetParser/RISCVISAInfo.h
+++ b/llvm/include/llvm/TargetParser/RISCVISAInfo.h
@@ -36,8 +36,7 @@ class RISCVISAInfo {
/// default version will be used (as ignoring the base is not possible).
static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
parseArchString(StringRef Arch, bool EnableExperimentalExtension,
- bool ExperimentalExtensionVersionCheck = true,
- bool IgnoreUnknown = false);
+ bool ExperimentalExtensionVersionCheck = true);
/// Parse RISC-V ISA info from an arch string that is already in normalized
/// form (as defined in the psABI). Unlike parseArchString, this function
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 869be57928890..b4fd067a1ed7a 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -522,8 +522,7 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
llvm::Expected<std::unique_ptr<RISCVISAInfo>>
RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
- bool ExperimentalExtensionVersionCheck,
- bool IgnoreUnknown) {
+ bool ExperimentalExtensionVersionCheck) {
// RISC-V ISA strings must be [a-z0-9_]
if (!llvm::all_of(
Arch, [](char C) { return isDigit(C) || isLower(C) || C == '_'; }))
@@ -567,7 +566,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
NewArch += ArchWithoutProfile.str();
}
return parseArchString(NewArch, EnableExperimentalExtension,
- ExperimentalExtensionVersionCheck, IgnoreUnknown);
+ ExperimentalExtensionVersionCheck);
}
}
@@ -601,16 +600,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// Baseline is `i` or `e`
if (auto E = getExtensionVersion(
StringRef(&Baseline, 1), Exts, Major, Minor, ConsumeLength,
- EnableExperimentalExtension, ExperimentalExtensionVersionCheck)) {
- if (!IgnoreUnknown)
- return std::move(E);
- // If IgnoreUnknown, then ignore an unrecognised version of the baseline
- // ISA and just use the default supported version.
- consumeError(std::move(E));
- auto Version = findDefaultVersion(StringRef(&Baseline, 1));
- Major = Version->Major;
- Minor = Version->Minor;
- }
+ EnableExperimentalExtension, ExperimentalExtensionVersionCheck))
+ return std::move(E);
// Postpone AddExtension until end of this function
SeenExtMap[StringRef(&Baseline, 1).str()] = {Major, Minor};
@@ -677,11 +668,10 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
Ext = StringRef();
assert(!Type.empty() && "Empty type?");
- if (!IgnoreUnknown && Name.size() == Type.size())
+ if (Name.size() == Type.size())
return createStringError(errc::invalid_argument,
Desc + " name missing after '" + Type + "'");
} else {
- // FIXME: Could it be ignored by IgnoreUnknown?
return createStringError(errc::invalid_argument,
"invalid standard user-level extension '" +
Twine(Ext.front()) + "'");
@@ -690,27 +680,17 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
unsigned Major, Minor, ConsumeLength;
if (auto E = getExtensionVersion(Name, Vers, Major, Minor, ConsumeLength,
EnableExperimentalExtension,
- ExperimentalExtensionVersionCheck)) {
- if (!IgnoreUnknown)
- return E;
-
- consumeError(std::move(E));
- if (Name.size() == 1)
- Ext = Ext.substr(ConsumeLength);
- continue;
- }
+ ExperimentalExtensionVersionCheck))
+ return E;
if (Name.size() == 1)
Ext = Ext.substr(ConsumeLength);
// Check if duplicated extension.
- if (!IgnoreUnknown && SeenExtMap.contains(Name.str()))
+ if (SeenExtMap.contains(Name.str()))
return createStringError(errc::invalid_argument,
"duplicated " + Desc + " '" + Name + "'");
- if (IgnoreUnknown && !RISCVISAInfo::isSupportedExtension(Name))
- continue;
-
SeenExtMap[Name.str()] = {Major, Minor};
} while (!Ext.empty());
}
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index f944ac87b2cb0..eb5437b1ae3d4 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -341,25 +341,6 @@ TEST(ParseArchString, RejectsUnrecognizedExtensionNamesByDefault) {
"unsupported non-standard user-level extension 'xmadeup'");
}
-TEST(ParseArchString, IgnoresUnrecognizedExtensionNamesWithIgnoreUnknown) {
- for (StringRef Input : {"rv32i_zmadeup", "rv64i_smadeup", "rv64i_xmadeup"}) {
- auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
- ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
- RISCVISAInfo &Info = **MaybeISAInfo;
- const auto &Exts = Info.getExtensions();
- EXPECT_EQ(Exts.size(), 1UL);
- EXPECT_TRUE(Exts.at("i") == (RISCVISAUtils::ExtensionVersion{2, 1}));
- }
-
- // Checks that supported extensions aren't incorrectly ignored when a
- // version is present (an early version of the patch had this mistake).
- auto MaybeISAInfo =
- RISCVISAInfo::parseArchString("rv32i_zbc1p0_xmadeup", true, false, true);
- ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
- const auto &Exts = (*MaybeISAInfo)->getExtensions();
- EXPECT_TRUE(Exts.at("zbc") == (RISCVISAUtils::ExtensionVersion{1, 0}));
-}
-
TEST(ParseArchString, AcceptsVersionInLongOrShortForm) {
for (StringRef Input : {"rv64i2p1"}) {
auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true);
@@ -393,35 +374,6 @@ TEST(ParseArchString, RejectsUnrecognizedExtensionVersionsByDefault) {
"unsupported version number 10.10 for extension 'zifencei'");
}
-TEST(ParseArchString,
- UsesDefaultVersionForUnrecognisedBaseISAVersionWithIgnoreUnknown) {
- for (StringRef Input : {"rv32i0p1", "rv32i99p99", "rv64i0p1", "rv64i99p99"}) {
- auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
- ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
- const auto &Exts = (*MaybeISAInfo)->getExtensions();
- EXPECT_EQ(Exts.size(), 1UL);
- EXPECT_TRUE(Exts.at("i") == (RISCVISAUtils::ExtensionVersion{2, 1}));
- }
- for (StringRef Input : {"rv32e0p1", "rv32e99p99", "rv64e0p1", "rv64e99p99"}) {
- auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
- ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
- const auto &Exts = (*MaybeISAInfo)->getExtensions();
- EXPECT_EQ(Exts.size(), 1UL);
- EXPECT_TRUE(Exts.at("e") == (RISCVISAUtils::ExtensionVersion{2, 0}));
- }
-}
-
-TEST(ParseArchString,
- IgnoresExtensionsWithUnrecognizedVersionsWithIgnoreUnknown) {
- for (StringRef Input : {"rv32im1p1", "rv64i_svnapot10p9", "rv32i_zicsr0p5"}) {
- auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
- ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
- const auto &Exts = (*MaybeISAInfo)->getExtensions();
- EXPECT_EQ(Exts.size(), 1UL);
- EXPECT_TRUE(Exts.at("i") == (RISCVISAUtils::ExtensionVersion{2, 1}));
- }
-}
-
TEST(ParseArchString, AcceptsUnderscoreSplittingExtensions) {
for (StringRef Input : {"rv32imafdczifencei", "rv32i_m_a_f_d_c_zifencei"}) {
auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true);
More information about the llvm-commits
mailing list