[clang] [llvm] [RISCV] Relax march string order constraint (PR #78120)

Piyou Chen via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 23 05:26:18 PST 2024


https://github.com/BeMg updated https://github.com/llvm/llvm-project/pull/78120

>From 88eef23588b545f29f3fe62a702ed2121b53c7cd Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Sun, 14 Jan 2024 19:41:59 -0800
Subject: [PATCH 1/6] [RISCV] Relax march string order constraint

---
 clang/test/Driver/riscv-arch.c              |  14 +-
 llvm/lib/Support/RISCVISAInfo.cpp           | 297 ++++++++++----------
 llvm/unittests/Support/RISCVISAInfoTest.cpp |  91 ++++--
 3 files changed, 226 insertions(+), 176 deletions(-)

diff --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index 0ac81ea982f1b61..38de95e4fbf7aa4 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -156,9 +156,8 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32L %s
 // RV32L: error: invalid arch name 'rv32l'
 
-// RUN: not %clang --target=riscv32-unknown-elf -march=rv32imadf -### %s \
-// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMADF %s
-// RV32IMADF: error: invalid arch name 'rv32imadf'
+// RUN: %clang --target=riscv32-unknown-elf -march=rv32imadf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
 
 // RUN: not %clang --target=riscv32-unknown-elf -march=rv32imm -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMM %s
@@ -184,9 +183,8 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64L %s
 // RV64L: error: invalid arch name 'rv64l'
 
-// RUN: not %clang --target=riscv64-unknown-elf -march=rv64imadf -### %s \
-// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64IMADF %s
-// RV64IMADF: error: invalid arch name 'rv64imadf'
+// RUN: %clang --target=riscv64-unknown-elf -march=rv64imadf -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck %s
 
 // RUN: not %clang --target=riscv64-unknown-elf -march=rv64imm -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64IMM %s
@@ -216,7 +214,7 @@
 // RUN: not %clang --target=riscv32-unknown-elf -march=rv32imcq -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ORDER %s
 // RV32-ORDER: error: invalid arch name 'rv32imcq',
-// RV32-ORDER: standard user-level extension not given in canonical order 'q'
+// RV32-ORDER: unsupported standard user-level extension 'q'
 
 // RUN: not %clang --target=riscv32-unknown-elf -march=rv32izvl64b -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-ZVL64B-ER %s
@@ -318,7 +316,7 @@
 // RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixabc_a -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-PREFIX %s
 // RV32-PREFIX: error: invalid arch name 'rv32ixabc_a',
-// RV32-PREFIX: invalid extension prefix 'a'
+// RV32-PREFIX: unsupported non-standard user-level extension 'xabc'
 
 // RUN: not %clang --target=riscv32-unknown-elf -march=rv32ixdef_sabc -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-ORDER %s
diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 390d950486a7956..865ca48aeb90d1a 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -695,6 +695,106 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
   return std::move(ISAInfo);
 }
 
