caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Tobias Stadler
2025-06-18 14:25:41 +01:00
committed by GitHub
parent c4d99704e2
commit 1f34d68c4f
19 changed files with 64 additions and 618 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
llvm/docs/CommandGuide/llvm-opt-report.rst
View File

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

32
llvm/docs/Remarks.rst
View File

@@ -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>``.

2
llvm/include/llvm/Remarks/RemarkFormat.h
View File

@@ -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);

5
llvm/include/llvm/Remarks/RemarkParser.h
View File

@@ -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

41
llvm/include/llvm/Remarks/YAMLRemarkSerializer.h
View File

@@ -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

7
llvm/lib/Remarks/BitstreamRemarkParser.cpp
View File

@@ -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);

7
llvm/lib/Remarks/BitstreamRemarkParser.h
View File

@@ -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 @@ private:
};
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

4
llvm/lib/Remarks/RemarkFormat.cpp
View File

@@ -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);

2
llvm/lib/Remarks/RemarkLinker.cpp
View File

@@ -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)

40
llvm/lib/Remarks/RemarkParser.cpp
View File

@@ -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(); }

5
llvm/lib/Remarks/RemarkSerializer.cpp
View File

@@ -26,8 +26,6 @@ remarks::createRemarkSerializer(Format RemarksFormat, SerializerMode Mode,
"Unknown remark serializer format.");
case Format::YAML:
return std::make_unique<YAMLRemarkSerializer>(OS, Mode);
case Format::YAMLStrTab:
return std::make_unique<YAMLStrTabRemarkSerializer>(OS, Mode);
case Format::Bitstream:
return std::make_unique<BitstreamRemarkSerializer>(OS, Mode);
}
@@ -43,9 +41,6 @@ remarks::createRemarkSerializer(Format RemarksFormat, SerializerMode Mode,
"Unknown remark serializer format.");
case Format::YAML:
return std::make_unique<YAMLRemarkSerializer>(OS, Mode, std::move(StrTab));
case Format::YAMLStrTab:
return std::make_unique<YAMLStrTabRemarkSerializer>(OS, Mode,
std::move(StrTab));
case Format::Bitstream:
return std::make_unique<BitstreamRemarkSerializer>(OS, Mode,
std::move(StrTab));

4
llvm/lib/Remarks/RemarkStreamer.cpp
View File

@@ -21,7 +21,7 @@ static cl::opt<cl::boolOrDefault> 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:

71
llvm/lib/Remarks/YAMLRemarkParser.cpp
View File

