Add a tool for diffing instruction count + stack size remarks

[paquette]

Summary

This is a tool which can handle bitstream and YAML remarks. The idea here is to provide more insight into which functions changed in a benchmark when testing compiler changes.

E.g. "foo got 20% bigger, so maybe we should look more closely at that."

To use the tool, you can use...

$ llvm-remark-size-diff remarks_file_a remarks_file_b --parser=yaml|bitstream

... on two remarks files containing at least instruction count remarks. This will output some data on instruction count change and also other relevant information such as stack size change from remarks_file_a to remarks_file_b.

This is a bit of a WIP so I'm happy to change the format etc. Ultimately I think it'd be best to have some JSON output which could be consumed by another tool. But some base-level, greppable output is very handy to have anyway.

The format I'm proposing here is

<which files?> <increase/decrease in instruction count?> <function name> <inst count change> <stack B change>

Where the files and increase/decrease are indicated like below:

• <which files> is one of ++ (file B), -- (file A), == (both)
• <increase/decrease in instruction count?> is one of > (increase) or < (decrease)

This makes it easy to grep for things like "which functions appeared in A but did not appear in B?" Or "what are all the instruction count decreases?"

[thegameg]

This is great, thanks! In general everything looks pretty good to me. I'll take a second look while others get some time to comment too!

llvm/tools/llvm-size-diff/SizeDiff.cpp

299

you can probably use remarks::Format::YAML and remarks::Format::Bitstream directly here

[jhenderson]

I've not looked into this patch at all, but my Herald rule got triggered, as I'm subscribed to llvm-size changes. That made me think it would be good if we could pick a different name that isn't going to have a prefix clash with other tools? This will help with some shells auto-completion, if nothing else! I don't have any concrete suggestions, but perhaps llvm-remarks-diff or something like that?

llvm/test/tools/llvm-size-diff/no-instruction-count-remarks.test

4

Nit: no new line at EOF

[paquette]

In D112940#3102141, @jhenderson wrote:

I've not looked into this patch at all, but my Herald rule got triggered, as I'm subscribed to llvm-size changes. That made me think it would be good if we could pick a different name that isn't going to have a prefix clash with other tools? This will help with some shells auto-completion, if nothing else! I don't have any concrete suggestions, but perhaps llvm-remarks-diff or something like that?

Sure.

Not sure if remarks diff works either, since there's already opt-diff etc, which have a slightly different purpose than this tool. I'll think of something though.

[paquette]

Address review comment + change name to avoid terminal completion issues with llvm-size

(I wanted to keep the "size" part in the name because I want to focus this tool around instruction count)

[jhenderson]

In D112940#3132444, @paquette wrote:

Address review comment + change name to avoid terminal completion issues with llvm-size

(I wanted to keep the "size" part in the name because I want to focus this tool around instruction count)

Thanks. Name change seems reasonable to me.

I noticed the pre-merge checks are failing. It seems to not like the new executable name, or there's something else missing (do you need to add this tool to the list of subsitutions lit performs?).

[paquette]

