[Remarks] Remove yaml-strtab format (#144527)

Background: The yaml-strtab format looks just like the yaml format,
except that the values in the key/value pairs of the remarks are
deduplicated and replaced by indices into a string table (see removed
test cases for examples). The motivation behind this format was to
reduce size of the remarks files. However, it was quickly superseded by
the bitstream format.

Therefore, remove the yaml-strtab format, as it doesn't have a good
usecase anymore:
  - It isn't particularly efficient
  - It isn't human-readable
  - It isn't straightforward to parse in external tools that can't use the
remarks library. We don't even support it in opt-viewer.

llvm-remarkutil is also missing options to parse/convert yaml-strtab, so
the chance that anyone is actually using this format is low.

Author: tobias-stadler

llvm/docs/CommandGuide/llvm-opt-report.rst

@@ -94,7 +94,6 @@ be sent to standard output.
  The Argument is one of the following:
 
  - yaml
- - yaml-strtab
  - bitstream
 
 .. option:: --no-demangle

llvm/docs/Remarks.rst

@@ -112,7 +112,6 @@ following options:
       Supported formats:
 
       * :ref:`yaml <yamlremarks>` (default)
-      * :ref:`yaml-strtab <yamlstrtabremarks>`
       * :ref:`bitstream <bitstreamremarks>`
 
 ``Content configuration``
@@ -213,30 +212,6 @@ fields are required:
 * ``<arg-line>``
 * ``<arg-column>``
 
-.. _yamlstrtabremarks:
-
-YAML with a string table
-------------------------
-
-The YAML serialization supports the usage of a string table by using the
-``yaml-strtab`` format.
-
-This format replaces strings in the YAML output with integers representing the
-index in the string table that can be provided separately through metadata.
-
-The following entries can take advantage of the string table while respecting
-YAML rules:
-
-* ``<pass>``
-* ``<name>``
-* ``<function>``
-* ``<file>``
-* ``<value>``
-* ``<arg-file>``
-
-Currently, none of the tools in :ref:`the opt-viewer directory <optviewer>`
-support this format.
-
 .. _optviewer:
 
 YAML metadata
@@ -246,9 +221,9 @@ The metadata used together with the YAML format is:
 
 * a magic number: "REMARKS\\0"
 * the version number: a little-endian uint64_t
-* the total size of the string table (the size itself excluded):
-  little-endian uint64_t
-* a list of null-terminated strings
+* 8 zero bytes. This space was previously used to encode the size of a string
+  table. String table support for YAML remarks has been removed, use the
+  bitstream format instead.
 
 Optional:
 
@@ -584,7 +559,6 @@ Emitting remark diagnostics in the object file
 A section containing metadata on remark diagnostics will be emitted for the
 following formats:
 
-* ``yaml-strtab``
 * ``bitstream``
 
 This can be overridden by using the flag ``-remarks-section=<bool>``.

llvm/include/llvm/Remarks/RemarkFormat.h

@@ -23,7 +23,7 @@ namespace remarks {
 constexpr StringLiteral Magic("REMARKS");
 
 /// The format used for serializing/deserializing remarks.
-enum class Format { Unknown, YAML, YAMLStrTab, Bitstream };
+enum class Format { Unknown, YAML, Bitstream };
 
 /// Parse and validate a string for the remark format.
 LLVM_ABI Expected<Format> parseFormat(StringRef FormatStr);

llvm/include/llvm/Remarks/RemarkParser.h

@@ -80,13 +80,8 @@ struct ParsedStringTable {
 LLVM_ABI Expected<std::unique_ptr<RemarkParser>>
 createRemarkParser(Format ParserFormat, StringRef Buf);
 
-LLVM_ABI Expected<std::unique_ptr<RemarkParser>>
-createRemarkParser(Format ParserFormat, StringRef Buf,
-                   ParsedStringTable StrTab);
-
 LLVM_ABI Expected<std::unique_ptr<RemarkParser>> createRemarkParserFromMeta(
     Format ParserFormat, StringRef Buf,
-    std::optional<ParsedStringTable> StrTab = std::nullopt,
     std::optional<StringRef> ExternalFilePrependPath = std::nullopt);
 
 } // end namespace remarks

llvm/include/llvm/Remarks/YAMLRemarkSerializer.h

@@ -63,47 +63,6 @@ struct LLVM_ABI YAMLMetaSerializer : public MetaSerializer {
   void emit() override;
 };
 
-/// Serialize the remarks to YAML using a string table. An remark entry looks
-/// like the regular YAML remark but instead of string entries it's using
-/// numbers that map to an index in the string table.
-struct LLVM_ABI YAMLStrTabRemarkSerializer : public YAMLRemarkSerializer {
-  /// Wether we already emitted the metadata in standalone mode.
-  /// This should be set to true after the first invocation of `emit`.
-  bool DidEmitMeta = false;
-
-  YAMLStrTabRemarkSerializer(raw_ostream &OS, SerializerMode Mode)
-      : YAMLRemarkSerializer(Format::YAMLStrTab, OS, Mode) {
-    // We always need a string table for this type of serializer.
-    StrTab.emplace();
-  }
-  YAMLStrTabRemarkSerializer(raw_ostream &OS, SerializerMode Mode,
-                             StringTable StrTab)
-      : YAMLRemarkSerializer(Format::YAMLStrTab, OS, Mode, std::move(StrTab)) {}
-
-  /// Override to emit the metadata if necessary.
-  void emit(const Remark &Remark) override;
-
-  std::unique_ptr<MetaSerializer> metaSerializer(
-      raw_ostream &OS,
-      std::optional<StringRef> ExternalFilename = std::nullopt) override;
-
-  static bool classof(const RemarkSerializer *S) {
-    return S->SerializerFormat == Format::YAMLStrTab;
-  }
-};
-
-struct LLVM_ABI YAMLStrTabMetaSerializer : public YAMLMetaSerializer {
-  /// The string table is part of the metadata.
-  const StringTable &StrTab;
-
-  YAMLStrTabMetaSerializer(raw_ostream &OS,
-                           std::optional<StringRef> ExternalFilename,
-                           const StringTable &StrTab)
-      : YAMLMetaSerializer(OS, ExternalFilename), StrTab(StrTab) {}
-
-  void emit() override;
-};
-
 } // end namespace remarks
 } // end namespace llvm
 

