[PATCH] D51435: [SLC] Support expanding pow(x, n+0.5) to x * x * ... * sqrt(x)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D51435#1222148, @fhahn wrote:

> In https://reviews.llvm.org/D51435#1219433, @spatel wrote:
>
> > Comment + extra tests look good.
> >
> > Given the revert of https://reviews.llvm.org/D49273, make the getSqrtCall() diff an NFC commit ahead of this patch + generalize that for any libcall, so we make sure that we're not generating libcalls when we're not allowed to?
>
>
> I had a look at other uses that could benefit from a general getSqrtCall, but I am not entirely sure what the scope of it should be. Also, for sqrt, we use the intrinsic when possible, and a lib call if it is available otherwise. But e.g. for the pow(2.0 ** n, x) -> exp2(n * x) transform we check if an exp2 lib func is available. If it is not, we also do not emit an intrinsic, even if it would be possible. Should the generic function behave similar to the exp2 behavior?


That's an interesting question. IIUC, we don't generate an exp2 intrinsic if the libcall is not available because we expect that most targets would end up lowering to that libcall anyway (they don't have hardware support for exp2). But that's probably not true for sqrt - either a compliant or estimate sqrt instruction probably is available even if the libcall is not. That question/problem is noted in the 'TODO' comment for the sqrt libcall test.

This is probably worth asking about on llvm-dev if not via another patch. Either way, I don't think we should hold this patch up while deciding how to fix that.


https://reviews.llvm.org/D51435