[clang] [llvm] [RISCV] Remove SeenExtMap from RISCVISAInfo::parseArchString. (PR #97506)

Craig Topper via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 2 19:17:44 PDT 2024


https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/97506

>From d64fc1ad1a214ce8cbba95836e2eced70b308c5e Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 2 Jul 2024 19:08:11 -0700
Subject: [PATCH 1/2] [RISCV] Remove SeenExtMap from
 RISCVISAInfo::parseArchString.

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 uknown 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 | 30 ++++++++++----------------
 2 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index ffd92e1f398c4..0f285f7c0033c 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 b4fd067a1ed7a..5be936a7c642c 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -577,9 +577,6 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
         "profile name");
 
   std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));
-  MapVector<std::string, RISCVISAUtils::ExtensionVersion,
-            std::map<std::string, unsigned>>
-      SeenExtMap;
 
   // The canonical order specified in ISA manual.
   // Ref: Table 22.1 in RISC-V User-Level ISA V2.2
@@ -603,8 +600,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.
@@ -618,11 +614,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;
   }
@@ -686,24 +682,20 @@ 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 getStringErrorForInvalidExt(Name);
+
+      // Insert and error for duplicates.
+      if (!ISAInfo->Exts
+               .emplace(Name.str(),
+                        RISCVISAUtils::ExtensionVersion{Major, Minor})
+               .second)
         return createStringError(errc::invalid_argument,
                                  "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 getStringErrorForInvalidExt(ExtName);
-    ISAInfo->Exts[ExtName] = SeenExtAndVers.second;
-  }
-
   return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
 }
 

>From 673a0ff3512eab9c1d86fb86fc62cfc533c81b32 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 2 Jul 2024 19:17:29 -0700
Subject: [PATCH 2/2] fixup! Remove MapVector/SetVector.h includes.

---
 llvm/lib/TargetParser/RISCVISAInfo.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 5be936a7c642c..c5249606dc671 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"



More information about the cfe-commits mailing list