-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[RISCV] Add wrapper function for getStringError in RISCVISAInfo. NFC #97478
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We use the same error code for all errors. Add a wrapper so we don't have to repeat it. r Please enter the commit message for your changes. Lines starting
@llvm/pr-subscribers-backend-risc-v Author: Craig Topper (topperc) ChangesWe use the same error code for all errors. Add a wrapper so we don't have to repeat it. Full diff: https://github.com/llvm/llvm-project/pull/97478.diff 1 Files Affected:
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index b4fd067a1ed7a..133c6852bc85e 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -282,15 +282,17 @@ std::vector<std::string> RISCVISAInfo::toFeatures(bool AddAllExtensions,
return Features;
}
-static Error getStringErrorForInvalidExt(StringRef ExtName) {
+static Error getError(const Twine &Message) {
+ return createStringError(errc::invalid_argument, Message);
+}
+
+static Error getErrorForInvalidExt(StringRef ExtName) {
if (ExtName.size() == 1) {
- return createStringError(errc::invalid_argument,
- "unsupported standard user-level extension '" +
- ExtName + "'");
+ return getError("unsupported standard user-level extension '" + ExtName +
+ "'");
}
- return createStringError(errc::invalid_argument,
- "unsupported " + getExtensionTypeDesc(ExtName) +
- " '" + ExtName + "'");
+ return getError("unsupported " + getExtensionTypeDesc(ExtName) + " '" +
+ ExtName + "'");
}
// Extensions may have a version number, and may be separated by
@@ -315,21 +317,18 @@ static Error getExtensionVersion(StringRef Ext, StringRef In, unsigned &Major,
// Expected 'p' to be followed by minor version number.
if (MinorStr.empty()) {
- return createStringError(
- errc::invalid_argument,
- "minor version number missing after 'p' for extension '" + Ext + "'");
+ return getError("minor version number missing after 'p' for extension '" +
+ Ext + "'");
}
}
if (!MajorStr.empty() && MajorStr.getAsInteger(10, Major))
- return createStringError(
- errc::invalid_argument,
- "Failed to parse major version number for extension '" + Ext + "'");
+ return getError("Failed to parse major version number for extension '" +
+ Ext + "'");
if (!MinorStr.empty() && MinorStr.getAsInteger(10, Minor))
- return createStringError(
- errc::invalid_argument,
- "Failed to parse minor version number for extension '" + Ext + "'");
+ return getError("Failed to parse minor version number for extension '" +
+ Ext + "'");
ConsumeLength = MajorStr.size();
@@ -340,24 +339,21 @@ static Error getExtensionVersion(StringRef Ext, StringRef In, unsigned &Major,
// subsequent characters (i.e. must either end string or be followed by
// an underscore).
if (Ext.size() > 1 && In.size())
- return createStringError(
- errc::invalid_argument,
+ return getError(
"multi-character extensions must be separated by underscores");
// If experimental extension, require use of current version number
if (auto ExperimentalExtension = isExperimentalExtension(Ext)) {
if (!EnableExperimentalExtension)
- return createStringError(errc::invalid_argument,
- "requires '-menable-experimental-extensions' "
- "for experimental extension '" +
- Ext + "'");
+ return getError("requires '-menable-experimental-extensions' "
+ "for experimental extension '" +
+ Ext + "'");
if (ExperimentalExtensionVersionCheck &&
(MajorStr.empty() && MinorStr.empty()))
- return createStringError(
- errc::invalid_argument,
+ return getError(
"experimental extension requires explicit version number `" + Ext +
- "`");
+ "`");
auto SupportedVers = *ExperimentalExtension;
if (ExperimentalExtensionVersionCheck &&
@@ -368,7 +364,7 @@ static Error getExtensionVersion(StringRef Ext, StringRef In, unsigned &Major,
Error += " for experimental extension '" + Ext.str() +
"' (this compiler supports " + utostr(SupportedVers.Major) +
"." + utostr(SupportedVers.Minor) + ")";
- return createStringError(errc::invalid_argument, Error);
+ return getError(Error);
}
return Error::success();
}
@@ -392,13 +388,13 @@ static Error getExtensionVersion(StringRef Ext, StringRef In, unsigned &Major,
return Error::success();
if (!RISCVISAInfo::isSupportedExtension(Ext))
- return getStringErrorForInvalidExt(Ext);
+ return getErrorForInvalidExt(Ext);
- std::string Error = "unsupported version number " + std::string(MajorStr);
+ std::string Error = "unsupported version number " + MajorStr.str();
if (!MinorStr.empty())
Error += "." + MinorStr.str();
Error += " for extension '" + Ext.str() + "'";
- return createStringError(errc::invalid_argument, Error);
+ return getError(Error);
}
llvm::Expected<std::unique_ptr<RISCVISAInfo>>
@@ -439,8 +435,7 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
// RISC-V ISA strings must be [a-z0-9_]
if (!llvm::all_of(
Arch, [](char C) { return isDigit(C) || isLower(C) || C == '_'; }))
- return createStringError(errc::invalid_argument,
- "string may only contain [a-z0-9_]");
+ return getError("string may only contain [a-z0-9_]");
// Must start with a valid base ISA name.
unsigned XLen = 0;
@@ -450,8 +445,7 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
XLen = 64;
if (XLen == 0 || Arch.empty() || (Arch[0] != 'i' && Arch[0] != 'e'))
- return createStringError(errc::invalid_argument,
- "arch string must begin with valid base ISA");
+ return getError("arch string must begin with valid base ISA");
std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));
@@ -461,8 +455,7 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
while (!Arch.empty()) {
if (Arch[0] == '_') {
if (Arch.size() == 1 || Arch[1] == '_')
- return createStringError(errc::invalid_argument,
- "extension name missing after separator '_'");
+ return getError("extension name missing after separator '_'");
Arch = Arch.drop_front();
}
@@ -473,12 +466,10 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
StringRef Prefix, MinorVersionStr;
std::tie(Prefix, MinorVersionStr) = Ext.rsplit('p');
if (MinorVersionStr.empty())
- return createStringError(errc::invalid_argument,
- "extension lacks version in expected format");
+ return getError("extension lacks version in expected format");
unsigned MajorVersion, MinorVersion;
if (MinorVersionStr.getAsInteger(10, MinorVersion))
- return createStringError(errc::invalid_argument,
- "failed to parse minor version number");
+ return getError("failed to parse minor version number");
// Split Prefix into the extension name and the major version number
// (the trailing digits of Prefix).
@@ -489,32 +480,27 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
--VersionStart;
}
if (VersionStart == Prefix.size())
- return createStringError(errc::invalid_argument,
- "extension lacks version in expected format");
+ return getError("extension lacks version in expected format");
if (VersionStart == 0)
- return createStringError(errc::invalid_argument,
- "missing extension name");
+ return getError("missing extension name");
StringRef ExtName = Prefix.slice(0, VersionStart);
StringRef MajorVersionStr = Prefix.slice(VersionStart, StringRef::npos);
if (MajorVersionStr.getAsInteger(10, MajorVersion))
- return createStringError(errc::invalid_argument,
- "failed to parse major version number");
+ return getError("failed to parse major version number");
if ((ExtName[0] == 'z' || ExtName[0] == 's' || ExtName[0] == 'x') &&
(ExtName.size() == 1 || isDigit(ExtName[1])))
- return createStringError(errc::invalid_argument,
- "'" + Twine(ExtName[0]) +
- "' must be followed by a letter");
+ return getError("'" + Twine(ExtName[0]) +
+ "' must be followed by a letter");
if (!ISAInfo->Exts
.emplace(
ExtName.str(),
RISCVISAUtils::ExtensionVersion{MajorVersion, MinorVersion})
.second)
- return createStringError(errc::invalid_argument,
- "duplicate extension '" + ExtName + "'");
+ return getError("duplicate extension '" + ExtName + "'");
}
ISAInfo->updateImpliedLengths();
return std::move(ISAInfo);
@@ -526,8 +512,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// RISC-V ISA strings must be [a-z0-9_]
if (!llvm::all_of(
Arch, [](char C) { return isDigit(C) || isLower(C) || C == '_'; }))
- return createStringError(errc::invalid_argument,
- "string may only contain [a-z0-9_]");
+ return getError("string may only contain [a-z0-9_]");
// ISA string must begin with rv32, rv64, or a profile.
unsigned XLen = 0;
@@ -548,10 +533,9 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
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 + "'");
+ return getError("requires '-menable-experimental-extensions' "
+ "for profile '" +
+ std::prev(I)->Name + "'");
}
}
if (FoundProfile) {
@@ -560,9 +544,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
StringRef ArchWithoutProfile = Arch.drop_front(I->Name.size());
if (!ArchWithoutProfile.empty()) {
if (ArchWithoutProfile.front() != '_')
- return createStringError(
- errc::invalid_argument,
- "additional extensions must be after separator '_'");
+ return getError("additional extensions must be after separator '_'");
NewArch += ArchWithoutProfile.str();
}
return parseArchString(NewArch, EnableExperimentalExtension,
@@ -571,8 +553,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
}
if (XLen == 0 || Arch.empty())
- return createStringError(
- errc::invalid_argument,
+ return getError(
"string must begin with rv32{i,e,g}, rv64{i,e,g}, or a supported "
"profile name");
@@ -592,9 +573,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// First letter should be 'e', 'i' or 'g'.
switch (Baseline) {
default:
- return createStringError(errc::invalid_argument,
- "first letter after \'rv" + Twine(XLen) +
- "\' should be 'e', 'i' or 'g'");
+ return getError("first letter after \'rv" + Twine(XLen) +
+ "\' should be 'e', 'i' or 'g'");
case 'e':
case 'i':
// Baseline is `i` or `e`
@@ -609,8 +589,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
case 'g':
// g expands to extensions in RISCVGImplications.
if (Arch.size() > 1 && isDigit(Arch[1]))
- return createStringError(errc::invalid_argument,
- "version not supported for 'g'");
+ return getError("version not supported for 'g'");
// Versions for g are disallowed, and this was checked for previously.
ConsumeLength = 0;
@@ -634,8 +613,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
while (!Exts.empty()) {
if (Exts.front() == '_') {
if (Exts.size() == 1 || Exts[1] == '_')
- return createStringError(errc::invalid_argument,
- "extension name missing after separator '_'");
+ return getError("extension name missing after separator '_'");
Exts = Exts.drop_front();
}
@@ -669,12 +647,10 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
assert(!Type.empty() && "Empty type?");
if (Name.size() == Type.size())
- return createStringError(errc::invalid_argument,
- Desc + " name missing after '" + Type + "'");
+ return getError(Desc + " name missing after '" + Type + "'");
} else {
- return createStringError(errc::invalid_argument,
- "invalid standard user-level extension '" +
- Twine(Ext.front()) + "'");
+ return getError("invalid standard user-level extension '" +
+ Twine(Ext.front()) + "'");
}
unsigned Major, Minor, ConsumeLength;
@@ -688,8 +664,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
// Check if duplicated extension.
if (SeenExtMap.contains(Name.str()))
- return createStringError(errc::invalid_argument,
- "duplicated " + Desc + " '" + Name + "'");
+ return getError("duplicated " + Desc + " '" + Name + "'");
SeenExtMap[Name.str()] = {Major, Minor};
} while (!Ext.empty());
@@ -700,7 +675,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
const std::string &ExtName = SeenExtAndVers.first;
if (!RISCVISAInfo::isSupportedExtension(ExtName))
- return getStringErrorForInvalidExt(ExtName);
+ return getErrorForInvalidExt(ExtName);
ISAInfo->Exts[ExtName] = SeenExtAndVers.second;
}
@@ -718,60 +693,49 @@ Error RISCVISAInfo::checkDependency() {
bool HasZcmt = Exts.count("zcmt") != 0;
if (HasI && HasE)
- return createStringError(errc::invalid_argument,
- "'I' and 'E' extensions are incompatible");
+ return getError("'I' and 'E' extensions are incompatible");
if (HasF && HasZfinx)
- return createStringError(errc::invalid_argument,
- "'f' and 'zfinx' extensions are incompatible");
+ return getError("'f' and 'zfinx' extensions are incompatible");
if (HasZvl && !HasVector)
- return createStringError(
- errc::invalid_argument,
+ return getError(
"'zvl*b' requires 'v' or 'zve*' extension to also be specified");
if (Exts.count("zvbb") && !HasVector)
- return createStringError(
- errc::invalid_argument,
+ return getError(
"'zvbb' requires 'v' or 'zve*' extension to also be specified");
if (Exts.count("zvbc") && !Exts.count("zve64x"))
- return createStringError(
- errc::invalid_argument,
+ return getError(
"'zvbc' requires 'v' or 'zve64*' extension to also be specified");
if ((Exts.count("zvkb") || Exts.count("zvkg") || Exts.count("zvkned") ||
Exts.count("zvknha") || Exts.count("zvksed") || Exts.count("zvksh")) &&
!HasVector)
- return createStringError(
- errc::invalid_argument,
+ return getError(
"'zvk*' requires 'v' or 'zve*' extension to also be specified");
if (Exts.count("zvknhb") && !Exts.count("zve64x"))
- return createStringError(
- errc::invalid_argument,
+ return getError(
"'zvknhb' requires 'v' or 'zve64*' extension to also be specified");
if ((HasZcmt || Exts.count("zcmp")) && Exts.count("d") &&
(HasC || Exts.count("zcd")))
- return createStringError(
- errc::invalid_argument,
- Twine("'") + (HasZcmt ? "zcmt" : "zcmp") +
- "' extension is incompatible with '" + (HasC ? "c" : "zcd") +
- "' extension when 'd' extension is enabled");
+ return getError(Twine("'") + (HasZcmt ? "zcmt" : "zcmp") +
+ "' extension is incompatible with '" +
+ (HasC ? "c" : "zcd") +
+ "' extension when 'd' extension is enabled");
if (XLen != 32 && Exts.count("zcf"))
- return createStringError(errc::invalid_argument,
- "'zcf' is only supported for 'rv32'");
+ return getError("'zcf' is only supported for 'rv32'");
if (Exts.count("zacas") && !(Exts.count("a") || Exts.count("zaamo")))
- return createStringError(
- errc::invalid_argument,
+ return getError(
"'zacas' requires 'a' or 'zaamo' extension to also be specified");
if (Exts.count("zabha") && !(Exts.count("a") || Exts.count("zaamo")))
- return createStringError(
- errc::invalid_argument,
+ return getError(
"'zabha' requires 'a' or 'zaamo' extension to also be specified");
return Error::success();
|
wangpc-pp
approved these changes
Jul 3, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
…lvm#97478) We use the same error code for all errors. Add a wrapper so we don't have to repeat it.
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
…lvm#97478) We use the same error code for all errors. Add a wrapper so we don't have to repeat it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We use the same error code for all errors. Add a wrapper so we don't have to repeat it.