From: Francis Visoiu Mistrih Date: Tue, 23 Jul 2019 22:50:08 +0000 (+0000) Subject: [Remarks] String tables should be move-only X-Git-Tag: llvmorg-11-init~13726 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4287c95b08ab9b10cce50ab8139f536ea2d72f1d;p=platform%2Fupstream%2Fllvm.git [Remarks] String tables should be move-only Copying them is expensive. This allows the tables to be moved around at lower cost, and allows a remarks::StringTable to be constructed from a remarks::ParsedStringTable. llvm-svn: 366864 --- diff --git a/llvm/include/llvm/Remarks/RemarkParser.h b/llvm/include/llvm/Remarks/RemarkParser.h index eb827d1..f53c04d 100644 --- a/llvm/include/llvm/Remarks/RemarkParser.h +++ b/llvm/include/llvm/Remarks/RemarkParser.h @@ -23,9 +23,6 @@ namespace llvm { namespace remarks { -struct ParserImpl; -struct ParsedStringTable; - class EndOfFileError : public ErrorInfo { public: static char ID; @@ -60,19 +57,28 @@ struct Parser { struct ParsedStringTable { /// The buffer mapped from the section contents. StringRef Buffer; - /// Collection of offsets in the buffer for each string entry. - SmallVector Offsets; + /// This object has high changes to be std::move'd around, so don't use a + /// SmallVector for once. + std::vector Offsets; - Expected operator[](size_t Index) const; ParsedStringTable(StringRef Buffer); + /// Disable copy. + ParsedStringTable(const ParsedStringTable &) = delete; + ParsedStringTable &operator=(const ParsedStringTable &) = delete; + /// Should be movable. + ParsedStringTable(ParsedStringTable &&) = default; + ParsedStringTable &operator=(ParsedStringTable &&) = default; + + size_t size() const { return Offsets.size(); } + Expected operator[](size_t Index) const; }; Expected> createRemarkParser(Format ParserFormat, StringRef Buf); -Expected> -createRemarkParser(Format ParserFormat, StringRef Buf, - const ParsedStringTable &StrTab); +Expected> createRemarkParser(Format ParserFormat, + StringRef Buf, + ParsedStringTable StrTab); } // end namespace remarks } // end namespace llvm diff --git a/llvm/include/llvm/Remarks/RemarkStringTable.h b/llvm/include/llvm/Remarks/RemarkStringTable.h index f9b4fdb..be2596d 100644 --- a/llvm/include/llvm/Remarks/RemarkStringTable.h +++ b/llvm/include/llvm/Remarks/RemarkStringTable.h @@ -18,7 +18,7 @@ #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/Allocator.h" +#include "llvm/Remarks/RemarkParser.h" #include namespace llvm { @@ -31,15 +31,24 @@ namespace remarks { /// This table can be for example serialized in a section to be consumed after /// the compilation. struct StringTable { - /// Allocator holding all the memory used by the map. - BumpPtrAllocator Allocator; /// The string table containing all the unique strings used in the output. /// It maps a string to an unique ID. - StringMap StrTab; + StringMap StrTab; /// Total size of the string table when serialized. size_t SerializedSize = 0; - StringTable() : Allocator(), StrTab(Allocator) {} + StringTable() = default; + + /// Disable copy. + StringTable(const StringTable &) = delete; + StringTable &operator=(const StringTable &) = delete; + /// Should be movable. + StringTable(StringTable &&) = default; + StringTable &operator=(StringTable &&) = default; + + /// Construct a string table from a ParsedStringTable. + StringTable(const ParsedStringTable &Other); + /// Add a string to the table. It returns an unique ID of the string. std::pair add(StringRef Str); /// Serialize the string table to a stream. It is serialized as a little diff --git a/llvm/lib/Remarks/RemarkParser.cpp b/llvm/lib/Remarks/RemarkParser.cpp index 96a4b42..ac46ae3 100644 --- a/llvm/lib/Remarks/RemarkParser.cpp +++ b/llvm/lib/Remarks/RemarkParser.cpp @@ -64,14 +64,14 @@ llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf) { Expected> llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf, - const ParsedStringTable &StrTab) { + 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 llvm::make_unique(Buf, StrTab); + return llvm::make_unique(Buf, std::move(StrTab)); case Format::Unknown: return createStringError(std::make_error_code(std::errc::invalid_argument), "Unknown remark parser format."); @@ -84,10 +84,10 @@ struct CParser { Optional Err; CParser(Format ParserFormat, StringRef Buf, - Optional StrTab = None) - : TheParser(cantFail(StrTab - ? createRemarkParser(ParserFormat, Buf, **StrTab) - : createRemarkParser(ParserFormat, Buf))) {} + Optional StrTab = None) + : TheParser(cantFail( + StrTab ? createRemarkParser(ParserFormat, Buf, std::move(*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/RemarkStringTable.cpp b/llvm/lib/Remarks/RemarkStringTable.cpp index 90a90e5..17aa394 100644 --- a/llvm/lib/Remarks/RemarkStringTable.cpp +++ b/llvm/lib/Remarks/RemarkStringTable.cpp @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// #include "llvm/Remarks/RemarkStringTable.h" +#include "llvm/Remarks/RemarkParser.h" #include "llvm/Support/EndianStream.h" #include "llvm/Support/Error.h" #include @@ -18,6 +19,14 @@ using namespace llvm; using namespace llvm::remarks; +StringTable::StringTable(const ParsedStringTable &Other) : StrTab() { + for (unsigned i = 0, e = Other.size(); i < e; ++i) + if (Expected MaybeStr = Other[i]) + add(*MaybeStr); + else + llvm_unreachable("Unexpected error while building remarks string table."); +} + std::pair StringTable::add(StringRef Str) { size_t NextID = StrTab.size(); auto KV = StrTab.insert({Str, NextID}); diff --git a/llvm/lib/Remarks/YAMLRemarkParser.cpp b/llvm/lib/Remarks/YAMLRemarkParser.cpp index 3cbb4932..fcb4cce 100644 --- a/llvm/lib/Remarks/YAMLRemarkParser.cpp +++ b/llvm/lib/Remarks/YAMLRemarkParser.cpp @@ -58,8 +58,8 @@ YAMLRemarkParser::YAMLRemarkParser(StringRef Buf) : YAMLRemarkParser(Buf, None) {} YAMLRemarkParser::YAMLRemarkParser(StringRef Buf, - Optional StrTab) - : Parser{Format::YAML}, StrTab(StrTab), LastErrorMessage(), + Optional StrTab) + : Parser{Format::YAML}, StrTab(std::move(StrTab)), LastErrorMessage(), SM(setupSM(LastErrorMessage)), Stream(Buf, SM), YAMLIt(Stream.begin()) {} Error YAMLRemarkParser::error(StringRef Message, yaml::Node &Node) { @@ -326,7 +326,7 @@ Expected YAMLStrTabRemarkParser::parseStr(yaml::KeyValueNode &Node) { else return MaybeStrID.takeError(); - if (Expected Str = (**StrTab)[StrID]) + if (Expected Str = (*StrTab)[StrID]) Result = *Str; else return Str.takeError(); diff --git a/llvm/lib/Remarks/YAMLRemarkParser.h b/llvm/lib/Remarks/YAMLRemarkParser.h index 82a86fa..da0e9de 100644 --- a/llvm/lib/Remarks/YAMLRemarkParser.h +++ b/llvm/lib/Remarks/YAMLRemarkParser.h @@ -48,7 +48,7 @@ private: /// Regular YAML to Remark parser. struct YAMLRemarkParser : public Parser { /// The string table used for parsing strings. - Optional StrTab; + 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; @@ -68,7 +68,7 @@ struct YAMLRemarkParser : public Parser { } protected: - YAMLRemarkParser(StringRef Buf, Optional StrTab); + 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. @@ -93,8 +93,8 @@ protected: /// YAML with a string table to Remark parser. struct YAMLStrTabRemarkParser : public YAMLRemarkParser { - YAMLStrTabRemarkParser(StringRef Buf, const ParsedStringTable &StrTab) - : YAMLRemarkParser(Buf, &StrTab) {} + YAMLStrTabRemarkParser(StringRef Buf, ParsedStringTable StrTab) + : YAMLRemarkParser(Buf, std::move(StrTab)) {} static bool classof(const Parser *P) { return P->ParserFormat == Format::YAMLStrTab; diff --git a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp index 84d1371..baa4814 100644 --- a/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp +++ b/llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp @@ -526,7 +526,8 @@ TEST(YAMLRemarks, ContentsStrTab) { remarks::ParsedStringTable StrTab(StrTabBuf); Expected> MaybeParser = - remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, StrTab); + remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, + std::move(StrTab)); EXPECT_FALSE(errorToBool(MaybeParser.takeError())); EXPECT_TRUE(*MaybeParser != nullptr); @@ -601,7 +602,8 @@ TEST(YAMLRemarks, ParsingBadStringTableIndex) { remarks::ParsedStringTable StrTab(StrTabBuf); Expected> MaybeParser = - remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, StrTab); + remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, + std::move(StrTab)); EXPECT_FALSE(errorToBool(MaybeParser.takeError())); EXPECT_TRUE(*MaybeParser != nullptr);