[PATCH] D66002: [RISCV] Move architecture parsing code into its own function
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371492: [RISCV] Move architecture parsing code into its own
function (authored by rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D66002?vs=214344=219482#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66002/new/
https://reviews.llvm.org/D66002
Files:
cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -189,167 +189,177 @@
}
}
-void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
- std::vector ) {
- if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
+// Returns false if an error is diagnosed.
+static bool getArchFeatures(const Driver , StringRef MArch,
+std::vector ,
+const ArgList ) {
+ // RISC-V ISA strings must be lowercase.
+ if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must be lowercase";
+return false;
+ }
-// RISC-V ISA strings must be lowercase.
-if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
- D.Diag(diag::err_drv_invalid_riscv_arch_name)
- << MArch << "string must be lowercase";
- return;
-}
+ // ISA string must begin with rv32 or rv64.
+ if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
+ (MArch.size() < 5)) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must begin with rv32{i,e,g} or rv64{i,g}";
+return false;
+ }
-// ISA string must begin with rv32 or rv64.
-if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
-(MArch.size() < 5)) {
- D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "string must begin with rv32{i,e,g} or rv64{i,g}";
- return;
-}
+ bool HasRV64 = MArch.startswith("rv64");
-bool HasRV64 = MArch.startswith("rv64");
+ // The canonical order specified in ISA manual.
+ // Ref: Table 22.1 in RISC-V User-Level ISA V2.2
+ StringRef StdExts = "mafdqlcbjtpvn";
+ bool HasF = false, HasD = false;
+ char Baseline = MArch[4];
+
+ // First letter should be 'e', 'i' or 'g'.
+ switch (Baseline) {
+ default:
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "first letter should be 'e', 'i' or 'g'";
+return false;
+ case 'e': {
+StringRef Error;
+// Currently LLVM does not support 'e'.
+// Extension 'e' is not allowed in rv64.
+if (HasRV64)
+ Error = "standard user-level extension 'e' requires 'rv32'";
+else
+ Error = "unsupported standard user-level extension 'e'";
+D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch << Error;
+return false;
+ }
+ case 'i':
+break;
+ case 'g':
+// g = imafd
+StdExts = StdExts.drop_front(4);
+Features.push_back("+m");
+Features.push_back("+a");
+Features.push_back("+f");
+Features.push_back("+d");
+HasF = true;
+HasD = true;
+break;
+ }
-// The canonical order specified in ISA manual.
-// Ref: Table 22.1 in RISC-V User-Level ISA V2.2
-StringRef StdExts = "mafdqlcbjtpvn";
-bool HasF = false, HasD = false;
-char Baseline = MArch[4];
+ // Skip rvxxx
+ StringRef Exts = MArch.substr(5);
-// First letter should be 'e', 'i' or 'g'.
-switch (Baseline) {
-default:
- D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "first letter should be 'e', 'i' or 'g'";
- return;
-case 'e': {
+ // Remove non-standard extensions and supervisor-level extensions.
+ // They have 'x', 's', 'sx' prefixes. Parse them at the end.
+ // Find the very first occurrence of 's' or 'x'.
+ StringRef OtherExts;
+ size_t Pos = Exts.find_first_of("sx");
+ if (Pos != StringRef::npos) {
+OtherExts = Exts.substr(Pos);
+Exts = Exts.substr(0, Pos);
+ }
+
+ std::string Major, Minor;
+ if (!getExtensionVersion(D, MArch, std::string(1, Baseline), Exts, Major,
+ Minor))
+return false;
+
+ // TODO: Use version number when setting target features
+ // and consume the underscore '_' that might follow.
+
+ auto StdExtsItr = StdExts.begin();
+ auto StdExtsEnd = StdExts.end();
+
+ for (auto I = Exts.begin(), E = Exts.end(); I != E; ++I) {
+char c = *I;
+
+// Check ISA extensions are specified in the canonical order.
+while (StdExtsItr != StdExtsEnd && *StdExtsItr != c)
+ ++StdExtsItr;
+
+if (StdExtsItr == StdExtsEnd) {
+ // Either c contains a valid extension but it was not given
[PATCH] D66002: [RISCV] Move architecture parsing code into its own function
rogfer01 added a comment. Herald added a subscriber: pzheng. Thanks for the review @luismarques I plan to commit this shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66002/new/ https://reviews.llvm.org/D66002 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66002: [RISCV] Move architecture parsing code into its own function
luismarques added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66002/new/ https://reviews.llvm.org/D66002 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66002: [RISCV] Move architecture parsing code into its own function
rogfer01 created this revision.
rogfer01 added reviewers: asb, lenary.
Herald added subscribers: cfe-commits, s.egerton, Jim, benna, psnobl, jocewei,
PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, MaskRay,
jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso,
rbar.
Herald added a project: clang.
I plan to reuse it in a later patch.
This is almost NFC except a small change in control flow when diagnosing `+d`
without `+f`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D66002
Files:
clang/lib/Driver/ToolChains/Arch/RISCV.cpp
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -189,167 +189,177 @@
}
}
-void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
- std::vector ) {
- if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
+// Returns false if an error is diagnosed.
+static bool getArchFeatures(const Driver , StringRef MArch,
+std::vector ,
+const ArgList ) {
+ // RISC-V ISA strings must be lowercase.
+ if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must be lowercase";
+return false;
+ }
-// RISC-V ISA strings must be lowercase.
-if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
- D.Diag(diag::err_drv_invalid_riscv_arch_name)
- << MArch << "string must be lowercase";
- return;
-}
+ // ISA string must begin with rv32 or rv64.
+ if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
+ (MArch.size() < 5)) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must begin with rv32{i,e,g} or rv64{i,g}";
+return false;
+ }
-// ISA string must begin with rv32 or rv64.
-if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
-(MArch.size() < 5)) {
- D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "string must begin with rv32{i,e,g} or rv64{i,g}";
- return;
-}
+ bool HasRV64 = MArch.startswith("rv64");
+
+ // The canonical order specified in ISA manual.
+ // Ref: Table 22.1 in RISC-V User-Level ISA V2.2
+ StringRef StdExts = "mafdqlcbjtpvn";
+ bool HasF = false, HasD = false;
+ char Baseline = MArch[4];
+
+ // First letter should be 'e', 'i' or 'g'.
+ switch (Baseline) {
+ default:
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "first letter should be 'e', 'i' or 'g'";
+return false;
+ case 'e': {
+StringRef Error;
+// Currently LLVM does not support 'e'.
+// Extension 'e' is not allowed in rv64.
+if (HasRV64)
+ Error = "standard user-level extension 'e' requires 'rv32'";
+else
+ Error = "unsupported standard user-level extension 'e'";
+D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch << Error;
+return false;
+ }
+ case 'i':
+break;
+ case 'g':
+// g = imafd
+StdExts = StdExts.drop_front(4);
+Features.push_back("+m");
+Features.push_back("+a");
+Features.push_back("+f");
+Features.push_back("+d");
+HasF = true;
+HasD = true;
+break;
+ }
-bool HasRV64 = MArch.startswith("rv64");
+ // Skip rvxxx
+ StringRef Exts = MArch.substr(5);
+
+ // Remove non-standard extensions and supervisor-level extensions.
+ // They have 'x', 's', 'sx' prefixes. Parse them at the end.
+ // Find the very first occurrence of 's' or 'x'.
+ StringRef OtherExts;
+ size_t Pos = Exts.find_first_of("sx");
+ if (Pos != StringRef::npos) {
+OtherExts = Exts.substr(Pos);
+Exts = Exts.substr(0, Pos);
+ }
-// The canonical order specified in ISA manual.
-// Ref: Table 22.1 in RISC-V User-Level ISA V2.2
-StringRef StdExts = "mafdqlcbjtpvn";
-bool HasF = false, HasD = false;
-char Baseline = MArch[4];
+ std::string Major, Minor;
+ if (!getExtensionVersion(D, MArch, std::string(1, Baseline), Exts, Major,
+ Minor))
+return false;
-// First letter should be 'e', 'i' or 'g'.
-switch (Baseline) {
-default:
- D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "first letter should be 'e', 'i' or 'g'";
- return;
-case 'e': {
+ // TODO: Use version number when setting target features
+ // and consume the underscore '_' that might follow.
+
+ auto StdExtsItr = StdExts.begin();
+ auto StdExtsEnd = StdExts.end();
+
+ for (auto I = Exts.begin(), E = Exts.end(); I != E; ++I) {
+char c = *I;
+
+// Check ISA extensions are specified in the canonical order.
+while (StdExtsItr != StdExtsEnd && *StdExtsItr != c)
+ ++StdExtsItr;
+
+if (StdExtsItr == StdExtsEnd) {
+
