-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[RISCV] Improve error message when the extension is not supported #83989
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
Conversation
If the "march" has some extension with version that is not supported, it returns the error message like: "error: invalid arch name 'some_arch', unsupported version number 2.0 for extension 'some_arch'", which is not precise enough, it should return the message that only tells users "the extension is not supported".
@llvm/pr-subscribers-clang @llvm/pr-subscribers-llvm-support Author: Brandon Wu (4vtomat) ChangesIf the "march" has some extension with version that is not supported, it Full diff: https://github.com/llvm/llvm-project/pull/83989.diff 2 Files Affected:
diff --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index c9e984e07cbea9..8399b4e97f86d5 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -306,7 +306,7 @@
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ist2p0 -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-SMINOR0 %s
// RV32-SMINOR0: error: invalid arch name 'rv32ist2p0',
-// RV32-SMINOR0: unsupported version number 2.0 for extension 'st'
+// RV32-SMINOR0: unsupported standard supervisor-level extension 'st'
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_ -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XSEP %s
@@ -397,7 +397,7 @@
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32izbb1p0zbs1p0 -menable-experimental-extensions -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-EXPERIMENTAL-ZBB-ZBS-UNDERSCORE %s
-// RV32-EXPERIMENTAL-ZBB-ZBS-UNDERSCORE: error: invalid arch name 'rv32izbb1p0zbs1p0', unsupported version number 1.0 for extension 'zbb1p0zbs'
+// RV32-EXPERIMENTAL-ZBB-ZBS-UNDERSCORE: error: invalid arch name 'rv32izbb1p0zbs1p0', unsupported standard user-level extension 'zbb1p0zbs'
// RUN: %clang --target=riscv32-unknown-elf -march=rv32izba1p0 -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZBA %s
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 54b4dcb22de8e0..6eec03fd6f7082 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -531,6 +531,17 @@ std::vector<std::string> RISCVISAInfo::toFeatures(bool AddAllExtensions,
return Features;
}
+static Error getStringErrorForInvalidExt(std::string_view ExtName) {
+ if (ExtName.size() == 1) {
+ return createStringError(errc::invalid_argument,
+ "unsupported standard user-level extension '" +
+ ExtName + "'");
+ }
+ return createStringError(errc::invalid_argument,
+ "unsupported " + getExtensionTypeDesc(ExtName) +
+ " '" + ExtName + "'");
+}
+
// Extensions may have a version number, and may be separated by
// an underscore '_' e.g.: rv32i2_m2.
// Version number is divided into major and minor version numbers,
@@ -629,6 +640,9 @@ static Error getExtensionVersion(StringRef Ext, StringRef In, unsigned &Major,
if (RISCVISAInfo::isSupportedExtension(Ext, Major, Minor))
return Error::success();
+ if (!RISCVISAInfo::isSupportedExtension(Ext))
+ return getStringErrorForInvalidExt(Ext);
+
std::string Error = "unsupported version number " + std::string(MajorStr);
if (!MinorStr.empty())
Error += "." + MinorStr.str();
@@ -965,16 +979,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
const std::string &ExtName = SeenExtAndVers.first;
RISCVISAInfo::ExtensionVersion ExtVers = SeenExtAndVers.second;
- if (!RISCVISAInfo::isSupportedExtension(ExtName)) {
- if (ExtName.size() == 1) {
- return createStringError(errc::invalid_argument,
- "unsupported standard user-level extension '" +
- ExtName + "'");
- }
- return createStringError(errc::invalid_argument,
- "unsupported " + getExtensionTypeDesc(ExtName) +
- " '" + ExtName + "'");
- }
+ if (!RISCVISAInfo::isSupportedExtension(ExtName))
+ return getStringErrorForInvalidExt(ExtName);
ISAInfo->addExtension(ExtName, ExtVers);
}
|
@llvm/pr-subscribers-backend-risc-v Author: Brandon Wu (4vtomat) ChangesIf the "march" has some extension with version that is not supported, it Full diff: https://github.com/llvm/llvm-project/pull/83989.diff 2 Files Affected:
diff --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index c9e984e07cbea9..8399b4e97f86d5 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -306,7 +306,7 @@
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ist2p0 -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-SMINOR0 %s
// RV32-SMINOR0: error: invalid arch name 'rv32ist2p0',
-// RV32-SMINOR0: unsupported version number 2.0 for extension 'st'
+// RV32-SMINOR0: unsupported standard supervisor-level extension 'st'
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_ -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XSEP %s
@@ -397,7 +397,7 @@
// RUN: not %clang --target=riscv32-unknown-elf -march=rv32izbb1p0zbs1p0 -menable-experimental-extensions -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-EXPERIMENTAL-ZBB-ZBS-UNDERSCORE %s
-// RV32-EXPERIMENTAL-ZBB-ZBS-UNDERSCORE: error: invalid arch name 'rv32izbb1p0zbs1p0', unsupported version number 1.0 for extension 'zbb1p0zbs'
+// RV32-EXPERIMENTAL-ZBB-ZBS-UNDERSCORE: error: invalid arch name 'rv32izbb1p0zbs1p0', unsupported standard user-level extension 'zbb1p0zbs'
// RUN: %clang --target=riscv32-unknown-elf -march=rv32izba1p0 -### %s \
// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZBA %s
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 54b4dcb22de8e0..6eec03fd6f7082 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -531,6 +531,17 @@ std::vector<std::string> RISCVISAInfo::toFeatures(bool AddAllExtensions,
return Features;
}
+static Error getStringErrorForInvalidExt(std::string_view ExtName) {
+ if (ExtName.size() == 1) {
+ return createStringError(errc::invalid_argument,
+ "unsupported standard user-level extension '" +
+ ExtName + "'");
+ }
+ return createStringError(errc::invalid_argument,
+ "unsupported " + getExtensionTypeDesc(ExtName) +
+ " '" + ExtName + "'");
+}
+
// Extensions may have a version number, and may be separated by
// an underscore '_' e.g.: rv32i2_m2.
// Version number is divided into major and minor version numbers,
@@ -629,6 +640,9 @@ static Error getExtensionVersion(StringRef Ext, StringRef In, unsigned &Major,
if (RISCVISAInfo::isSupportedExtension(Ext, Major, Minor))
return Error::success();
+ if (!RISCVISAInfo::isSupportedExtension(Ext))
+ return getStringErrorForInvalidExt(Ext);
+
std::string Error = "unsupported version number " + std::string(MajorStr);
if (!MinorStr.empty())
Error += "." + MinorStr.str();
@@ -965,16 +979,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
const std::string &ExtName = SeenExtAndVers.first;
RISCVISAInfo::ExtensionVersion ExtVers = SeenExtAndVers.second;
- if (!RISCVISAInfo::isSupportedExtension(ExtName)) {
- if (ExtName.size() == 1) {
- return createStringError(errc::invalid_argument,
- "unsupported standard user-level extension '" +
- ExtName + "'");
- }
- return createStringError(errc::invalid_argument,
- "unsupported " + getExtensionTypeDesc(ExtName) +
- " '" + ExtName + "'");
- }
+ if (!RISCVISAInfo::isSupportedExtension(ExtName))
+ return getStringErrorForInvalidExt(ExtName);
ISAInfo->addExtension(ExtName, ExtVers);
}
|
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
perhaps the llvm libSupport prats of this change should be unit tested in LLVM, rather than only tested indirectly in clang? |
Good point, thanks! |
If the "march" has some extension with version that is not supported, it
returns the error message like: "error: invalid arch name 'some_arch',
unsupported version number 2.0 for extension 'some_arch'", which is not
precise enough, it should return the message that only tells users "the
extension is not supported".