[llvm-branch-commits] [llvm] 7b13394 - [RISCV] Allow llvm-objdump to disassemble objects with unrecognised versions of known extensions

Tom Stellard via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Apr 4 10:33:56 PDT 2023


Author: Alex Bradbury
Date: 2023-04-04T10:33:20-07:00
New Revision: 7b133944eb8912767b31695774ed018c52222177

URL: https://github.com/llvm/llvm-project/commit/7b133944eb8912767b31695774ed018c52222177
DIFF: https://github.com/llvm/llvm-project/commit/7b133944eb8912767b31695774ed018c52222177.diff

LOG: [RISCV] Allow llvm-objdump to disassemble objects with unrecognised versions of known extensions

This Moves ELFObjectFile to using
RISCVISAInfo::parseNormalizedArchString which does not ignore or produce
an error for ISA extensions with a version that isn't
recognised/supported by LLVM. As current GCC is marking its objects with
a higher version of the A, F, and D extensions than LLVM (see [extension
versioning
discussion](https://discourse.llvm.org/t/rfc-resolving-issues-related-to-extension-versioning-in-risc-v/68472)
this massively improves the usability of llvm-objdump with such
binaries.

This patch is functionally a cherry-pick of
af602edf0ecb4e1d7de4ccce8ebe1be8bb53443d and
91c6174ce35969d7f0d73c645fa47b813e0d99d3, though I've constructed the
backport manually because other changes unrelated to this specific fix
make a direct cherry pick difficult.

Added: 
    llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s

Modified: 
    llvm/lib/Object/ELFObjectFile.cpp
    llvm/lib/Support/RISCVISAInfo.cpp
    llvm/unittests/Support/RISCVISAInfoTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Object/ELFObjectFile.cpp b/llvm/lib/Object/ELFObjectFile.cpp
index ebc57bd04be74..c6d536188391b 100644
--- a/llvm/lib/Object/ELFObjectFile.cpp
+++ b/llvm/lib/Object/ELFObjectFile.cpp
@@ -303,12 +303,7 @@ Expected<SubtargetFeatures> ELFObjectFileBase::getRISCVFeatures() const {
   std::optional<StringRef> Attr =
       Attributes.getAttributeString(RISCVAttrs::ARCH);
   if (Attr) {
-    // Suppress version checking for experimental extensions to prevent erroring
-    // when getting any unknown version of experimental extension.
-    auto ParseResult = RISCVISAInfo::parseArchString(
-        *Attr, /*EnableExperimentalExtension=*/true,
-        /*ExperimentalExtensionVersionCheck=*/false,
-        /*IgnoreUnknown=*/true);
+    auto ParseResult = RISCVISAInfo::parseNormalizedArchString(*Attr);
     if (!ParseResult)
       return ParseResult.takeError();
     auto &ISAInfo = *ParseResult;

diff  --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp
index b14fe1358d1f8..7cb1147d4265b 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -1060,6 +1060,8 @@ std::vector<std::string> RISCVISAInfo::toFeatureVector() const {
     std::string ExtName = Ext.first;
     if (ExtName == "i") // i is not recognized in clang -cc1
       continue;
+    if (!isSupportedExtension(ExtName))
+      continue;
     std::string Feature = isExperimentalExtension(ExtName)
                               ? "+experimental-" + ExtName
                               : "+" + ExtName;

diff  --git a/llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s b/llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
new file mode 100644
index 0000000000000..5a47dea0b00dd
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s
@@ -0,0 +1,93 @@
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+m,+f,+d,+v noncanonicalized_arch.s -o noncanonicalized_arch.o
+# RUN: not llvm-objdump -d noncanonicalized_arch.o 2>&1 | FileCheck %s -DFILE=noncanonicalized_arch.o --check-prefix=NONCANON
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 invalid_arch.s -o invalid_arch.o
+# RUN: not llvm-objdump -d invalid_arch.o 2>&1 | FileCheck %s -DFILE=invalid_arch.o --check-prefix=INVALID
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32 unknown_i_version.s -o unknown_i_version.o
+# 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
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 unknown_ext_name.s -o unknown_ext_name.o
+# RUN: llvm-objdump -d unknown_ext_name.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN-EXT-NAME
+
+#--- noncanonicalized_arch.s
+# NONCANON: error: '[[FILE]]': arch string must begin with valid base ISA
+# NONCANON-NOT: {{.}}
+vsetvli a3, a2, e8, m8, tu, mu
+
+.section .riscv.attributes,"", at 0x70000003
+.byte 0x41
+.long .Lend-.riscv.attributes-1
+.asciz "riscv"  # vendor
+.Lbegin:
+.byte 1  # Tag_File
+.long .Lend-.Lbegin
+.byte 5  # Tag_RISCV_arch
+.asciz "rv64gcv"
+.Lend:
+
+#--- invalid_arch.s
+# INVALID: error: '[[FILE]]': arch string must begin with valid base ISA
+# INVALID-NOT: {{.}}
+nop
+
+.section .riscv.attributes,"", at 0x70000003
+.byte 0x41
+.long .Lend-.riscv.attributes-1
+.asciz "riscv"  # vendor
+.Lbegin:
+.byte 1  # Tag_File
+.long .Lend-.Lbegin
+.byte 5  # Tag_RISCV_arch
+.asciz "nonsense"
+.Lend:
+
+#--- unknown_i_version.s
+# UNKNOWN-I-VERSION: nop
+nop
+
+.section .riscv.attributes,"", at 0x70000003
+.byte 0x41
+.long .Lend-.riscv.attributes-1
+.asciz "riscv"  # vendor
+.Lbegin:
+.byte 1  # Tag_File
+.long .Lend-.Lbegin
+.byte 5  # Tag_RISCV_arch
+.asciz "rv32i99p99"
+.Lend:
+
+#--- unknown_ext_version.s
+# UNKNOWN-EXT-VERSION: cbo.clean (t0)
+cbo.clean (t0)
+
+.section .riscv.attributes,"", at 0x70000003
+.byte 0x41
+.long .Lend-.riscv.attributes-1
+.asciz "riscv"  # vendor
+.Lbegin:
+.byte 1  # Tag_File
+.long .Lend-.Lbegin
+.byte 5  # Tag_RISCV_arch
+.asciz "rv32i2p0_zicbom0p1"
+.Lend:
+
+#--- unknown_ext_name.s
+# UNKNOWN-EXT-NAME: nop
+nop
+
+.section .riscv.attributes,"", at 0x70000003
+.byte 0x41
+.long .Lend-.riscv.attributes-1
+.asciz "riscv"  # vendor
+.Lbegin:
+.byte 1  # Tag_File
+.long .Lend-.Lbegin
+.byte 5  # Tag_RISCV_arch
+.asciz "rv32i2p0_zmadeup1p0_smadeup1p0_xmadeup1p0_sxmadeup1p0"
+.Lend:

diff  --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp
index 18620180e87f1..8a9e0ca68b9de 100644
--- a/llvm/unittests/Support/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/Support/RISCVISAInfoTest.cpp
@@ -10,6 +10,8 @@
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
+using ::testing::ElementsAre;
+
 using namespace llvm;
 
 bool operator==(const llvm::RISCVExtensionInfo &A,
@@ -103,3 +105,24 @@ TEST(ParseNormalizedArchString, UpdatesFLenMinVLenMaxELen) {
   EXPECT_EQ(Info.getMinVLen(), 64U);
   EXPECT_EQ(Info.getMaxELen(), 64U);
 }
+
+TEST(ToFeatureVector, IIsDroppedAndExperimentalExtensionsArePrefixed) {
+  auto MaybeISAInfo1 =
+      RISCVISAInfo::parseArchString("rv64im_zihintntl", true, false);
+  ASSERT_THAT_EXPECTED(MaybeISAInfo1, Succeeded());
+  EXPECT_THAT((*MaybeISAInfo1)->toFeatureVector(),
+              ElementsAre("+m", "+experimental-zihintntl"));
+
+  auto MaybeISAInfo2 = RISCVISAInfo::parseArchString(
+      "rv32e_zihintntl_xventanacondops", true, false);
+  ASSERT_THAT_EXPECTED(MaybeISAInfo2, Succeeded());
+  EXPECT_THAT((*MaybeISAInfo2)->toFeatureVector(),
+              ElementsAre("+e", "+experimental-zihintntl", "+xventanacondops"));
+}
+
+TEST(ToFeatureVector, UnsupportedExtensionsAreDropped) {
+  auto MaybeISAInfo =
+      RISCVISAInfo::parseNormalizedArchString("rv64i2p0_m2p0_xmadeup1p0");
+  ASSERT_THAT_EXPECTED(MaybeISAInfo, Succeeded());
+  EXPECT_THAT((*MaybeISAInfo)->toFeatureVector(), ElementsAre("+m"));
+}


        


More information about the llvm-branch-commits mailing list