Add the tool to the lit tool substitutions. (Not sure if this is the correct way to do this, let's see!)

[jhenderson]

In D112940#3135470, @paquette wrote:

Add the tool to the lit tool substitutions. (Not sure if this is the correct way to do this, let's see!)

I think what you've done is right, but apparently it's not the issue.

I just noticed that all the failures are on debian machines, but I think there's also Windows machines that run the pre-merge checks. This could be a red-herring though. Sorry, I'm out of further ideas.

[jhenderson]

In D112940#3136809, @jhenderson wrote:

In D112940#3135470, @paquette wrote:

Add the tool to the lit tool substitutions. (Not sure if this is the correct way to do this, let's see!)

I think what you've done is right, but apparently it's not the issue.

I just noticed that all the failures are on debian machines, but I think there's also Windows machines that run the pre-merge checks. This could be a red-herring though. Sorry, I'm out of further ideas.

Ignore what I just said there about Windows. For some reason, some parts aren't showing up, but the Windows ones are failing in the same way. On further inspection of the build log, the reason that the tool isn't being found is because for whatever reason, it isn't being built, based on my reading of the log at least. Again, I'm not sure why. I'd blame something in cmake, but I don't know why you wouldn't see the failure locally in this case.

[paquette]

Maybe the pre-merge check is incremental, and it needs a ninja clean? At least on my machine, the tool required that to build.

[paquette]

Adding the tool to llvm/test/CMakeLists.txt to see if that helps (as suggested by @fhahn offline)

[fhahn]

Great to see this coming along :)

Do you think there is a way to generalize this to diff arbitrary remarks or are you envisioning having separate tools for diffing separate remarks? (I didn't read the full discussion, apologies if this was already covered)

[paquette]

I was thinking about keeping it focused on instruction count as the most important metric, with a greppable output to start.

I think it could be generalized though. I think that a really important next step for this is to have a machine-consumable output mode (e.g. JSON) which could have custom views built around it.

Theoretically, this greppable output could be changed into a custom view which is built from the, say, JSON.

I guess that once we have a machine-consumable output, there's really no reason to say that instruction count would be the most important thing.

[mehdi_amini]

llvm/tools/llvm-remark-size-diff/RemarkSizeDiff.cpp

239 ↗

(On Diff #388009)

You need to use long long for this variable to match the signature of getAsSignedInteger apparently

[paquette]

Use long long explicitly in getIntValFromKey

[thegameg]

In D112940#3138560, @paquette wrote:

I think it could be generalized though. I think that a really important next step for this is to have a machine-consumable output mode (e.g. JSON) which could have custom views built around it.

Maybe having python bindings that work with bitstream on top of the C API can make this even easier? Going back to a textual format seems wasteful and won't scale when a lot more remarks are enabled.

[paquette]

Hmm yeah I think you're right.

[thegameg]

LGTM on my side. If this needs to grow into a more generic tool I think we can go with the python bindings and other things at that time.

[sylvestre.ledru]

if this isn't the case, it would be nice to add this to the release notes :)

llvm/test/tools/llvm-size-diff/Inputs/1-func-1-instr-1-stack.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-size-diff/Inputs/1-func-2-instr-2-stack.yaml

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

llvm/test/tools/llvm-size-diff/Inputs/2-identical-func-1-instr-1-stack.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'
+...
+--- !Analysis
+Pass:            prologepilog
+Name:            StackSize
+Function:        func1
+Args:
+  - NumStackBytes:   '1'
+  - String:          ' stack bytes in function'
+...
+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+Function:        func1
+Args:
+  - NumInstructions: '1'
+  - String:          ' instructions in function'
+...

llvm/test/tools/llvm-size-diff/Inputs/inconvertible-integer.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: 'a'
+  - String:          ' instructions in function'
+...

llvm/test/tools/llvm-size-diff/Inputs/no-instruction-count-remarks.yaml

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

llvm/test/tools/llvm-size-diff/Inputs/unexpected-key.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:
+  - Wrong: '1'
+  - String:          ' instructions in function'
+...

llvm/test/tools/llvm-size-diff/add-remove-func.test

+RUN: llvm-size-diff %p/Inputs/1-func-1-instr-1-stack.yaml %p/Inputs/2-identical-func-1-instr-1-stack.yaml --parser=yaml | FileCheck -strict-whitespace %s --check-prefix=ADD
+RUN: llvm-size-diff %p/Inputs/2-identical-func-1-instr-1-stack.yaml %p/Inputs/1-func-1-instr-1-stack.yaml  --parser=yaml | FileCheck -strict-whitespace %s --check-prefix=REMOVE
+
+; The "two-identical-one-instr-funcs" file contains a single-instruction
+; function which does not appear in the other file.
+
+; ADD: ++ > func1, 1 instrs, 1 stack B
+; ADD-DAG: instruction count: 1 (100.00%)
+; ADD-DAG: stack byte usage: 1 (100.00%)
+
+; REMOVE: -- < func1, -1 instrs, -1 stack B
+; REMOVE-DAG: instruction count: -1 (-50.00%)
+; REMOVE-DAG: stack byte usage: -1 (-50.00%)

llvm/test/tools/llvm-size-diff/empty-file.test

+RUN: not llvm-size-diff %p/Inputs/empty-file.yaml %p/Inputs/1-func-2-instr-2-stack.yaml --parser=yaml 2>&1 | FileCheck %s
+RUN: not llvm-size-diff %p/Inputs/1-func-2-instr-2-stack.yaml %p/Inputs/empty-file.yaml --parser=yaml 2>&1 | FileCheck %s
+
+; CHECK: error: document root is not of mapping type.

llvm/test/tools/llvm-size-diff/inconvertible-integer.test

+RUN: not llvm-size-diff %p/Inputs/inconvertible-integer.yaml %p/Inputs/1-func-1-instr-1-stack.yaml --parser=yaml 2>&1 | FileCheck %s
+
+; CHECK: Could not convert string to signed integer: a

llvm/test/tools/llvm-size-diff/increase-decrease-inst-count.test

+RUN: llvm-size-diff %p/Inputs/1-func-1-instr-1-stack.yaml %p/Inputs/1-func-2-instr-2-stack.yaml --parser=yaml | FileCheck -strict-whitespace %s --check-prefix=INCREASE
+RUN: llvm-size-diff %p/Inputs/1-func-2-instr-2-stack.yaml %p/Inputs/1-func-1-instr-1-stack.yaml --parser=yaml | FileCheck -strict-whitespace %s --check-prefix=DECREASE
+
+; Test a size increase/decrease of one instruction + 1 stack byte.
+
+; INCREASE: == > func0, 1 instrs, 1 stack B
+; INCREASE-DAG: instruction count: 1 (100.00%)
+; INCREASE-NEXT: stack byte usage: 1 (100.00%)
+
+; DECREASE: == < func0, -1 instrs, -1 stack B
+; DECREASE-DAG: instruction count: -1 (-50.00%)
+; DECREASE-NEXT: stack byte usage: -1 (-50.00%)

llvm/test/tools/llvm-size-diff/no-difference.test

+RUN: llvm-size-diff %p/Inputs/1-func-1-instr-1-stack.yaml %p/Inputs/1-func-1-instr-1-stack.yaml --parser=yaml | FileCheck -strict-whitespace %s
+
+; Same file passed twice -> no changes reported.
+
+; CHECK-NOT: {{[0-9]+}}
+; CHECK: instruction count: None
+; CHECK: stack byte usage: None

llvm/test/tools/llvm-size-diff/no-instruction-count-remarks.test

+RUN: not llvm-size-diff %p/Inputs/no-instruction-count-remarks.yaml %p/Inputs/no-instruction-count-remarks.yaml --parser=yaml 2>&1 | FileCheck %s
+
+; CHECK: error: File {{.*}} did not contain any instruction-count remarks!
 No newline at end of file

llvm/test/tools/llvm-size-diff/unexpected-key.test

+RUN: not llvm-size-diff %p/Inputs/unexpected-key.yaml %p/Inputs/1-func-1-instr-1-stack.yaml --parser=yaml 2>&1 | FileCheck %s
+
+; CHECK: Expected 'NumInstructions', got 'Wrong'

llvm/tools/llvm-size-diff/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS Core Demangle Object Remarks Support)
+
+add_llvm_tool(llvm-size-diff
+  SizeDiff.cpp
+)

llvm/tools/llvm-size-diff/SizeDiff.cpp

+//===------------------ llvm-size-diff/SizeDiff.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
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Diffs instruction count and stack size remarks between two remark files.
+///
+/// This is intended for use by compiler developers who want to see how their
+/// changes impact program code size.
+///
+/// TODO: Add structured output (JSON, or YAML, or something...)
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm-c/Remarks.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/Remarks/Remark.h"
+#include "llvm/Remarks/RemarkParser.h"
+#include "llvm/Remarks/RemarkSerializer.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
+
+enum ParserFormatOptions { yaml, bitstream };
+static cl::OptionCategory SizeDiffCategory("llvm-size-diff options");
+static cl::opt<std::string> InputFileNameA(cl::Positional, cl::Required,
+                                           cl::cat(SizeDiffCategory),
+                                           cl::desc("remarks_a"));
+static cl::opt<std::string> InputFileNameB(cl::Positional, cl::Required,
+                                           cl::cat(SizeDiffCategory),
+                                           cl::desc("remarks_b"));
+static cl::opt<std::string> OutputFilename("o", cl::init("-"),
+                                           cl::cat(SizeDiffCategory),
+                                           cl::desc("Output"),
+                                           cl::value_desc("file"));
+static cl::opt<ParserFormatOptions>
+    ParserFormat("parser", cl::cat(SizeDiffCategory), cl::init(bitstream),
+                 cl::desc("Set the remark parser format:"),
+                 cl::values(clEnumVal(yaml, "YAML format"),
+                            clEnumVal(bitstream, "Bitstream format")));
+
+/// Contains information from size remarks.
+// This is a little nicer to read than a std::pair.
+struct InstCountAndStackSize {
+  int64_t InstCount = 0;
+  int64_t StackSize = 0;
+};
+
+/// Represents which files a function appeared in.
+enum FilesPresent { A, B, BOTH };
+
+/// Contains the data from the remarks in file A and file B for some function.
+/// E.g. instruction count, stack size...
+struct FunctionDiff {
+  /// Function name from the remark.
+  std::string FuncName;
+  // Idx 0 = A, Idx 1 = B.
+  int64_t InstCount[2] = {0, 0};
+  int64_t StackSize[2] = {0, 0};
+
+  // Calculate diffs between the first and second files.
+  int64_t getInstDiff() const { return InstCount[1] - InstCount[0]; }
+  int64_t getStackDiff() const { return StackSize[1] - StackSize[0]; }
+
+  // Accessors for the remarks from the first file.
+  int64_t getInstCountA() const { return InstCount[0]; }
+  int64_t getStackSizeA() const { return StackSize[0]; }
+
+  // Accessors for the remarks from the second file.
+  int64_t getInstCountB() const { return InstCount[1]; }
+  int64_t getStackSizeB() const { return StackSize[1]; }
+
+  /// \returns which files this function was present in.
+  FilesPresent getFilesPresent() const {
+    if (getInstCountA() == 0)
+      return B;
+    if (getInstCountB() == 0)
+      return A;
+    return BOTH;
+  }
+
+  FunctionDiff(StringRef FuncName, const InstCountAndStackSize &A,
+               const InstCountAndStackSize &B)
+      : FuncName(FuncName) {
+    InstCount[0] = A.InstCount;
+    InstCount[1] = B.InstCount;
+    StackSize[0] = A.StackSize;
+    StackSize[1] = B.StackSize;
+  }
+};
+
+/// Organizes the diffs into 3 categories:
+/// - Functions which only appeared in the first file
+/// - Functions which only appeared in the second file
+/// - Functions which appeared in both files
+struct DiffsCategorizedByFilesPresent {
+  /// Diffs for functions which only appeared in the first file.
+  SmallVector<FunctionDiff> OnlyInA;
+
+  /// Diffs for functions which only appeared in the second file.
+  SmallVector<FunctionDiff> OnlyInB;
+
+  /// Diffs for functions which appeared in both files.
+  SmallVector<FunctionDiff> InBoth;
+
+  /// Add a diff to the appropriate list.
+  void addDiff(FunctionDiff &FD) {
+    switch (FD.getFilesPresent()) {
+    case A:
+      OnlyInA.push_back(FD);
+      break;
+    case B:
+      OnlyInB.push_back(FD);
+      break;
+    case BOTH:
+      InBoth.push_back(FD);
+      break;
+    }
+  }
+};
+
+static void printFunctionDiff(const FunctionDiff &FD, llvm::raw_ostream &OS) {
+  // Describe which files the function had remarks in.
+  auto FilesPresent = FD.getFilesPresent();
+  const auto &FuncName = FD.FuncName;
+  const int64_t InstDiff = FD.getInstDiff();
+  assert(InstDiff && "Shouldn't get functions with no size change?");
+  const int64_t StackDiff = FD.getStackDiff();
+  // Output an indicator denoting which files the function was present in.
+  switch (FilesPresent) {
+  case FilesPresent::A:
+    OS << "-- ";
+    break;
+  case FilesPresent::B:
+    OS << "++ ";
+    break;
+  case FilesPresent::BOTH:
+    OS << "== ";
+    break;
+  }
+  // Output an indicator denoting if a function changed in size.
+  if (InstDiff > 0)
+    OS << "> ";
+  else
+    OS << "< ";
+  OS << FuncName << ", ";
+  OS << InstDiff << " instrs, ";
+  OS << StackDiff << " stack B";
+  OS << "\n";
+}
+
+/// Print an item in the summary section.
+///
+/// \p TotalA - Total count of the metric in file A.
+/// \p TotalB - Total count of the metric in file B.
+/// \p Metric - Name of the metric we want to print (e.g. instruction
+/// count).
+/// \p OS - The output stream.
+static void printSummaryItem(int64_t TotalA, int64_t TotalB, StringRef Metric,
+                             llvm::raw_ostream &OS) {
+  OS << "  " << Metric << ": ";
+  int64_t TotalDiff = TotalB - TotalA;
+  if (TotalDiff == 0) {
+    OS << "None\n";
+    return;
+  }
+  OS << TotalDiff << " (" << formatv("{0:p}", TotalDiff / (double)TotalA)
+     << ")\n";
+}
+
+/// Print all contents of \p Diff and a high-level summary of the differences.
+static void printDiffsCategorizedByFilesPresent(
+    DiffsCategorizedByFilesPresent &DiffsByFilesPresent,
+    llvm::raw_ostream &OS) {
+  int64_t InstrsA = 0;
+  int64_t InstrsB = 0;
+  int64_t StackA = 0;
+  int64_t StackB = 0;
+  // Helper lambda to sort + print a list of diffs.
+  auto PrintDiffList = [&](SmallVector<FunctionDiff> &FunctionDiffList) {
+    if (FunctionDiffList.empty())
+      return;
+    stable_sort(FunctionDiffList,
+                [](const FunctionDiff &LHS, const FunctionDiff &RHS) {
+                  return LHS.getInstDiff() < RHS.getInstDiff();
+                });
+    for (const auto &FuncDiff : FunctionDiffList) {
+      // If there is a difference in instruction count, then print out info for
+      // the function.
+      if (FuncDiff.getInstDiff())
+        printFunctionDiff(FuncDiff, OS);
+      InstrsA += FuncDiff.getInstCountA();
+      InstrsB += FuncDiff.getInstCountB();
+      StackA += FuncDiff.getStackSizeA();
+      StackB += FuncDiff.getStackSizeB();
+    }
+  };
+  PrintDiffList(DiffsByFilesPresent.OnlyInA);
+  PrintDiffList(DiffsByFilesPresent.OnlyInB);
+  PrintDiffList(DiffsByFilesPresent.InBoth);
+  OS << "\n### Summary ###\n";
+  OS << "Total change: \n";
+  printSummaryItem(InstrsA, InstrsB, "instruction count", OS);
+  printSummaryItem(StackA, StackB, "stack byte usage", OS);
+}
+
+/// Collects an expected integer value from a given argument index in a remark.
+///
+/// \p Remark - The remark.
+/// \p ArgIdx - The index where the integer value should be found.
+/// \p ExpectedKeyName - The expected key name for the index
+/// (e.g. "InstructionCount")
+///
+/// \returns the integer value at the index if it exists, and the key-value pair
+/// is what is expected. Otherwise, returns an Error.
+static Expected<int64_t> getIntValFromKey(const remarks::Remark &Remark,
+                                          unsigned ArgIdx,
+                                          StringRef ExpectedKeyName) {
+  auto KeyName = Remark.Args[ArgIdx].Key;
+  if (KeyName != ExpectedKeyName)
+    return createStringError(
+        inconvertibleErrorCode(),
+        Twine("Unexpected key at argument index " + std::to_string(ArgIdx) +
+              ": Expected '" + ExpectedKeyName + "', got '" + KeyName + "'"));
+  int64_t Val;
+  auto ValStr = Remark.Args[ArgIdx].Val;
+  if (getAsSignedInteger(ValStr, 0, Val))
+    return createStringError(
+        inconvertibleErrorCode(),
+        Twine("Could not convert string to signed integer: " + ValStr));
+  return Val;
+}
+
+/// Collects relevant size information from \p Remark if it is an size-related
+/// remark of some kind (e.g. instruction count). Otherwise records nothing.
+///
+/// \p Remark - The remark.
+/// \p FuncNameToSizeInfo - Maps function names to relevant size info.
+/// \p NumInstCountRemarksParsed - Keeps track of the number of instruction
+/// count remarks parsed. We need at least 1 in both files to produce a diff.
+static Error processRemark(const remarks::Remark &Remark,
+                           StringMap<InstCountAndStackSize> &FuncNameToSizeInfo,
+                           unsigned &NumInstCountRemarksParsed) {
+  const auto &RemarkName = Remark.RemarkName;
+  const auto &PassName = Remark.PassName;
+  // Collect remarks which contain the number of instructions in a function.
+  if (PassName == "asm-printer" && RemarkName == "InstructionCount") {
+    // Expecting the 0-th argument to have the key "NumInstructions" and an
+    // integer value.
+    auto MaybeInstCount =
+        getIntValFromKey(Remark, /*ArgIdx = */ 0, "NumInstructions");
+    if (!MaybeInstCount)
+      return MaybeInstCount.takeError();
+    FuncNameToSizeInfo[Remark.FunctionName].InstCount = *MaybeInstCount;
+    ++NumInstCountRemarksParsed;
+  }
+  // Collect remarks which contain the stack size of a function.
+  else if (PassName == "prologepilog" && RemarkName == "StackSize") {
+    // Expecting the 0-th argument to have the key "NumStackBytes" and an
+    // integer value.
+    auto MaybeStackSize =
+        getIntValFromKey(Remark, /*ArgIdx = */ 0, "NumStackBytes");
+    if (!MaybeStackSize)
+      return MaybeStackSize.takeError();
+    FuncNameToSizeInfo[Remark.FunctionName].StackSize = *MaybeStackSize;
+  }
+  // Either we collected a remark, or it's something we don't care about. In
+  // both cases, this is a success.
+  return Error::success();
+}
+
+/// Process all of the size-related remarks in a file.
+///
+/// \param[in] InputFileName - Name of file to read from.
+/// \param[in, out] FuncNameToSizeInfo - Maps function names to relevant
+/// size info.
+static Error readFileAndProcessRemarks(
+    StringRef InputFileName,
+    StringMap<InstCountAndStackSize> &FuncNameToSizeInfo) {
+  auto Buf = MemoryBuffer::getFile(InputFileName);
+  if (auto EC = Buf.getError())
+    return createStringError(
+        EC, Twine("Cannot open file '" + InputFileName + "': " + EC.message()));
+  auto Format =
+      remarks::parseFormat(ParserFormat == bitstream ? "bitstream" : "yaml");
+  if (!Format)
+    return Format.takeError();
+  auto MaybeParser =
+      remarks::createRemarkParserFromMeta(*Format, (*Buf)->getBuffer());
+  if (!MaybeParser)
+    return MaybeParser.takeError();
+  auto &Parser = **MaybeParser;
+  auto MaybeRemark = Parser.next();
+  unsigned NumInstCountRemarksParsed = 0;
+  for (; MaybeRemark; MaybeRemark = Parser.next()) {
+    if (auto E = processRemark(**MaybeRemark, FuncNameToSizeInfo,
+                               NumInstCountRemarksParsed))
+      return E;
+  }
+  auto E = MaybeRemark.takeError();
+  if (!E.isA<remarks::EndOfFileError>())
+    return E;
+  consumeError(std::move(E));
+  // We need at least one instruction count remark in each file to produce a
+  // meaningful diff.
+  if (NumInstCountRemarksParsed == 0)
+    return createStringError(
+        inconvertibleErrorCode(),
+        "File '" + InputFileName +
+            "' did not contain any instruction-count remarks!");
+  return Error::success();
+}
+
+/// Wrapper function for readFileAndProcessRemarks which handles errors.
+///
+/// \param[in] InputFileName - Name of file to read from.
+/// \param[out] FuncNameToSizeInfo - Populated with information from size
+/// remarks in the input file.
+///
+/// \returns true if readFileAndProcessRemarks returned no errors. False
+/// otherwise.
+static bool tryReadFileAndProcessRemarks(
+    StringRef InputFileName,
+    StringMap<InstCountAndStackSize> &FuncNameToSizeInfo) {
+  if (Error E = readFileAndProcessRemarks(InputFileName, FuncNameToSizeInfo)) {
+    handleAllErrors(std::move(E), [&](const ErrorInfoBase &PE) {
+      PE.log(WithColor::error());
+      errs() << '\n';
+    });
+    return false;
+  }
+  return true;
+}
+
+/// Populates \p FuncDiffs with the difference between \p
+/// FuncNameToSizeInfoA and \p FuncNameToSizeInfoB.
+///
+/// \param[in] FuncNameToSizeInfoA - Size info collected from the first
+/// remarks file.
+/// \param[in] FuncNameToSizeInfoB - Size info collected from
+/// the second remarks file.
+/// \param[out] D - Filled with the diff between \p FuncNameToSizeInfoA and
+/// \p FuncNameToSizeInfoB.
+static void
+computeDiff(const StringMap<InstCountAndStackSize> &FuncNameToSizeInfoA,
+            const StringMap<InstCountAndStackSize> &FuncNameToSizeInfoB,
+            DiffsCategorizedByFilesPresent &DiffsByFilesPresent) {
+  SmallSet<std::string, 10> FuncNames;
+  for (const auto &FuncName : FuncNameToSizeInfoA.keys())
+    FuncNames.insert(FuncName.str());
+  for (const auto &FuncName : FuncNameToSizeInfoB.keys())
+    FuncNames.insert(FuncName.str());
+  for (const std::string &FuncName : FuncNames) {
+    const auto &SizeInfoA = FuncNameToSizeInfoA.lookup(FuncName);
+    const auto &SizeInfoB = FuncNameToSizeInfoB.lookup(FuncName);
+    FunctionDiff FuncDiff(FuncName, SizeInfoA, SizeInfoB);
+    DiffsByFilesPresent.addDiff(FuncDiff);
+  }
+}
+
+/// Attempt to get the output stream for writing the diff.
+static ErrorOr<std::unique_ptr<ToolOutputFile>> getOutputStream() {
+  if (OutputFilename == "")
+    OutputFilename = "-";
+  std::error_code EC;
+  auto Out = std::make_unique<ToolOutputFile>(OutputFilename, EC,
+                                              sys::fs::OF_TextWithCRLF);
+  if (!EC)
+    return std::move(Out);
+  return EC;
+}
+
+/// Output all diffs in \p DiffsByFilesPresent.
+/// \returns Error::success() on success, and an Error otherwise.
+static Error
+outputAllDiffs(DiffsCategorizedByFilesPresent &DiffsByFilesPresent) {
+  auto MaybeOF = getOutputStream();
+  if (std::error_code EC = MaybeOF.getError())
+    return errorCodeToError(EC);
+  std::unique_ptr<ToolOutputFile> OF = std::move(*MaybeOF);
+  printDiffsCategorizedByFilesPresent(DiffsByFilesPresent, OF->os());
+  OF->keep();
+  return Error::success();
+}
+
+/// Boolean wrapper for outputDiff which handles errors.
+static bool
+tryOutputAllDiffs(DiffsCategorizedByFilesPresent &DiffsByFilesPresent) {
+  if (Error E = outputAllDiffs(DiffsByFilesPresent)) {
+    handleAllErrors(std::move(E), [&](const ErrorInfoBase &PE) {
+      PE.log(WithColor::error());
+      errs() << '\n';
+    });
+    return false;
+  }
+  return true;
+}
+
+int main(int argc, const char **argv) {
+  InitLLVM X(argc, argv);
+  cl::HideUnrelatedOptions(SizeDiffCategory);
+  cl::ParseCommandLineOptions(argc, argv,
+                              "Diff instruction count and stack size remarks "
+                              "between two remark files.\n");
+  StringMap<InstCountAndStackSize> FuncNameToSizeInfoA;
+  StringMap<InstCountAndStackSize> FuncNameToSizeInfoB;
+  if (!tryReadFileAndProcessRemarks(InputFileNameA, FuncNameToSizeInfoA) ||
+      !tryReadFileAndProcessRemarks(InputFileNameB, FuncNameToSizeInfoB))
+    return 1;
+  DiffsCategorizedByFilesPresent DiffsByFilesPresent;
+  computeDiff(FuncNameToSizeInfoA, FuncNameToSizeInfoB, DiffsByFilesPresent);
+  if (!tryOutputAllDiffs(DiffsByFilesPresent))
+    return 1;
+}