+static Error splitExtsByUnderscore(StringRef Exts,
+                                   std::vector<std::string> &SplitedExts) {
+  SmallVector<StringRef, 8> Split;
+  if (Exts.empty())
+    return Error::success();
+
+  Exts.split(Split, "_");
+
+  for (auto Ext : Split) {
+    if (Ext.empty())
+      return createStringError(errc::invalid_argument,
+                               "extension name missing after separator '_'");
+
+    SplitedExts.push_back(Ext.str());
+  }
+  return Error::success();
+}
+
+static Error processMultiLetterExtension(
+    StringRef RawExt, SmallVector<std::string, 8> &SeenExts,
+    SmallVector<RISCVISAInfo::ExtensionVersion, 8> &ExtsVersion,
+    bool IgnoreUnknown, bool EnableExperimentalExtension,
+    bool ExperimentalExtensionVersionCheck) {
+  StringRef Type = getExtensionType(RawExt);
+  StringRef Desc = getExtensionTypeDesc(RawExt);
+  auto Pos = findLastNonVersionCharacter(RawExt) + 1;
+  StringRef Name(RawExt.substr(0, Pos));
+  StringRef Vers(RawExt.substr(Pos));
+
+  if (Type.empty()) {
+    if (IgnoreUnknown)
+      return Error::success();
+    return createStringError(errc::invalid_argument,
+                             "invalid extension prefix '" + RawExt + "'");
+  }
+
+  if (!IgnoreUnknown && Name.size() == Type.size())
+    return createStringError(errc::invalid_argument,
+                             "%s name missing after '%s'", Desc.str().c_str(),
+                             Type.str().c_str());
+
+  unsigned Major, Minor, ConsumeLength;
+  if (auto E = getExtensionVersion(Name, Vers, Major, Minor, ConsumeLength,
+                                   EnableExperimentalExtension,
+                                   ExperimentalExtensionVersionCheck)) {
+    if (IgnoreUnknown) {
+      consumeError(std::move(E));
+      return Error::success();
+    }
+    return E;
+  }
+
+  // Check if duplicated extension.
+  if (!IgnoreUnknown && llvm::is_contained(SeenExts, Name))
+    return createStringError(errc::invalid_argument, "duplicated %s '%s'",
+                             Desc.str().c_str(), Name.str().c_str());
+
+  if (IgnoreUnknown && !RISCVISAInfo::isSupportedExtension(Name))
+    return Error::success();
+
+  SeenExts.push_back(Name.str());
+  ExtsVersion.push_back({Major, Minor});
+  return Error::success();
+}
+
+static Error processSingleLetterExtension(
+    StringRef &RawExt, SmallVector<std::string, 8> &SeenExts,
+    SmallVector<RISCVISAInfo::ExtensionVersion, 8> &ExtsVersion,
+    bool IgnoreUnknown, bool EnableExperimentalExtension,
+    bool ExperimentalExtensionVersionCheck) {
+  unsigned Major, Minor, ConsumeLength;
+  StringRef Name = RawExt.take_front(1);
+  RawExt.consume_front(Name);
+  if (auto E = getExtensionVersion(Name, RawExt, Major, Minor, ConsumeLength,
+                                   EnableExperimentalExtension,
+                                   ExperimentalExtensionVersionCheck)) {
+    if (IgnoreUnknown) {
+      consumeError(std::move(E));
+      RawExt = RawExt.substr(ConsumeLength);
+      return Error::success();
+    }
+    return E;
+  }
+
+  RawExt = RawExt.substr(ConsumeLength);
+
+  // Check if duplicated extension.
+  if (!IgnoreUnknown && llvm::is_contained(SeenExts, Name))
+    return createStringError(errc::invalid_argument,
+                             "duplicated standard user-level extension '%s'",
+                             Name.str().c_str());
+
+  if (IgnoreUnknown && !RISCVISAInfo::isSupportedExtension(Name))
+    return Error::success();
+
+  SeenExts.push_back(Name.str());
+  ExtsVersion.push_back({Major, Minor});
+  return Error::success();
+}
+
 llvm::Expected<std::unique_ptr<RISCVISAInfo>>
 RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
                               bool ExperimentalExtensionVersionCheck,
@@ -715,6 +815,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
 
   unsigned XLen = HasRV64 ? 64 : 32;
   std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));
+  SmallVector<std::string, 8> SeenExts;
+  SmallVector<RISCVISAInfo::ExtensionVersion, 8> ExtsVersion;
 
   // The canonical order specified in ISA manual.
   // Ref: Table 22.1 in RISC-V User-Level ISA V2.2
@@ -745,17 +847,6 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
   // Skip rvxxx
   StringRef Exts = Arch.substr(5);
 
-  // Remove multi-letter standard extensions, non-standard extensions and
-  // supervisor-level extensions. They have 'z', 'x', 's' prefixes.
-  // Parse them at the end.
-  // Find the very first occurrence of 's', 'x' or 'z'.
-  StringRef OtherExts;
-  size_t Pos = Exts.find_first_of("zsx");
-  if (Pos != StringRef::npos) {
-    OtherExts = Exts.substr(Pos);
-    Exts = Exts.substr(0, Pos);
-  }
-
   unsigned Major, Minor, ConsumeLength;
   if (Baseline == 'g') {
     // Versions for g are disallowed, and this was checked for previously.
@@ -765,9 +856,11 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
     // version since the we don't have clear version scheme for that on
     // ISA spec.
     for (const auto *Ext : RISCVGImplications) {
-      if (auto Version = findDefaultVersion(Ext))
-        ISAInfo->addExtension(Ext, *Version);
-      else
+      if (auto Version = findDefaultVersion(Ext)) {
+        // Postpone AddExtension until end of this function
+        SeenExts.push_back(Ext);
+        ExtsVersion.push_back({Version->Major, Version->Minor});
+      } else
         llvm_unreachable("Default extension version not found?");
     }
   } else {
@@ -785,7 +878,9 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
       Minor = Version->Minor;
     }
 
