[PATCH] D116145: [FuncSpec] Improve specializing direct constant

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

Just a general comment.
I am not surprised this doesn't work. The main goal of the work thus far has been to support a very specific use-case: promote indirect calls to direct calls, i.e. specialise on arguments that are function pointers. The main benefit we get out of this is inlining later on. The cost-model is tuned to this: it calculates the inlining cost/benefits.

Inlining "direct constants" is a very different use case from that. Nothing wrong with that, but again, it's just different. :-)
The main concern with this different use-case is:

- the cost model is tuned for it,
- and as as result, this is going to lead to more specialisations, resulting in increase compile-times which will be the main obstacle to get this pass enabled by default.

That's the reason we have this:

  // TODO: This needs checking to see the impact on compile-times, which is why
  // this is off by default for now.
  static cl::opt<bool> EnableSpecializationForLiteralConstant(

Adding support for literal constants is probably the easy part. I.e., somewhere some logic needs a bit of changing, but the real question is what the cost-model is going to be? So, if we want to support literal constants, I would rather start some work on thinking what this cost-model could be. On the other hand, I also didn't want to stop any work or experimentation in this direction, so that's why I pushed for this option and it to be off by default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116145/new/

https://reviews.llvm.org/D116145