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

[Alex Bradbury via llvm-commits]

Author: Alex Bradbury
Date: 2023-03-27T04:32:58+01:00
New Revision: 062065888f645bbe6a8eec99ba58487a539afd62

URL: https://github.com/llvm/llvm-project/commit/062065888f645bbe6a8eec99ba58487a539afd62
DIFF: https://github.com/llvm/llvm-project/commit/062065888f645bbe6a8eec99ba58487a539afd62.diff

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

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,
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.

Differential Revision: https://reviews.llvm.org/D146070

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/RISCVISAInfo.h b/llvm/include/llvm/Support/RISCVISAInfo.h
index 331b6455f8ced..b0712d8321757 100644
--- a/llvm/include/llvm/Support/RISCVISAInfo.h
+++ b/llvm/include/llvm/Support/RISCVISAInfo.h
@@ -45,6 +45,10 @@ class RISCVISAInfo {
       : 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,

diff  --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index 994fba36b13c6..875b975ff53ea 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -658,10 +658,18 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
         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);
   }

diff  --git a/llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s b/llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
index d15b675450a90..9cf324ca3b2df 100644
--- a/llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
+++ b/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 @@ nop
 .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

diff  --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 27b00709f2bb5..d9d3e45c1668a 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -279,11 +279,22 @@ TEST(ParseArchString, RejectsUnrecognizedExtensionVersionsByDefault) {
             "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,