Add a remarks-based code size diffing tool

[paquette]

Summary

This is a little remarks-based size diffing tool I hacked out using the new remarks section stuff. The idea here is to provide insight into function size changes that show up due to compiler changes rather than source code changes.

What this tool does is diffs the instruction counts produced by the asm-printer's remarks. This allows you to quickly see which functions changed in size between two compiles of the same program.

This is just the basic functionality for now.

I can imagine adding, say...

• Detailed information using size-info remarks
• Information from other pass' remarks, e.g the inliner (with remarks, we can directly access this information!)
• Some nice statistical output (e.g. foo contains 25% of the instructions in the program)

Some detailed output for reference:

https://hastebin.com/hucexocuxi.md

Detailed output shows the differences in instruction counts between compiling stepanov_vector for AArch64 at -Os and -O3 with Clang. (-Os count is first, -O3 count is second, delta is third.)

[echristo]

Haven't done much of a review, one drive by, but in general I like the idea. Would it make sense to have a general "remarks in object files" tool rather than just a size one? (It can, of course, only do size at the beginning.)

Thoughts?

llvm/tools/sizediff/sizediff.cpp

238–241

This looks awkward? Late reformat?

[MaskRay]

+@pcc for https://lists.llvm.org/pipermail/llvm-dev/2019-April/131811.html

[thegameg]

llvm/test/tools/sizediff/basic-input.s

4

| FileCheck %s ?

llvm/tools/sizediff/sizediff.cpp

42

Maybe use llvm::Error and llvm::Expected instead?

[JDevlieghere]

llvm/tools/sizediff/sizediff.cpp

53

I would use two integers with descriptive names, unless you expect to extend this in the future? You could keep collectRemarks generic and pass an enum instead of the index as the second argument, and have a setter with a switch.

91

What happens when you pass a fat binary?

[thegameg]

llvm/tools/sizediff/CMakeLists.txt

2

Do we usually sort the components list?

12

Any reason why both sizeinfo and sizediff are not prefixed with llvm-?

llvm/tools/sizediff/sizediff.cpp

91

Even more, we should make sure this is a always an object file since the __LLVM,__remarks section is not linked in the final binary.

142

I would check for the return value of getAsInteger and continue; if it fails.

245

StringRef?

[paquette]

In D63306#1543020, @echristo wrote:

Haven't done much of a review, one drive by, but in general I like the idea. Would it make sense to have a general "remarks in object files" tool rather than just a size one? (It can, of course, only do size at the beginning.)

Thoughts?

Hmmm. That sounds similar to a more detailed version of opt-diff? (With added support for object files)

(I suppose that basically every pass can impact code size though, so I could see basically every remark being correlated with size.)

Do you have anything in particular in mind for what a more general tool would look like?

llvm/tools/sizediff/CMakeLists.txt

12

sizeinfo is a typo and shouldn't even be there, whoops.

sizediff is the best name I could think of, and I am entirely open to changing the name of this.

I think that llvm- is usually used to differentiate from existing UNIX/GNU tools (e.g. nm, size, etc.), but if people prefer the prefix, I don't mind at all.

llvm/tools/sizediff/sizediff.cpp

53

I think descriptive names is a good idea.

I have to think about making collectRemarks generic. I'll play with teaching it to recognize some other types of remarks and see what it looks like.

[echristo]

In D63306#1543925, @paquette wrote:

In D63306#1543020, @echristo wrote:

Haven't done much of a review, one drive by, but in general I like the idea. Would it make sense to have a general "remarks in object files" tool rather than just a size one? (It can, of course, only do size at the beginning.)

Thoughts?

Hmmm. That sounds similar to a more detailed version of opt-diff? (With added support for object files)

(I suppose that basically every pass can impact code size though, so I could see basically every remark being correlated with size.)

Do you have anything in particular in mind for what a more general tool would look like?

Nothing in particular, I figure growing an existing related tool makes as much sense as creating a new one? Up to you ultimately though.

llvm/test/tools/sizediff/Inputs/basic-a.opt.yaml

+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+DebugLoc:        { File: foo.c, Line: 4, Column: 0 }
+Function:        foo
+Args:
+  - NumInstructions: '17'
+  - String:          ' instructions in function'
+...
+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+DebugLoc:        { File: foo.c, Line: 10, Column: 0 }
+Function:        bar
+Args:
+  - NumInstructions: '26'
+  - String:          ' instructions in function'
+...
+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+DebugLoc:        { File: foo.c, Line: 10, Column: 0 }
+Function:        pluto
+Args:
+  - NumInstructions: '24'
+  - String:          ' instructions in function'
+...

