[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 created this revision.
asb added reviewers: kito-cheng, reames, MaskRay, craig.topper.
Herald added subscribers: jobnoorman, luke, wingo, pmatos, VincentWu, vkmr, frasercrmck, jdoerfert, 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, emaste.
Herald added a reviewer: jhenderson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD.
Herald added a project: LLVM.

Tools such as llvm-objdump will currently inputs when the base ISA has an unrecognised version. I addressed a similar issue in LLD in D144353 <https://reviews.llvm.org/D144353>, introducing parseArchStringNormalized. While it would make sense to migrate `llvm/lib/Object/ELFObjectFile.cpp` to using `parseArchStringNormalized` as well, this patch takes a less ambitious initial step. By tweaking the behaviour of `parseArchString` when `IgnoreUnknown` is true (which only has one in-tree user), we use the default supported ISA version when a base ISA with unrecognised version is encountered.

This means that llvm-objdump and related tools will function better for objects produced from a recent GCC. This isn't a full fix, as IgnoreUnknown means that an imafd object with attributes specifying newer A/F/D versions will have those extensions ignored.


Repository:
  rG LLVM Github Monorepo

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{"i", 2, 0}));
+  }
+  for (StringRef Input : {"rv32e0p1", "rv32e99p99"}) {
+    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{"e", 1, 9}));
+  }
 }
 
 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
@@ -46,6 +46,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.505150.patch
Type: text/x-patch
Size: 4615 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230314/13688fa5/attachment.bin>