From 062065888f645bbe6a8eec99ba58487a539afd62 Mon Sep 17 00:00:00 2001 From: Alex Bradbury Date: Mon, 27 Mar 2023 04:32:58 +0100 Subject: [PATCH] [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 --- llvm/include/llvm/Support/RISCVISAInfo.h | 4 ++++ llvm/lib/Support/RISCVISAInfo.cpp | 16 ++++++++++++---- .../tools/llvm-objdump/ELF/RISCV/riscv-attributes.s | 4 ++-- llvm/unittests/Support/RISCVISAInfoTest.cpp | 21 ++++++++++++++++----- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/llvm/include/llvm/Support/RISCVISAInfo.h b/llvm/include/llvm/Support/RISCVISAInfo.h index 331b645..b0712d8 100644 --- a/llvm/include/llvm/Support/RISCVISAInfo.h +++ b/llvm/include/llvm/Support/RISCVISAInfo.h @@ -45,6 +45,10 @@ public: : 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> parseArchString(StringRef Arch, bool EnableExperimentalExtension, bool ExperimentalExtensionVersionCheck = true, diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp index 994fba3..875b975 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 d15b675..9cf324c 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,"",@0x70000003 diff --git a/llvm/unittests/Support/RISCVISAInfoTest.cpp b/llvm/unittests/Support/RISCVISAInfoTest.cpp index 27b0070..d9d3e45 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, -- 2.7.4