-    ISAInfo->addExtension(StringRef(&Baseline, 1), {Major, Minor});
+    // Postpone AddExtension until end of this function
+    SeenExts.push_back(StringRef(&Baseline, 1).str());
+    ExtsVersion.push_back({Major, Minor});
   }
 
   // Consume the base ISA version number and any '_' between rvxxx and the
@@ -793,145 +888,51 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
   Exts = Exts.drop_front(ConsumeLength);
   Exts.consume_front("_");
 
-  auto StdExtsItr = StdExts.begin();
-  auto StdExtsEnd = StdExts.end();
-  auto GoToNextExt = [](StringRef::iterator &I, unsigned ConsumeLength,
-                        StringRef::iterator E) {
-    I += 1 + ConsumeLength;
-    if (I != E && *I == '_')
-      ++I;
-  };
-  for (auto I = Exts.begin(), E = Exts.end(); I != E;) {
-    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 in
-      // canonical order or it is an invalid extension.
-      if (StdExts.contains(C)) {
-        return createStringError(
-            errc::invalid_argument,
-            "standard user-level extension not given in canonical order '%c'",
-            C);
-      }
-
-      return createStringError(errc::invalid_argument,
-                               "invalid standard user-level extension '%c'", C);
-    }
-
-    // Move to next char to prevent repeated letter.
-    ++StdExtsItr;
-
-    StringRef Next;
-    unsigned Major, Minor, ConsumeLength;
-    if (std::next(I) != E)
-      Next = StringRef(std::next(I), E - std::next(I));
-    if (auto E = getExtensionVersion(StringRef(&C, 1), Next, Major, Minor,
-                                     ConsumeLength, EnableExperimentalExtension,
-                                     ExperimentalExtensionVersionCheck)) {
-      if (IgnoreUnknown) {
-        consumeError(std::move(E));
-        GoToNextExt(I, ConsumeLength, Exts.end());
-        continue;
-      }
-      return std::move(E);
-    }
-
-    // The order is OK, then push it into features.
-    // Currently LLVM supports only "mafdcvh".
-    if (!isSupportedExtension(StringRef(&C, 1))) {
-      if (IgnoreUnknown) {
-        GoToNextExt(I, ConsumeLength, Exts.end());
-        continue;
-      }
-      return createStringError(errc::invalid_argument,
-                               "unsupported standard user-level extension '%c'",
-                               C);
-    }
-    ISAInfo->addExtension(StringRef(&C, 1), {Major, Minor});
-
-    // Consume full extension name and version, including any optional '_'
-    // between this extension and the next
-    GoToNextExt(I, ConsumeLength, Exts.end());
-  }
-
-  // Handle other types of extensions other than the standard
-  // general purpose and standard user-level extensions.
-  // Parse the ISA string containing non-standard user-level
-  // extensions, standard supervisor-level extensions and
-  // non-standard supervisor-level extensions.
-  // These extensions start with 'z', 's', 'x' prefixes, might have a version
-  // number (major, minor) and are separated by a single underscore '_'. We do
-  // not enforce a canonical order for them.
-  // Set the hardware features for the extensions that are supported.
-
-  // Multi-letter extensions are seperated by a single underscore
-  // as described in RISC-V User-Level ISA V2.2.
-  SmallVector<StringRef, 8> Split;
-  OtherExts.split(Split, '_');
-
-  SmallVector<StringRef, 8> AllExts;
-  if (Split.size() > 1 || Split[0] != "") {
-    for (StringRef Ext : Split) {
-      if (Ext.empty())
-        return createStringError(errc::invalid_argument,
-                                 "extension name missing after separator '_'");
-
-      StringRef Type = getExtensionType(Ext);
-      StringRef Desc = getExtensionTypeDesc(Ext);
-      auto Pos = findLastNonVersionCharacter(Ext) + 1;
-      StringRef Name(Ext.substr(0, Pos));
-      StringRef Vers(Ext.substr(Pos));
-
-      if (Type.empty()) {
-        if (IgnoreUnknown)
-          continue;
-        return createStringError(errc::invalid_argument,
-                                 "invalid extension prefix '" + Ext + "'");
-      }
-
-      if (!IgnoreUnknown && Name.size() == Type.size()) {
+  std::vector<std::string> SplitedExts;
+  if (auto E = splitExtsByUnderscore(Exts, SplitedExts))
+    return std::move(E);
+
+  for (auto Ext : SplitedExts) {
+    StringRef CurrExt = Ext;
+    while (!CurrExt.empty()) {
+      if (AllStdExts.contains(CurrExt.front())) {
+        if (auto E = processSingleLetterExtension(
+                CurrExt, SeenExts, ExtsVersion, IgnoreUnknown,
+                EnableExperimentalExtension, ExperimentalExtensionVersionCheck))
+          return E;
+      } else if (CurrExt.front() == 'z' || CurrExt.front() == 's' ||
+                 CurrExt.front() == 'x') {
+        if (auto E = processMultiLetterExtension(
+                CurrExt, SeenExts, ExtsVersion, IgnoreUnknown,
+                EnableExperimentalExtension, ExperimentalExtensionVersionCheck))
+          return E;
+        // Multi-letter extension must be seperate following extension with
+        // underscore
+        break;
+      } else {
+        // FIXME: Could it be ignored by IgnoreUnknown?
         return createStringError(errc::invalid_argument,
-                                 "%s name missing after '%s'",
-                                 Desc.str().c_str(), Type.str().c_str());
-      }
-
-      unsigned Major, Minor, ConsumeLength;
-      if (auto E = getExtensionVersion(Name, Vers, Major, Minor, ConsumeLength,
-                                       EnableExperimentalExtension,
-                                       ExperimentalExtensionVersionCheck)) {
-        if (IgnoreUnknown) {
-          consumeError(std::move(E));
-          continue;
-        }
-        return std::move(E);
-      }
-
-      // Check if duplicated extension.
-      if (!IgnoreUnknown && llvm::is_contained(AllExts, Name)) {
-        return createStringError(errc::invalid_argument, "duplicated %s '%s'",
-                                 Desc.str().c_str(), Name.str().c_str());
+                                 "invalid standard user-level extension '%c'",
+                                 CurrExt.front());
       }
-
-      if (IgnoreUnknown && !isSupportedExtension(Name))
-        continue;
-
-      ISAInfo->addExtension(Name, {Major, Minor});
-      // Extension format is correct, keep parsing the extensions.
-      // TODO: Save Type, Name, Major, Minor to avoid parsing them later.
-      AllExts.push_back(Name);
     }
   }
 
-  for (auto Ext : AllExts) {
-    if (!isSupportedExtension(Ext)) {
-      StringRef Desc = getExtensionTypeDesc(getExtensionType(Ext));
-      return createStringError(errc::invalid_argument, "unsupported %s '%s'",
-                               Desc.str().c_str(), Ext.str().c_str());
+  // Check all Extensions are supported.
+  for (size_t Idx = 0; Idx < SeenExts.size(); Idx++) {
+    if (!RISCVISAInfo::isSupportedExtension(SeenExts[Idx])) {
+      if (SeenExts[Idx].size() == 1) {
+        return createStringError(
+            errc::invalid_argument,
+            "unsupported standard user-level extension '%s'",
+            SeenExts[Idx].c_str());
+      }
+      return createStringError(
+          errc::invalid_argument, "unsupported %s '%s'",
+          getExtensionTypeDesc(SeenExts[Idx]).str().c_str(),
+          SeenExts[Idx].c_str());
     }
+    ISAInfo->addExtension(SeenExts[Idx], ExtsVersion[Idx]);
   }
 
   return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 997551e5c44c092..41d40f143eb3911 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -203,24 +203,6 @@ TEST(ParseArchString, AcceptsSupportedBaseISAsAndSetsXLenAndFLen) {
   EXPECT_EQ(InfoRV64G.getFLen(), 64U);
 }
 
-TEST(ParseArchString, RequiresCanonicalOrderForSingleLetterExtensions) {
-  EXPECT_EQ(
-      toString(RISCVISAInfo::parseArchString("rv64idf", true).takeError()),
-      "standard user-level extension not given in canonical order 'f'");
-  EXPECT_EQ(
-      toString(RISCVISAInfo::parseArchString("rv32iam", true).takeError()),
-      "standard user-level extension not given in canonical order 'm'");
-  EXPECT_EQ(
-      toString(
-          RISCVISAInfo::parseArchString("rv32i_zfinx_a", true).takeError()),
-      "invalid extension prefix 'a'");
-  // Canonical ordering not required for z*, s*, and x* extensions.
-  EXPECT_THAT_EXPECTED(
-      RISCVISAInfo::parseArchString(
-          "rv64imafdc_xsfvcp_zicsr_xtheadba_svnapot_zawrs", true),
-      Succeeded());
-}
-
 TEST(ParseArchString, RejectsUnrecognizedExtensionNamesByDefault) {
   EXPECT_EQ(toString(RISCVISAInfo::parseArchString("rv64ib", true).takeError()),
             "unsupported standard user-level extension 'b'");
@@ -337,10 +319,79 @@ TEST(ParseArchString, AcceptsUnderscoreSplittingExtensions) {
   }
 }
 
+TEST(ParseArchString, AcceptsRelaxSingleLetterExtensions) {
+  for (StringRef Input :
+       {"rv32imfad", "rv32im_fa_d", "rv32im2p0fad", "rv32i2p1m2p0fad"}) {
+    auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true);
+    ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+    RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
+    EXPECT_EQ(Exts.size(), 6UL);
+    EXPECT_EQ(Exts.count("i"), 1U);
+    EXPECT_EQ(Exts.count("m"), 1U);
+    EXPECT_EQ(Exts.count("f"), 1U);
+    EXPECT_EQ(Exts.count("a"), 1U);
+    EXPECT_EQ(Exts.count("d"), 1U);
+    EXPECT_EQ(Exts.count("zicsr"), 1U);
+  }
+}
+
+TEST(ParseArchString, AcceptsRelaxMixedLetterExtensions) {
+  for (StringRef Input :
+       {"rv32i_zihintntl_m_a_f_d_svinval", "rv32izihintntl_mafdsvinval",
+        "rv32i_zihintntl_mafd_svinval"}) {
+    auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true);
+    ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+    RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
+    EXPECT_EQ(Exts.size(), 8UL);
+    EXPECT_EQ(Exts.count("i"), 1U);
+    EXPECT_EQ(Exts.count("m"), 1U);
+    EXPECT_EQ(Exts.count("a"), 1U);
+    EXPECT_EQ(Exts.count("f"), 1U);
+    EXPECT_EQ(Exts.count("d"), 1U);
+    EXPECT_EQ(Exts.count("zihintntl"), 1U);
+    EXPECT_EQ(Exts.count("svinval"), 1U);
+    EXPECT_EQ(Exts.count("zicsr"), 1U);
+  }
+}
+
+TEST(ParseArchString, AcceptsAmbiguousFromRelaxExtensions) {
+  for (StringRef Input : {"rv32i_zba_m", "rv32izba_m", "rv32izba1p0_m2p0"}) {
+    auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true);
+    ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+    RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
+    EXPECT_EQ(Exts.size(), 3UL);
+    EXPECT_EQ(Exts.count("i"), 1U);
+    EXPECT_EQ(Exts.count("zba"), 1U);
+    EXPECT_EQ(Exts.count("m"), 1U);
+  }
+  for (StringRef Input :
+       {"rv32ia_zba_m", "rv32iazba_m", "rv32ia2p1zba1p0_m2p0"}) {
+    auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true);
+    ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+    RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
+    EXPECT_EQ(Exts.size(), 4UL);
+    EXPECT_EQ(Exts.count("i"), 1U);
+    EXPECT_EQ(Exts.count("zba"), 1U);
+    EXPECT_EQ(Exts.count("m"), 1U);
+    EXPECT_EQ(Exts.count("a"), 1U);
+  }
+}
+
+TEST(ParseArchString,
+     RejectsMultiLetterExtensionFollowBySingleLetterExtensions) {
+  for (StringRef Input : {"rv32izbam", "rv32i_zbam"})
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "unsupported standard user-level extension 'zbam'");
+  EXPECT_EQ(
+      toString(
+          RISCVISAInfo::parseArchString("rv32i_zba1p0m", true).takeError()),
+      "unsupported standard user-level extension 'zba1p0m'");
+}
+
 TEST(ParseArchString, RejectsDoubleOrTrailingUnderscore) {
   EXPECT_EQ(
       toString(RISCVISAInfo::parseArchString("rv64i__m", true).takeError()),
-      "invalid standard user-level extension '_'");
+      "extension name missing after separator '_'");
 
   for (StringRef Input :
        {"rv32ezicsr__zifencei", "rv32i_", "rv32izicsr_", "rv64im_"}) {
@@ -356,7 +407,7 @@ TEST(ParseArchString, RejectsDuplicateExtensionNames) {
             "invalid standard user-level extension 'e'");
   EXPECT_EQ(
       toString(RISCVISAInfo::parseArchString("rv64imm", true).takeError()),
-      "standard user-level extension not given in canonical order 'm'");
+      "duplicated standard user-level extension 'm'");
   EXPECT_EQ(
       toString(
           RISCVISAInfo::parseArchString("rv32i_zicsr_zicsr", true).takeError()),

>From 8a6c112c6194317f5d9ed3939d290d73b5261692 Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Sun, 21 Jan 2024 03:42:03 -0800
Subject: [PATCH 2/6] Indent continuation lines by 2 spaces

---
 clang/test/Driver/riscv-arch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/test/Driver/riscv-arch.c b/clang/test/Driver/riscv-arch.c
index 38de95e4fbf7aa4..c9e984e07cbea98 100644
--- a/clang/test/Driver/riscv-arch.c
+++ b/clang/test/Driver/riscv-arch.c
@@ -157,7 +157,7 @@
 // RV32L: error: invalid arch name 'rv32l'
 
 // RUN: %clang --target=riscv32-unknown-elf -march=rv32imadf -### %s \
-// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN:   -fsyntax-only 2>&1 | FileCheck %s
 
 // RUN: not %clang --target=riscv32-unknown-elf -march=rv32imm -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32IMM %s
@@ -184,7 +184,7 @@
 // RV64L: error: invalid arch name 'rv64l'
 
 // RUN: %clang --target=riscv64-unknown-elf -march=rv64imadf -### %s \
-// RUN: -fsyntax-only 2>&1 | FileCheck %s
+// RUN:   -fsyntax-only 2>&1 | FileCheck %s
 
 // RUN: not %clang --target=riscv64-unknown-elf -march=rv64imm -### %s \
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64IMM %s

>From 207fa3e39f472c129f81c6715f8cf7e31d5152e1 Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Sun, 21 Jan 2024 03:57:25 -0800
Subject: [PATCH 3/6] Keepd comment about extension start with 'z' 's' 'x'

---
 llvm/lib/Support/RISCVISAInfo.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 865ca48aeb90d1a..fd1b962fc56f4f6 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -902,6 +902,14 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
           return E;
       } else if (CurrExt.front() == 'z' || CurrExt.front() == 's' ||
                  CurrExt.front() == 'x') {
+        // Handle other types of extensions other than the standard
+        // general purpose and standard user-level extensions.
+        // Parse the ISA string containing non-standard user-level
+        // extensions, standard supervisor-level extensions and
+        // non-standard supervisor-level extensions.
+        // These extensions start with 'z', 's', 'x' prefixes, might have a
+        // version number (major, minor) and are separated by a single
+        // underscore '_'. We do not enforce a canonical order for them.
         if (auto E = processMultiLetterExtension(
                 CurrExt, SeenExts, ExtsVersion, IgnoreUnknown,
                 EnableExperimentalExtension, ExperimentalExtensionVersionCheck))

>From 7cff39e515d32c7b556a0f0a23e43ce624f0e402 Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Sun, 21 Jan 2024 04:48:07 -0800
Subject: [PATCH 4/6] Add testcase for i after zba

---
 llvm/unittests/Support/RISCVISAInfoTest.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 41d40f143eb3911..113e565c18408ec 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -377,11 +377,23 @@ TEST(ParseArchString, AcceptsAmbiguousFromRelaxExtensions) {
   }
 }
 
