[llvm] [RISCV] Detect duplicate extensions in parseNormalizedArchString. (PR #91416)

[Craig Topper via llvm-commits]

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

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.

>From faed9feed445cb73d16153727b8ac35e618cfaa6 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 7 May 2024 18:31:43 -0700
Subject: [PATCH] [RISCV] Detect duplicate extensions in
 parseNormalizedArchString.

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.
---
 llvm/include/llvm/TargetParser/RISCVISAInfo.h    | 2 +-
 llvm/lib/TargetParser/RISCVISAInfo.cpp           | 8 +++++---
 llvm/unittests/TargetParser/RISCVISAInfoTest.cpp | 8 ++++++++
 3 files changed, 14 insertions(+), 4 deletions(-)

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());