[PATCH] D145954: [RISCV] Reject 'g' with explicitly version in parseArchString

[Alex Bradbury via Phabricator via llvm-commits]

asb created this revision.
asb added reviewers: kito-cheng, MaskRay, reames, craig.topper.
Herald added subscribers: jobnoorman, luke, wingo, pmatos, VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD.
Herald added a project: LLVM.

There is no versioning scheme for the 'g' shorthand for imafd (or in current ISA specs, imafd_zifencei_zicsr). As such, the only sensible behaviour to me seems to be to reject a version for it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145954

Files:
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/unittests/Support/RISCVISAInfoTest.cpp


Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===================================================================
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -397,14 +397,11 @@
       "(this compiler supports 0.2)");
 }
 
-TEST(ParseArchString, AcceptsExtensionVersionForG) {
-  // FIXME: As there is no versioning scheme for G, arguably an error should
-  // be produced.
-  auto MaybeISAInfo = RISCVISAInfo::parseArchString("rv64g9p9", true, false);
-  ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
-  RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
-  EXPECT_EQ(Exts.size(), 5UL);
-  EXPECT_TRUE(Exts.at("i") == (RISCVExtensionInfo{"i", 2, 0}));
+TEST(ParseArchString, RejectsExtensionVersionForG) {
+  for (StringRef Input : {"rv32g1c", "rv64g2p0"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "version not supported for 'g'");
+  }
 }
 
 TEST(ParseArchString, AddsImpliedExtensions) {
Index: llvm/lib/Support/RISCVISAInfo.cpp
===================================================================
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -622,6 +622,9 @@
     break;
   case 'g':
     // g = imafd
+    if (Arch.size() > 5 && isdigit(Arch[5]))
+      return createStringError(errc::invalid_argument,
+                               "version not supported for 'g'");
     StdExts = StdExts.drop_front(4);
     break;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145954.504712.patch
Type: text/x-patch
Size: 1505 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230313/bb9b10c3/attachment.bin>