+TEST(ParseArchString, RejectsRelaxExtensionsNotStartWithEorIorG) {
+  EXPECT_EQ(
+      toString(RISCVISAInfo::parseArchString("rv32zba_im", true).takeError()),
+      "first letter should be 'e', 'i' or 'g'");
+}
+
 TEST(ParseArchString,
      RejectsMultiLetterExtensionFollowBySingleLetterExtensions) {
   for (StringRef Input : {"rv32izbam", "rv32i_zbam"})
     EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
               "unsupported standard user-level extension 'zbam'");
+  EXPECT_EQ(
+      toString(RISCVISAInfo::parseArchString("rv32izbai_m", true).takeError()),
+      "unsupported standard user-level extension 'zbai'");
+  EXPECT_EQ(
+      toString(RISCVISAInfo::parseArchString("rv32izbaim", true).takeError()),
+      "unsupported standard user-level extension 'zbaim'");
   EXPECT_EQ(
       toString(
           RISCVISAInfo::parseArchString("rv32i_zba1p0m", true).takeError()),

>From 3ccdb9b67a7819b96dbce24c162a866db170b9f7 Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Tue, 23 Jan 2024 02:47:37 -0800
Subject: [PATCH 5/6] Rename the SplitedExts with SplitExts

---
 llvm/lib/Support/RISCVISAInfo.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index fd1b962fc56f4f6..098b4fece91c9a5 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -696,7 +696,7 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
 }
 
 static Error splitExtsByUnderscore(StringRef Exts,
-                                   std::vector<std::string> &SplitedExts) {
+                                   std::vector<std::string> &SplitExts) {
   SmallVector<StringRef, 8> Split;
   if (Exts.empty())
     return Error::success();
@@ -708,7 +708,7 @@ static Error splitExtsByUnderscore(StringRef Exts,
       return createStringError(errc::invalid_argument,
                                "extension name missing after separator '_'");
 
-    SplitedExts.push_back(Ext.str());
+    SplitExts.push_back(Ext.str());
   }
   return Error::success();
 }

