[SimplifyLibCalls] pow(x, 0.5) -> fabs(sqrt(x)) (finite-only-mode)

[davide]

Summary

This is a new version of http://reviews.llvm.org/D16833 as mgrang has no more time to work on the feature.
This patch substantially differ from the previous version. The original patch applied the transformation if:
if (Op1C->getValueAPF().isFinite())

I don't think this is the correct way of checking we're in finite-mode only. Instead, checked that the function has the attributes no-nans-fp-math and no-infs-fp-math both set to true. I hope I wasn't entirely wrong in my analysis. In any case, comments welcome.

Also, this adds a test (which wasn't included in the original revision).

cc:ing Steve Canon so that he can confirm/deny if this makes sense from a mathematical point of view.

[majnemer]

lib/Transforms/Utils/SimplifyLibCalls.cpp

1071 ↗

(On Diff #63107)

Hmm. Why not check to see if the call-site is annotated with ninf nnan?

[davide]

Like this or you meant something different?

[davide]

ping

[davide]

Also, maybe we can do this separately, but after discussing with Sanjay I concluded that maybe it's a good idea to emit intrinsics instead of libcalls here as optimizeSqrt() does? David, what do you think?

[davide]

This doesn't preserve FMF across calls, need to modify to take that in account (thanks to Sanjay for the suggestion offline)

[spatel]

Sorry - didn't mean to back-channel a review. I just noticed that the test case starts with an LLVM intrinsic and ends with libcalls. That means we've artificially (if it's ok for scalars, then it's ok for vectors) excluded the vector variant of an 'llvm.pow' from this transform. There was also a proposal to change all libcalls to intrinsics when possible, so I think we're just creating extra work for that pass if we create libcalls here.

Possible changes to make this transform more general:

1. Switch the dyn_cast check for ConstantFP to "match(X, m_SpecificFP(0.5))" (this should work with a vector splat constant).
2. Use llvm.fabs/sqrt intrinsics so the transform works with vectors.
3. Check the 'nsz' FMF too; ie, for the case that James and Eli noted, 'nsz' should let us eliminate the fabs.

[davide]

It's not obvious this transformation buys something, abandoning for now.

llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp

 Value *LibCallSimplifier::optimizePow(CallInst *CI, IRBuilder<> &B) {
   Function *Callee = CI->getCalledFunction();
   Value *Ret = nullptr;
   StringRef Name = Callee->getName();
   if (UnsafeFPShrink && Name == "pow" && hasFloatVersion(Name))
     Ret = optimizeUnaryDoubleFP(CI, B, true);
 
   Value *Op1 = CI->getArgOperand(0), *Op2 = CI->getArgOperand(1);
-  if (ConstantFP *Op1C = dyn_cast<ConstantFP>(Op1)) {
-    // pow(1.0, x) -> 1.0
-    if (Op1C->isExactlyValue(1.0))
-      return Op1C;
-    // pow(2.0, x) -> exp2(x)
-    if (Op1C->isExactlyValue(2.0) &&
-        hasUnaryFloatFn(TLI, Op1->getType(), LibFunc::exp2, LibFunc::exp2f,
-                        LibFunc::exp2l))
-      return emitUnaryFloatFnCall(Op2, TLI->getName(LibFunc::exp2), B,
-                                  Callee->getAttributes());
+
+  if (match(Op1, m_SpecificFP(1.0)))
+    return Op1;
+  // pow(2.0, x) -> llvm.exp2(x)
+  if (match(Op1, m_SpecificFP(2.0))) {
+    Value *Exp2 = Intrinsic::getDeclaration(CI->getModule(), Intrinsic::exp2,
+                                            CI->getType());
+    return B.CreateCall(Exp2, Op2, "exp2");
+  }
+
+  // There's no llvm.exp10 intrinsic yet, but, maybe, some day there will
+  // be one.
+  if (ConstantFP *Op1C = dyn_cast<ConstantFP>(Op1)) {
     // pow(10.0, x) -> exp10(x)
     if (Op1C->isExactlyValue(10.0) &&
         hasUnaryFloatFn(TLI, Op1->getType(), LibFunc::exp10, LibFunc::exp10f,
                         LibFunc::exp10l))
       return emitUnaryFloatFnCall(Op2, TLI->getName(LibFunc::exp10), B,
                                   Callee->getAttributes());
   }
 

llvm/trunk/test/Transforms/InstCombine/pow-1.ll

 ; CHECK-NEXT: ret double 1.000000e+00
 }
 
 ; Check pow(2.0, x) -> exp2(x).
 
 define float @test_simplify3(float %x) {
 ; CHECK-LABEL: @test_simplify3(
   %retval = call float @powf(float 2.0, float %x)
-; CHECK-NEXT: [[EXP2F:%[a-z0-9]+]] = call float @exp2f(float %x) [[NUW_RO:#[0-9]+]]
+; CHECK-NEXT: [[EXP2F:%[a-z0-9]+]] = call float @llvm.exp2.f32(float %x)
   ret float %retval
 ; CHECK-NEXT: ret float [[EXP2F]]
 }
 
 define double @test_simplify4(double %x) {
 ; CHECK-LABEL: @test_simplify4(
   %retval = call double @pow(double 2.0, double %x)
-; CHECK-NEXT: [[EXP2:%[a-z0-9]+]] = call double @exp2(double %x) [[NUW_RO]]
+; CHECK-NEXT: [[EXP2:%[a-z0-9]+]] = call double @llvm.exp2.f64(double %x)
   ret double %retval
 ; CHECK-NEXT: ret double [[EXP2]]
 }
 
 ; Check pow(x, 0.0) -> 1.0.
 
 define float @test_simplify5(float %x) {
 ; CHECK-LABEL: @test_simplify5(

 ; CHECK-NEXT: ret double 1.000000e+00
 }
 
 ; Check pow(x, 0.5) -> fabs(sqrt(x)), where x != -infinity.
 
 define float @test_simplify7(float %x) {
 ; CHECK-LABEL: @test_simplify7(
   %retval = call float @powf(float %x, float 0.5)
-; CHECK-NEXT: [[SQRTF:%[a-z0-9]+]] = call float @sqrtf(float %x) [[NUW_RO]]
+; CHECK-NEXT: [[SQRTF:%[a-z0-9]+]] = call float @sqrtf(float %x) [[NUW_RO:#[0-9]+]]
 ; CHECK-NEXT: [[FABSF:%[a-z0-9]+]] = call float @fabsf(float [[SQRTF]]) [[NUW_RO]]
 ; CHECK-NEXT: [[FCMP:%[a-z0-9]+]] = fcmp oeq float %x, 0xFFF0000000000000
 ; CHECK-NEXT: [[SELECT:%[a-z0-9]+]] = select i1 [[FCMP]], float 0x7FF0000000000000, float [[FABSF]]
   ret float %retval
 ; CHECK-NEXT: ret float [[SELECT]]
 }
 
 define double @test_simplify8(double %x) {