[PATCH] D146070: [RISCV] Enable tools such as llvm-objdump to process objects with unrecognised base ISA versions

[Alex Bradbury via Phabricator via llvm-commits]

asb updated this revision to Diff 508418.
asb added a comment.

Rebase (to account for removal of string from RISCVExtensionInfo. The following patches have been approved, so just waiting for approval of this to land the series.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146070/new/

https://reviews.llvm.org/D146070

Files:
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
  llvm/unittests/Support/RISCVISAInfoTest.cpp


Index: llvm/unittests/Support/RISCVISAInfoTest.cpp
===================================================================
--- llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -279,11 +279,22 @@
             "unsupported version number 10.10 for extension 'zifencei'");
 }
 
-TEST(ParseArchString, RejectsUnrecognisedBaseISAVersionEvenWithIgnoreUnknown) {
-  EXPECT_EQ(
-      toString(RISCVISAInfo::parseArchString("rv64i1p0", true, false, true)
-                   .takeError()),
-      "unsupported version number 1.0 for extension 'i'");
+TEST(ParseArchString,
+     UsesDefaultVersionForUnrecognisedBaseISAVersionWithIgnoreUnknown) {
+  for (StringRef Input : {"rv32i0p1", "rv32i99p99", "rv64i0p1", "rv64i99p99"}) {
+    auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
+    ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+    RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
+    EXPECT_EQ(Exts.size(), 1UL);
+    EXPECT_TRUE(Exts.at("i") == (RISCVExtensionInfo{2, 0}));
+  }
+  for (StringRef Input : {"rv32e0p1", "rv32e99p99", "rv64e0p1", "rv64e99p99"}) {
+    auto MaybeISAInfo = RISCVISAInfo::parseArchString(Input, true, false, true);
+    ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+    RISCVISAInfo::OrderedExtensionMap Exts = (*MaybeISAInfo)->getExtensions();
+    EXPECT_EQ(Exts.size(), 1UL);
+    EXPECT_TRUE(Exts.at("e") == (RISCVExtensionInfo{2, 0}));
+  }
 }
 
 TEST(ParseArchString,
Index: llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
===================================================================
--- llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
+++ llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
@@ -7,7 +7,7 @@
 # RUN: not llvm-objdump -d invalid_arch.o 2>&1 | FileCheck %s --check-prefix=INVALID
 
 # RUN: llvm-mc -filetype=obj -triple=riscv32 unknown_i_version.s -o unknown_i_version.o
-# RUN: not llvm-objdump -d unknown_i_version.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN-I-VERSION
+# RUN: llvm-objdump -d unknown_i_version.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN-I-VERSION
 
 # RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+zicbom unknown_ext_version.s -o unknown_ext_version.o
 # RUN: llvm-objdump -d unknown_ext_version.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN-EXT-VERSION
@@ -46,7 +46,7 @@
 .Lend:
 
 #--- unknown_i_version.s
-# UNKNOWN-I-VERSION: unsupported version number 99.99 for extension 'i'
+# UNKNOWN-I-VERSION: nop
 nop
 
 .section .riscv.attributes,"", at 0x70000003
Index: llvm/lib/Support/RISCVISAInfo.cpp
===================================================================
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -658,10 +658,18 @@
         llvm_unreachable("Default extension version not found?");
   } else {
     // Baseline is `i` or `e`
-    if (auto E = getExtensionVersion(std::string(1, Baseline), Exts, Major, Minor,
-                                     ConsumeLength, EnableExperimentalExtension,
-                                     ExperimentalExtensionVersionCheck))
-      return std::move(E);
+    if (auto E = getExtensionVersion(
+            std::string(1, Baseline), Exts, Major, Minor, ConsumeLength,
+            EnableExperimentalExtension, ExperimentalExtensionVersionCheck)) {
+      if (!IgnoreUnknown)
+        return std::move(E);
+      // If IgnoreUnknown, then ignore an unrecognised version of the baseline
+      // ISA and just use the default supported version.
+      consumeError(std::move(E));
+      auto Version = findDefaultVersion(std::string(1, Baseline));
+      Major = Version->Major;
+      Minor = Version->Minor;
+    }
 
     ISAInfo->addExtension(std::string(1, Baseline), Major, Minor);
   }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===================================================================
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -45,6 +45,10 @@
       : XLen(XLen), FLen(0), MinVLen(0), MaxELen(0), MaxELenFp(0), Exts(Exts) {}
 
   /// Parse RISCV ISA info from arch string.
+  /// If IgnoreUnknown is set, any unrecognised extension names or
+  /// extensions with unrecognised versions will be silently dropped, except
+  /// for the special case of the base 'i' and 'e' extensions, where the
+  /// default version will be used (as ignoring the base is not possible).
   static llvm::Expected<std::unique_ptr<RISCVISAInfo>>
   parseArchString(StringRef Arch, bool EnableExperimentalExtension,
                   bool ExperimentalExtensionVersionCheck = true,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146070.508418.patch
Type: text/x-patch
Size: 4631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230326/cdb322f2/attachment.bin>