[PATCH] D106426: [FuncSpec] Support specialising recursive functions

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

In D106426#2906213 <https://reviews.llvm.org/D106426#2906213>, @ChuanqiXu wrote:

> 4 times is an arbitrary number. I thought the best would be 1 time only. But I worried if it is too strict. So I said 4 times. Now I think we should be more strict. So I think it would better to specialize it only 1 time.

I can't follow any this. There are only arbitrary numbers here. I can't follow the decision making behind this, because no rationale is given.
Taking your example, specialising 4 times, and after cleanup passes like inline, instcombine, and simplify cfg, this would exactly give the code I would expect.
And that's the problem of this discussion, it is based on a idea, and not based on an any data that we can verify. For *this* example, I don't see which high level criteria is the problem:

- compile-times,
- code-size,
- performance.

>> how things work for recursive function, I think we need to discuss how the cost-model changes would look like
>
> I think we need to handle recursive function specially. Since it is common that recursive function f  and its specialized one f' have the same cost and bonus. We need to record the function specialized (and specialized argument). Then we need to specialize a function (for specific argument) we had specialized, we could forbid it or give it a much higher penalty. The detail is missing. But the key point is that we need to handle recursive function specially instead of tuning or refactoring the cost model.

To help this discussion and make this concrete, can you take your own example, and point exactly on what code-generation criteria I mentioned before you would like to specialise this only once?

The problem I have with this discussion is that it is based on an idea that I cannot verify with data. While I understand the general idea, perhaps, and implementing a penalty is easy but I simply don't know what that should be. I can put a random/arbitrary number in, but why would we do that? That's why I still prefer the approach of getting this foundation in place, because it is opt-in, then we can tune this further.


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

https://reviews.llvm.org/D106426