>From 8bf3ccbfa961167c8b5763f25acd1f091c2cf6e8 Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Tue, 23 Jan 2024 04:12:30 -0800
Subject: [PATCH 6/6] Use MapVector instead of SmallVector

---
 llvm/lib/Support/RISCVISAInfo.cpp | 61 ++++++++++++++++---------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 098b4fece91c9a5..e33cac397e1d61e 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/RISCVISAInfo.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringExtras.h"
@@ -714,8 +715,9 @@ static Error splitExtsByUnderscore(StringRef Exts,
 }
 
 static Error processMultiLetterExtension(
-    StringRef RawExt, SmallVector<std::string, 8> &SeenExts,
-    SmallVector<RISCVISAInfo::ExtensionVersion, 8> &ExtsVersion,
+    StringRef RawExt,
+    MapVector<std::string, RISCVISAInfo::ExtensionVersion,
+              std::map<std::string, unsigned>> &SeenExtMap,
     bool IgnoreUnknown, bool EnableExperimentalExtension,
     bool ExperimentalExtensionVersionCheck) {
   StringRef Type = getExtensionType(RawExt);
@@ -748,21 +750,21 @@ static Error processMultiLetterExtension(
   }
 
   // Check if duplicated extension.
-  if (!IgnoreUnknown && llvm::is_contained(SeenExts, Name))
+  if (!IgnoreUnknown && (SeenExtMap.find(Name.str()) != SeenExtMap.end()))
     return createStringError(errc::invalid_argument, "duplicated %s '%s'",
                              Desc.str().c_str(), Name.str().c_str());
 
   if (IgnoreUnknown && !RISCVISAInfo::isSupportedExtension(Name))
     return Error::success();
 
-  SeenExts.push_back(Name.str());
-  ExtsVersion.push_back({Major, Minor});
+  SeenExtMap[Name.str()] = {Major, Minor};
   return Error::success();
 }
 
 static Error processSingleLetterExtension(
-    StringRef &RawExt, SmallVector<std::string, 8> &SeenExts,
-    SmallVector<RISCVISAInfo::ExtensionVersion, 8> &ExtsVersion,
+    StringRef &RawExt,
+    MapVector<std::string, RISCVISAInfo::ExtensionVersion,
+              std::map<std::string, unsigned>> &SeenExtMap,
     bool IgnoreUnknown, bool EnableExperimentalExtension,
     bool ExperimentalExtensionVersionCheck) {
   unsigned Major, Minor, ConsumeLength;
@@ -782,7 +784,7 @@ static Error processSingleLetterExtension(
   RawExt = RawExt.substr(ConsumeLength);
 
   // Check if duplicated extension.
-  if (!IgnoreUnknown && llvm::is_contained(SeenExts, Name))
+  if (!IgnoreUnknown && (SeenExtMap.find(Name.str()) != SeenExtMap.end()))
     return createStringError(errc::invalid_argument,
                              "duplicated standard user-level extension '%s'",
                              Name.str().c_str());
@@ -790,8 +792,7 @@ static Error processSingleLetterExtension(
   if (IgnoreUnknown && !RISCVISAInfo::isSupportedExtension(Name))
     return Error::success();
 
-  SeenExts.push_back(Name.str());
-  ExtsVersion.push_back({Major, Minor});
+  SeenExtMap[Name.str()] = {Major, Minor};
   return Error::success();
 }
 
@@ -815,8 +816,9 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
 
   unsigned XLen = HasRV64 ? 64 : 32;
   std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo(XLen));
-  SmallVector<std::string, 8> SeenExts;
-  SmallVector<RISCVISAInfo::ExtensionVersion, 8> ExtsVersion;
+  MapVector<std::string, RISCVISAInfo::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
@@ -858,8 +860,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
     for (const auto *Ext : RISCVGImplications) {
       if (auto Version = findDefaultVersion(Ext)) {
         // Postpone AddExtension until end of this function
-        SeenExts.push_back(Ext);
-        ExtsVersion.push_back({Version->Major, Version->Minor});
+        SeenExtMap[Ext] = {Version->Major, Version->Minor};
       } else
         llvm_unreachable("Default extension version not found?");
     }
@@ -879,8 +880,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
     }
 
     // Postpone AddExtension until end of this function
