[PATCH] Add similar function merging to MergeFunctions

Stepan Dyatkovskiy stpworld at narod.ru
Sat Apr 5 22:54:59 PDT 2014


  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
  >

http://reviews.llvm.org/D2591






More information about the llvm-commits mailing list