[llvm] [RISCV] Don't use std::vector<std::string> for split extensions in RISCVISAInfo::parseArchString. NFC (PR #91538)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Wed May 8 14:38:28 PDT 2024


https://github.com/topperc created https://github.com/llvm/llvm-project/pull/91538

We can use a SmallVector of StringRef.

Adjust the code so we check for empty strings in the loop instead of making a copy of the vector returned from StringRef::split.

This overlaps with #91532 which also removed the std::vector, but may be more controversial.

>From b4fcf8bcec1d9f4ee1f58e3667c8a31214915b03 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 8 May 2024 14:32:35 -0700
Subject: [PATCH] [RISCV] Don't use std::vector<std::string> for split
 extensions in RISCVISAInfo::parseArchString. NFC

We can use a SmallVector of StringRef.

Adjust the code so we check for empty strings in the loop instead
of making a copy of the vector returned from StringRef::split.
---
 llvm/lib/TargetParser/RISCVISAInfo.cpp | 49 ++++++++++----------------
 1 file changed, 18 insertions(+), 31 deletions(-)

diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 96590745b2ebc..c553e330a878b 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -500,24 +500,6 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
   return std::move(ISAInfo);
 }
 
-static Error splitExtsByUnderscore(StringRef Exts,
-                                   std::vector<std::string> &SplitExts) {
-  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 '_'");
-
-    SplitExts.push_back(Ext.str());
-  }
-  return Error::success();
-}
-
 static Error processMultiLetterExtension(
     StringRef RawExt,
     MapVector<std::string, RISCVISAUtils::ExtensionVersion,
@@ -714,20 +696,25 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
   Exts = Exts.drop_front(ConsumeLength);
   Exts.consume_front("_");
 
-  std::vector<std::string> SplitExts;
-  if (auto E = splitExtsByUnderscore(Exts, SplitExts))
-    return std::move(E);
+  SmallVector<StringRef, 8> SplitExts;
+  // Only split if the string is not empty. Otherwise the split will push an
+  // empty string into the vector.
+  if (!Exts.empty())
+    Exts.split(SplitExts, '_');
+
+  for (auto Ext : SplitExts) {
+    if (Ext.empty())
+      return createStringError(errc::invalid_argument,
+                               "extension name missing after separator '_'");
 
-  for (auto &Ext : SplitExts) {
-    StringRef CurrExt = Ext;
-    while (!CurrExt.empty()) {
-      if (RISCVISAUtils::AllStdExts.contains(CurrExt.front())) {
+    do {
+      if (RISCVISAUtils::AllStdExts.contains(Ext.front())) {
         if (auto E = processSingleLetterExtension(
-                CurrExt, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
+                Ext, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
                 ExperimentalExtensionVersionCheck))
           return std::move(E);
-      } else if (CurrExt.front() == 'z' || CurrExt.front() == 's' ||
-                 CurrExt.front() == 'x') {
+      } else if (Ext.front() == 'z' || Ext.front() == 's' ||
+                 Ext.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
@@ -737,7 +724,7 @@ 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, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
+                Ext, SeenExtMap, IgnoreUnknown, EnableExperimentalExtension,
                 ExperimentalExtensionVersionCheck))
           return std::move(E);
         // Multi-letter extension must be seperate following extension with
@@ -747,9 +734,9 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
         // FIXME: Could it be ignored by IgnoreUnknown?
         return createStringError(errc::invalid_argument,
                                  "invalid standard user-level extension '" +
-                                     Twine(CurrExt.front()) + "'");
+                                     Twine(Ext.front()) + "'");
       }
-    }
+    } while (!Ext.empty());
   }
 
   // Check all Extensions are supported.



More information about the llvm-commits mailing list