Add a utility for converting between different types of remarks

[paquette]

Summary

This adds llvm-remarkutil. This is intended to be a general tool for doing stuff with/to remark files.

This patch gives it the following powers:

• bitstream2yaml - To convert bitstream remarks to YAML
• yaml2bitstream - To convert YAML remarks to bitstream remarks

These are both implemented as subcommands, like

llvm-remarkutil bitstream2yaml <input_file> -o -

I ran into an issue where I had some bitstream remarks coming from CI, and I wanted to be able to do stuff with them (e.g. visualize them) But then I noticed we didn't have any tooling for doing that, so I decided to write this thing.

Being able to output YAML as a start seemed like a good idea, since it would allow people to reuse any tooling they may have written based around YAML remarks.

Hopefully it can grow into a more featureful remark utility. :)

Currently there are is an outstanding performance issue (see the TODO) with the bitstream2yaml case. I decided that I'd keep the tool small to start with and have the yaml2bitstream and bitstream2yaml cases be symmetric.

Also I moved the remarks documentation to its own header because it seems a little out of place with "basic commands" and "developer tools"; it's really kind of its own thing.

[thegameg]

Looks straightforward and pretty good to me. Thanks for working on this.

One suggestion would be to have a convert subcommand that takes bitstream2yaml or yaml2bitstream, assuming this tool will be extended to do more (for example, one thing I want to add is extract from a mach-o object file). I don't have strong opinions though so if everyone is fine with this I'm also fine.

llvm/tools/llvm-remarkutil/RemarkUtil.cpp

50

Does this need a static/const/constexpr?

[paquette]

Make some stuff static that should have been static

[paquette]

In D133646#3782446, @thegameg wrote:

Looks straightforward and pretty good to me. Thanks for working on this.

One suggestion would be to have a convert subcommand that takes bitstream2yaml or yaml2bitstream, assuming this tool will be extended to do more (for example, one thing I want to add is extract from a mach-o object file). I don't have strong opinions though so if everyone is fine with this I'm also fine.

What would this use-case look like in the (proposed) UI?

[thegameg]

In D133646#3782500, @paquette wrote:

In D133646#3782446, @thegameg wrote:

One suggestion would be to have a convert subcommand that takes bitstream2yaml or yaml2bitstream, assuming this tool will be extended to do more (for example, one thing I want to add is extract from a mach-o object file). I don't have strong opinions though so if everyone is fine with this I'm also fine.

What would this use-case look like in the (proposed) UI?

$ llvm-remarkutil -convert bitstream2yaml a.opt.bitstream -o a.opt.yaml
$ llvm-remarkutil -convert yaml2bitstream a.opt.bitstream -o a.opt.yaml
$ llvm-remarkutil -extract a.o -o a.opt.bitstream`

is what I had in mind. Maybe having -input-format=bitstream -output-format=yaml is better for other use cases?

[thegameg]

Also maybe something you would want to do in a separate patch, adding a -dump option that only calls llvm::remarks::Remark::dump() on all the parsed remarks would be very helpful (with maybe a -filter too)

[jroelofs]

LGTM

llvm/tools/llvm-remarkutil/RemarkUtil.cpp

129

const?

[paquette]

In D133646#3782507, @thegameg wrote:

In D133646#3782500, @paquette wrote:

In D133646#3782446, @thegameg wrote:

One suggestion would be to have a convert subcommand that takes bitstream2yaml or yaml2bitstream, assuming this tool will be extended to do more (for example, one thing I want to add is extract from a mach-o object file). I don't have strong opinions though so if everyone is fine with this I'm also fine.

What would this use-case look like in the (proposed) UI?

$ llvm-remarkutil -convert bitstream2yaml a.opt.bitstream -o a.opt.yaml
$ llvm-remarkutil -convert yaml2bitstream a.opt.bitstream -o a.opt.yaml
$ llvm-remarkutil -extract a.o -o a.opt.bitstream`

is what I had in mind. Maybe having -input-format=bitstream -output-format=yaml is better for other use cases?

I don't think you'd actually need either of those because you could just pipe in/out of the tool, or am I missing something?

I was thinking it'd also be useful to have options like

llvm-remarkutil stats --input=yaml <file>.yaml
llvm-remarkutil stats --input=bitstream <file>.bitstream

or

llvm-remarkutil stats -y <file>.yaml
llvm-remarkutil stats -b <file>.bitstream

which could print out some stats/facts about the remarks as well.

That'd be pretty separate from conversion, which is why I'm somewhat inclined to keep these as suboptions versus regular flags.

[thegameg]

In D133646#3784306, @paquette wrote:

