In D50900#1204771, @fhahn wrote:

We were also thinking about expanding pow(x, n+0.5) -> x * x * x * ... * sqrt(x). Is this something you have also planned on adding?

Can you please rebase it, for D50036 touched the same function?

Rebase after the preliminary patches rL340060 and rL340061.

Thank you.

I'll commit the data type changes increasing type diversity on the side.

The dependent patches can be found in the Stack below: D49273 and D50036.

Thank you.

Added test for shrinking pow() when simplifying libcalls is disabled.

Remove additional simplification when exp10() is the base in pow(), since it's not enabled by many targets.

OK, then.

Closed by commit rL339354

Ping! 🔔

Ping! 🔔

Ping! 🔔

Keep the double precision pow() when the result is too.

Refactored the helper functions for shrinking unary and binary functions into a single one, while keeping all their functionality, in rL338905.

Alternatively, optimizeBinaryDoubleFP() could be fixed to shrink just float f = pow(), while keeping its other users, f{min,max}() and copysign() happy, which should be easy to satisfy, given how simple these functions are.

In D50113#1186263, @efriedma wrote:

I'd like to get this right... both the CheckRetType check, and the "infinite loop" check from optimizeUnaryDoubleFP are probably relevant. Can we share code between the two functions?

Maybe the CheckRetType check isn't critical if we're only doing this transform by default when afn is enabled, but it's still losing a lot of bits, particularly if the input is supposed to be an exact number. pow(2.f, 0.5f) is a lot different from (double)powf(2.f, 0.5f).

(If you want to fix the potential miscompile for 7.0, I'd accept a patch to just completely disable shrinking for pow.)

Added test cases for vector types in pow(x, n).

In D50113#1184429, @spatel wrote:

I think we should commit this based on fixing the miscompile alone; we can debate the other diff in a follow-on patch if needed.

A specialized transcendental function is inherently faster than a generic one. In the case of cbrt() this is measurable in popular benchmarks, like CPU2000 and CPU2006. I can see the appeal of converging particular cases back to one generic case in the middle end, but IMO it's the opposite way in the run time environment.

Agreed.

In D49306#1182662, @spatel wrote:

Why is this the right IR canonicalization?

Committed rL338152 to add the base line test case pow-cbrt.ll.

OK. Please, stay tuned.

¡Ping! 🔔🔔

¡Ping! 🔔🔔

¡Ping! 🔔🔔

Ping! 🔔

Thank you for this fix.

Ping! 🔔