From c5b5cc4575452463360a16c80e1fce25e7e06d31 Mon Sep 17 00:00:00 2001 From: Francis Visoiu Mistrih Date: Tue, 23 Jul 2019 20:42:46 +0000 Subject: [PATCH] [Remarks] Introduce a new format: yaml-strtab This exposes better support to use a string table with a format through an actual new remark::Format, called yaml-strtab. This can now be used with -fsave-optimization-record=yaml-strtab. llvm-svn: 366849 --- llvm/docs/Remarks.rst | 29 ++++++++++++++ llvm/include/llvm/Remarks/RemarkFormat.h | 2 +- llvm/include/llvm/Remarks/RemarkParser.h | 5 ++- llvm/include/llvm/Remarks/RemarkSerializer.h | 3 -- llvm/include/llvm/Remarks/YAMLRemarkSerializer.h | 13 +++++- llvm/lib/IR/RemarkStreamer.cpp | 4 +- llvm/lib/Remarks/RemarkFormat.cpp | 1 + llvm/lib/Remarks/RemarkParser.cpp | 28 +++++++++++-- llvm/lib/Remarks/YAMLRemarkParser.cpp | 46 ++++++++++++++-------- llvm/lib/Remarks/YAMLRemarkParser.h | 22 +++++++++-- llvm/lib/Remarks/YAMLRemarkSerializer.cpp | 11 +----- llvm/test/CodeGen/X86/remarks-section.ll | 2 +- llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp | 4 +- .../Remarks/YAMLRemarksSerializerTest.cpp | 15 +++---- 14 files changed, 134 insertions(+), 51 deletions(-) diff --git a/llvm/docs/Remarks.rst b/llvm/docs/Remarks.rst index e3d088d..5646547 100644 --- a/llvm/docs/Remarks.rst +++ b/llvm/docs/Remarks.rst @@ -112,6 +112,7 @@ following options: Supported formats: * :ref:`yaml ` (default) + * :ref:`yaml-strtab ` ``Content configuration`` @@ -191,12 +192,40 @@ 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: + opt-viewer ========== The ``opt-viewer`` directory contains a collection of tools that visualize and summarize serialized remarks. +The tools only support the ``yaml`` format. + .. _optviewerpy: opt-viewer.py diff --git a/llvm/include/llvm/Remarks/RemarkFormat.h b/llvm/include/llvm/Remarks/RemarkFormat.h index e167d99..87f5fa2 100644 --- a/llvm/include/llvm/Remarks/RemarkFormat.h +++ b/llvm/include/llvm/Remarks/RemarkFormat.h @@ -22,7 +22,7 @@ namespace remarks { constexpr StringRef Magic("REMARKS", 7); /// The format used for serializing/deserializing remarks. -enum class Format { Unknown, YAML }; +enum class Format { Unknown, YAML, YAMLStrTab }; /// Parse and validate a string for the remark format. Expected parseFormat(StringRef FormatStr); diff --git a/llvm/include/llvm/Remarks/RemarkParser.h b/llvm/include/llvm/Remarks/RemarkParser.h index 671e1ab..eb827d1 100644 --- a/llvm/include/llvm/Remarks/RemarkParser.h +++ b/llvm/include/llvm/Remarks/RemarkParser.h @@ -67,9 +67,12 @@ struct ParsedStringTable { ParsedStringTable(StringRef Buffer); }; +Expected> createRemarkParser(Format ParserFormat, + StringRef Buf); + Expected> createRemarkParser(Format ParserFormat, StringRef Buf, - Optional StrTab = None); + const ParsedStringTable &StrTab); } // end namespace remarks } // end namespace llvm diff --git a/llvm/include/llvm/Remarks/RemarkSerializer.h b/llvm/include/llvm/Remarks/RemarkSerializer.h index 3eddac4..bcf6381 100644 --- a/llvm/include/llvm/Remarks/RemarkSerializer.h +++ b/llvm/include/llvm/Remarks/RemarkSerializer.h @@ -36,9 +36,6 @@ struct Serializer { virtual void emit(const Remark &Remark) = 0; }; -/// Wether the serializer should use a string table while emitting. -enum class UseStringTable { No, Yes }; - } // end namespace remarks } // end namespace llvm diff --git a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h b/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h index 5b81732..9bb5c6e 100644 --- a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h +++ b/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h @@ -34,13 +34,22 @@ struct YAMLSerializer : public Serializer { /// The YAML streamer. yaml::Output YAMLOutput; - YAMLSerializer(raw_ostream &OS, - UseStringTable UseStringTable = remarks::UseStringTable::No); + YAMLSerializer(raw_ostream &OS); /// Emit a remark to the stream. void emit(const Remark &Remark) 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 YAMLStrTabSerializer : public YAMLSerializer { + YAMLStrTabSerializer(raw_ostream &OS) : YAMLSerializer(OS) { + // Having a string table set up enables the serializer to use it. + StrTab.emplace(); + } +}; + } // end namespace remarks } // end namespace llvm diff --git a/llvm/lib/IR/RemarkStreamer.cpp b/llvm/lib/IR/RemarkStreamer.cpp index 9470f3e..c71487d 100644 --- a/llvm/lib/IR/RemarkStreamer.cpp +++ b/llvm/lib/IR/RemarkStreamer.cpp @@ -110,11 +110,13 @@ char RemarkSetupFormatError::ID = 0; static std::unique_ptr formatToSerializer(remarks::Format RemarksFormat, raw_ostream &OS) { switch (RemarksFormat) { - default: + case remarks::Format::Unknown: llvm_unreachable("Unknown remark serializer format."); return nullptr; case remarks::Format::YAML: return llvm::make_unique(OS); + case remarks::Format::YAMLStrTab: + return llvm::make_unique(OS); }; } diff --git a/llvm/lib/Remarks/RemarkFormat.cpp b/llvm/lib/Remarks/RemarkFormat.cpp index bcd0f75..4e9ada6 100644 --- a/llvm/lib/Remarks/RemarkFormat.cpp +++ b/llvm/lib/Remarks/RemarkFormat.cpp @@ -19,6 +19,7 @@ using namespace llvm::remarks; Expected llvm::remarks::parseFormat(StringRef FormatStr) { auto Result = StringSwitch(FormatStr) .Cases("", "yaml", Format::YAML) + .Cases("", "yaml-strtab", Format::YAMLStrTab) .Default(Format::Unknown); if (Result == Format::Unknown) diff --git a/llvm/lib/Remarks/RemarkParser.cpp b/llvm/lib/Remarks/RemarkParser.cpp index f674640..96a4b42 100644 --- a/llvm/lib/Remarks/RemarkParser.cpp +++ b/llvm/lib/Remarks/RemarkParser.cpp @@ -48,16 +48,34 @@ Expected ParsedStringTable::operator[](size_t Index) const { } Expected> +llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf) { + switch (ParserFormat) { + case Format::YAML: + return llvm::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::Unknown: + return createStringError(std::make_error_code(std::errc::invalid_argument), + "Unknown remark parser format."); + } +} + +Expected> llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf, - Optional StrTab) { + const ParsedStringTable &StrTab) { switch (ParserFormat) { case Format::YAML: - return llvm::make_unique(Buf, StrTab); + 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 llvm::make_unique(Buf, StrTab); case Format::Unknown: return createStringError(std::make_error_code(std::errc::invalid_argument), "Unknown remark parser format."); } - llvm_unreachable("unknown format"); } // Wrapper that holds the state needed to interact with the C API. @@ -67,7 +85,9 @@ struct CParser { CParser(Format ParserFormat, StringRef Buf, Optional StrTab = None) - : TheParser(cantFail(createRemarkParser(ParserFormat, Buf, StrTab))) {} + : TheParser(cantFail(StrTab + ? createRemarkParser(ParserFormat, Buf, **StrTab) + : createRemarkParser(ParserFormat, Buf))) {} void handleError(Error E) { Err.emplace(toString(std::move(E))); } bool hasError() const { return Err.hasValue(); } diff --git a/llvm/lib/Remarks/YAMLRemarkParser.cpp b/llvm/lib/Remarks/YAMLRemarkParser.cpp index ed78b7b..3cbb4932 100644 --- a/llvm/lib/Remarks/YAMLRemarkParser.cpp +++ b/llvm/lib/Remarks/YAMLRemarkParser.cpp @@ -54,6 +54,9 @@ static SourceMgr setupSM(std::string &LastErrorMessage) { return SM; } +YAMLRemarkParser::YAMLRemarkParser(StringRef Buf) + : YAMLRemarkParser(Buf, None) {} + YAMLRemarkParser::YAMLRemarkParser(StringRef Buf, Optional StrTab) : Parser{Format::YAML}, StrTab(StrTab), LastErrorMessage(), @@ -179,22 +182,7 @@ Expected YAMLRemarkParser::parseStr(yaml::KeyValueNode &Node) { auto *Value = dyn_cast(Node.getValue()); if (!Value) return error("expected a value of scalar type.", Node); - StringRef Result; - if (!StrTab) { - Result = Value->getRawValue(); - } else { - // 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(); - } + StringRef Result = Value->getRawValue(); if (Result.front() == '\'') Result = Result.drop_front(); @@ -325,3 +313,29 @@ Expected> YAMLRemarkParser::next() { return std::move(*MaybeResult); } + +Expected YAMLStrTabRemarkParser::parseStr(yaml::KeyValueNode &Node) { + auto *Value = dyn_cast(Node.getValue()); + if (!Value) + return error("expected a value of scalar type.", Node); + StringRef Result; + // 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(); + + if (Result.front() == '\'') + Result = Result.drop_front(); + + if (Result.back() == '\'') + Result = Result.drop_back(); + + return Result; +} diff --git a/llvm/lib/Remarks/YAMLRemarkParser.h b/llvm/lib/Remarks/YAMLRemarkParser.h index cea76e6..82a86fa 100644 --- a/llvm/lib/Remarks/YAMLRemarkParser.h +++ b/llvm/lib/Remarks/YAMLRemarkParser.h @@ -59,8 +59,7 @@ struct YAMLRemarkParser : public Parser { /// Iterator in the YAML stream. yaml::document_iterator YAMLIt; - YAMLRemarkParser(StringRef Buf, - Optional StrTab = None); + YAMLRemarkParser(StringRef Buf); Expected> next() override; @@ -68,7 +67,8 @@ struct YAMLRemarkParser : public Parser { return P->ParserFormat == Format::YAML; } -private: +protected: + YAMLRemarkParser(StringRef Buf, Optional StrTab); /// Create a YAMLParseError error from an existing error generated by the YAML /// parser. /// If there is no error, this returns Success. @@ -82,7 +82,7 @@ private: /// Parse one key to a string. Expected parseKey(yaml::KeyValueNode &Node); /// Parse one value to a string. - Expected parseStr(yaml::KeyValueNode &Node); + virtual Expected parseStr(yaml::KeyValueNode &Node); /// Parse one value to an unsigned. Expected parseUnsigned(yaml::KeyValueNode &Node); /// Parse a debug location. @@ -90,6 +90,20 @@ private: /// Parse an argument. Expected parseArg(yaml::Node &Node); }; + +/// YAML with a string table to Remark parser. +struct YAMLStrTabRemarkParser : public YAMLRemarkParser { + YAMLStrTabRemarkParser(StringRef Buf, const ParsedStringTable &StrTab) + : YAMLRemarkParser(Buf, &StrTab) {} + + static bool classof(const Parser *P) { + return P->ParserFormat == Format::YAMLStrTab; + } + +protected: + /// Parse one value to a string. + Expected parseStr(yaml::KeyValueNode &Node) override; +}; } // end namespace remarks } // end namespace llvm diff --git a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp index e437974..1297fbc 100644 --- a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp +++ b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp @@ -17,10 +17,6 @@ using namespace llvm; using namespace llvm::remarks; -cl::opt RemarksYAMLStringTable( - "remarks-yaml-string-table", cl::init(false), cl::Hidden, - cl::desc("Enable the usage of a string table with YAML remarks.")); - // Use the same keys whether we use a string table or not (respectively, T is an // unsigned or a StringRef). template @@ -153,11 +149,8 @@ template <> struct MappingTraits { LLVM_YAML_IS_SEQUENCE_VECTOR(Argument) -YAMLSerializer::YAMLSerializer(raw_ostream &OS, UseStringTable UseStringTable) - : Serializer(OS), YAMLOutput(OS, reinterpret_cast(this)) { - if (UseStringTable == remarks::UseStringTable::Yes || RemarksYAMLStringTable) - StrTab.emplace(); -} +YAMLSerializer::YAMLSerializer(raw_ostream &OS) + : Serializer(OS), YAMLOutput(OS, reinterpret_cast(this)) {} void YAMLSerializer::emit(const Remark &Remark) { // Again, YAMLTraits expect a non-const object for inputting, but we're not diff --git a/llvm/test/CodeGen/X86/remarks-section.ll b/llvm/test/CodeGen/X86/remarks-section.ll index 6d7957a..3ff2fb2 100644 --- a/llvm/test/CodeGen/X86/remarks-section.ll +++ b/llvm/test/CodeGen/X86/remarks-section.ll @@ -1,6 +1,6 @@ ; RUN: llc < %s -mtriple=x86_64-linux -remarks-section -pass-remarks-output=%/t.yaml | FileCheck -DPATH=%/t.yaml %s ; 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 -remarks-section -remarks-yaml-string-table -pass-remarks-output=%/t.yaml | FileCheck --check-prefix=CHECK-DARWIN-STRTAB -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 ; CHECK-LABEL: func1: diff --git a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp index 8b79dfd..84d1371 100644 --- a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp +++ b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp @@ -526,7 +526,7 @@ TEST(YAMLRemarks, ContentsStrTab) { remarks::ParsedStringTable StrTab(StrTabBuf); Expected> MaybeParser = - remarks::createRemarkParser(remarks::Format::YAML, Buf, &StrTab); + remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, StrTab); EXPECT_FALSE(errorToBool(MaybeParser.takeError())); EXPECT_TRUE(*MaybeParser != nullptr); @@ -601,7 +601,7 @@ TEST(YAMLRemarks, ParsingBadStringTableIndex) { remarks::ParsedStringTable StrTab(StrTabBuf); Expected> MaybeParser = - remarks::createRemarkParser(remarks::Format::YAML, Buf, &StrTab); + remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, StrTab); EXPECT_FALSE(errorToBool(MaybeParser.takeError())); EXPECT_TRUE(*MaybeParser != nullptr); diff --git a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp index c5f222f..629811a 100644 --- a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp +++ b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp @@ -16,16 +16,17 @@ static void check(const remarks::Remark &R, StringRef Expected, Optional ExpectedStrTab = None) { std::string Buf; raw_string_ostream OS(Buf); - remarks::UseStringTable UseStrTab = ExpectedStrTab.hasValue() - ? remarks::UseStringTable::Yes - : remarks::UseStringTable::No; - remarks::YAMLSerializer S(OS, UseStrTab); - S.emit(R); + bool UseStrTab = ExpectedStrTab.hasValue(); + std::unique_ptr S = + UseStrTab ? llvm::make_unique(OS) + : llvm::make_unique(OS); + + S->emit(R); EXPECT_EQ(OS.str(), Expected); if (ExpectedStrTab) { Buf.clear(); - EXPECT_TRUE(S.StrTab); - S.StrTab->serialize(OS); + EXPECT_TRUE(S->StrTab); + S->StrTab->serialize(OS); EXPECT_EQ(OS.str(), *ExpectedStrTab); } } -- 2.7.4