-    SeenExts.push_back(StringRef(&Baseline, 1).str());
-    ExtsVersion.push_back({Major, Minor});
+    SeenExtMap[StringRef(&Baseline, 1).str()] = {Major, Minor};
   }
 
   // Consume the base ISA version number and any '_' between rvxxx and the
@@ -897,8 +897,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
     while (!CurrExt.empty()) {
       if (AllStdExts.contains(CurrExt.front())) {
         if (auto E = processSingleLetterExtension(
-                CurrExt, SeenExts, ExtsVersion, IgnoreUnknown,
-                EnableExperimentalExtension, ExperimentalExtensionVersionCheck))
+                CurrExt, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
+                ExperimentalExtensionVersionCheck))
           return E;
       } else if (CurrExt.front() == 'z' || CurrExt.front() == 's' ||
                  CurrExt.front() == 'x') {
@@ -911,8 +911,8 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
         // version number (major, minor) and are separated by a single
         // underscore '_'. We do not enforce a canonical order for them.
         if (auto E = processMultiLetterExtension(
-                CurrExt, SeenExts, ExtsVersion, IgnoreUnknown,
-                EnableExperimentalExtension, ExperimentalExtensionVersionCheck))
+                CurrExt, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
+                ExperimentalExtensionVersionCheck))
           return E;
         // Multi-letter extension must be seperate following extension with
         // underscore
@@ -927,20 +927,21 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
   }
 
   // Check all Extensions are supported.
-  for (size_t Idx = 0; Idx < SeenExts.size(); Idx++) {
-    if (!RISCVISAInfo::isSupportedExtension(SeenExts[Idx])) {
-      if (SeenExts[Idx].size() == 1) {
+  for (auto SeenExtAndVers : SeenExtMap) {
+    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 '%s'",
-            SeenExts[Idx].c_str());
+            "unsupported standard user-level extension '%s'", ExtName.c_str());
       }
-      return createStringError(
-          errc::invalid_argument, "unsupported %s '%s'",
-          getExtensionTypeDesc(SeenExts[Idx]).str().c_str(),
-          SeenExts[Idx].c_str());
+      return createStringError(errc::invalid_argument, "unsupported %s '%s'",
+                               getExtensionTypeDesc(ExtName).str().c_str(),
+                               ExtName.c_str());
     }
-    ISAInfo->addExtension(SeenExts[Idx], ExtsVersion[Idx]);
+    ISAInfo->addExtension(ExtName, ExtVers);
   }
 
   return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));



More information about the llvm-commits mailing list