llvm/test/tools/sizediff/Inputs/basic-b.opt.yaml

+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+DebugLoc:        { File: foo.c, Line: 4, Column: 0 }
+Function:        foo
+Args:
+  - NumInstructions: '16'
+  - String:          ' instructions in function'
+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+DebugLoc:        { File: foo.c, Line: 10, Column: 0 }
+Function:        bar
+Args:
+  - NumInstructions: '22'
+  - String:          ' instructions in function'
+...
+--- !Analysis
+Pass:            asm-printer
+Name:            InstructionCount
+DebugLoc:        { File: foo.c, Line: 10, Column: 0 }
+Function:        baz
+Args:
+  - NumInstructions: '42'
+  - String:          ' instructions in function'
+...

llvm/test/tools/sizediff/basic-input.s

+// REQUIRES: x86-registered-target
+// RUN: sed s!FILEPATH!%/p/Inputs/basic-a.opt.yaml! %s | llvm-mc -triple x86_64-apple-darwin --filetype=obj -o=%t1
+// RUN: sed s!FILEPATH!%/p/Inputs/basic-b.opt.yaml! %s | llvm-mc -triple x86_64-apple-darwin --filetype=obj -o=%t2
+// RUN: sizediff %t1 %t2
+
+// Check that we produce a valid size diff from basic-a.opt.yaml and basic-b.opt.yaml.
+
+// CHECK: pluto 24 0 -24
+// CHECK-NEXT: bar 26 22 -4
+// CHECK-NEXT: foo 17 16 -1
+// CHECK-NEXT: baz 0 42 42
+
+  .section  __TEXT,__text,regular,pure_instructions
+  .globl  _foo
+  .p2align  4, 0x90
+_foo:
+  .cfi_startproc
+  retq
+  .cfi_endproc
+  .section  __LLVM,__remarks,regular,debug
+  .ascii  "REMARKS"
+  .byte 0
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .ascii  "FILEPATH"
+  .byte 0
+
+.subsections_via_symbols

llvm/test/tools/sizediff/no-remarks.s

+// REQUIRES: x86-registered-target
+// RUN: llvm-mc -triple x86_64-apple-darwin --filetype=obj -o=%t1
+// RUN: llvm-mc -triple x86_64-linux-elf --filetype=obj -o=%t2
+// RUN: echo "heck" > %t3
+
+// RUN: not sizediff %t1 %t1 2>&1 | FileCheck %s --check-prefix=NO-REMARKS
+// RUN: not sizediff %t1 2>&1 | FileCheck %s --check-prefix=NEED-TWO
+// RUN: not sizediff %t2 %t2 2>&1 | FileCheck %s --check-prefix=ELF
+// RUN: not sizediff %t3 %t3 2>&1 | FileCheck %s --check-prefix=GARBAGE
+
+// Check basic failure conditions.
+// NO-REMARKS: Could not find remarks section
+// NEED-TWO: Not enough positional command line arguments specified!
+// ELF: Currently, only Mach-O files are supported
+// GARBAGE: The file was not recognized as a valid object file
+
+  .section  __TEXT,__text,regular,pure_instructions
+  .globl  _foo
+  .p2align  4, 0x90
+_foo:
+  .cfi_startproc
+  retq
+  .cfi_endproc
+
+.subsections_via_symbols

llvm/tools/sizediff/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Object
+  Support
+  Remarks
+  )
+
+add_llvm_tool(sizediff
+  sizediff.cpp
+  )
+
+if(LLVM_INSTALL_BINUTILS_SYMLINKS)
+  add_llvm_tool_symlink(sizeinfo sizediff)
+endif()

llvm/tools/sizediff/LLVMBuild.txt

+[component_0]
+type = Tool
+name = sizediff
+parent = Tools
+required_libraries = Object Support Remarks

llvm/tools/sizediff/sizediff.cpp

