This is an archive of the discontinued LLVM Phabricator instance.

[MergeFunctions] Merge small functions if possible without a thunk
ClosedPublic

Authored by whitequark on Jun 28 2017, 9:40 PM.

Details

Summary

This can result in significant code size savings in some cases,
e.g. an interrupt table all filled with the same assembly stub
in a certain Cortex-M BSP results in code blowup by a factor of 2.5.

Tests depend on D34805.

Diff Detail

Repository
rL LLVM

Event Timeline

whitequark created this revision.Jun 28 2017, 9:40 PM
whitequark edited the summary of this revision. (Show Details)

Cosmetic change in debug output.

jfb accepted this revision.Jun 28 2017, 9:49 PM

A few questions, but this looks good.

lib/Transforms/IPO/MergeFunctions.cpp
653

So I know that you're just moving code, but why these numbers? What's the usual thunk size? You also need to consider alignment (both of the function and thunks). IIRC there was a bunch of waste with alignment even when merge funcs ran, and I don't think it got fixed.

785

Weird that this code was way late here.

This fixme isn't relevant right? It's handled at line 631 it seems like.

This revision is now accepted and ready to land.Jun 28 2017, 9:49 PM
whitequark added inline comments.Jun 28 2017, 10:24 PM
lib/Transforms/IPO/MergeFunctions.cpp
653

This checks for one basic block with one instruction in it apart from terminator. I think the idea is that you don't want to write a thunk for a thunk, and also a call instruction is typically larger than simple arithmetics, so you also don't want to write a thunk for that.

Regarding the alignment, I'm not sure I know all implications of changing that.

785

Yeah, I fixed it in D34805, on which this patch is based on.

This revision was automatically updated to reflect the committed changes.