From c156d421851d175805309a83bf703ad304955847 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 3 Jul 2024 11:49:08 -0700 Subject: [PATCH] [RISCV] Remove SeenExtMap from RISCVISAInfo::parseArchString. (#97506) Use the Exts map directly instead of adding to a temporary MapVector first. There are a couple functional change from this. -If an unknown extension is duplicated, we will now print an error for it being unknown instead of an error for it being duplicated. -If an unknown extension is followed by an underscore with no extension after it, we will error for the unknown extension instead of the dangling underscore. These don't seem like serious changes to me. I've updated tests accordingly. --- clang/test/Driver/riscv-arch.c | 8 +++---- llvm/lib/TargetParser/RISCVISAInfo.cpp | 32 +++++++++----------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c index ffd92e1f398c45..0f285f7c0033cf 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=rv32ixabc_ -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XSEP %s // RV32-XSEP: error: invalid arch name 'rv32ixabc_', -// RV32-XSEP: extension name missing after separator '_' +// RV32-XSEP: unsupported non-standard user-level extension 'xabc' // RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_a -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-PREFIX %s @@ -318,10 +318,10 @@ // RV32-X-ORDER: error: invalid arch name 'rv32ixdef_sabc', // RV32-X-ORDER unsupported non-standard user-level extension 'xdef' -// RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_xabc -### %s \ +// RUN: not %clang --target=riscv32-unknown-elf -march=rv32im_m -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-XDUP %s -// RV32-XDUP: error: invalid arch name 'rv32ixabc_xabc', -// RV32-XDUP: duplicated non-standard user-level extension 'xabc' +// RV32-XDUP: error: invalid arch name 'rv32im_m', +// RV32-XDUP: duplicated standard user-level extension 'm' // RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_xdef -### %s \ // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-X-INVAL %s diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp index 133c6852bc85e8..ee8024b3bea36c 100644 --- a/llvm/lib/TargetParser/RISCVISAInfo.cpp +++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp @@ -7,9 +7,7 @@ //===----------------------------------------------------------------------===// #include "llvm/TargetParser/RISCVISAInfo.h" -#include "llvm/ADT/MapVector.h" #include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SetVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Errc.h" @@ -558,9 +556,6 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, "profile name"); std::unique_ptr ISAInfo(new RISCVISAInfo(XLen)); - MapVector> - SeenExtMap; // The canonical order specified in ISA manual. // Ref: Table 22.1 in RISC-V User-Level ISA V2.2 @@ -583,8 +578,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, EnableExperimentalExtension, ExperimentalExtensionVersionCheck)) return std::move(E); - // Postpone AddExtension until end of this function - SeenExtMap[StringRef(&Baseline, 1).str()] = {Major, Minor}; + ISAInfo->Exts[std::string(1, Baseline)] = {Major, Minor}; break; case 'g': // g expands to extensions in RISCVGImplications. @@ -597,11 +591,11 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, // No matter which version is given to `g`, we always set imafd to default // version since the we don't have clear version scheme for that on // ISA spec. - for (const auto *Ext : RISCVGImplications) { + for (const char *Ext : RISCVGImplications) { auto Version = findDefaultVersion(Ext); assert(Version && "Default extension version not found?"); // Postpone AddExtension until end of this function - SeenExtMap[Ext] = {Version->Major, Version->Minor}; + ISAInfo->Exts[std::string(Ext)] = {Version->Major, Version->Minor}; } break; } @@ -662,23 +656,19 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, if (Name.size() == 1) Ext = Ext.substr(ConsumeLength); - // Check if duplicated extension. - if (SeenExtMap.contains(Name.str())) + if (!RISCVISAInfo::isSupportedExtension(Name)) + return getErrorForInvalidExt(Name); + + // Insert and error for duplicates. + if (!ISAInfo->Exts + .emplace(Name.str(), + RISCVISAUtils::ExtensionVersion{Major, Minor}) + .second) return getError("duplicated " + Desc + " '" + Name + "'"); - SeenExtMap[Name.str()] = {Major, Minor}; } while (!Ext.empty()); } - // Check all Extensions are supported. - for (auto &SeenExtAndVers : SeenExtMap) { - const std::string &ExtName = SeenExtAndVers.first; - - if (!RISCVISAInfo::isSupportedExtension(ExtName)) - return getErrorForInvalidExt(ExtName); - ISAInfo->Exts[ExtName] = SeenExtAndVers.second; - } - return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo)); }