diff --git a/include/llvm/Remarks/RemarkSerializer.h b/include/llvm/Remarks/RemarkSerializer.h index 362d81b459d..a8e083f5458 100644 --- a/include/llvm/Remarks/RemarkSerializer.h +++ b/include/llvm/Remarks/RemarkSerializer.h @@ -20,6 +20,8 @@ namespace llvm { namespace remarks { +struct MetaSerializer; + /// This is the base class for a remark serializer. /// It includes support for using a string table while emitting. struct RemarkSerializer { @@ -33,7 +35,24 @@ struct RemarkSerializer { /// This is just an interface. virtual ~RemarkSerializer() = default; + /// Emit a remark to the stream. virtual void emit(const Remark &Remark) = 0; + /// Return the corresponding metadata serializer. + virtual std::unique_ptr + metaSerializer(raw_ostream &OS, + Optional ExternalFilename = None) = 0; +}; + +/// This is the base class for a remark metadata serializer. +struct MetaSerializer { + /// The open raw_ostream that the metadata is emitted to. + raw_ostream &OS; + + MetaSerializer(raw_ostream &OS) : OS(OS) {} + + /// This is just an interface. + virtual ~MetaSerializer() = default; + virtual void emit() = 0; }; /// Create a remark serializer. diff --git a/include/llvm/Remarks/YAMLRemarkSerializer.h b/include/llvm/Remarks/YAMLRemarkSerializer.h index cc09805a335..5d68bb4c6eb 100644 --- a/include/llvm/Remarks/YAMLRemarkSerializer.h +++ b/include/llvm/Remarks/YAMLRemarkSerializer.h @@ -36,8 +36,19 @@ struct YAMLRemarkSerializer : public RemarkSerializer { YAMLRemarkSerializer(raw_ostream &OS); - /// Emit a remark to the stream. void emit(const Remark &Remark) override; + std::unique_ptr + metaSerializer(raw_ostream &OS, + Optional ExternalFilename = None) override; +}; + +struct YAMLMetaSerializer : public MetaSerializer { + Optional ExternalFilename; + + YAMLMetaSerializer(raw_ostream &OS, Optional ExternalFilename) + : MetaSerializer(OS), ExternalFilename(ExternalFilename) {} + + void emit() override; }; /// Serialize the remarks to YAML using a string table. An remark entry looks @@ -52,6 +63,21 @@ struct YAMLStrTabRemarkSerializer : public YAMLRemarkSerializer { : YAMLRemarkSerializer(OS) { StrTab = std::move(StrTabIn); } + std::unique_ptr + metaSerializer(raw_ostream &OS, + Optional ExternalFilename = None) override; +}; + +struct YAMLStrTabMetaSerializer : public YAMLMetaSerializer { + /// The string table is part of the metadata. + StringTable StrTab; + + YAMLStrTabMetaSerializer(raw_ostream &OS, + Optional ExternalFilename, + StringTable StrTab) + : YAMLMetaSerializer(OS, ExternalFilename), StrTab(std::move(StrTab)) {} + + void emit() override; }; } // end namespace remarks diff --git a/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index 1d22ea8f294..cc85cb940e6 100644 --- a/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -1349,60 +1349,25 @@ void AsmPrinter::emitRemarksSection(Module &M) { RemarkStreamer *RS = M.getContext().getRemarkStreamer(); if (!RS) return; - const remarks::RemarkSerializer &RemarkSerializer = RS->getSerializer(); + remarks::RemarkSerializer &RemarkSerializer = RS->getSerializer(); + + StringRef FilenameRef = RS->getFilename(); + SmallString<128> Filename = FilenameRef; + sys::fs::make_absolute(Filename); + assert(!Filename.empty() && "The filename can't be empty."); + + std::string Buf; + raw_string_ostream OS(Buf); + std::unique_ptr MetaSerializer = + RemarkSerializer.metaSerializer(OS, StringRef(Filename)); + MetaSerializer->emit(); // Switch to the right section: .remarks/__remarks. MCSection *RemarksSection = OutContext.getObjectFileInfo()->getRemarksSection(); OutStreamer->SwitchSection(RemarksSection); - // Emit the magic number. - OutStreamer->EmitBytes(remarks::Magic); - // Explicitly emit a '\0'. - OutStreamer->EmitIntValue(/*Value=*/0, /*Size=*/1); - - // Emit the version number: little-endian uint64_t. - // The version number is located at the offset 0x0 in the section. - std::array Version; - support::endian::write64le(Version.data(), remarks::Version); - OutStreamer->EmitBinaryData(StringRef(Version.data(), Version.size())); - - // Emit the string table in the section. - // Note: we need to use the streamer here to emit it in the section. We can't - // just use the serialize function with a raw_ostream because of the way - // MCStreamers work. - uint64_t StrTabSize = - RemarkSerializer.StrTab ? RemarkSerializer.StrTab->SerializedSize : 0; - // Emit the total size of the string table (the size itself excluded): - // little-endian uint64_t. - // The total size is located after the version number. - // Note: even if no string table is used, emit 0. - std::array StrTabSizeBuf; - support::endian::write64le(StrTabSizeBuf.data(), StrTabSize); - OutStreamer->EmitBinaryData( - StringRef(StrTabSizeBuf.data(), StrTabSizeBuf.size())); - - if (const Optional &StrTab = RemarkSerializer.StrTab) { - std::vector StrTabStrings = StrTab->serialize(); - // Emit a list of null-terminated strings. - // Note: the order is important here: the ID used in the remarks corresponds - // to the position of the string in the section. - for (StringRef Str : StrTabStrings) { - OutStreamer->EmitBytes(Str); - // Explicitly emit a '\0'. - OutStreamer->EmitIntValue(/*Value=*/0, /*Size=*/1); - } - } - - // Emit the null-terminated absolute path to the remark file. - // The path is located at the offset 0x4 in the section. - StringRef FilenameRef = RS->getFilename(); - SmallString<128> Filename = FilenameRef; - sys::fs::make_absolute(Filename); - assert(!Filename.empty() && "The filename can't be empty."); - OutStreamer->EmitBytes(Filename); - // Explicitly emit a '\0'. - OutStreamer->EmitIntValue(/*Value=*/0, /*Size=*/1); + OutStreamer->EmitBinaryData(OS.str()); } bool AsmPrinter::doFinalization(Module &M) { diff --git a/lib/Remarks/YAMLRemarkSerializer.cpp b/lib/Remarks/YAMLRemarkSerializer.cpp index c8a4ffe01d9..725ac15d7ea 100644 --- a/lib/Remarks/YAMLRemarkSerializer.cpp +++ b/lib/Remarks/YAMLRemarkSerializer.cpp @@ -158,3 +158,68 @@ void YAMLRemarkSerializer::emit(const Remark &Remark) { auto R = const_cast(&Remark); YAMLOutput << R; } + +std::unique_ptr +YAMLRemarkSerializer::metaSerializer(raw_ostream &OS, + Optional ExternalFilename) { + return llvm::make_unique(OS, ExternalFilename); +} + +std::unique_ptr YAMLStrTabRemarkSerializer::metaSerializer( + raw_ostream &OS, Optional ExternalFilename) { + assert(StrTab); + return llvm::make_unique(OS, ExternalFilename, + std::move(*StrTab)); +} + +static void emitMagic(raw_ostream &OS) { + // Emit the magic number. + OS << remarks::Magic; + // Explicitly emit a '\0'. + OS.write('\0'); +} + +static void emitVersion(raw_ostream &OS) { + // Emit the version number: little-endian uint64_t. + std::array Version; + support::endian::write64le(Version.data(), remarks::Version); + OS.write(Version.data(), Version.size()); +} + +static void emitStrTab(raw_ostream &OS, const Optional &StrTab) { + // Emit the string table in the section. + uint64_t StrTabSize = StrTab ? StrTab->SerializedSize : 0; + // Emit the total size of the string table (the size itself excluded): + // little-endian uint64_t. + // Note: even if no string table is used, emit 0. + std::array StrTabSizeBuf; + support::endian::write64le(StrTabSizeBuf.data(), StrTabSize); + OS.write(StrTabSizeBuf.data(), StrTabSizeBuf.size()); + if (StrTab) + StrTab->serialize(OS); +} + +static void emitExternalFile(raw_ostream &OS, StringRef Filename) { + // Emit the null-terminated absolute path to the remark file. + SmallString<128> FilenameBuf = Filename; + sys::fs::make_absolute(FilenameBuf); + assert(!FilenameBuf.empty() && "The filename can't be empty."); + OS.write(FilenameBuf.data(), FilenameBuf.size()); + OS.write('\0'); +} + +void YAMLMetaSerializer::emit() { + emitMagic(OS); + emitVersion(OS); + emitStrTab(OS, None); + if (ExternalFilename) + emitExternalFile(OS, *ExternalFilename); +} + +void YAMLStrTabMetaSerializer::emit() { + emitMagic(OS); + emitVersion(OS); + emitStrTab(OS, std::move(StrTab)); + if (ExternalFilename) + emitExternalFile(OS, *ExternalFilename); +} diff --git a/test/CodeGen/X86/remarks-section.ll b/test/CodeGen/X86/remarks-section.ll index 3ff2fb2b9b1..7c4d40220a0 100644 --- a/test/CodeGen/X86/remarks-section.ll +++ b/test/CodeGen/X86/remarks-section.ll @@ -5,75 +5,13 @@ ; CHECK-LABEL: func1: ; CHECK: .section .remarks,"e",@progbits -; The magic number: -; CHECK-NEXT: .ascii "REMARKS" -; Null-terminator: -; CHECK-NEXT: .byte 0 -; The version: -; CHECK-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; CHECK-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; The string table size: -; CHECK-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; CHECK-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; The string table: -; EMPTY -; The remark file path: -; CHECK-NEXT: .ascii "[[PATH]]" -; Null-terminator: -; CHECK-NEXT: .byte 0 +; CHECK-NEXT: .byte ; CHECK-DARWIN: .section __LLVM,__remarks,regular,debug -; The magic number: -; CHECK-DARWIN-NEXT: .ascii "REMARKS" -; Null-terminator: -; CHECK-DARWIN-NEXT: .byte 0 -; The version: -; CHECK-DARWIN-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; CHECK-DARWIN-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; The string table size: -; CHECK-DARWIN-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; CHECK-DARWIN-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; The string table: -; EMPTY -; The remark file path: -; CHECK-DARWIN-NEXT: .ascii "[[PATH]]" -; Null-terminator: -; CHECK-DARWIN-NEXT: .byte 0 +; CHECK-DARWIN-NEXT: .byte ; CHECK-DARWIN-STRTAB: .section __LLVM,__remarks,regular,debug -; The magic number: -; CHECK-DARWIN-STRTAB-NEXT: .ascii "REMARKS" -; Null-terminator: -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; The version: -; CHECK-DARWIN-STRTAB-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; CHECK-DARWIN-STRTAB-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; The size of the string table: -; CHECK-DARWIN-STRTAB-NEXT: .byte 0x71, 0x00, 0x00, 0x00 -; CHECK-DARWIN-STRTAB-NEXT: .byte 0x00, 0x00, 0x00, 0x00 -; The string table: -; CHECK-DARWIN-STRTAB-NEXT: .ascii "prologepilog" -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .ascii "StackSize" -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .ascii "func1" -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .byte 48 -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .ascii " stack bytes in function" -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .ascii "asm-printer" -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .ascii "InstructionCount" -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .byte 49 -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; CHECK-DARWIN-STRTAB-NEXT: .ascii " instructions in function" -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 -; The remark file path: -; CHECK-DARWIN-STRTAB-NEXT: .ascii "[[PATH]]" -; Null-terminator: -; CHECK-DARWIN-STRTAB-NEXT: .byte 0 +; CHECK-DARWIN-STRTAB-NEXT: .byte define void @func1() { ret void } diff --git a/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/unittests/Remarks/YAMLRemarksSerializerTest.cpp index 12c754ee2c7..811c1b2a455 100644 --- a/unittests/Remarks/YAMLRemarksSerializerTest.cpp +++ b/unittests/Remarks/YAMLRemarksSerializerTest.cpp @@ -11,14 +11,21 @@ #include "llvm/Support/Error.h" #include "gtest/gtest.h" +// We need to supprt Windows paths as well. In order to have paths with the same +// length, use a different path according to the platform. +#ifdef _WIN32 +#define EXTERNALFILETESTPATH "C:/externalfi" +#else +#define EXTERNALFILETESTPATH "/externalfile" +#endif + using namespace llvm; static void check(const remarks::Remark &R, StringRef ExpectedR, - Optional ExpectedStrTab = None, + StringRef ExpectedMeta, bool UseStrTab = false, Optional StrTab = None) { std::string Buf; raw_string_ostream OS(Buf); - bool UseStrTab = ExpectedStrTab.hasValue(); Expected> MaybeS = [&] { if (UseStrTab) { if (StrTab) @@ -34,12 +41,12 @@ static void check(const remarks::Remark &R, StringRef ExpectedR, S->emit(R); EXPECT_EQ(OS.str(), ExpectedR); - if (ExpectedStrTab) { - Buf.clear(); - EXPECT_TRUE(S->StrTab); - S->StrTab->serialize(OS); - EXPECT_EQ(OS.str(), *ExpectedStrTab); - } + + Buf.clear(); + std::unique_ptr MS = + S->metaSerializer(OS, StringRef(EXTERNALFILETESTPATH)); + MS->emit(); + EXPECT_EQ(OS.str(), ExpectedMeta); } TEST(YAMLRemarks, SerializerRemark) { @@ -57,17 +64,23 @@ TEST(YAMLRemarks, SerializerRemark) { R.Args.back().Key = "keydebug"; R.Args.back().Val = "valuedebug"; R.Args.back().Loc = remarks::RemarkLocation{"argpath", 6, 7}; - check(R, "--- !Missed\n" - "Pass: pass\n" - "Name: name\n" - "DebugLoc: { File: path, Line: 3, Column: 4 }\n" - "Function: func\n" - "Hotness: 5\n" - "Args:\n" - " - key: value\n" - " - keydebug: valuedebug\n" - " DebugLoc: { File: argpath, Line: 6, Column: 7 }\n" - "...\n"); + check(R, + "--- !Missed\n" + "Pass: pass\n" + "Name: name\n" + "DebugLoc: { File: path, Line: 3, Column: 4 }\n" + "Function: func\n" + "Hotness: 5\n" + "Args:\n" + " - key: value\n" + " - keydebug: valuedebug\n" + " DebugLoc: { File: argpath, Line: 6, Column: 7 }\n" + "...\n", + StringRef("REMARKS\0" + "\0\0\0\0\0\0\0\0" + "\0\0\0\0\0\0\0\0" + EXTERNALFILETESTPATH"\0", + 38)); } TEST(YAMLRemarks, SerializerRemarkStrTab) { @@ -97,7 +110,13 @@ TEST(YAMLRemarks, SerializerRemarkStrTab) { " - keydebug: 5\n" " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" "...\n", - StringRef("pass\0name\0func\0path\0value\0valuedebug\0argpath\0", 45)); + StringRef("REMARKS\0" + "\0\0\0\0\0\0\0\0" + "\x2d\0\0\0\0\0\0\0" + "pass\0name\0func\0path\0value\0valuedebug\0argpath\0" + EXTERNALFILETESTPATH"\0", + 83), + /*UseStrTab=*/true); } TEST(YAMLRemarks, SerializerRemarkParsedStrTab) { @@ -128,5 +147,12 @@ TEST(YAMLRemarks, SerializerRemarkParsedStrTab) { " - keydebug: 5\n" " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" "...\n", - StrTab, remarks::StringTable(remarks::ParsedStringTable(StrTab))); + StringRef("REMARKS\0" + "\0\0\0\0\0\0\0\0" + "\x2d\0\0\0\0\0\0\0" + "pass\0name\0func\0path\0value\0valuedebug\0argpath\0" + EXTERNALFILETESTPATH"\0", + 83), + /*UseStrTab=*/true, + remarks::StringTable(remarks::ParsedStringTable(StrTab))); }