[PATCH] Add similar function merging to MergeFunctions

[Stepan Dyatkovskiy]

Hi Tobias,

You idea is really great! Thanks for working on it.
Though, you patch still contains some things to be fixed:

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140127/203297.html

Perhaps it would be good to implement similar merging itself as separate 
pass, that is based on MergeFunctions results. All you need from 
MergeFunctions is kind of FunctionToFunction map? Right? Now it mostly 
implemented as SimilarFnMap in your patch. I think, I could prepare that 
kind of data for you in my patches.

In mean time I'd propose you to focus on merging process, since I got a 
lot of failures on my machine with your patch applied. Merging process 
is most delicate and dainty here. Averything depends on quality of this 
part. Note, there is a buildbot, that tests MergeFunctions pass:
http://lab.llvm.org:8011/builders/clang-mergefunc-x86_64-freeBSD9.2

We could boost things up and move your patch into trunk faster, if you 
prepare clean and well made patch for second part: merging itself. On 
the top, I think, it should look somewhat like:

void mergeSimilarFunctions(Function *FL, Function *FR, Instruction 
*MergeStart, Instruction *MergeEnd);

-Stepan

Tobias von Koch wrote:
>
>    Oh right - didn't realise commits wasn't on there! Also CCing Nick.
>
>    There has been some work on MergeFunctions recently by Stepan so it probably needs to be rebased.
>
>    Stepan - are you still working on this? If I recall correctly, you were considering implementing similar function merging on top of your ordering-based approach. I currently don't have the resources to completely rewrite the patch. If we want to support similar function merging right now, I see two options:
>
>    a) try and add this as-is to MergeFunctions but this would likely mean we can't do your ordering-based merging,
>    b) split MergeFunctions into two passes, one for merging identical functions (it could use your faster algorithm) and another just for similar function merging (based on parts of this patch).
>
>    It would be nice to get this into trunk, because the results are good and it is in active use by customers. We also have an upcoming paper at LCTES with a more thorough evaluation.
>
> http://reviews.llvm.org/D2591
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>