That'd be pretty separate from conversion, which is why I'm somewhat inclined to keep these as suboptions versus regular flags.

👍

[paquette]

• Make vector const
• Disable bitstream conversion test on non-Darwin platforms (attempting to fix pre-merge checks before pushing)

[paquette]

Francis pointed out to me that the Darwin assumption for bitstream only holds when embedding metadata in Separate mode.

• Drop the Darwin check
• Try to modify the test to see if we can make Debian and Windows happy in the pre-merge checks.

Current assumption is that piping binary data is weird in the tests, although it Works On My Machine™

[paquette]

Add missing std::move when returning an Error. That was the only issue that showed up when I built the patch inside a Debian Docker container.

It looks like the tests fail cause applying the patch won't actually create the correct binary files. Going to push now.

[MatzeB]

On a somewhat random note: I have a remarks2sql utility here that converts remarks into an sqlite3 database which I found works great when analyzing bigger codebases where you pretty much always want to do various amount of filtering, sorting and aggregation... I should probably clean that up and add it to llvm/utils if there's demand from others... Unfortunately it's written in python, so can't easily be integrated into this tool here...

[paquette]

In D133646#3785158, @MatzeB wrote:

On a somewhat random note: I have a remarks2sql utility here that converts remarks into an sqlite3 database which I found works great when analyzing bigger codebases where you pretty much always want to do various amount of filtering, sorting and aggregation... I should probably clean that up and add it to llvm/utils if there's demand from others... Unfortunately it's written in python, so can't easily be integrated into this tool here...

I think that would be extremely handy to have. I thought something like a

llvm-remarkutil query <file> <query>

option would be really handy, but an actual database would probably be better-suited to that task than basically anything else...

llvm/docs/CommandGuide/LLVMRemarkUtil.rst

+llvm-remarkutil -
+==============================================================
+
+.. program:: llvm-remarkutil
+
+SYNOPSIS
+--------
+
+:program:`llvm-remarkutil` [*subcommmand*] [*options*]
+
+DESCRIPTION
+-----------
+
+Utility for displaying information from, and converting between different
+`remark <https://llvm.org/docs/Remarks.html>`_ formats.
+
+Subcommands
+-----------
+
+  * :ref:`bitstream2yaml_subcommand` - Reserialize bitstream remarks to YAML.
+  * :ref:`yaml2bitstream_subcommand` - Reserialize YAML remarks to bitstream.
+
+.. _bitstream2yaml_subcommand:
+
+bitstream2yaml
+~~~~~~
+
+.. program:: llvm-remarkutil bitstream2yaml
+
+USAGE: :program:`llvm-remarkutil` bitstream2yaml <input file> -o <output file>
+
+Summary
+^^^^^^^^^^^
+
+Takes a bitstream remark file as input, and reserializes that file as YAML.
+
+.. _yaml2bitstream_subcommand:
+
+yaml2bitstream
+~~~~~~
+
+.. program:: llvm-remarkutil yaml2bitstream
+
+USAGE: :program:`llvm-remarkutil` yaml2bitstream <input file> -o <output file>
+
+Summary
+^^^^^^^^^^^
+
+Takes a YAML remark file as input, and reserializes that file in the bitstream
+format.

llvm/docs/CommandGuide/index.rst

.. toctree::
    llvm-lib
    llvm-libtool-darwin
    llvm-link
    llvm-lipo
    llvm-mca
    llvm-otool
    llvm-profdata
    llvm-readobj
-   llvm-remark-size-diff
    llvm-stress
    llvm-symbolizer
    opt
 
 GNU binutils replacements
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
 .. toctree::
.. toctree::
    mlir-tblgen
    lit
    llvm-exegesis
    llvm-ifs
    llvm-locstats
    llvm-pdbutil
    llvm-profgen
    llvm-tli-checker
+
+Remarks Tools
+~~~~~~~~~~~~~~
+
+.. toctree::
+   :maxdepth: 1
+
+   llvm-remark-size-diff
+   llvm-remarkutil

llvm/test/CMakeLists.txt

