diff --git a/llvm/include/llvm/Remarks/Remark.h b/llvm/include/llvm/Remarks/Remark.h --- a/llvm/include/llvm/Remarks/Remark.h +++ b/llvm/include/llvm/Remarks/Remark.h @@ -110,6 +110,21 @@ DEFINE_SIMPLE_CONVERSION_FUNCTIONS(Remark, LLVMRemarkEntryRef) /// Comparison operators for Remark objects and dependent objects. + +template +bool operator<(const Optional &LHS, const Optional &RHS) { + // Sorting based on optionals should result in all `None` entries to appear + // before the valid entries. For example, remarks with no debug location will + // appear first. + if (!LHS && !RHS) + return false; + if (!LHS && RHS) + return true; + if (LHS && !RHS) + return false; + return *LHS < *RHS; +} + inline bool operator==(const RemarkLocation &LHS, const RemarkLocation &RHS) { return LHS.SourceFilePath == RHS.SourceFilePath && LHS.SourceLine == RHS.SourceLine && @@ -120,6 +135,11 @@ return !(LHS == RHS); } +inline bool operator<(const RemarkLocation &LHS, const RemarkLocation &RHS) { + return std::make_tuple(LHS.SourceFilePath, LHS.SourceLine, LHS.SourceColumn) < + std::make_tuple(RHS.SourceFilePath, RHS.SourceLine, RHS.SourceColumn); +} + inline bool operator==(const Argument &LHS, const Argument &RHS) { return LHS.Key == RHS.Key && LHS.Val == RHS.Val && LHS.Loc == RHS.Loc; } @@ -128,6 +148,11 @@ return !(LHS == RHS); } +inline bool operator<(const Argument &LHS, const Argument &RHS) { + return std::make_tuple(LHS.Key, LHS.Val, LHS.Loc) < + std::make_tuple(RHS.Key, RHS.Val, RHS.Loc); +} + inline bool operator==(const Remark &LHS, const Remark &RHS) { return LHS.RemarkType == RHS.RemarkType && LHS.PassName == RHS.PassName && LHS.RemarkName == RHS.RemarkName && @@ -139,6 +164,13 @@ return !(LHS == RHS); } +inline bool operator<(const Remark &LHS, const Remark &RHS) { + return std::make_tuple(LHS.RemarkType, LHS.PassName, LHS.RemarkName, + LHS.FunctionName, LHS.Loc, LHS.Hotness, LHS.Args) < + std::make_tuple(RHS.RemarkType, RHS.PassName, RHS.RemarkName, + RHS.FunctionName, RHS.Loc, RHS.Hotness, RHS.Args); +} + } // end namespace remarks } // end namespace llvm diff --git a/llvm/include/llvm/Remarks/RemarkFormat.h b/llvm/include/llvm/Remarks/RemarkFormat.h --- a/llvm/include/llvm/Remarks/RemarkFormat.h +++ b/llvm/include/llvm/Remarks/RemarkFormat.h @@ -27,6 +27,9 @@ /// Parse and validate a string for the remark format. Expected parseFormat(StringRef FormatStr); +/// Parse and validate a magic number to a remark format. +Expected magicToFormat(StringRef Magic); + } // end namespace remarks } // end namespace llvm diff --git a/llvm/include/llvm/Remarks/RemarkLinker.h b/llvm/include/llvm/Remarks/RemarkLinker.h new file mode 100644 --- /dev/null +++ b/llvm/include/llvm/Remarks/RemarkLinker.h @@ -0,0 +1,100 @@ +//===-- llvm/Remarks/RemarkLinker.h -----------------------------*- C++/-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file provides an interface to link together multiple remark files. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_REMARKS_REMARK_LINKER_H +#define LLVM_REMARKS_REMARK_LINKER_H + +#include "llvm/Object/ObjectFile.h" +#include "llvm/Remarks/Remark.h" +#include "llvm/Remarks/RemarkFormat.h" +#include "llvm/Remarks/RemarkStringTable.h" +#include "llvm/Support/Error.h" +#include +#include + +namespace llvm { +namespace remarks { + +struct RemarkLinker { +private: + /// Compare through the pointers. + struct RemarkPtrCompare { + bool operator()(const std::unique_ptr &LHS, + const std::unique_ptr &RHS) const { + assert(LHS && RHS && "Invalid pointers to compare."); + return *LHS < *RHS; + }; + }; + + /// The main string table for the remarks. + /// Note: all remarks should use the strings from this string table to avoid + /// dangling references. + StringTable StrTab; + + /// A set holding unique remarks. + /// FIXME: std::set is probably not the most appropriate data structure here. + /// Due to the limitation of having a move-only key, there isn't another + /// obvious choice for now. + std::set, RemarkPtrCompare> Remarks; + + /// A path to append before the external file path found in remark metadata. + Optional PrependPath; + + /// Keep this remark. If it's already in the set, discard it. + Remark &keep(std::unique_ptr Remark); + +public: + /// Set a path to prepend to the external file path. + void setExternalFilePrependPath(StringRef PrependPath); + + /// Link the remarks found in \p Buffer. + /// If \p RemarkFormat is not provided, try to deduce it from the metadata in + /// \p Buffer. + /// \p Buffer can be either a standalone remark container or just + /// metadata. This takes care of uniquing and merging the remarks. + Error link(StringRef Buffer, Optional RemarkFormat = None); + + /// Link the remarks found in \p Obj by looking for the right section and + /// calling the method above. + Error link(const object::ObjectFile &Obj, + Optional RemarkFormat = None); + + /// Serialize the linked remarks to the stream \p OS, using the format \p + /// RemarkFormat. + /// This clears internal state such as the string table. + /// Note: this implies that the serialization mode is standalone. + Error serialize(raw_ostream &OS, Format RemarksFormat) const; + + /// Check whether there are any remarks linked. + bool empty() const { return Remarks.empty(); } + + /// Return a collection of the linked unique remarks to iterate on. + /// Ex: + /// for (const Remark &R : RL.remarks() { [...] } + using iterator = + pointee_iterator>::iterator>; + + iterator_range remarks() const { + return {Remarks.begin(), Remarks.end()}; + } +}; + +/// Returns a buffer with the contents of the remarks section depending on the +/// format of the file. If the section doesn't exist, this returns an empty +/// optional. +Expected> +getRemarksSectionContents(const object::ObjectFile &Obj); + +} // end namespace remarks +} // end namespace llvm + +#endif /* LLVM_REMARKS_REMARK_LINKER_H */ diff --git a/llvm/lib/Remarks/CMakeLists.txt b/llvm/lib/Remarks/CMakeLists.txt --- a/llvm/lib/Remarks/CMakeLists.txt +++ b/llvm/lib/Remarks/CMakeLists.txt @@ -3,6 +3,7 @@ BitstreamRemarkSerializer.cpp Remark.cpp RemarkFormat.cpp + RemarkLinker.cpp RemarkParser.cpp RemarkSerializer.cpp RemarkStringTable.cpp diff --git a/llvm/lib/Remarks/RemarkFormat.cpp b/llvm/lib/Remarks/RemarkFormat.cpp --- a/llvm/lib/Remarks/RemarkFormat.cpp +++ b/llvm/lib/Remarks/RemarkFormat.cpp @@ -12,6 +12,7 @@ #include "llvm/Remarks/RemarkFormat.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Remarks/BitstreamRemarkContainer.h" using namespace llvm; using namespace llvm::remarks; @@ -30,3 +31,17 @@ return Result; } + +Expected llvm::remarks::magicToFormat(StringRef Magic) { + auto Result = + StringSwitch(Magic) + .StartsWith("--- ", Format::YAML) // This is only an assumption. + .StartsWith(remarks::Magic, Format::YAMLStrTab) + .StartsWith(remarks::ContainerMagic, Format::Bitstream) + .Default(Format::Unknown); + + if (Result == Format::Unknown) + return createStringError(std::make_error_code(std::errc::invalid_argument), + "Unknown remark magic: '%s'", Magic.data()); + return Result; +} diff --git a/llvm/lib/Remarks/RemarkLinker.cpp b/llvm/lib/Remarks/RemarkLinker.cpp new file mode 100644 --- /dev/null +++ b/llvm/lib/Remarks/RemarkLinker.cpp @@ -0,0 +1,126 @@ +//===- RemarkLinker.cpp ---------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file provides an implementation of the remark linker. +// +//===----------------------------------------------------------------------===// + +#include "llvm/Remarks/RemarkLinker.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Remarks/BitstreamRemarkContainer.h" +#include "llvm/Remarks/RemarkParser.h" +#include "llvm/Remarks/RemarkSerializer.h" +#include "llvm/Support/Error.h" + +using namespace llvm; +using namespace llvm::remarks; + +static Expected +getRemarksSectionName(const object::ObjectFile &Obj) { + if (Obj.isMachO()) + return StringRef("__remarks"); + // ELF -> .remarks, but there is no ELF support at this point. + return createStringError(std::errc::illegal_byte_sequence, + "Unsupported file format."); +} + +Expected> +llvm::remarks::getRemarksSectionContents(const object::ObjectFile &Obj) { + Expected SectionName = getRemarksSectionName(Obj); + if (!SectionName) + return SectionName.takeError(); + + for (const object::SectionRef &Section : Obj.sections()) { + Expected MaybeName = Section.getName(); + if (!MaybeName) + return MaybeName.takeError(); + if (*MaybeName != *SectionName) + continue; + + if (Expected Contents = Section.getContents()) + return *Contents; + else + return Contents.takeError(); + } + return Optional{}; +} + +Remark &RemarkLinker::keep(std::unique_ptr Remark) { + StrTab.internalize(*Remark); + auto Inserted = Remarks.insert(std::move(Remark)); + return **Inserted.first; +} + +void RemarkLinker::setExternalFilePrependPath(StringRef PrependPathIn) { + PrependPath = PrependPathIn; +} + +// Discard remarks with no source location. +static bool shouldKeepRemark(const Remark &R) { return R.Loc.hasValue(); } + +Error RemarkLinker::link(StringRef Buffer, Optional RemarkFormat) { + if (!RemarkFormat) { + Expected ParserFormat = magicToFormat(Buffer); + if (!ParserFormat) + return ParserFormat.takeError(); + RemarkFormat = *ParserFormat; + } + + Expected> MaybeParser = + createRemarkParserFromMeta( + *RemarkFormat, Buffer, /*StrTab=*/None, + PrependPath ? Optional(StringRef(*PrependPath)) + : Optional(None)); + if (!MaybeParser) + return MaybeParser.takeError(); + + RemarkParser &Parser = **MaybeParser; + + while (true) { + Expected> Next = Parser.next(); + if (Error E = Next.takeError()) { + if (E.isA()) { + consumeError(std::move(E)); + break; + } + return E; + } + + assert(*Next != nullptr); + + if (shouldKeepRemark(**Next)) + keep(std::move(*Next)); + } + return Error::success(); +} + +Error RemarkLinker::link(const object::ObjectFile &Obj, + Optional RemarkFormat) { + Expected> SectionOrErr = getRemarksSectionContents(Obj); + if (!SectionOrErr) + return SectionOrErr.takeError(); + + if (Optional Section = *SectionOrErr) + return link(*Section, RemarkFormat); + return Error::success(); +} + +Error RemarkLinker::serialize(raw_ostream &OS, Format RemarksFormat) const { + Expected> MaybeSerializer = + createRemarkSerializer(RemarksFormat, SerializerMode::Standalone, OS, + std::move(const_cast(StrTab))); + if (!MaybeSerializer) + return MaybeSerializer.takeError(); + + std::unique_ptr Serializer = + std::move(*MaybeSerializer); + + for (const Remark &R : remarks()) + Serializer->emit(R); + return Error::success(); +} diff --git a/llvm/unittests/Remarks/CMakeLists.txt b/llvm/unittests/Remarks/CMakeLists.txt --- a/llvm/unittests/Remarks/CMakeLists.txt +++ b/llvm/unittests/Remarks/CMakeLists.txt @@ -9,6 +9,7 @@ BitstreamRemarksParsingTest.cpp BitstreamRemarksSerializerTest.cpp RemarksAPITest.cpp + RemarksLinkingTest.cpp RemarksStrTabParsingTest.cpp YAMLRemarksParsingTest.cpp YAMLRemarksSerializerTest.cpp diff --git a/llvm/unittests/Remarks/RemarksLinkingTest.cpp b/llvm/unittests/Remarks/RemarksLinkingTest.cpp new file mode 100644 --- /dev/null +++ b/llvm/unittests/Remarks/RemarksLinkingTest.cpp @@ -0,0 +1,217 @@ +//===- unittest/Support/RemarksLinkingTest.cpp - Linking tests ------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "llvm/Bitcode/BitcodeAnalyzer.h" +#include "llvm/Remarks/RemarkLinker.h" +#include "llvm/Remarks/RemarkSerializer.h" +#include "llvm/Support/raw_ostream.h" +#include "gtest/gtest.h" +#include + +using namespace llvm; + +static void serializeAndCheck(remarks::RemarkLinker &RL, + remarks::Format OutputFormat, + StringRef ExpectedOutput) { + // 1. Create a serializer. + // 2. Serialize all the remarks from the linker. + // 3. Check that it matches the output. + std::string Buf; + raw_string_ostream OS(Buf); + Error E = RL.serialize(OS, OutputFormat); + EXPECT_FALSE(static_cast(E)); + + // For bitstream, run it through the analyzer. + if (OutputFormat == remarks::Format::Bitstream) { + std::string AnalyzeBuf; + raw_string_ostream AnalyzeOS(AnalyzeBuf); + BCDumpOptions O(AnalyzeOS); + O.ShowBinaryBlobs = true; + BitcodeAnalyzer BA(OS.str()); + EXPECT_FALSE(BA.analyze(O)); // Expect no errors. + EXPECT_EQ(AnalyzeOS.str(), ExpectedOutput); + } else { + EXPECT_EQ(OS.str(), ExpectedOutput); + } +} + +static void check(remarks::Format InputFormat, StringRef Input, + remarks::Format OutputFormat, StringRef ExpectedOutput) { + remarks::RemarkLinker RL; + EXPECT_FALSE(RL.link(Input, InputFormat)); + serializeAndCheck(RL, OutputFormat, ExpectedOutput); +} + +static void check(remarks::Format InputFormat, StringRef Input, + remarks::Format InputFormat2, StringRef Input2, + remarks::Format OutputFormat, StringRef ExpectedOutput) { + remarks::RemarkLinker RL; + EXPECT_FALSE(RL.link(Input, InputFormat)); + EXPECT_FALSE(RL.link(Input2, InputFormat2)); + serializeAndCheck(RL, OutputFormat, ExpectedOutput); +} + +TEST(Remarks, LinkingGoodYAML) { + // One YAML remark. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n", + remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n"); + + // Check that we don't keep remarks without debug locations. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "Function: foo\n" + "...\n", + remarks::Format::YAML, ""); + + // Check that we deduplicate remarks. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n" + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n", + remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n"); +} + +TEST(Remarks, LinkingGoodBitstream) { + // One YAML remark. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n", + remarks::Format::Bitstream, + "\n" + "\n" + " \n" + " \n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n" + "\n" + "\n" + " \n" + " \n" + "\n"); + + // Check that we deduplicate remarks. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n" + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n", + remarks::Format::Bitstream, + "\n" + "\n" + " \n" + " \n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n" + "\n" + "\n" + " \n" + " \n" + "\n"); +} + +TEST(Remarks, LinkingGoodStrTab) { + // Check that remarks from different entries use the same strtab. + check(remarks::Format::YAML, + "--- !Missed\n" + "Pass: inline\n" + "Name: NoDefinition\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n", + remarks::Format::YAML, + "--- !Passed\n" + "Pass: inline\n" + "Name: Ok\n" + "DebugLoc: { File: file.c, Line: 3, Column: 12 }\n" + "Function: foo\n" + "...\n", + remarks::Format::YAMLStrTab, + StringRef("REMARKS\0\0\0\0\0\0\0\0\0\x22\0\0\0\0\0\0\0" + "inline\0NoDefinition\0foo\0file.c\0Ok\0" + "--- !Passed\n" + "Pass: 0\n" + "Name: 4\n" + "DebugLoc: { File: 3, Line: 3, Column: 12 }\n" + "Function: 2\n" + "...\n" + "--- !Missed\n" + "Pass: 0\n" + "Name: 1\n" + "DebugLoc: { File: 3, Line: 3, Column: 12 }\n" + "Function: 2\n" + "...\n", + 304)); +} + +// Check that we propagate parsing errors. +TEST(Remarks, LinkingError) { + remarks::RemarkLinker RL; + { + Error E = RL.link("badyaml", remarks::Format::YAML); + EXPECT_TRUE(static_cast(E)); + EXPECT_EQ(toString(std::move(E)), + "YAML:1:1: error: document root is not of mapping type.\n" + "\n" + "badyaml\n" + "^~~~~~~\n" + "\n"); + } + + { + // Check that the prepend path is propagated and fails with the full path. + RL.setExternalFilePrependPath("/baddir/"); + Error E = RL.link( + StringRef("REMARKS\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0badfile.opt.yaml", + 40), + remarks::Format::YAMLStrTab); + EXPECT_TRUE(static_cast(E)); + EXPECT_EQ(toString(std::move(E)), + "'/baddir/badfile.opt.yaml': No such file or directory"); + } +}