[PATCH] D93838: [SCCP] Add Function Specialization pass

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

In D93838#2770470 <https://reviews.llvm.org/D93838#2770470>, @xbolva00 wrote:

>>> getting this all right and enabled by default with the first commit is probably not achievable
>
> Maybe yes, well…
>
> If we could pick some the most obvious and profitable cases and enable this pass for them (simple heuristics, otherwise be conservative), plus, if we have PGO data, specialize very hot functions?
>
> Yeah, I see concerns that another off by default pass is not ideal. Just look how many passes are in tree and disabled.
>
> So +1 if we can run this pass even with conservative heuristics.

PGO is definitely interesting. Actually quite a few interesting ideas have been brought up during discussions:

- An idea was expressed that a function specialisation attribute on arguments, we could have a direct way of specialising, avoiding the cost-model,
- Currently we only support constants, not constant ranges,
- and I see that PGO could definitely help.

But I think these things can be best added once we have an initial implementation.

Thanks @nikic for your thoughts:

> 'm pretty sure @fhahn did not mean to suggest that the pass is actually default enabled when it lands, merely that there is some viable path towards enabling it in the future. Adding a pass and enabling it in the default pipeline are always two separate steps. And it does sound to me like there is a viable path forward here, so this is mainly waiting on someone to perform a technical review of the implementation (that someone would preferably be @fhahn, as the local IPSCCP expert :)

I agree with this and I think I have shown that there is a viable path to get this enabled (again, this our goal, because we want to reach parity with GCC here).
We have performed quite a few round of code reviews. Although there's obviously more to do, my impression was that people were reluctant to approve this as a first version  because of the "on by default" discussion and not so much because of other things.


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

https://reviews.llvm.org/D93838