[PATCH] D34806: [MergeFunctions] Merge small functions if possible without a thunk

[whitequark via Phabricator via llvm-commits]

whitequark added inline comments.


================
Comment at: lib/Transforms/IPO/MergeFunctions.cpp:653
+  if (F->size() == 1) {
+    if (F->front().size() <= 2) {
+      DEBUG(dbgs() << "writeThunk: " << F->getName() 
----------------
jfb wrote:
> 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.
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.


================
Comment at: lib/Transforms/IPO/MergeFunctions.cpp:785
-  // FIXME: Should still merge them if they are unnamed_addr and produce an
-  // alias.
-  if (NewFunction->size() == 1) {
----------------
jfb wrote:
> Weird that this code was way late here.
> 
> This fixme isn't relevant right? It's handled at line 631 it seems like.
Yeah, I fixed it in D34805, on which this patch is based on.


Repository:
  rL LLVM

https://reviews.llvm.org/D34806