@@ -95,21 +95,8 @@ static Expected<uint64_t> parseStrTabSize(StringRef &Buf) {
return StrTabSize;
}
static Expected<ParsedStringTable> 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<ParsedStringTable>(std::move(Result));
}
Expected<std::unique_ptr<YAMLRemarkParser>> remarks::createYAMLParserFromMeta(
StringRef Buf, std::optional<ParsedStringTable> StrTab,
std::optional<StringRef> ExternalFilePrependPath) {
StringRef Buf, std::optional<StringRef> ExternalFilePrependPath) {
// We now have a magic number. The metadata has to be correct.
Expected<bool> isMeta = parseMagic(Buf);
if (!isMeta)
@@ -125,15 +112,9 @@ Expected<std::unique_ptr<YAMLRemarkParser>> 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<ParsedStringTable> 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<std::unique_ptr<YAMLRemarkParser>> remarks::createYAMLParserFromMeta(
}
std::unique_ptr<YAMLRemarkParser> Result =
StrTab
? std::make_unique<YAMLStrTabRemarkParser>(Buf, std::move(*StrTab))
: std::make_unique<YAMLRemarkParser>(Buf);
std::make_unique<YAMLRemarkParser>(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<ParsedStringTable> 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<YAMLParseError>(Message, SM, Stream, Node);
@@ -208,8 +183,8 @@ YAMLRemarkParser::parseRemark(yaml::Document &RemarkEntry) {
Expected<Type> 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<std::unique_ptr<Remark>> YAMLRemarkParser::next() {
return std::move(*MaybeResult);
}
Expected<StringRef> YAMLStrTabRemarkParser::parseStr(yaml::KeyValueNode &Node) {
auto *Value = dyn_cast<yaml::ScalarNode>(Node.getValue());
yaml::BlockScalarNode *ValueBlock;
StringRef Result;
if (!Value) {
// Try to parse the value as a block node.
ValueBlock = dyn_cast<yaml::BlockScalarNode>(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<unsigned> MaybeStrID = parseUnsigned(Node))
StrID = *MaybeStrID;
else
return MaybeStrID.takeError();
if (Expected<StringRef> Str = (*StrTab)[StrID])
Result = *Str;
else
return Str.takeError();
Result.consume_front("\'");
Result.consume_back("\'");
return Result;
}

19
llvm/lib/Remarks/YAMLRemarkParser.h
View File

@@ -46,8 +46,6 @@ private:
/// Regular YAML to Remark parser.
struct YAMLRemarkParser : public RemarkParser {
/// The string table used for parsing strings.
std::optional<ParsedStringTable> 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<ParsedStringTable> 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 @@ protected:
Expected<Argument> 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<StringRef> parseStr(yaml::KeyValueNode &Node) override;
};
Expected<std::unique_ptr<YAMLRemarkParser>> createYAMLParserFromMeta(
StringRef Buf, std::optional<ParsedStringTable> StrTab = std::nullopt,
StringRef Buf,
std::optional<StringRef> ExternalFilePrependPath = std::nullopt);
} // end namespace remarks

103
llvm/lib/Remarks/YAMLRemarkSerializer.cpp
View File

@@ -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 <typename T>
static void mapRemarkHeader(yaml::IO &io, T PassName, T RemarkName,
std::optional<RemarkLocation> RL, T FunctionName,
std::optional<uint64_t> Hotness,
ArrayRef<Argument> Args) {
static void
mapRemarkHeader(yaml::IO &io, StringRef PassName, StringRef RemarkName,
std::optional<RemarkLocation> RL, StringRef FunctionName,
std::optional<uint64_t> Hotness, ArrayRef<Argument> Args) {
io.mapRequired("Pass", PassName);
io.mapRequired("Name", RemarkName);
io.mapOptional("DebugLoc", RL);
@@ -58,19 +57,8 @@ template <> struct MappingTraits<remarks::Remark *> {
else
llvm_unreachable("Unknown remark type");
if (auto *Serializer = dyn_cast<YAMLStrTabRemarkSerializer>(
reinterpret_cast<RemarkSerializer *>(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<RemarkLocation> {
unsigned Line = RL.SourceLine;
unsigned Col = RL.SourceColumn;
if (auto *Serializer = dyn_cast<YAMLStrTabRemarkSerializer>(
reinterpret_cast<RemarkSerializer *>(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<Argument> {
static void mapping(IO &io, Argument &A) {
assert(io.outputting() && "input not yet implemented");
if (auto *Serializer = dyn_cast<YAMLStrTabRemarkSerializer>(
reinterpret_cast<RemarkSerializer *>(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<StringTable> StrTabIn)
: YAMLRemarkSerializer(Format::YAML, OS, Mode, std::move(StrTabIn)) {}
YAMLRemarkSerializer::YAMLRemarkSerializer(Format SerializerFormat,
raw_ostream &OS, SerializerMode Mode,
std::optional<StringTable> StrTabIn)
: RemarkSerializer(SerializerFormat, OS, Mode),
: RemarkSerializer(Format::YAML, OS, Mode),
YAMLOutput(OS, reinterpret_cast<void *>(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<remarks::Remark *>(&Remark);
auto *R = const_cast<remarks::Remark *>(&Remark);
YAMLOutput << R;
}
@@ -181,27 +150,6 @@ std::unique_ptr<MetaSerializer> YAMLRemarkSerializer::metaSerializer(
return std::make_unique<YAMLMetaSerializer>(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 =
metaSerializer(OS, /*ExternalFilename=*/std::nullopt);
MetaSerializer->emit();
DidEmitMeta = true;
}
// Then do the usual remark emission.
YAMLRemarkSerializer::emit(Remark);
}
std::unique_ptr<MetaSerializer> YAMLStrTabRemarkSerializer::metaSerializer(
raw_ostream &OS, std::optional<StringRef> ExternalFilename) {
assert(StrTab);
return std::make_unique<YAMLStrTabMetaSerializer>(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<const 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;
@@ -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<char, 8> StrTabSizeBuf;
support::endian::write64le(StrTabSizeBuf.data(), StrTabSize);
OS.write(StrTabSizeBuf.data(), StrTabSizeBuf.size());
if (ExternalFilename)
emitExternalFile(OS, *ExternalFilename);
}

8
llvm/test/CodeGen/X86/remarks-section.ll
View File

@@ -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

35
llvm/unittests/Remarks/RemarksLinkingTest.cpp
View File

@@ -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,
"<BLOCKINFO_BLOCK/>\n"
"<Meta BlockID=8 NumWords=13 BlockCodeSize=3>\n"
" <Container info codeid=1 abbrevid=4 op0=0 op1=2/>\n"
" <Remark version codeid=2 abbrevid=5 op0=0/>\n"
" <String table codeid=3 abbrevid=6/> blob data = "
"'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00Ok\\x00'\n"
"</Meta>\n"
"<Remark BlockID=9 NumWords=4 BlockCodeSize=4>\n"
" <Remark header codeid=5 abbrevid=4 op0=1 op1=4 op2=0 op3=2/>\n"
" <Remark debug location codeid=6 abbrevid=5 op0=3 op1=3 op2=12/>\n"
"</Remark>\n"
"<Remark BlockID=9 NumWords=4 BlockCodeSize=4>\n"
" <Remark header codeid=5 abbrevid=4 op0=2 op1=1 op2=0 op3=2/>\n"
" <Remark debug location codeid=6 abbrevid=5 op0=3 op1=3 op2=12/>\n"
"</Remark>\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<bool>(E));
std::string ErrorMessage = toString(std::move(E));
EXPECT_EQ(StringRef(ErrorMessage).lower(),

133
llvm/unittests/Remarks/YAMLRemarksParsingTest.cpp
View File

@@ -77,7 +77,6 @@ void parseExpectErrorMeta(
Expected<std::unique_ptr<remarks::RemarkParser>> 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<std::unique_ptr<remarks::RemarkParser>> MaybeParser =
remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf,
std::move(StrTab));
EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
EXPECT_TRUE(*MaybeParser != nullptr);
remarks::RemarkParser &Parser = **MaybeParser;
Expected<std::unique_ptr<remarks::Remark>> 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<remarks::EndOfFileError>());
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<std::unique_ptr<remarks::RemarkParser>> MaybeParser =
remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf,
std::move(StrTab));
EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
EXPECT_TRUE(*MaybeParser != nullptr);
remarks::RemarkParser &Parser = **MaybeParser;
Expected<std::unique_ptr<remarks::Remark>> 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"

163
llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp
View File

@@ -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<remarks::Remark, 2> 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));
}