diff --git a/llvm/docs/CommandGuide/llvm-opt-report.rst b/llvm/docs/CommandGuide/llvm-opt-report.rst index 4a666a4aa7afd..ba10ba34578aa 100644 --- a/llvm/docs/CommandGuide/llvm-opt-report.rst +++ b/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 diff --git a/llvm/docs/Remarks.rst b/llvm/docs/Remarks.rst index b6cec12b326f8..3be66e5adac95 100644 --- a/llvm/docs/Remarks.rst +++ b/llvm/docs/Remarks.rst @@ -112,7 +112,6 @@ following options: Supported formats: * :ref:`yaml ` (default) - * :ref:`yaml-strtab ` * :ref:`bitstream ` ``Content configuration`` @@ -213,30 +212,6 @@ fields are required: * ```` * ```` -.. _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: - -* ```` -* ```` -* ```` -* ```` -* ```` -* ```` - -Currently, none of the tools in :ref:`the opt-viewer directory ` -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=``. diff --git a/llvm/include/llvm/Remarks/RemarkFormat.h b/llvm/include/llvm/Remarks/RemarkFormat.h index 64d08bcc9b8a1..a39a013dcf905 100644 --- a/llvm/include/llvm/Remarks/RemarkFormat.h +++ b/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 parseFormat(StringRef FormatStr); diff --git a/llvm/include/llvm/Remarks/RemarkParser.h b/llvm/include/llvm/Remarks/RemarkParser.h index abb1fb86a87e9..e3df74436348e 100644 --- a/llvm/include/llvm/Remarks/RemarkParser.h +++ b/llvm/include/llvm/Remarks/RemarkParser.h @@ -80,13 +80,8 @@ struct ParsedStringTable { LLVM_ABI Expected> createRemarkParser(Format ParserFormat, StringRef Buf); -LLVM_ABI Expected> -createRemarkParser(Format ParserFormat, StringRef Buf, - ParsedStringTable StrTab); - LLVM_ABI Expected> createRemarkParserFromMeta( Format ParserFormat, StringRef Buf, - std::optional StrTab = std::nullopt, std::optional ExternalFilePrependPath = std::nullopt); } // end namespace remarks diff --git a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h b/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h index a2214c349e1cf..d80464c0fe74a 100644 --- a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h +++ b/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( - raw_ostream &OS, - std::optional 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 ExternalFilename, - const StringTable &StrTab) - : YAMLMetaSerializer(OS, ExternalFilename), StrTab(StrTab) {} - - void emit() override; -}; - } // end namespace remarks } // end namespace llvm diff --git a/llvm/lib/Remarks/BitstreamRemarkParser.cpp b/llvm/lib/Remarks/BitstreamRemarkParser.cpp index 6dd032f07e72d..312886013598d 100644 --- a/llvm/lib/Remarks/BitstreamRemarkParser.cpp +++ b/llvm/lib/Remarks/BitstreamRemarkParser.cpp @@ -308,8 +308,7 @@ static Error advanceToMetaBlock(BitstreamParserHelper &Helper) { Expected> remarks::createBitstreamParserFromMeta( - StringRef Buf, std::optional StrTab, - std::optional ExternalFilePrependPath) { + StringRef Buf, std::optional ExternalFilePrependPath) { BitstreamParserHelper Helper(Buf); Expected> 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(Buf, std::move(*StrTab)) - : std::make_unique(Buf); + auto Parser = std::make_unique(Buf); if (ExternalFilePrependPath) Parser->ExternalFilePrependPath = std::string(*ExternalFilePrependPath); diff --git a/llvm/lib/Remarks/BitstreamRemarkParser.h b/llvm/lib/Remarks/BitstreamRemarkParser.h index fc786fc57622d..f6f79ef199f7e 100644 --- a/llvm/lib/Remarks/BitstreamRemarkParser.h +++ b/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> next() override; static bool classof(const RemarkParser *P) { @@ -77,7 +72,7 @@ struct BitstreamRemarkParser : public RemarkParser { }; Expected> createBitstreamParserFromMeta( - StringRef Buf, std::optional StrTab = std::nullopt, + StringRef Buf, std::optional ExternalFilePrependPath = std::nullopt); } // end namespace remarks diff --git a/llvm/lib/Remarks/RemarkFormat.cpp b/llvm/lib/Remarks/RemarkFormat.cpp index 5006421a3c638..800f5bffe70da 100644 --- a/llvm/lib/Remarks/RemarkFormat.cpp +++ b/llvm/lib/Remarks/RemarkFormat.cpp @@ -20,7 +20,6 @@ using namespace llvm::remarks; Expected llvm::remarks::parseFormat(StringRef FormatStr) { auto Result = StringSwitch(FormatStr) .Cases("", "yaml", Format::YAML) - .Case("yaml-strtab", Format::YAMLStrTab) .Case("bitstream", Format::Bitstream) .Default(Format::Unknown); @@ -36,7 +35,8 @@ Expected llvm::remarks::magicToFormat(StringRef MagicStr) { auto Result = StringSwitch(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); diff --git a/llvm/lib/Remarks/RemarkLinker.cpp b/llvm/lib/Remarks/RemarkLinker.cpp index b70b06d706bdf..b8395aa135d82 100644 --- a/llvm/lib/Remarks/RemarkLinker.cpp +++ b/llvm/lib/Remarks/RemarkLinker.cpp @@ -76,7 +76,7 @@ Error RemarkLinker::link(StringRef Buffer, std::optional RemarkFormat) { Expected> MaybeParser = createRemarkParserFromMeta( - *RemarkFormat, Buffer, /*StrTab=*/std::nullopt, + *RemarkFormat, Buffer, PrependPath ? std::optional(StringRef(*PrependPath)) : std::optional()); if (!MaybeParser) diff --git a/llvm/lib/Remarks/RemarkParser.cpp b/llvm/lib/Remarks/RemarkParser.cpp index 7fccb94014b9a..5c1690aaa0fe6 100644 --- a/llvm/lib/Remarks/RemarkParser.cpp +++ b/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(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(Buf); case Format::Unknown: @@ -66,38 +62,15 @@ llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf) { llvm_unreachable("unhandled ParseFormat"); } -Expected> -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(Buf, std::move(StrTab)); - case Format::Bitstream: - return std::make_unique(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> llvm::remarks::createRemarkParserFromMeta( - Format ParserFormat, StringRef Buf, std::optional StrTab, + Format ParserFormat, StringRef Buf, std::optional 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 TheParser; std::optional Err; - CParser(Format ParserFormat, StringRef Buf, - std::optional 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(); } diff --git a/llvm/lib/Remarks/RemarkSerializer.cpp b/llvm/lib/Remarks/RemarkSerializer.cpp index ab19c84bbadbd..cc10b91f287af 100644 --- a/llvm/lib/Remarks/RemarkSerializer.cpp +++ b/llvm/lib/Remarks/RemarkSerializer.cpp @@ -26,8 +26,6 @@ remarks::createRemarkSerializer(Format RemarksFormat, SerializerMode Mode, "Unknown remark serializer format."); case Format::YAML: return std::make_unique(OS, Mode); - case Format::YAMLStrTab: - return std::make_unique(OS, Mode); case Format::Bitstream: return std::make_unique(OS, Mode); } @@ -43,9 +41,6 @@ remarks::createRemarkSerializer(Format RemarksFormat, SerializerMode Mode, "Unknown remark serializer format."); case Format::YAML: return std::make_unique(OS, Mode, std::move(StrTab)); - case Format::YAMLStrTab: - return std::make_unique(OS, Mode, - std::move(StrTab)); case Format::Bitstream: return std::make_unique(OS, Mode, std::move(StrTab)); diff --git a/llvm/lib/Remarks/RemarkStreamer.cpp b/llvm/lib/Remarks/RemarkStreamer.cpp index 9f4676ce37ab9..bb62c8b5c2fdc 100644 --- a/llvm/lib/Remarks/RemarkStreamer.cpp +++ b/llvm/lib/Remarks/RemarkStreamer.cpp @@ -21,7 +21,7 @@ static cl::opt EnableRemarksSection( "remarks-section", cl::desc( "Emit a section containing remark diagnostics metadata. By default, " - "this is enabled for the following formats: yaml-strtab, bitstream."), + "this is enabled for the following formats: bitstream."), cl::init(cl::BOU_UNSET), cl::Hidden); RemarkStreamer::RemarkStreamer( @@ -63,9 +63,7 @@ bool RemarkStreamer::needsSection() const { // Only some formats need a section: // * bitstream - // * yaml-strtab switch (RemarkSerializer->SerializerFormat) { - case remarks::Format::YAMLStrTab: case remarks::Format::Bitstream: return true; default: diff --git a/llvm/lib/Remarks/YAMLRemarkParser.cpp b/llvm/lib/Remarks/YAMLRemarkParser.cpp index a287ef5742556..5ff42fe6b9a9c 100644 --- a/llvm/lib/Remarks/YAMLRemarkParser.cpp +++ b/llvm/lib/Remarks/YAMLRemarkParser.cpp @@ -95,21 +95,8 @@ static Expected parseStrTabSize(StringRef &Buf) { return StrTabSize; } -static Expected parseStrTab(StringRef &Buf, - uint64_t StrTabSize) { - if (Buf.size() < StrTabSize) - return createStringError(std::errc::illegal_byte_sequence, - "Expecting string table."); - - // Attach the string table to the parser. - ParsedStringTable Result(StringRef(Buf.data(), StrTabSize)); - Buf = Buf.drop_front(StrTabSize); - return Expected(std::move(Result)); -} - Expected> remarks::createYAMLParserFromMeta( - StringRef Buf, std::optional StrTab, - std::optional ExternalFilePrependPath) { + StringRef Buf, std::optional ExternalFilePrependPath) { // We now have a magic number. The metadata has to be correct. Expected isMeta = parseMagic(Buf); if (!isMeta) @@ -125,15 +112,9 @@ Expected> remarks::createYAMLParserFromMeta( if (!StrTabSize) return StrTabSize.takeError(); - // If the size of string table is not 0, try to build one. if (*StrTabSize != 0) { - if (StrTab) - return createStringError(std::errc::illegal_byte_sequence, - "String table already provided."); - Expected MaybeStrTab = parseStrTab(Buf, *StrTabSize); - if (!MaybeStrTab) - return MaybeStrTab.takeError(); - StrTab = std::move(*MaybeStrTab); + return createStringError(std::errc::illegal_byte_sequence, + "String table unsupported for YAML format."); } // If it starts with "---", there is no external file. if (!Buf.starts_with("---")) { @@ -157,21 +138,15 @@ Expected> remarks::createYAMLParserFromMeta( } std::unique_ptr Result = - StrTab - ? std::make_unique(Buf, std::move(*StrTab)) - : std::make_unique(Buf); + std::make_unique(Buf); if (SeparateBuf) Result->SeparateBuf = std::move(SeparateBuf); return std::move(Result); } YAMLRemarkParser::YAMLRemarkParser(StringRef Buf) - : YAMLRemarkParser(Buf, std::nullopt) {} - -YAMLRemarkParser::YAMLRemarkParser(StringRef Buf, - std::optional StrTab) - : RemarkParser{Format::YAML}, StrTab(std::move(StrTab)), - SM(setupSM(LastErrorMessage)), Stream(Buf, SM), YAMLIt(Stream.begin()) {} + : RemarkParser{Format::YAML}, SM(setupSM(LastErrorMessage)), + Stream(Buf, SM), YAMLIt(Stream.begin()) {} Error YAMLRemarkParser::error(StringRef Message, yaml::Node &Node) { return make_error(Message, SM, Stream, Node); @@ -208,8 +183,8 @@ YAMLRemarkParser::parseRemark(yaml::Document &RemarkEntry) { Expected T = parseType(*Root); if (!T) return T.takeError(); - else - TheRemark.RemarkType = *T; + + TheRemark.RemarkType = *T; // Then, parse the fields, one by one. for (yaml::KeyValueNode &RemarkField : *Root) { @@ -428,33 +403,3 @@ Expected> YAMLRemarkParser::next() { return std::move(*MaybeResult); } - -Expected YAMLStrTabRemarkParser::parseStr(yaml::KeyValueNode &Node) { - auto *Value = dyn_cast(Node.getValue()); - yaml::BlockScalarNode *ValueBlock; - StringRef Result; - if (!Value) { - // Try to parse the value as a block node. - ValueBlock = dyn_cast(Node.getValue()); - if (!ValueBlock) - return error("expected a value of scalar type.", Node); - Result = ValueBlock->getValue(); - } else - Result = Value->getRawValue(); - // If we have a string table, parse it as an unsigned. - unsigned StrID = 0; - if (Expected MaybeStrID = parseUnsigned(Node)) - StrID = *MaybeStrID; - else - return MaybeStrID.takeError(); - - if (Expected Str = (*StrTab)[StrID]) - Result = *Str; - else - return Str.takeError(); - - Result.consume_front("\'"); - Result.consume_back("\'"); - - return Result; -} diff --git a/llvm/lib/Remarks/YAMLRemarkParser.h b/llvm/lib/Remarks/YAMLRemarkParser.h index 8ef72e16be74e..9a30e9e295cb2 100644 --- a/llvm/lib/Remarks/YAMLRemarkParser.h +++ b/llvm/lib/Remarks/YAMLRemarkParser.h @@ -46,8 +46,6 @@ class YAMLParseError : public ErrorInfo { /// Regular YAML to Remark parser. struct YAMLRemarkParser : public RemarkParser { - /// The string table used for parsing strings. - std::optional StrTab; /// Last error message that can come from the YAML parser diagnostics. /// We need this for catching errors in the constructor. std::string LastErrorMessage; @@ -70,7 +68,6 @@ struct YAMLRemarkParser : public RemarkParser { } protected: - YAMLRemarkParser(StringRef Buf, std::optional StrTab); /// Create a YAMLParseError error from an existing error generated by the YAML /// parser. /// If there is no error, this returns Success. @@ -93,22 +90,8 @@ struct YAMLRemarkParser : public RemarkParser { Expected parseArg(yaml::Node &Node); }; -/// YAML with a string table to Remark parser. -struct YAMLStrTabRemarkParser : public YAMLRemarkParser { - YAMLStrTabRemarkParser(StringRef Buf, ParsedStringTable StrTab) - : YAMLRemarkParser(Buf, std::move(StrTab)) {} - - static bool classof(const RemarkParser *P) { - return P->ParserFormat == Format::YAMLStrTab; - } - -protected: - /// Parse one value to a string. - Expected parseStr(yaml::KeyValueNode &Node) override; -}; - Expected> createYAMLParserFromMeta( - StringRef Buf, std::optional StrTab = std::nullopt, + StringRef Buf, std::optional ExternalFilePrependPath = std::nullopt); } // end namespace remarks diff --git a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp index 68285c3dde1bf..846a72182d8f0 100644 --- a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp +++ b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp @@ -21,11 +21,10 @@ using namespace llvm::remarks; // Use the same keys whether we use a string table or not (respectively, T is an // unsigned or a StringRef). -template -static void mapRemarkHeader(yaml::IO &io, T PassName, T RemarkName, - std::optional RL, T FunctionName, - std::optional Hotness, - ArrayRef Args) { +static void +mapRemarkHeader(yaml::IO &io, StringRef PassName, StringRef RemarkName, + std::optional RL, StringRef FunctionName, + std::optional Hotness, ArrayRef Args) { io.mapRequired("Pass", PassName); io.mapRequired("Name", RemarkName); io.mapOptional("DebugLoc", RL); @@ -58,19 +57,8 @@ template <> struct MappingTraits { else llvm_unreachable("Unknown remark type"); - if (auto *Serializer = dyn_cast( - reinterpret_cast(io.getContext()))) { - assert(Serializer->StrTab && "YAMLStrTabSerializer with no StrTab."); - StringTable &StrTab = *Serializer->StrTab; - unsigned PassID = StrTab.add(Remark->PassName).first; - unsigned NameID = StrTab.add(Remark->RemarkName).first; - unsigned FunctionID = StrTab.add(Remark->FunctionName).first; - mapRemarkHeader(io, PassID, NameID, Remark->Loc, FunctionID, - Remark->Hotness, Remark->Args); - } else { - mapRemarkHeader(io, Remark->PassName, Remark->RemarkName, Remark->Loc, - Remark->FunctionName, Remark->Hotness, Remark->Args); - } + mapRemarkHeader(io, Remark->PassName, Remark->RemarkName, Remark->Loc, + Remark->FunctionName, Remark->Hotness, Remark->Args); } }; @@ -82,15 +70,7 @@ template <> struct MappingTraits { unsigned Line = RL.SourceLine; unsigned Col = RL.SourceColumn; - if (auto *Serializer = dyn_cast( - reinterpret_cast(io.getContext()))) { - assert(Serializer->StrTab && "YAMLStrTabSerializer with no StrTab."); - StringTable &StrTab = *Serializer->StrTab; - unsigned FileID = StrTab.add(File).first; - io.mapRequired("File", FileID); - } else { - io.mapRequired("File", File); - } + io.mapRequired("File", File); io.mapRequired("Line", Line); io.mapRequired("Column", Col); @@ -136,13 +116,7 @@ template <> struct MappingTraits { static void mapping(IO &io, Argument &A) { assert(io.outputting() && "input not yet implemented"); - if (auto *Serializer = dyn_cast( - reinterpret_cast(io.getContext()))) { - assert(Serializer->StrTab && "YAMLStrTabSerializer with no StrTab."); - StringTable &StrTab = *Serializer->StrTab; - auto ValueID = StrTab.add(A.Val).first; - io.mapRequired(A.Key.data(), ValueID); - } else if (StringRef(A.Val).count('\n') > 1) { + if (StringRef(A.Val).count('\n') > 1) { StringBlockVal S(A.Val); io.mapRequired(A.Key.data(), S); } else { @@ -159,12 +133,7 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(Argument) YAMLRemarkSerializer::YAMLRemarkSerializer(raw_ostream &OS, SerializerMode Mode, std::optional StrTabIn) - : YAMLRemarkSerializer(Format::YAML, OS, Mode, std::move(StrTabIn)) {} - -YAMLRemarkSerializer::YAMLRemarkSerializer(Format SerializerFormat, - raw_ostream &OS, SerializerMode Mode, - std::optional StrTabIn) - : RemarkSerializer(SerializerFormat, OS, Mode), + : RemarkSerializer(Format::YAML, OS, Mode), YAMLOutput(OS, reinterpret_cast(this)) { StrTab = std::move(StrTabIn); } @@ -172,7 +141,7 @@ YAMLRemarkSerializer::YAMLRemarkSerializer(Format SerializerFormat, void YAMLRemarkSerializer::emit(const Remark &Remark) { // Again, YAMLTraits expect a non-const object for inputting, but we're not // using that here. - auto R = const_cast(&Remark); + auto *R = const_cast(&Remark); YAMLOutput << R; } @@ -181,27 +150,6 @@ std::unique_ptr YAMLRemarkSerializer::metaSerializer( return std::make_unique(OS, ExternalFilename); } -void YAMLStrTabRemarkSerializer::emit(const Remark &Remark) { - // In standalone mode, for the serializer with a string table, emit the - // metadata first and set DidEmitMeta to avoid emitting it again. - if (Mode == SerializerMode::Standalone && !DidEmitMeta) { - std::unique_ptr MetaSerializer = - metaSerializer(OS, /*ExternalFilename=*/std::nullopt); - MetaSerializer->emit(); - DidEmitMeta = true; - } - - // Then do the usual remark emission. - YAMLRemarkSerializer::emit(Remark); -} - -std::unique_ptr YAMLStrTabRemarkSerializer::metaSerializer( - raw_ostream &OS, std::optional ExternalFilename) { - assert(StrTab); - return std::make_unique(OS, ExternalFilename, - *StrTab); -} - static void emitMagic(raw_ostream &OS) { // Emit the magic number. OS << remarks::Magic; @@ -216,20 +164,6 @@ static void emitVersion(raw_ostream &OS) { OS.write(Version.data(), Version.size()); } -static void emitStrTab(raw_ostream &OS, - std::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; @@ -242,15 +176,16 @@ static void emitExternalFile(raw_ostream &OS, StringRef Filename) { void YAMLMetaSerializer::emit() { emitMagic(OS); emitVersion(OS); - emitStrTab(OS, std::nullopt); - if (ExternalFilename) - emitExternalFile(OS, *ExternalFilename); -} -void YAMLStrTabMetaSerializer::emit() { - emitMagic(OS); - emitVersion(OS); - emitStrTab(OS, &StrTab); + // Emit StringTable with size 0. This is left over after removing StringTable + // support from the YAML format. For now, don't unnecessarily change how the + // the metadata is serialized. When changing the format, we should think about + // just reusing the bitstream remark meta for this. + uint64_t StrTabSize = 0; + std::array StrTabSizeBuf; + support::endian::write64le(StrTabSizeBuf.data(), StrTabSize); + + OS.write(StrTabSizeBuf.data(), StrTabSizeBuf.size()); if (ExternalFilename) emitExternalFile(OS, *ExternalFilename); } diff --git a/llvm/test/CodeGen/X86/remarks-section.ll b/llvm/test/CodeGen/X86/remarks-section.ll index dba20d428a695..e67c3579b7593 100644 --- a/llvm/test/CodeGen/X86/remarks-section.ll +++ b/llvm/test/CodeGen/X86/remarks-section.ll @@ -1,8 +1,6 @@ ; RUN: llc < %s -mtriple=x86_64-darwin -remarks-section -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN -DPATH=%/t.yaml %s -; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=yaml-strtab -remarks-section -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-STRTAB -DPATH=%/t.yaml %s ; RUN: llc < %s -mtriple=x86_64-darwin -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-DEFAULT %s -; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=yaml-strtab -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-DEFAULT-YAML-STRTAB %s ; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=bitstream -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-DEFAULT-BITSTREAM %s ; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=bitstream -remarks-section=false -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-OVERRIDE-BITSTREAM %s ; RUN: llc < %s -mtriple=x86_64-darwin --pass-remarks-format=yaml -remarks-section=true -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-OVERRIDE-YAML %s @@ -10,15 +8,9 @@ ; CHECK-DARWIN: .section __LLVM,__remarks,regular,debug ; CHECK-DARWIN-NEXT: .byte -; CHECK-DARWIN-STRTAB: .section __LLVM,__remarks,regular,debug -; CHECK-DARWIN-STRTAB-NEXT: .byte - ; By default, the format is YAML which does not need a section. ; CHECK-DARWIN-DEFAULT-NOT: .section __LLVM,__remarks -; yaml-strtab needs a section. -; CHECK-DARWIN-DEFAULT-YAML-STRTAB: .section __LLVM,__remarks - ; bitstream needs a section. ; CHECK-DARWIN-DEFAULT-BITSTREAM: .section __LLVM,__remarks diff --git a/llvm/unittests/Remarks/RemarksLinkingTest.cpp b/llvm/unittests/Remarks/RemarksLinkingTest.cpp index ff2aec669f2fd..dcd598aaeb5cd 100644 --- a/llvm/unittests/Remarks/RemarksLinkingTest.cpp +++ b/llvm/unittests/Remarks/RemarksLinkingTest.cpp @@ -207,22 +207,22 @@ TEST(Remarks, LinkingGoodStrTab) { "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)); + remarks::Format::Bitstream, + "\n" + "\n" + " \n" + " \n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00Ok\\x00'\n" + "\n" + "\n" + " \n" + " \n" + "\n" + "\n" + " \n" + " \n" + "\n"); } // Check that we propagate parsing errors. @@ -241,11 +241,12 @@ TEST(Remarks, LinkingError) { { // Check that the prepend path is propagated and fails with the full path. + // Also ensures that the remark format is correctly auto-detected. 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); + /*RemarkFormat=*/std::nullopt); EXPECT_TRUE(static_cast(E)); std::string ErrorMessage = toString(std::move(E)); EXPECT_EQ(StringRef(ErrorMessage).lower(), diff --git a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp index 3c740ddc8a555..824813aa5af7c 100644 --- a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp +++ b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp @@ -77,7 +77,6 @@ void parseExpectErrorMeta( Expected> MaybeParser = remarks::createRemarkParserFromMeta(remarks::Format::YAML, Buf, - /*StrTab=*/std::nullopt, std::move(ExternalFilePrependPath)); handleAllErrors(MaybeParser.takeError(), [&](const ErrorInfoBase &EIB) { EIB.log(Stream); }); @@ -558,124 +557,6 @@ TEST(YAMLRemarks, ContentsCAPI) { LLVMRemarkParserDispose(Parser); } -TEST(YAMLRemarks, ContentsStrTab) { - StringRef Buf = "\n" - "--- !Missed\n" - "Pass: 0\n" - "Name: 1\n" - "DebugLoc: { File: 2, Line: 3, Column: 12 }\n" - "Function: 3\n" - "Hotness: 4\n" - "Args:\n" - " - Callee: 5\n" - " - String: 7\n" - " - Caller: 3\n" - " DebugLoc: { File: 2, Line: 2, Column: 0 }\n" - " - String: 8\n" - "\n"; - - StringRef StrTabBuf = - StringRef("inline\0NoDefinition\0file.c\0foo\0Callee\0bar\0String\0 " - "will not be inlined into \0 because its definition is " - "unavailable", - 115); - - remarks::ParsedStringTable StrTab(StrTabBuf); - Expected> MaybeParser = - remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, - std::move(StrTab)); - EXPECT_FALSE(errorToBool(MaybeParser.takeError())); - EXPECT_TRUE(*MaybeParser != nullptr); - - remarks::RemarkParser &Parser = **MaybeParser; - Expected> MaybeRemark = Parser.next(); - EXPECT_FALSE( - errorToBool(MaybeRemark.takeError())); // Check for parsing errors. - EXPECT_TRUE(*MaybeRemark != nullptr); // At least one remark. - - const remarks::Remark &Remark = **MaybeRemark; - EXPECT_EQ(Remark.RemarkType, remarks::Type::Missed); - EXPECT_EQ(checkStr(Remark.PassName, 6), "inline"); - EXPECT_EQ(checkStr(Remark.RemarkName, 12), "NoDefinition"); - EXPECT_EQ(checkStr(Remark.FunctionName, 3), "foo"); - EXPECT_TRUE(Remark.Loc); - const remarks::RemarkLocation &RL = *Remark.Loc; - EXPECT_EQ(checkStr(RL.SourceFilePath, 6), "file.c"); - EXPECT_EQ(RL.SourceLine, 3U); - EXPECT_EQ(RL.SourceColumn, 12U); - EXPECT_TRUE(Remark.Hotness); - EXPECT_EQ(*Remark.Hotness, 4U); - EXPECT_EQ(Remark.Args.size(), 4U); - - unsigned ArgID = 0; - for (const remarks::Argument &Arg : Remark.Args) { - switch (ArgID) { - case 0: - EXPECT_EQ(checkStr(Arg.Key, 6), "Callee"); - EXPECT_EQ(checkStr(Arg.Val, 3), "bar"); - EXPECT_FALSE(Arg.Loc); - break; - case 1: - EXPECT_EQ(checkStr(Arg.Key, 6), "String"); - EXPECT_EQ(checkStr(Arg.Val, 26), " will not be inlined into "); - EXPECT_FALSE(Arg.Loc); - break; - case 2: { - EXPECT_EQ(checkStr(Arg.Key, 6), "Caller"); - EXPECT_EQ(checkStr(Arg.Val, 3), "foo"); - EXPECT_TRUE(Arg.Loc); - const remarks::RemarkLocation &RL = *Arg.Loc; - EXPECT_EQ(checkStr(RL.SourceFilePath, 6), "file.c"); - EXPECT_EQ(RL.SourceLine, 2U); - EXPECT_EQ(RL.SourceColumn, 0U); - break; - } - case 3: - EXPECT_EQ(checkStr(Arg.Key, 6), "String"); - EXPECT_EQ(checkStr(Arg.Val, 38), - " because its definition is unavailable"); - EXPECT_FALSE(Arg.Loc); - break; - default: - break; - } - ++ArgID; - } - - MaybeRemark = Parser.next(); - Error E = MaybeRemark.takeError(); - EXPECT_TRUE(E.isA()); - EXPECT_TRUE(errorToBool(std::move(E))); // Check for parsing errors. -} - -TEST(YAMLRemarks, ParsingBadStringTableIndex) { - StringRef Buf = "\n" - "--- !Missed\n" - "Pass: 50\n" - "\n"; - - StringRef StrTabBuf = StringRef("inline"); - - remarks::ParsedStringTable StrTab(StrTabBuf); - Expected> MaybeParser = - remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, - std::move(StrTab)); - EXPECT_FALSE(errorToBool(MaybeParser.takeError())); - EXPECT_TRUE(*MaybeParser != nullptr); - - remarks::RemarkParser &Parser = **MaybeParser; - Expected> MaybeRemark = Parser.next(); - EXPECT_FALSE(MaybeRemark); // Expect an error here. - - std::string ErrorStr; - raw_string_ostream Stream(ErrorStr); - handleAllErrors(MaybeRemark.takeError(), - [&](const ErrorInfoBase &EIB) { EIB.log(Stream); }); - EXPECT_TRUE( - StringRef(Stream.str()) - .contains("String with index 50 is out of bounds (size = 1).")); -} - TEST(YAMLRemarks, ParsingGoodMeta) { // No metadata should also work. parseGoodMeta("--- !Missed\n" @@ -692,17 +573,6 @@ TEST(YAMLRemarks, ParsingGoodMeta) { "Name: NoDefinition\n" "Function: foo\n", 82)); - - // Use the string table from the metadata. - parseGoodMeta(StringRef("REMARKS\0" - "\0\0\0\0\0\0\0\0" - "\x02\0\0\0\0\0\0\0" - "a\0" - "--- !Missed\n" - "Pass: 0\n" - "Name: 0\n" - "Function: 0\n", - 66)); } TEST(YAMLRemarks, ParsingBadMeta) { @@ -727,7 +597,8 @@ TEST(YAMLRemarks, ParsingBadMeta) { "\0\0\0\0\0\0\0\0" "\x01\0\0\0\0\0\0\0", 24), - "Expecting string table.", CmpType::Equal); + "String table unsupported for YAML format.", + CmpType::Equal); parseExpectErrorMeta(StringRef("REMARKS\0" "\0\0\0\0\0\0\0\0" diff --git a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp index 442c24b9fd95d..7e994ac4d58bc 100644 --- a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp +++ b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp @@ -131,78 +131,6 @@ TEST(YAMLRemarks, SerializerRemarkStandalone) { "...\n")); } -TEST(YAMLRemarks, SerializerRemarkStrTab) { - remarks::Remark R; - R.RemarkType = remarks::Type::Missed; - R.PassName = "pass"; - R.RemarkName = "name"; - R.FunctionName = "func"; - R.Loc = remarks::RemarkLocation{"path", 3, 4}; - R.Hotness = 5; - R.Args.emplace_back(); - R.Args.back().Key = "key"; - R.Args.back().Val = "value"; - R.Args.emplace_back(); - R.Args.back().Key = "keydebug"; - R.Args.back().Val = "valuedebug"; - R.Args.back().Loc = remarks::RemarkLocation{"argpath", 6, 7}; - check(remarks::Format::YAMLStrTab, R, - "--- !Missed\n" - "Pass: 0\n" - "Name: 1\n" - "DebugLoc: { File: 3, Line: 3, Column: 4 }\n" - "Function: 2\n" - "Hotness: 5\n" - "Args:\n" - " - key: 4\n" - " - 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" EXTERNALFILETESTPATH "\0", - 83)); -} - -TEST(YAMLRemarks, SerializerRemarkParsedStrTab) { - StringRef StrTab("pass\0name\0func\0path\0value\0valuedebug\0argpath\0", 45); - remarks::Remark R; - R.RemarkType = remarks::Type::Missed; - R.PassName = "pass"; - R.RemarkName = "name"; - R.FunctionName = "func"; - R.Loc = remarks::RemarkLocation{"path", 3, 4}; - R.Hotness = 5; - R.Args.emplace_back(); - R.Args.back().Key = "key"; - R.Args.back().Val = "value"; - R.Args.emplace_back(); - R.Args.back().Key = "keydebug"; - R.Args.back().Val = "valuedebug"; - R.Args.back().Loc = remarks::RemarkLocation{"argpath", 6, 7}; - check(remarks::Format::YAMLStrTab, R, - "--- !Missed\n" - "Pass: 0\n" - "Name: 1\n" - "DebugLoc: { File: 3, Line: 3, Column: 4 }\n" - "Function: 2\n" - "Hotness: 5\n" - "Args:\n" - " - key: 4\n" - " - 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" EXTERNALFILETESTPATH "\0", - 83), - remarks::StringTable(remarks::ParsedStringTable(StrTab))); -} - TEST(YAMLRemarks, SerializerRemarkParsedStrTabStandaloneNoStrTab) { // Check that we don't use the string table even if it was provided. StringRef StrTab("pass\0name\0func\0path\0value\0valuedebug\0argpath\0", 45); @@ -237,94 +165,3 @@ TEST(YAMLRemarks, SerializerRemarkParsedStrTabStandaloneNoStrTab) { "...\n"), std::move(PreFilledStrTab)); } - -TEST(YAMLRemarks, SerializerRemarkParsedStrTabStandalone) { - StringRef StrTab("pass\0name\0func\0path\0value\0valuedebug\0argpath\0", 45); - remarks::ParsedStringTable ParsedStrTab(StrTab); - remarks::StringTable PreFilledStrTab(ParsedStrTab); - remarks::Remark R; - R.RemarkType = remarks::Type::Missed; - R.PassName = "pass"; - R.RemarkName = "name"; - R.FunctionName = "func"; - R.Loc = remarks::RemarkLocation{"path", 3, 4}; - R.Hotness = 5; - R.Args.emplace_back(); - R.Args.back().Key = "key"; - R.Args.back().Val = "value"; - R.Args.emplace_back(); - R.Args.back().Key = "keydebug"; - R.Args.back().Val = "valuedebug"; - R.Args.back().Loc = remarks::RemarkLocation{"argpath", 6, 7}; - checkStandalone( - remarks::Format::YAMLStrTab, R, - 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" - "--- !Missed\n" - "Pass: 0\n" - "Name: 1\n" - "DebugLoc: { File: 3, Line: 3, Column: 4 }\n" - "Function: 2\n" - "Hotness: 5\n" - "Args:\n" - " - key: 4\n" - " - keydebug: 5\n" - " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" - "...\n", - 315), - std::move(PreFilledStrTab)); -} - -TEST(YAMLRemarks, SerializerRemarkParsedStrTabStandaloneMultipleRemarks) { - StringRef StrTab("pass\0name\0func\0path\0value\0valuedebug\0argpath\0", 45); - remarks::ParsedStringTable ParsedStrTab(StrTab); - remarks::StringTable PreFilledStrTab(ParsedStrTab); - SmallVector Rs; - remarks::Remark R; - R.RemarkType = remarks::Type::Missed; - R.PassName = "pass"; - R.RemarkName = "name"; - R.FunctionName = "func"; - R.Loc = remarks::RemarkLocation{"path", 3, 4}; - R.Hotness = 5; - R.Args.emplace_back(); - R.Args.back().Key = "key"; - R.Args.back().Val = "value"; - R.Args.emplace_back(); - R.Args.back().Key = "keydebug"; - R.Args.back().Val = "valuedebug"; - R.Args.back().Loc = remarks::RemarkLocation{"argpath", 6, 7}; - Rs.emplace_back(R.clone()); - Rs.emplace_back(std::move(R)); - check(remarks::Format::YAMLStrTab, remarks::SerializerMode::Standalone, Rs, - 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" - "--- !Missed\n" - "Pass: 0\n" - "Name: 1\n" - "DebugLoc: { File: 3, Line: 3, Column: 4 }\n" - "Function: 2\n" - "Hotness: 5\n" - "Args:\n" - " - key: 4\n" - " - keydebug: 5\n" - " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" - "...\n" - "--- !Missed\n" - "Pass: 0\n" - "Name: 1\n" - "DebugLoc: { File: 3, Line: 3, Column: 4 }\n" - "Function: 2\n" - "Hotness: 5\n" - "Args:\n" - " - key: 4\n" - " - keydebug: 5\n" - " DebugLoc: { File: 6, Line: 6, Column: 7 }\n" - "...\n", - 561), - /*ExpectedMeta=*/std::nullopt, std::move(PreFilledStrTab)); -}