[RISCV] Detect duplicate extensions in parseNormalizedArchString.

[topperc]

This detects the same extension name being added twice. Mostly I'm worried about the case that the same string appears with two different versions. We will only preserve one of the versions.

We could allow the same version to be repeated, but that doesn't seem useful at the moment.

I've updated addExtension to use map::emplace instead of map::operator[]. This means we only keep the first version if there are duplicates. Previously we kept the last version.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

This detects the same extension name being added twice. Mostly I'm worried about the case, that the same string appears with two different versions. We will only preserve one of the versions.

We could allow the same version to be repeated, but that doesn't seem useful at the moment.

I've updated addExtension to use map::emplace instead of map::operator[]. This means we only keep the first version if there are duplicates. Previously we kept the last version.

---

Full diff: https://github.com/llvm/llvm-project/pull/91416.diff

3 Files Affected:

• (modified) llvm/include/llvm/TargetParser/RISCVISAInfo.h (+1-1)
• (modified) llvm/lib/TargetParser/RISCVISAInfo.cpp (+5-3)
• (modified) llvm/unittests/TargetParser/RISCVISAInfoTest.cpp (+8)

diff --git a/llvm/include/llvm/TargetParser/RISCVISAInfo.h b/llvm/include/llvm/TargetParser/RISCVISAInfo.h
index 36617a9b62597..12f6b46fb3cee 100644
--- a/llvm/include/llvm/TargetParser/RISCVISAInfo.h
+++ b/llvm/include/llvm/TargetParser/RISCVISAInfo.h
@@ -87,7 +87,7 @@ class RISCVISAInfo {
 
   RISCVISAUtils::OrderedExtensionMap Exts;
 
-  void addExtension(StringRef ExtName, RISCVISAUtils::ExtensionVersion Version);
+  bool addExtension(StringRef ExtName, RISCVISAUtils::ExtensionVersion Version);
 
   Error checkDependency();
 
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 9c2ac8c3893f1..96590745b2ebc 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -159,9 +159,9 @@ findDefaultVersion(StringRef ExtName) {
   return std::nullopt;
 }
 
-void RISCVISAInfo::addExtension(StringRef ExtName,
+bool RISCVISAInfo::addExtension(StringRef ExtName,
                                 RISCVISAUtils::ExtensionVersion Version) {
-  Exts[ExtName.str()] = Version;
+  return Exts.emplace(ExtName, Version).second;
 }
 
 static StringRef getExtensionTypeDesc(StringRef Ext) {
@@ -492,7 +492,9 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
                                "'" + Twine(ExtName[0]) +
                                    "' must be followed by a letter");
 
-    ISAInfo->addExtension(ExtName, {MajorVersion, MinorVersion});
+    if (!ISAInfo->addExtension(ExtName, {MajorVersion, MinorVersion}))
+      return createStringError(errc::invalid_argument,
+                               "duplicate extension '" + ExtName + "'");
   }
   ISAInfo->updateImpliedLengths();
   return std::move(ISAInfo);
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index 83b52d0527c3a..0e807cfb8e3b8 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -78,6 +78,14 @@ TEST(ParseNormalizedArchString, RejectsBadX) {
   }
 }
 
+TEST(ParseNormalizedArchString, DuplicateExtension) {
+  for (StringRef Input : {"rv64i2p0_a2p0_a1p0"}) {
+    EXPECT_EQ(
+        toString(RISCVISAInfo::parseNormalizedArchString(Input).takeError()),
+        "duplicate extension 'a'");
+  }
+}
+
 TEST(ParseNormalizedArchString, AcceptsValidBaseISAsAndSetsXLen) {
   auto MaybeRV32I = RISCVISAInfo::parseNormalizedArchString("rv32i2p0");
   ASSERT_THAT_EXPECTED(MaybeRV32I, Succeeded());

[dtcxzyw]

LGTM.

[yetingk]

LGTM.