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

[Alex Bradbury via Phabricator via llvm-commits]

asb created this revision.
asb added reviewers: craig.topper, kito-cheng, reames, MaskRay, jrtc27.
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, 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.

This Moves ELFObjectFile to using RISCVISAInfo::parseNormalizedArchString which is not an NFC, as the test changes show. D144353 <https://reviews.llvm.org/D144353> transitioned LLD to using this function, which is specialised to parsing arch strings in the normalised format specified in the psABI rather than user-authored strings accepted in `-march`, which has greater flexibility.

parseNormalizedArchString 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 also solves the issues addressed in D146070 <https://reviews.llvm.org/D146070>, though I've opted to post both because I suspect this patch may attract more discussion about questions such as if/when warning should be produced, while D140670 <https://reviews.llvm.org/D140670> is hopefully more straightforward to land. One challenge with producing warnings for unrecognised versions is that LLVM lacks a good infrastructure for doing this from utility code like RISCVISAInfo.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146114

Files:
  llvm/lib/Object/ELFObjectFile.cpp
  llvm/test/tools/llvm-objdump/ELF/RISCV/riscv-attributes.s


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
@@ -1,7 +1,7 @@
 # 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: llvm-objdump -d noncanonicalized_arch.o | FileCheck %s --check-prefix=NONCANON
+# RUN: not llvm-objdump -d noncanonicalized_arch.o 2>&1 | FileCheck %s --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 --check-prefix=INVALID
@@ -16,7 +16,7 @@
 # RUN: llvm-objdump -d unknown_ext_name.o 2>&1 | FileCheck %s --check-prefix=UNKNOWN-EXT-NAME
 
 #--- noncanonicalized_arch.s
-# NONCANON: vsetvli a3, a2, e8, m8, tu, mu
+# NONCANON: arch string must begin with valid base ISA
 vsetvli a3, a2, e8, m8, tu, mu
 
 .section .riscv.attributes,"", at 0x70000003
@@ -31,7 +31,7 @@
 .Lend:
 
 #--- invalid_arch.s
-# INVALID: string must begin with rv32{i,e,g} or rv64{i,g} 
+# INVALID: arch string must begin with valid base ISA
 nop
 
 .section .riscv.attributes,"", at 0x70000003
@@ -61,7 +61,7 @@
 .Lend:
 
 #--- unknown_ext_version.s
-# UNKNOWN-EXT-VERSION: <unknown>
+# UNKNOWN-EXT-VERSION: cbo.clean (t0)
 cbo.clean (t0)
 
 .section .riscv.attributes,"", at 0x70000003
Index: llvm/lib/Object/ELFObjectFile.cpp
===================================================================
--- llvm/lib/Object/ELFObjectFile.cpp
+++ llvm/lib/Object/ELFObjectFile.cpp
@@ -303,12 +303,7 @@
   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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146114.505376.patch
Type: text/x-patch
Size: 2343 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230315/d092bdc0/attachment.bin>