Page MenuHomePhabricator

Add a remarks-based code size diffing tool
Needs ReviewPublic

Authored by paquette on Jun 13 2019, 4:25 PM.

Details

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

Diff Detail

Event Timeline

paquette created this revision.Jun 13 2019, 4:25 PM
ormris added a subscriber: ormris.Jun 13 2019, 4:36 PM

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?

thegameg added inline comments.Jun 13 2019, 10:59 PM
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 added inline comments.Jun 13 2019, 11:22 PM
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 added inline comments.Jun 14 2019, 9:17 AM
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 marked 3 inline comments as done.Jun 14 2019, 10:43 AM

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.

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.