From 60dd9229ecb2c0296cf8eba5869d71267babf098 Mon Sep 17 00:00:00 2001 From: Simon Pilgrim Date: Thu, 25 Jul 2019 10:20:39 +0000 Subject: [PATCH] Revert rL366946 : [Remarks] Add support for serializing metadata for every remark streamer This allows every serializer format to implement metaSerializer() and return the corresponding meta serializer. ........ Fix windows build bots http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@367004 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Remarks/RemarkSerializer.h | 19 ------ include/llvm/Remarks/YAMLRemarkSerializer.h | 28 +------- lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 61 +++++++++++++---- lib/Remarks/YAMLRemarkSerializer.cpp | 65 ------------------ test/CodeGen/X86/remarks-section.ll | 68 ++++++++++++++++++- .../Remarks/YAMLRemarksSerializerTest.cpp | 60 ++++++---------- 6 files changed, 135 insertions(+), 166 deletions(-) diff --git a/include/llvm/Remarks/RemarkSerializer.h b/include/llvm/Remarks/RemarkSerializer.h index a8e083f5458..362d81b459d 100644 --- a/include/llvm/Remarks/RemarkSerializer.h +++ b/include/llvm/Remarks/RemarkSerializer.h @@ -20,8 +20,6 @@ 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 { @@ -35,24 +33,7 @@ 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 5d68bb4c6eb..cc09805a335 100644 --- a/include/llvm/Remarks/YAMLRemarkSerializer.h +++ b/include/llvm/Remarks/YAMLRemarkSerializer.h @@ -36,19 +36,8 @@ 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 @@ -63,21 +52,6 @@ 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 cc85cb940e6..1d22ea8f294 100644 --- a/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -1349,25 +1349,60 @@ void AsmPrinter::emitRemarksSection(Module &M) { RemarkStreamer *RS = M.getContext().getRemarkStreamer(); if (!RS) return; - 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(); + const remarks::RemarkSerializer &RemarkSerializer = RS->getSerializer(); // Switch to the right section: .remarks/__remarks. MCSection *RemarksSection = OutContext.getObjectFileInfo()->getRemarksSection(); OutStreamer->SwitchSection(RemarksSection); - OutStreamer->EmitBinaryData(OS.str()); + // 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); } bool AsmPrinter::doFinalization(Module &M) { diff --git a/lib/Remarks/YAMLRemarkSerializer.cpp b/lib/Remarks/YAMLRemarkSerializer.cpp index 725ac15d7ea..c8a4ffe01d9 100644 --- a/lib/Remarks/YAMLRemarkSerializer.cpp +++ b/lib/Remarks/YAMLRemarkSerializer.cpp @@ -158,68 +158,3 @@ 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 7c4d40220a0..3ff2fb2b9b1 100644 --- a/test/CodeGen/X86/remarks-section.ll +++ b/test/CodeGen/X86/remarks-section.ll @@ -5,13 +5,75 @@ ; CHECK-LABEL: func1: ; CHECK: .section .remarks,"e",@progbits -; CHECK-NEXT: .byte +; 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-DARWIN: .section __LLVM,__remarks,regular,debug -; CHECK-DARWIN-NEXT: .byte +; 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-STRTAB: .section __LLVM,__remarks,regular,debug -; CHECK-DARWIN-STRTAB-NEXT: .byte +; 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 define void @func1() { ret void } diff --git a/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/unittests/Remarks/YAMLRemarksSerializerTest.cpp index 2d080483d48..12c754ee2c7 100644 --- a/unittests/Remarks/YAMLRemarksSerializerTest.cpp +++ b/unittests/Remarks/YAMLRemarksSerializerTest.cpp @@ -14,10 +14,11 @@ using namespace llvm; static void check(const remarks::Remark &R, StringRef ExpectedR, - StringRef ExpectedMeta, bool UseStrTab = false, + Optional ExpectedStrTab = None, Optional StrTab = None) { std::string Buf; raw_string_ostream OS(Buf); + bool UseStrTab = ExpectedStrTab.hasValue(); Expected> MaybeS = [&] { if (UseStrTab) { if (StrTab) @@ -33,12 +34,12 @@ static void check(const remarks::Remark &R, StringRef ExpectedR, S->emit(R); EXPECT_EQ(OS.str(), ExpectedR); - - Buf.clear(); - std::unique_ptr MS = - S->metaSerializer(OS, StringRef("/externalfile")); - MS->emit(); - EXPECT_EQ(OS.str(), ExpectedMeta); + if (ExpectedStrTab) { + Buf.clear(); + EXPECT_TRUE(S->StrTab); + S->StrTab->serialize(OS); + EXPECT_EQ(OS.str(), *ExpectedStrTab); + } } TEST(YAMLRemarks, SerializerRemark) { @@ -56,23 +57,17 @@ 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", - StringRef("REMARKS\0" - "\0\0\0\0\0\0\0\0" - "\0\0\0\0\0\0\0\0" - "/externalfile\0", - 38)); + 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"); } TEST(YAMLRemarks, SerializerRemarkStrTab) { @@ -102,13 +97,7 @@ TEST(YAMLRemarks, SerializerRemarkStrTab) { " - keydebug: 5\n" " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" "...\n", - 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" - "/externalfile\0", - 83), - /*UseStrTab=*/true); + StringRef("pass\0name\0func\0path\0value\0valuedebug\0argpath\0", 45)); } TEST(YAMLRemarks, SerializerRemarkParsedStrTab) { @@ -139,12 +128,5 @@ TEST(YAMLRemarks, SerializerRemarkParsedStrTab) { " - keydebug: 5\n" " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" "...\n", - 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" - "/externalfile\0", - 83), - /*UseStrTab=*/true, - remarks::StringTable(remarks::ParsedStringTable(StrTab))); + StrTab, remarks::StringTable(remarks::ParsedStringTable(StrTab))); } -- 2.40.0