set(LLVM_TEST_DEPENDS
           llvm-profdata
           llvm-profgen
           llvm-ranlib
           llvm-rc
           llvm-readobj
           llvm-readelf
           llvm-reduce
           llvm-remark-size-diff
+          llvm-remarkutil
           llvm-rtdyld
           llvm-sim
           llvm-size
           llvm-split
           llvm-stress
           llvm-strings
           llvm-strip
           llvm-symbolizer

llvm/test/lit.cfg.py

tools.extend([
     'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mca',
     'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump', 'llvm-otool',
     'llvm-pdbutil', 'llvm-profdata', 'llvm-profgen', 'llvm-ranlib', 'llvm-rc', 'llvm-readelf',
     'llvm-readobj', 'llvm-remark-size-diff', 'llvm-rtdyld', 'llvm-sim',
     'llvm-size', 'llvm-split', 'llvm-stress', 'llvm-strings', 'llvm-strip',
     'llvm-tblgen', 'llvm-tapi-diff', 'llvm-undname', 'llvm-windres',
     'llvm-c-test', 'llvm-cxxfilt', 'llvm-xray', 'yaml2obj', 'obj2yaml',
     'yaml-bench', 'verify-uselistorder', 'bugpoint', 'llc', 'llvm-symbolizer',
-    'opt', 'sancov', 'sanstats'])
+    'opt', 'sancov', 'sanstats', 'llvm-remarkutil'])
 
 # The following tools are optional
 tools.extend([
     ToolSubst('llvm-go', unresolved='ignore'),
     ToolSubst('llvm-mt', unresolved='ignore'),
     ToolSubst('Kaleidoscope-Ch3', unresolved='ignore'),
     ToolSubst('Kaleidoscope-Ch4', unresolved='ignore'),
     ToolSubst('Kaleidoscope-Ch5', unresolved='ignore'),

llvm/test/tools/llvm-remarkutil/Inputs/broken-remark

+--- !Analysis
+Name:            StackSize
+Function:        func0
+Args:
+  - NumStackBytes:   '1'
+  - String:          ' stack bytes in function'
+...
+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+Function:        func0
+Args:
+  - NumInstructions: '1'
+  - String:          ' instructions in function'
+...

llvm/test/tools/llvm-remarkutil/Inputs/two-remarks.yaml

+--- !Analysis
+Pass:            prologepilog
+Name:            StackSize
+Function:        func0
+Args:
+  - NumStackBytes:   '1'
+  - String:          ' stack bytes in function'
+...
+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+Function:        func0
+Args:
+  - NumInstructions: '1'
+  - String:          ' instructions in function'
+...

llvm/test/tools/llvm-remarkutil/broken-bitstream-remark.test

+RUN: not llvm-remarkutil bitstream2yaml %p/Inputs/broken-remark -o - 2>&1 | FileCheck %s
+CHECK: error: Unknown magic number: expecting RMRK, got --- .

llvm/test/tools/llvm-remarkutil/broken-yaml-remark.test

+RUN: not llvm-remarkutil yaml2bitstream %p/Inputs/broken-remark -o - 2>&1 | FileCheck %s
+CHECK: error: Type, Pass, Name or Function missing

llvm/test/tools/llvm-remarkutil/convert.test

+RUN: llvm-remarkutil bitstream2yaml %p/Inputs/two-remarks.bitstream -o - | FileCheck %s -strict-whitespace
+RUN: llvm-remarkutil yaml2bitstream %p/Inputs/two-remarks.yaml -o %t
+RUN: llvm-remarkutil bitstream2yaml %t -o - | FileCheck %s -strict-whitespace
+
+; CHECK: --- !Analysis
+; CHECK-NEXT: Pass:            prologepilog
+; CHECK-NEXT: Name:            StackSize
+; CHECK-NEXT: Function:        func0
+; CHECK-NEXT: Args:
+; CHECK-NEXT:   - NumStackBytes:   '1'
+; CHECK-NEXT:   - String:          ' stack bytes in function'
+; CHECK-NEXT: ...
+; CHECK-NEXT: --- !Analysis
+; CHECK-NEXT: Pass:            asm-printer
+; CHECK-NEXT: Name:            InstructionCount
+; CHECK-NEXT: Function:        func0
+; CHECK-NEXT: Args:
+; CHECK-NEXT:   - NumInstructions: '1'
+; CHECK-NEXT:   - String:          ' instructions in function'
+; CHECK-NEXT: ...

llvm/test/tools/llvm-remarkutil/empty-file.test

+RUN: not llvm-remarkutil yaml2bitstream %p/Inputs/empty-file -o - 2>&1 | FileCheck %s --check-prefix=YAML2BITSTREAM
+RUN: llvm-remarkutil bitstream2yaml %p/Inputs/empty-file -o - 2>&1 | FileCheck %s --allow-empty --check-prefix=BITSTREAM2YAML
+
+; YAML2BITSTREAM: error: document root is not of mapping type.
+
+; An empty bitstream file is valid.
+; BITSTREAM2YAML-NOT: error

llvm/test/tools/llvm-remarkutil/file-does-not-exist.test

+RUN: not llvm-remarkutil bitstream2yaml %p/Inputs/i-do-not-exist -o - 2>&1 | FileCheck %s
+RUN: not llvm-remarkutil yaml2bitstream %p/Inputs/i-do-not-exist -o - 2>&1 | FileCheck %s
+CHECK: error: Cannot open file

llvm/test/tools/llvm-remarkutil/missing-subcommand.test

+RUN: not llvm-remarkutil 2>&1 | FileCheck %s
+CHECK: error: Please specify a subcommand. (See -help for options)

llvm/tools/llvm-remarkutil/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS Core Demangle Object Remarks Support)
+
+add_llvm_tool(llvm-remarkutil
+  RemarkUtil.cpp
+)

llvm/tools/llvm-remarkutil/RemarkUtil.cpp

+//===--------- llvm-remarkutil/RemarkUtil.cpp -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+/// Utility for remark files.
+//===----------------------------------------------------------------------===//
+
+#include "llvm-c/Remarks.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Remarks/Remark.h"
+#include "llvm/Remarks/RemarkFormat.h"
+#include "llvm/Remarks/RemarkParser.h"
+#include "llvm/Remarks/YAMLRemarkSerializer.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/WithColor.h"
+
+using namespace llvm;
+using namespace remarks;
+
+static ExitOnError ExitOnErr;
+static cl::OptionCategory RemarkUtilCategory("llvm-remarkutil options");
+namespace subopts {
+static cl::SubCommand
+    YAML2Bitstream("yaml2bitstream",
+                   "Convert YAML remarks to bitstream remarks");
+static cl::SubCommand
+    Bitstream2YAML("bitstream2yaml",
+                   "Convert bitstream remarks to YAML remarks");
+} // namespace subopts
+
+// Conversions have the same command line options. AFAIK there is no way to
+// reuse them, so to avoid duplication, let's just stick this in a hideous
+// macro.
+#define CONVERSION_COMMAND_LINE_OPTIONS(SUBOPT)                                \
+  static cl::opt<std::string> InputFileName(                                   \
+      cl::Positional, cl::cat(RemarkUtilCategory), cl::init("-"),              \
+      cl::desc("<input file>"), cl::sub(SUBOPT));                              \
+  static cl::opt<std::string> OutputFileName(                                  \
+      "o", cl::init("-"), cl::cat(RemarkUtilCategory), cl::desc("Output"),     \
+      cl::value_desc("filename"), cl::sub(SUBOPT));
+namespace yaml2bitstream {
+/// Remark format to parse.
+static constexpr Format InputFormat = Format::YAML;
+/// Remark format to output.
+static constexpr Format OutputFormat = Format::Bitstream;
+CONVERSION_COMMAND_LINE_OPTIONS(subopts::YAML2Bitstream)
+} // namespace yaml2bitstream
+
+namespace bitstream2yaml {
+/// Remark format to parse.
+static constexpr Format InputFormat = Format::Bitstream;
+/// Remark format to output.
+static constexpr Format OutputFormat = Format::YAML;
+CONVERSION_COMMAND_LINE_OPTIONS(subopts::Bitstream2YAML)
+} // namespace bitstream2yaml
+
+/// \returns A MemoryBuffer for the input file on success, and an Error
+/// otherwise.
+static Expected<std::unique_ptr<MemoryBuffer>>
+getInputMemoryBuffer(StringRef InputFileName) {
+  auto MaybeBuf = MemoryBuffer::getFileOrSTDIN(InputFileName);
+  if (auto ErrorCode = MaybeBuf.getError())
+    return createStringError(ErrorCode,
+                             Twine("Cannot open file '" + InputFileName +
+                                   "': " + ErrorCode.message()));
+  return std::move(*MaybeBuf);
+}
+
+/// Parses all remarks in the input file.
+/// \p [out] StrTab - A string table populated for later remark serialization.
+/// \returns A vector of parsed remarks on success, and an Error otherwise.
+static Expected<std::vector<std::unique_ptr<Remark>>>
+tryParseRemarksFromInputFile(StringRef InputFileName, Format InputFormat,
+                             StringTable &StrTab) {
+  auto MaybeBuf = getInputMemoryBuffer(InputFileName);
+  if (!MaybeBuf)
+    return MaybeBuf.takeError();
+  auto MaybeParser = createRemarkParser(InputFormat, (*MaybeBuf)->getBuffer());
+  if (!MaybeParser)
+    return MaybeParser.takeError();
+  auto &Parser = **MaybeParser;
+  auto MaybeRemark = Parser.next();
+  // TODO: If we are converting from bitstream to YAML, we don't need to parse
+  // early because the string table is not necessary.
+  std::vector<std::unique_ptr<Remark>> ParsedRemarks;
+  for (; MaybeRemark; MaybeRemark = Parser.next()) {
+    StrTab.internalize(**MaybeRemark);
+    ParsedRemarks.push_back(std::move(*MaybeRemark));
+  }
+  auto E = MaybeRemark.takeError();
+  if (!E.isA<EndOfFileError>())
+    return std::move(E);
+  consumeError(std::move(E));
+  return ParsedRemarks;
+}
+
+/// \returns A ToolOutputFile which can be used for writing remarks on success,
+/// and an Error otherwise.
+static Expected<std::unique_ptr<ToolOutputFile>>
+getOutputFile(StringRef OutputFileName, Format OutputFormat) {
+  if (OutputFileName == "")
+    OutputFileName = "-";
+  auto Flags = OutputFormat == Format::YAML ? sys::fs::OF_TextWithCRLF
+                                            : sys::fs::OF_None;
+  std::error_code ErrorCode;
+  auto OF = std::make_unique<ToolOutputFile>(OutputFileName, ErrorCode, Flags);
+  if (ErrorCode)
+    return errorCodeToError(ErrorCode);
+  return std::move(OF);
+}
+
+/// Reserialize a list of remarks into the desired output format, and output
+/// to the user-specified output file.
+/// \p ParsedRemarks - A list of remarks.
+/// \p StrTab - The string table for the remarks.
+/// \returns Error::success() on success.
+static Error tryReserializeParsedRemarks(
+    StringRef OutputFileName, Format OutputFormat,
+    const std::vector<std::unique_ptr<Remark>> &ParsedRemarks,
+    StringTable &StrTab) {
+  auto MaybeOF = getOutputFile(OutputFileName, OutputFormat);
+  if (!MaybeOF)
+    return MaybeOF.takeError();
+  auto OF = std::move(*MaybeOF);
+  auto MaybeSerializer = createRemarkSerializer(
+      OutputFormat, SerializerMode::Standalone, OF->os(), std::move(StrTab));
+  if (!MaybeSerializer)
+    return MaybeSerializer.takeError();
+  auto Serializer = std::move(*MaybeSerializer);
+  for (const auto &Remark : ParsedRemarks)
+    Serializer->emit(*Remark);
+  OF->keep();
+  return Error::success();
+}
+
+/// Parses remarks in the input format, and reserializes them in the desired
+/// output format.
+/// \returns Error::success() on success, and an Error otherwise.
+static Error tryReserialize(StringRef InputFileName, StringRef OutputFileName,
+                            Format InputFormat, Format OutputFormat) {
+  StringTable StrTab;
+  auto MaybeParsedRemarks =
+      tryParseRemarksFromInputFile(InputFileName, InputFormat, StrTab);
+  if (!MaybeParsedRemarks)
+    return MaybeParsedRemarks.takeError();
+  return tryReserializeParsedRemarks(OutputFileName, OutputFormat,
+                                     *MaybeParsedRemarks, StrTab);
+}
+
+/// Reserialize bitstream remarks as YAML remarks.
+/// \returns An Error if reserialization fails, or Error::success() on success.
+static Error tryBitstream2YAML() {
+  // Use the namespace to get the correct command line globals.
+  using namespace bitstream2yaml;
+  return tryReserialize(InputFileName, OutputFileName, InputFormat,
+                        OutputFormat);
+}
+
+/// Reserialize YAML remarks as bitstream remarks.
+/// \returns An Error if reserialization fails, or Error::success() on success.
+static Error tryYAML2Bitstream() {
+  // Use the namespace to get the correct command line globals.
+  using namespace yaml2bitstream;
+  return tryReserialize(InputFileName, OutputFileName, InputFormat,
+                        OutputFormat);
+}
+
+/// Handle user-specified suboptions (e.g. yaml2bitstream, bitstream2yaml).
+/// \returns An Error if the specified suboption fails or if no suboption was
+/// specified. Otherwise, Error::success().
+static Error handleSuboptions() {
+  if (subopts::Bitstream2YAML)
+    return tryBitstream2YAML();
+  if (subopts::YAML2Bitstream)
+    return tryYAML2Bitstream();
+  return make_error<StringError>(
+      "Please specify a subcommand. (See -help for options)",
+      inconvertibleErrorCode());
+}
+
+int main(int argc, const char **argv) {
+  InitLLVM X(argc, argv);
+  cl::HideUnrelatedOptions(RemarkUtilCategory);
+  cl::ParseCommandLineOptions(argc, argv, "Remark file utilities\n");
+  ExitOnErr.setBanner(std::string(argv[0]) + ": error: ");
+  ExitOnErr(handleSuboptions());
+}