[InstCombine] For pow(x, +/-0.5), stop falling into pow(x, 1.5), etc. case

[hubert.reinterpretcast]

Summary

The current code for handling pow(x, y) where y is an integer plus 0.5 is not explicitly guarded against attempting to transform the case where abs(y) is exactly 0.5.

The latter case is meant to be handled by replacePowWithSqrt. Indeed, if the pow(x, integer+0.5) case proceeds past a certain point, it will hit an assertion by attempting to form pow(x, 0) using getPow.

This patch adds an explicit check to prevent attempting the pow(x, integer+0.5) transformation on pow(x, +/-0.5) as suggested during the review of D87877. This has the effect of retaining the shrinking of pow to powf when the sqrt libcall cannot be formed.

[spatel]

LGTM

llvm/test/Transforms/InstCombine/pow-4.ll

240–241

That looks like another bug. We formed an extra sqrtf() call from a not-quite-dead pow() call?

[hubert.reinterpretcast]

llvm/test/Transforms/InstCombine/pow-4.ll

240–241

I'm not sure I'll be able to chase this one. Do you have a recommendation on how to mark/track it?

[spatel]

llvm/test/Transforms/InstCombine/pow-4.ll

240–241

A 'FIXME' comment in the test file should be ok. Bonus points for filing a bugzilla. :)

llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Value *LibCallSimplifier::optimizePow(CallInst *Pow, IRBuilderBase &B) {
   if (match(Expo, m_SpecificFP(2.0)))
     return B.CreateFMul(Base, Base, "square");
 
   if (Value *Sqrt = replacePowWithSqrt(Pow, B))
     return Sqrt;
 
   // pow(x, n) -> x * x * x * ...
   const APFloat *ExpoF;
-  if (AllowApprox && match(Expo, m_APFloat(ExpoF))) {
+  if (AllowApprox && match(Expo, m_APFloat(ExpoF)) &&
+      !ExpoF->isExactlyValue(0.5) && !ExpoF->isExactlyValue(-0.5)) {
     // We limit to a max of 7 multiplications, thus the maximum exponent is 32.
     // If the exponent is an integer+0.5 we generate a call to sqrt and an
     // additional fmul.
     // TODO: This whole transformation should be backend specific (e.g. some
     //       backends might prefer libcalls or the limit for the exponent might
     //       be different) and it should also consider optimizing for size.
     APFloat LimF(ExpoF->getSemantics(), 33),
             ExpoA(abs(*ExpoF));

llvm/test/Transforms/InstCombine/pow-4.ll

 ; CHECK-NEXT:    [[TMP1:%.*]] = fmul fast <4 x float> [[SQUARE]], [[X]]
 ; CHECK-NEXT:    [[TMP2:%.*]] = fmul fast <4 x float> [[TMP1]], [[SQRT]]
 ; CHECK-NEXT:    ret <4 x float> [[TMP2]]
 ;
   %1 = call fast <4 x float> @llvm.pow.v4f32(<4 x float> %x, <4 x float> <float 3.500000e+00, float 3.500000e+00, float 3.500000e+00, float 3.500000e+00>)
   ret <4 x float> %1
 }
 
+; (float)pow((double)(float)x, 0.5)
+; FIXME: One call to `sqrtf` would suffice (PR47613).
+define float @shrink_pow_libcall_half(float %x) {
+; SQRT-LABEL: @shrink_pow_libcall_half(
+; SQRT-NEXT:    [[SQRTF:%.*]] = call fast float @sqrtf(float [[X:%.*]])
+; SQRT-NEXT:    [[SQRTF1:%.*]] = call fast float @sqrtf(float [[X]])
+; SQRT-NEXT:    ret float [[SQRTF1]]
+;
+; NOSQRT-LABEL: @shrink_pow_libcall_half(
+; NOSQRT-NEXT:    [[SQRTF:%.*]] = call fast float @sqrtf(float [[X:%.*]])
+; NOSQRT-NEXT:    ret float [[SQRTF]]
+;
+  %dx = fpext float %x to double
+  %call = call fast double @pow(double %dx, double 0.5)
+  %fr = fptrunc double %call to float
+  ret float %fr
+}
+
 ; Make sure that -0.0 exponent is always simplified.
 
 define double @PR43233(double %x) {
 ; CHECK-LABEL: @PR43233(
 ; CHECK-NEXT:    ret double 1.000000e+00
 ;
   %r = call fast double @llvm.pow.f64(double %x, double -0.0)
   ret double %r
 }