Revert rL366946 : [Remarks] Add support for serializing metadata for every remark streamer

RKSimon · RKSimon · commit 55fd57ba95a3 · 2019-07-25T10:20:39.000Z
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 llvm-svn: 367004
diff --git a/llvm/include/llvm/Remarks/RemarkSerializer.h b/llvm/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>
-  metaSerializer(raw_ostream &OS,
-                 Optional<StringRef> 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/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h b/llvm/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>
-  metaSerializer(raw_ostream &OS,
-                 Optional<StringRef> ExternalFilename = None) override;
-};
-
-struct YAMLMetaSerializer : public MetaSerializer {
-  Optional<StringRef> ExternalFilename;
-
-  YAMLMetaSerializer(raw_ostream &OS, Optional<StringRef> 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>
-  metaSerializer(raw_ostream &OS,
-                 Optional<StringRef> ExternalFilename = None) override;
-};
-
-struct YAMLStrTabMetaSerializer : public YAMLMetaSerializer {
-  /// The string table is part of the metadata.
-  StringTable StrTab;
-
-  YAMLStrTabMetaSerializer(raw_ostream &OS,
-                           Optional<StringRef> ExternalFilename,
-                           StringTable StrTab)
-      : YAMLMetaSerializer(OS, ExternalFilename), StrTab(std::move(StrTab)) {}
-
-  void emit() override;
 };
 
 } // end namespace remarks
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/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<remarks::MetaSerializer> 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<char, 8> 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<char, 8> StrTabSizeBuf;
+  support::endian::write64le(StrTabSizeBuf.data(), StrTabSize);
+  OutStreamer->EmitBinaryData(
+      StringRef(StrTabSizeBuf.data(), StrTabSizeBuf.size()));
+
+  if (const Optional<remarks::StringTable> &StrTab = RemarkSerializer.StrTab) {
+    std::vector<StringRef> 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/llvm/lib/Remarks/YAMLRemarkSerializer.cpp b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp
@@ -158,68 +158,3 @@ void YAMLRemarkSerializer::emit(const Remark &Remark) {
   auto R = const_cast<remarks::Remark *>(&Remark);
   YAMLOutput << R;
 }
-
-std::unique_ptr<MetaSerializer>
-YAMLRemarkSerializer::metaSerializer(raw_ostream &OS,
-                                     Optional<StringRef> ExternalFilename) {
-  return llvm::make_unique<YAMLMetaSerializer>(OS, ExternalFilename);
-}
-
-std::unique_ptr<MetaSerializer> YAMLStrTabRemarkSerializer::metaSerializer(
-    raw_ostream &OS, Optional<StringRef> ExternalFilename) {
-  assert(StrTab);
-  return llvm::make_unique<YAMLStrTabMetaSerializer>(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<char, 8> Version;
-  support::endian::write64le(Version.data(), remarks::Version);
-  OS.write(Version.data(), Version.size());
-}
-
-static void emitStrTab(raw_ostream &OS, const Optional<StringTable> &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<char, 8> 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/llvm/test/CodeGen/X86/remarks-section.ll b/llvm/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/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/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<StringRef> ExpectedStrTab = None,
                   Optional<remarks::StringTable> StrTab = None) {
   std::string Buf;
   raw_string_ostream OS(Buf);
+  bool UseStrTab = ExpectedStrTab.hasValue();
   Expected<std::unique_ptr<remarks::RemarkSerializer>> 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<remarks::MetaSerializer> 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)));
 }
