From 537cdf92c4344d9d41284b1b49ee8cc7f6bcd40d Mon Sep 17 00:00:00 2001 From: Elena Lepilkina Date: Wed, 7 Dec 2022 09:47:51 +0300 Subject: [PATCH] [llvm-objdump][RISCV] Use new common method to parse ARCH RISCV attribute Differential Revision: https://reviews.llvm.org/D139553 --- llvm/include/llvm/MC/SubtargetFeature.h | 2 + llvm/include/llvm/Object/COFF.h | 4 +- llvm/include/llvm/Object/ELFObjectFile.h | 4 +- llvm/include/llvm/Object/MachO.h | 4 +- llvm/include/llvm/Object/ObjectFile.h | 2 +- llvm/include/llvm/Object/Wasm.h | 2 +- llvm/include/llvm/Object/XCOFFObjectFile.h | 2 +- llvm/include/llvm/Support/RISCVISAInfo.h | 3 +- .../DebugInfo/LogicalView/Readers/LVELFReader.cpp | 11 +++-- llvm/lib/MC/SubtargetFeature.cpp | 5 ++ llvm/lib/Object/ELFObjectFile.cpp | 55 ++++++++-------------- llvm/lib/Object/WasmObjectFile.cpp | 2 +- llvm/lib/Object/XCOFFObjectFile.cpp | 2 +- llvm/lib/Support/RISCVISAInfo.cpp | 55 +++++++++++++++++----- .../tools/llvm-objdump/ELF/RISCV/extensions.test | 6 +-- .../tools/llvm-objdump/ELF/RISCV/tag-riscv-arch.s | 21 +++++++++ .../llvm-objdump/ELF/RISCV/unknown-arch-attr.test | 6 +-- llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp | 6 ++- llvm/tools/llvm-objdump/llvm-objdump.cpp | 5 +- llvm/tools/llvm-profgen/ProfiledBinary.cpp | 6 ++- 20 files changed, 131 insertions(+), 72 deletions(-) create mode 100644 llvm/test/tools/llvm-objdump/ELF/RISCV/tag-riscv-arch.s diff --git a/llvm/include/llvm/MC/SubtargetFeature.h b/llvm/include/llvm/MC/SubtargetFeature.h index c952070..642fb2c 100644 --- a/llvm/include/llvm/MC/SubtargetFeature.h +++ b/llvm/include/llvm/MC/SubtargetFeature.h @@ -189,6 +189,8 @@ public: /// Adds Features. void AddFeature(StringRef String, bool Enable = true); + void addFeaturesVector(const ArrayRef OtherFeatures); + /// Returns the vector of individual subtarget features. const std::vector &getFeatures() const { return Features; } diff --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h index 8ad065d..89e12f4 100644 --- a/llvm/include/llvm/Object/COFF.h +++ b/llvm/include/llvm/Object/COFF.h @@ -980,7 +980,9 @@ public: StringRef getFileFormatName() const override; Triple::ArchType getArch() const override; Expected getStartAddress() const override; - SubtargetFeatures getFeatures() const override { return SubtargetFeatures(); } + Expected getFeatures() const override { + return SubtargetFeatures(); + } import_directory_iterator import_directory_begin() const; import_directory_iterator import_directory_end() const; diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h index debdaa9..8baf6f4 100644 --- a/llvm/include/llvm/Object/ELFObjectFile.h +++ b/llvm/include/llvm/Object/ELFObjectFile.h @@ -55,7 +55,7 @@ class ELFObjectFileBase : public ObjectFile { SubtargetFeatures getMIPSFeatures() const; SubtargetFeatures getARMFeatures() const; - SubtargetFeatures getRISCVFeatures() const; + Expected getRISCVFeatures() const; SubtargetFeatures getLoongArchFeatures() const; StringRef getAMDGPUCPUName() const; @@ -87,7 +87,7 @@ public: static bool classof(const Binary *v) { return v->isELF(); } - SubtargetFeatures getFeatures() const override; + Expected getFeatures() const override; std::optional tryGetCPUName() const override; diff --git a/llvm/include/llvm/Object/MachO.h b/llvm/include/llvm/Object/MachO.h index 00e2fc6..56e7c85 100644 --- a/llvm/include/llvm/Object/MachO.h +++ b/llvm/include/llvm/Object/MachO.h @@ -514,7 +514,9 @@ public: StringRef getFileFormatName() const override; Triple::ArchType getArch() const override; - SubtargetFeatures getFeatures() const override { return SubtargetFeatures(); } + Expected getFeatures() const override { + return SubtargetFeatures(); + } Triple getArchTriple(const char **McpuDefault = nullptr) const; relocation_iterator section_rel_begin(unsigned Index) const; diff --git a/llvm/include/llvm/Object/ObjectFile.h b/llvm/include/llvm/Object/ObjectFile.h index fc77aed..a010133 100644 --- a/llvm/include/llvm/Object/ObjectFile.h +++ b/llvm/include/llvm/Object/ObjectFile.h @@ -336,7 +336,7 @@ public: virtual StringRef getFileFormatName() const = 0; virtual Triple::ArchType getArch() const = 0; - virtual SubtargetFeatures getFeatures() const = 0; + virtual Expected getFeatures() const = 0; virtual std::optional tryGetCPUName() const { return std::nullopt; }; diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h index a26f703..3c8c8a2 100644 --- a/llvm/include/llvm/Object/Wasm.h +++ b/llvm/include/llvm/Object/Wasm.h @@ -203,7 +203,7 @@ public: uint8_t getBytesInAddress() const override; StringRef getFileFormatName() const override; Triple::ArchType getArch() const override; - SubtargetFeatures getFeatures() const override; + Expected getFeatures() const override; bool isRelocatableObject() const override; bool isSharedObject() const; diff --git a/llvm/include/llvm/Object/XCOFFObjectFile.h b/llvm/include/llvm/Object/XCOFFObjectFile.h index 5a66c46..1424780 100644 --- a/llvm/include/llvm/Object/XCOFFObjectFile.h +++ b/llvm/include/llvm/Object/XCOFFObjectFile.h @@ -619,7 +619,7 @@ public: uint8_t getBytesInAddress() const override; StringRef getFileFormatName() const override; Triple::ArchType getArch() const override; - SubtargetFeatures getFeatures() const override; + Expected getFeatures() const override; Expected getStartAddress() const override; StringRef mapDebugSectionName(StringRef Name) const override; bool isRelocatableObject() const override; diff --git a/llvm/include/llvm/Support/RISCVISAInfo.h b/llvm/include/llvm/Support/RISCVISAInfo.h index 50eca53..9070b88 100644 --- a/llvm/include/llvm/Support/RISCVISAInfo.h +++ b/llvm/include/llvm/Support/RISCVISAInfo.h @@ -48,7 +48,8 @@ public: /// Parse RISCV ISA info from arch string. static llvm::Expected> parseArchString(StringRef Arch, bool EnableExperimentalExtension, - bool ExperimentalExtensionVersionCheck = true); + bool ExperimentalExtensionVersionCheck = true, + bool IgnoreUnknown = false); /// Parse RISCV ISA info from feature vector. static llvm::Expected> diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp index 8baff14..7746bc5 100644 --- a/llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp +++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVELFReader.cpp @@ -1155,9 +1155,14 @@ Error LVELFReader::loadTargetInfo(const ObjectFile &Obj) { TT.setOS(Triple::UnknownOS); // Features to be passed to target/subtarget - SubtargetFeatures Features = Obj.getFeatures(); - - return loadGenericTargetInfo(TT.str(), Features.getString()); + Expected Features = Obj.getFeatures(); + SubtargetFeatures FeaturesValue; + if (!Features) { + consumeError(Features.takeError()); + FeaturesValue = SubtargetFeatures(); + } + FeaturesValue = *Features; + return loadGenericTargetInfo(TT.str(), FeaturesValue.getString()); } void LVELFReader::mapRangeAddress(const ObjectFile &Obj) { diff --git a/llvm/lib/MC/SubtargetFeature.cpp b/llvm/lib/MC/SubtargetFeature.cpp index e874216..d0ddfc7 100644 --- a/llvm/lib/MC/SubtargetFeature.cpp +++ b/llvm/lib/MC/SubtargetFeature.cpp @@ -42,6 +42,11 @@ void SubtargetFeatures::AddFeature(StringRef String, bool Enable) { : (Enable ? "+" : "-") + String.lower()); } +void SubtargetFeatures::addFeaturesVector( + const ArrayRef OtherFeatures) { + Features.insert(Features.cend(), OtherFeatures.begin(), OtherFeatures.end()); +} + SubtargetFeatures::SubtargetFeatures(StringRef Initial) { // Break up string into separate features Split(Features, Initial); diff --git a/llvm/lib/Object/ELFObjectFile.cpp b/llvm/lib/Object/ELFObjectFile.cpp index 6b29703..c293fdd 100644 --- a/llvm/lib/Object/ELFObjectFile.cpp +++ b/llvm/lib/Object/ELFObjectFile.cpp @@ -25,6 +25,7 @@ #include "llvm/Support/MathExtras.h" #include "llvm/Support/RISCVAttributeParser.h" #include "llvm/Support/RISCVAttributes.h" +#include "llvm/Support/RISCVISAInfo.h" #include #include #include @@ -286,7 +287,7 @@ SubtargetFeatures ELFObjectFileBase::getARMFeatures() const { return Features; } -SubtargetFeatures ELFObjectFileBase::getRISCVFeatures() const { +Expected ELFObjectFileBase::getRISCVFeatures() const { SubtargetFeatures Features; unsigned PlatformFlags = getPlatformFlags(); @@ -294,50 +295,32 @@ SubtargetFeatures ELFObjectFileBase::getRISCVFeatures() const { Features.AddFeature("c"); } - // Add features according to the ELF attribute section. - // If there are any unrecognized features, ignore them. RISCVAttributeParser Attributes; if (Error E = getBuildAttributes(Attributes)) { - // TODO Propagate Error. - consumeError(std::move(E)); - return Features; // Keep "c" feature if there is one in PlatformFlags. + return E; } std::optional Attr = Attributes.getAttributeString(RISCVAttrs::ARCH); if (Attr) { - // The Arch pattern is [rv32|rv64][i|e]version(_[m|a|f|d|c]version)* - // Version string pattern is (major)p(minor). Major and minor are optional. - // For example, a version number could be 2p0, 2, or p92. - StringRef Arch = *Attr; - if (Arch.consume_front("rv32")) + // 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); + if (!ParseResult) + return ParseResult.takeError(); + auto &ISAInfo = *ParseResult; + + if (ISAInfo->getXLen() == 32) Features.AddFeature("64bit", false); - else if (Arch.consume_front("rv64")) + else if (ISAInfo->getXLen() == 64) Features.AddFeature("64bit"); + else + llvm_unreachable("XLEN should be 32 or 64."); - while (!Arch.empty()) { - switch (Arch[0]) { - default: - break; // Ignore unexpected features. - case 'i': - Features.AddFeature("e", false); - break; - case 'd': - Features.AddFeature("f"); // D-ext will imply F-ext. - [[fallthrough]]; - case 'e': - case 'm': - case 'a': - case 'f': - case 'c': - Features.AddFeature(Arch.take_front()); - break; - } - - // FIXME: Handle version numbers. - Arch = Arch.drop_until([](char c) { return c == '_' || c == '\0'; }); - Arch = Arch.drop_while([](char c) { return c == '_'; }); - } + Features.addFeaturesVector(ISAInfo->toFeatureVector()); } return Features; @@ -361,7 +344,7 @@ SubtargetFeatures ELFObjectFileBase::getLoongArchFeatures() const { return Features; } -SubtargetFeatures ELFObjectFileBase::getFeatures() const { +Expected ELFObjectFileBase::getFeatures() const { switch (getEMachine()) { case ELF::EM_MIPS: return getMIPSFeatures(); diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp index 0e24ac9..1e98de7 100644 --- a/llvm/lib/Object/WasmObjectFile.cpp +++ b/llvm/lib/Object/WasmObjectFile.cpp @@ -1833,7 +1833,7 @@ Triple::ArchType WasmObjectFile::getArch() const { return HasMemory64 ? Triple::wasm64 : Triple::wasm32; } -SubtargetFeatures WasmObjectFile::getFeatures() const { +Expected WasmObjectFile::getFeatures() const { return SubtargetFeatures(); } diff --git a/llvm/lib/Object/XCOFFObjectFile.cpp b/llvm/lib/Object/XCOFFObjectFile.cpp index d93e9e7..68baefc 100644 --- a/llvm/lib/Object/XCOFFObjectFile.cpp +++ b/llvm/lib/Object/XCOFFObjectFile.cpp @@ -712,7 +712,7 @@ Triple::ArchType XCOFFObjectFile::getArch() const { return is64Bit() ? Triple::ppc64 : Triple::ppc; } -SubtargetFeatures XCOFFObjectFile::getFeatures() const { +Expected XCOFFObjectFile::getFeatures() const { return SubtargetFeatures(); } diff --git a/llvm/lib/Support/RISCVISAInfo.cpp b/llvm/lib/Support/RISCVISAInfo.cpp index a4e6a89..6b9d4ca 100644 --- a/llvm/lib/Support/RISCVISAInfo.cpp +++ b/llvm/lib/Support/RISCVISAInfo.cpp @@ -504,7 +504,8 @@ RISCVISAInfo::parseFeatures(unsigned XLen, llvm::Expected> RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, - bool ExperimentalExtensionVersionCheck) { + bool ExperimentalExtensionVersionCheck, + bool IgnoreUnknown) { // RISC-V ISA strings must be lowercase. if (llvm::any_of(Arch, isupper)) { return createStringError(errc::invalid_argument, @@ -589,6 +590,11 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, auto StdExtsItr = StdExts.begin(); auto StdExtsEnd = StdExts.end(); + auto GoToNextExt = [](StringRef::iterator &I, unsigned ConsumeLength) { + I += 1 + ConsumeLength; + if (*I == '_') + ++I; + }; for (auto I = Exts.begin(), E = Exts.end(); I != E;) { char C = *I; @@ -619,25 +625,32 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, Next = std::string(std::next(I), E); if (auto E = getExtensionVersion(std::string(1, C), Next, Major, Minor, ConsumeLength, EnableExperimentalExtension, - ExperimentalExtensionVersionCheck)) + ExperimentalExtensionVersionCheck)) { + if (IgnoreUnknown) { + consumeError(std::move(E)); + GoToNextExt(I, ConsumeLength); + continue; + } return std::move(E); + } // The order is OK, then push it into features. // TODO: Use version number when setting target features // Currently LLVM supports only "mafdcvh". - StringRef SupportedStandardExtension = "mafdcvh"; - if (!SupportedStandardExtension.contains(C)) + if (!isSupportedExtension(StringRef(&C, 1))) { + if (IgnoreUnknown) { + GoToNextExt(I, ConsumeLength); + continue; + } return createStringError(errc::invalid_argument, "unsupported standard user-level extension '%c'", C); + } ISAInfo->addExtension(std::string(1, C), Major, Minor); // Consume full extension name and version, including any optional '_' // between this extension and the next - ++I; - I += ConsumeLength; - if (*I == '_') - ++I; + GoToNextExt(I, ConsumeLength); } // Handle other types of extensions other than the standard @@ -671,20 +684,28 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, StringRef Name(Ext.substr(0, Pos)); StringRef Vers(Ext.substr(Pos)); - if (Type.empty()) + if (Type.empty()) { + if (IgnoreUnknown) + continue; return createStringError(errc::invalid_argument, "invalid extension prefix '" + Ext + "'"); + } // Check ISA extensions are specified in the canonical order. while (I != E && *I != Type) ++I; - if (I == E) + if (I == E) { + if (IgnoreUnknown) + continue; return createStringError(errc::invalid_argument, "%s not given in canonical order '%s'", Desc.str().c_str(), Ext.str().c_str()); + } - if (Name.size() == Type.size()) { + if (!IgnoreUnknown && Name.size() == Type.size()) { + if (IgnoreUnknown) + continue; return createStringError(errc::invalid_argument, "%s name missing after '%s'", Desc.str().c_str(), Type.str().c_str()); @@ -693,13 +714,21 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension, unsigned Major, Minor, ConsumeLength; if (auto E = getExtensionVersion(Name, Vers, Major, Minor, ConsumeLength, EnableExperimentalExtension, - ExperimentalExtensionVersionCheck)) + ExperimentalExtensionVersionCheck)) { + if (IgnoreUnknown) { + consumeError(std::move(E)); + continue; + } return std::move(E); + } // Check if duplicated extension. - if (llvm::is_contained(AllExts, Name)) + if (!IgnoreUnknown && llvm::is_contained(AllExts, Name)) { + if (IgnoreUnknown) + continue; return createStringError(errc::invalid_argument, "duplicated %s '%s'", Desc.str().c_str(), Name.str().c_str()); + } ISAInfo->addExtension(Name, Major, Minor); // Extension format is correct, keep parsing the extensions. diff --git a/llvm/test/tools/llvm-objdump/ELF/RISCV/extensions.test b/llvm/test/tools/llvm-objdump/ELF/RISCV/extensions.test index 1a19138..c7142d2 100644 --- a/llvm/test/tools/llvm-objdump/ELF/RISCV/extensions.test +++ b/llvm/test/tools/llvm-objdump/ELF/RISCV/extensions.test @@ -9,13 +9,13 @@ # RUN: | FileCheck %s --check-prefixes=DISASM # DISASM-LABEL: -# DISASM: +# DISASM: clmul a0, a0, a1 # DISASM-LABEL: -# DISASM: +# DISASM: clmulh a0, a0, a1 # DISASM-LABEL: -# DISASM: +# DISASM: clmulr a0, a0, a1 --- !ELF FileHeader: diff --git a/llvm/test/tools/llvm-objdump/ELF/RISCV/tag-riscv-arch.s b/llvm/test/tools/llvm-objdump/ELF/RISCV/tag-riscv-arch.s new file mode 100644 index 0000000..b223c8e --- /dev/null +++ b/llvm/test/tools/llvm-objdump/ELF/RISCV/tag-riscv-arch.s @@ -0,0 +1,21 @@ +## llvm-objdump should decode instructions that are supported by extensions that are used in Tag_RISCV_arch attribute. +# RUN: llvm-mc -filetype=obj -triple riscv64 %s | \ +# RUN: llvm-objdump -d -M no-aliases - | \ +# RUN: FileCheck %s + .attribute 5, "rv64gcv" +# CHECK-LABEL: : +foo: +# CHECK: vsetvli a3, a2, e8, m8, tu, mu +vsetvli a3, a2, e8, m8, tu, mu + +# CHECK: fadd.s fs10, fs11, ft8 +fadd.s f26, f27, f28 + +# CHECK: fld ft0, 12(a0) +fld f0, 12(a0) + +# CHECK: fmul.d ft0, ft1, ft2, dyn +fmul.d ft0, ft1, ft2, dyn + +# CHECK: vfsub.vv v8, v4, v20, v0.t +vfsub.vv v8, v4, v20, v0.t \ No newline at end of file diff --git a/llvm/test/tools/llvm-objdump/ELF/RISCV/unknown-arch-attr.test b/llvm/test/tools/llvm-objdump/ELF/RISCV/unknown-arch-attr.test index 23e10f8..35c8c62 100644 --- a/llvm/test/tools/llvm-objdump/ELF/RISCV/unknown-arch-attr.test +++ b/llvm/test/tools/llvm-objdump/ELF/RISCV/unknown-arch-attr.test @@ -3,7 +3,7 @@ ## The expected behavior is to ignore the unrecognized arch feature and ## continue to process the following arch features. ## -## The object file has the "rv32i2p0_x1p0_m2p0" arch feature. "x1p0" is an +## The object file has the "rv32i2p0_m2p0_x1p0" arch feature. "x1p0" is an ## unrecognized architecture extension. llvm-objdump will ignore it and decode ## "mul" instruction correctly according to "m2p0" in the arch feature. ## @@ -34,5 +34,5 @@ Sections: Content: 3385C502 - Name: .riscv.attributes Type: SHT_RISCV_ATTRIBUTES -## The content is the encoding of the arch feature "rv32i2p0_x1p0_m2p0" - Content: 412300000072697363760001190000000572763332693270305F783170305F6D32703000 +## The content is the encoding of the arch feature "rv32i2p0_m2p0_x1p0" + Content: 412300000072697363760001190000000572763332693270305F6D3270305F7831703000 diff --git a/llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp b/llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp index fbba8db..3e03c82 100644 --- a/llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp +++ b/llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp @@ -96,7 +96,11 @@ Expected FileAnalysis::Create(StringRef Filename) { } Analysis.ObjectTriple = Analysis.Object->makeTriple(); - Analysis.Features = Analysis.Object->getFeatures(); + Expected Features = Analysis.Object->getFeatures(); + if (!Features) + return Features.takeError(); + + Analysis.Features = *Features; // Init the rest of the object. if (auto InitResponse = Analysis.initialiseDisassemblyMembers()) diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp index 4e78e26..8c2be29 100644 --- a/llvm/tools/llvm-objdump/llvm-objdump.cpp +++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp @@ -2009,7 +2009,10 @@ static void disassembleObject(ObjectFile *Obj, bool InlineRelocs) { const Target *TheTarget = getTarget(Obj); // Package up features to be passed to target/subtarget - SubtargetFeatures Features = Obj->getFeatures(); + Expected FeaturesValue = Obj->getFeatures(); + if (!FeaturesValue) + WithColor::error(errs(), ToolName) << FeaturesValue.takeError(); + SubtargetFeatures Features = *FeaturesValue; if (!MAttrs.empty()) { for (unsigned I = 0; I != MAttrs.size(); ++I) Features.AddFeature(MAttrs[I]); diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp index 20837f2..00e9d50 100644 --- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp +++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp @@ -611,9 +611,11 @@ void ProfiledBinary::setUpDisassembler(const ELFObjectFileBase *Obj) { if (!AsmInfo) exitWithError("no assembly info for target " + TripleName, FileName); - SubtargetFeatures Features = Obj->getFeatures(); + Expected Features = Obj->getFeatures(); + if (!Features) + exitWithError(Features.takeError(), FileName); STI.reset( - TheTarget->createMCSubtargetInfo(TripleName, "", Features.getString())); + TheTarget->createMCSubtargetInfo(TripleName, "", Features->getString())); if (!STI) exitWithError("no subtarget info for target " + TripleName, FileName); -- 2.7.4