llvm/lib/Remarks/BitstreamRemarkParser.cpp

@@ -308,8 +308,7 @@ static Error advanceToMetaBlock(BitstreamParserHelper &Helper) {
 
 Expected<std::unique_ptr<BitstreamRemarkParser>>
 remarks::createBitstreamParserFromMeta(
-    StringRef Buf, std::optional<ParsedStringTable> StrTab,
-    std::optional<StringRef> ExternalFilePrependPath) {
+    StringRef Buf, std::optional<StringRef> ExternalFilePrependPath) {
   BitstreamParserHelper Helper(Buf);
   Expected<std::array<char, 4>> MagicNumber = Helper.parseMagic();
   if (!MagicNumber)
@@ -319,9 +318,7 @@ remarks::createBitstreamParserFromMeta(
           StringRef(MagicNumber->data(), MagicNumber->size())))
     return std::move(E);
 
-  auto Parser =
-      StrTab ? std::make_unique<BitstreamRemarkParser>(Buf, std::move(*StrTab))
-             : std::make_unique<BitstreamRemarkParser>(Buf);
+  auto Parser = std::make_unique<BitstreamRemarkParser>(Buf);
 
   if (ExternalFilePrependPath)
     Parser->ExternalFilePrependPath = std::string(*ExternalFilePrependPath);

llvm/lib/Remarks/BitstreamRemarkParser.h

@@ -48,11 +48,6 @@ struct BitstreamRemarkParser : public RemarkParser {
   explicit BitstreamRemarkParser(StringRef Buf)
       : RemarkParser(Format::Bitstream), ParserHelper(Buf) {}
 
-  /// Create a parser that uses a pre-parsed string table.
-  BitstreamRemarkParser(StringRef Buf, ParsedStringTable StrTab)
-      : RemarkParser(Format::Bitstream), ParserHelper(Buf),
-        StrTab(std::move(StrTab)) {}
-
   Expected<std::unique_ptr<Remark>> next() override;
 
   static bool classof(const RemarkParser *P) {
@@ -77,7 +72,7 @@ struct BitstreamRemarkParser : public RemarkParser {
 };
 
 Expected<std::unique_ptr<BitstreamRemarkParser>> createBitstreamParserFromMeta(
-    StringRef Buf, std::optional<ParsedStringTable> StrTab = std::nullopt,
+    StringRef Buf,
     std::optional<StringRef> ExternalFilePrependPath = std::nullopt);
 
 } // end namespace remarks

llvm/lib/Remarks/RemarkFormat.cpp

@@ -20,7 +20,6 @@ using namespace llvm::remarks;
 Expected<Format> llvm::remarks::parseFormat(StringRef FormatStr) {
   auto Result = StringSwitch<Format>(FormatStr)
                     .Cases("", "yaml", Format::YAML)
-                    .Case("yaml-strtab", Format::YAMLStrTab)
                     .Case("bitstream", Format::Bitstream)
                     .Default(Format::Unknown);
 
@@ -36,7 +35,8 @@ Expected<Format> llvm::remarks::magicToFormat(StringRef MagicStr) {
   auto Result =
       StringSwitch<Format>(MagicStr)
           .StartsWith("--- ", Format::YAML) // This is only an assumption.
-          .StartsWith(remarks::Magic, Format::YAMLStrTab)
+          .StartsWith(remarks::Magic,
+                      Format::YAML) // Needed for remark meta section
           .StartsWith(remarks::ContainerMagic, Format::Bitstream)
           .Default(Format::Unknown);
 

llvm/lib/Remarks/RemarkLinker.cpp

@@ -76,7 +76,7 @@ Error RemarkLinker::link(StringRef Buffer, std::optional<Format> RemarkFormat) {
 
   Expected<std::unique_ptr<RemarkParser>> MaybeParser =
       createRemarkParserFromMeta(
-          *RemarkFormat, Buffer, /*StrTab=*/std::nullopt,
+          *RemarkFormat, Buffer,
           PrependPath ? std::optional<StringRef>(StringRef(*PrependPath))
                       : std::optional<StringRef>());
   if (!MaybeParser)

llvm/lib/Remarks/RemarkParser.cpp

@@ -53,10 +53,6 @@ llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf) {
   switch (ParserFormat) {
   case Format::YAML:
     return std::make_unique<YAMLRemarkParser>(Buf);
-  case Format::YAMLStrTab:
-    return createStringError(
-        std::make_error_code(std::errc::invalid_argument),
-        "The YAML with string table format requires a parsed string table.");
   case Format::Bitstream:
     return std::make_unique<BitstreamRemarkParser>(Buf);
   case Format::Unknown:
@@ -66,38 +62,15 @@ llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf) {
   llvm_unreachable("unhandled ParseFormat");
 }
 
-Expected<std::unique_ptr<RemarkParser>>
-llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf,
-                                  ParsedStringTable StrTab) {
-  switch (ParserFormat) {
-  case Format::YAML:
-    return createStringError(std::make_error_code(std::errc::invalid_argument),
-                             "The YAML format can't be used with a string "
-                             "table. Use yaml-strtab instead.");
-  case Format::YAMLStrTab:
-    return std::make_unique<YAMLStrTabRemarkParser>(Buf, std::move(StrTab));
-  case Format::Bitstream:
-    return std::make_unique<BitstreamRemarkParser>(Buf, std::move(StrTab));
-  case Format::Unknown:
-    return createStringError(std::make_error_code(std::errc::invalid_argument),
-                             "Unknown remark parser format.");
-  }
-  llvm_unreachable("unhandled ParseFormat");
-}
-
 Expected<std::unique_ptr<RemarkParser>>
 llvm::remarks::createRemarkParserFromMeta(
-    Format ParserFormat, StringRef Buf, std::optional<ParsedStringTable> StrTab,
+    Format ParserFormat, StringRef Buf,
     std::optional<StringRef> ExternalFilePrependPath) {
   switch (ParserFormat) {
-  // Depending on the metadata, the format can be either yaml or yaml-strtab,
-  // regardless of the input argument.
   case Format::YAML:
-  case Format::YAMLStrTab:
-    return createYAMLParserFromMeta(Buf, std::move(StrTab),
-                                    std::move(ExternalFilePrependPath));
+    return createYAMLParserFromMeta(Buf, std::move(ExternalFilePrependPath));
   case Format::Bitstream:
-    return createBitstreamParserFromMeta(Buf, std::move(StrTab),
+    return createBitstreamParserFromMeta(Buf,
                                          std::move(ExternalFilePrependPath));
   case Format::Unknown:
     return createStringError(std::make_error_code(std::errc::invalid_argument),
@@ -112,11 +85,8 @@ struct CParser {
   std::unique_ptr<RemarkParser> TheParser;
   std::optional<std::string> Err;
 
-  CParser(Format ParserFormat, StringRef Buf,
-          std::optional<ParsedStringTable> StrTab = std::nullopt)
-      : TheParser(cantFail(
-            StrTab ? createRemarkParser(ParserFormat, Buf, std::move(*StrTab))
-                   : createRemarkParser(ParserFormat, Buf))) {}
+  CParser(Format ParserFormat, StringRef Buf)
+      : TheParser(cantFail(createRemarkParser(ParserFormat, Buf))) {}
 
   void handleError(Error E) { Err.emplace(toString(std::move(E))); }
   bool hasError() const { return Err.has_value(); }