[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) { +