+//===- 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
+//
+//===----------------------------------------------------------------------===//
+//
+// A size diffing tool for object files based on LLVM optimization remarks.
+//
+// This tool outputs information about individual functions in two object files
+// produced from the same (or very similar) source files. The object files must
+// contain LLVM optimization remarks. This tool allows the user to see which
+// functions changed the most in size between two compilations of their program.
+//
+// This can be useful for people experimenting with different optimization
+// levels, for example. It can also be used by compiler developers interested in
+// diagnosing code size changes.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Object/Binary.h"
+#include "llvm/Object/MachO.h"
+#include "llvm/Object/ObjectFile.h"
+#include "llvm/Remarks/RemarkParser.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
+#include <assert.h>
+#include <string.h>
+
+using namespace llvm;
+using namespace object;
+using namespace remarks;
+
+namespace {
+bool HadError = false;
+std::string ToolName;
+cl::opt<std::string> OldFile(cl::Positional, cl::desc("<old object file>"),
+                             cl::Required);
+cl::opt<std::string> NewFile(cl::Positional, cl::desc("<new object file>"),
+                             cl::Required);
+
+// Helper struct for instruction counts.
+struct FunctionInstrCount {
+  // First element = count in the old file. Second element = count in the
+  // new file.
+  int Count[2] = {0, 0};
+
+  // Convenience function to get the difference between the old file and the
+  // new file.
+  int getDelta() const { return Count[1] - Count[0]; }
+};
+} // Anonymous namespace.
+
+static void error(Twine Message, Twine Path = Twine()) {
+  HadError = true;
+  WithColor::error(errs(), ToolName) << Path << ": " << Message << ".\n";
+}
+
+static bool error(std::error_code EC, Twine Path = Twine()) {
+  if (EC) {
+    error(EC.message(), Path);
+    return true;
+  }
+  return false;
+}
+
+static void error(Error E, StringRef FileName) {
+  HadError = true;
+  WithColor::error(errs(), ToolName) << FileName << ":";
+  std::string Buf;
+  raw_string_ostream OS(Buf);
+  logAllUnhandledErrors(std::move(E), OS);
+  OS.flush();
+  errs() << " " << Buf << "\n";
+}
+
+/// Find the remarks section in \p Obj and return it if it exists. Otherwise,
+/// return an error.
+static Expected<SectionRef> getRemarksSection(ObjectFile &Obj) {
+  // TODO: This should work with other object file formats.
+  // For now, let's be conservative and just support Mach-O.
+  if (!isa<MachOObjectFile>(&Obj))
+    return make_error<StringError>(
+        "Currently, only Mach-O files are supported",
+        std::make_error_code(std::errc::invalid_argument));
+
+  // Find the remarks section.
+  section_iterator RemarksIt =
+      find_if(Obj.sections(), [](const SectionRef &SecRef) {
+        StringRef Name;
+
+        // Skip over sections that don't have names.
+        if (auto EC = SecRef.getName(Name))
+          return false;
+        return Name == "__remarks";
+      });
+
+  // No remarks section, bail.
+  if (RemarksIt == Obj.section_end())
+    return make_error<StringError>(
+        "Could not find remarks section",
+        std::make_error_code(std::errc::invalid_argument));
+  return *RemarksIt;
+}
+
+static bool getRemarksFromYAML(StringMap<FunctionInstrCount> &InstrCounts,
+                               int CurrFile,
+                               std::unique_ptr<MemoryBuffer> &YAMLBuf,
+                               StringRef RemarkFile) {
+  Parser RemarkParser(YAMLBuf->getBuffer());
+  Expected<const Remark *> RemarkOrErr = RemarkParser.getNext();
+
+  // Walk over remarks until we either run out of them, or hit an error.
+  for (; RemarkOrErr; RemarkOrErr = RemarkParser.getNext()) {
+    const Remark *Rmk = RemarkOrErr.get();
+
+    // We're at the end of the remark stream, so we're done.
+    if (!Rmk)
+      return true;
+
+    // Is this an asm-printer remark?
+    if (Rmk->PassName != "asm-printer")
+      continue;
+
+    // Yep. Grab the number of instructions in the function.
+    auto NumInstrs = find_if(Rmk->Args, [](const Argument &A) {
+      return A.Key == "NumInstructions";
+    });
+    assert("Didn't find number of instructions?" &&
+           NumInstrs != Rmk->Args.end());
+
+    // Convert the string from the YAML file to an integer so we can save it in
+    // the map, do comparisons on it later, etc.
+    int InstrCount;
+    NumInstrs->Val.getAsInteger(0, InstrCount);
+
+    // Did we already add this function to the map?
+    if (InstrCounts.count(Rmk->FunctionName)) {
+      // Yes. Just update the existing value.
+      InstrCounts[Rmk->FunctionName].Count[CurrFile] = InstrCount;
+    } else {
+      // No. Add a new value.
+      FunctionInstrCount FC;
+      FC.Count[CurrFile] = InstrCount;
+      InstrCounts[Rmk->FunctionName] = FC;
+    }
+  }
+
+  error(RemarkOrErr.takeError(), RemarkFile);
+  return false;
+}
+
+static bool collectRemarks(std::string &Filename, int CurrFile,
+                           StringMap<FunctionInstrCount> &InstrCounts) {
+
+  // Get the object file.
+  ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
+      MemoryBuffer::getFile(Filename);
+  if (error(BufferOrErr.getError(), Filename))
+    return false;
+
+  Expected<std::unique_ptr<ObjectFile>> ObjOrErr(
+      ObjectFile::createObjectFile(BufferOrErr.get()->getMemBufferRef()));
+  if (!ObjOrErr) {
+    error(ObjOrErr.takeError(), Filename);
+    return false;
+  }
+
+  // Get the remarks section in the object file if it exists.
+  ObjectFile &Obj = *ObjOrErr.get();
+  Expected<SectionRef> RemarksSectionOrErr = getRemarksSection(Obj);
+  if (!RemarksSectionOrErr) {
+    error(RemarksSectionOrErr.takeError(), Filename);
+    return false;
+  }
+
+  // We have a remarks section. Get whatever is in it so we can process it.
+  Expected<StringRef> SectionContentsOrErr =
+      RemarksSectionOrErr.get().getContents();
+  if (!SectionContentsOrErr) {
+    error(SectionContentsOrErr.takeError(), Filename);
+    return false;
+  }
+
+  // The remarks section points to a YAML file. Move past the special remarks
+  // header to get the path to the file.
+  StringRef RemarkFile = SectionContentsOrErr.get().trim("REMARKS").drop_while(
+      [](const char &C) { return C == '\0'; });
+  ErrorOr<std::unique_ptr<MemoryBuffer>> YAMLBufferOrErr =
+      MemoryBuffer::getFile(RemarkFile.str());
+  if (error(BufferOrErr.getError(), RemarkFile.str()))
+    return false;
+
+  // Collect results from the YAML file.
+  return getRemarksFromYAML(InstrCounts, CurrFile, YAMLBufferOrErr.get(),
+                            RemarkFile);
+}
+
+/// Output the instruction count differences between the two files.
+void outputResults(StringMap<FunctionInstrCount> &InstrCounts) {
+  typedef std::pair<std::string, FunctionInstrCount> NameAndSizeInfo;
+
+  // Copy the counts over to a vector so that we can sort them.
+  std::vector<NameAndSizeInfo> CountVec;
+  for (const StringMapEntry<FunctionInstrCount> &FnAndSize : InstrCounts)
+    CountVec.push_back({FnAndSize.first(), FnAndSize.getValue()});
+
+  // Sort by delta.
+  stable_sort(CountVec,
+              [](const NameAndSizeInfo &LHS, const NameAndSizeInfo &RHS) {
+                return LHS.second.getDelta() < RHS.second.getDelta();
+              });
+
+  // Output results.
+  for (const NameAndSizeInfo &FnAndSize : CountVec) {
+    FunctionInstrCount FIC = FnAndSize.second;
+    int Delta = FIC.getDelta();
+    // Only print things that changed.
+    if (Delta == 0)
+      continue;
+    outs() << FnAndSize.first << " " << FIC.Count[0] << " " << FIC.Count[1]
+           << " " << Delta << "\n";
+  }
+}
+
+int main(int argc, char **argv) {
+  InitLLVM X(argc, argv);
+  ToolName = argv[0];
+  cl::ParseCommandLineOptions(
+      argc, argv,
+      "Remark-based size diffing tool.\nThis tool takes in two object files "
+      "produced "
+      "with optimization remarks.\nBoth files must contain a __remarks "
+      "section.\n");
+  StringMap<FunctionInstrCount> InstrCounts;
+
+  // Read in each file. Bail out if an error is encountered at any point.
+  std::string Files[2] = {OldFile, NewFile};
+  for (unsigned Idx = 0; Idx < 2; ++Idx) {
+    collectRemarks(Files[Idx], Idx, InstrCounts);
+    if (HadError)
+      return 1;
+  }
+
+  outputResults(InstrCounts);
+}