[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

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.

lib/Transforms/Utils/SimplifyLibCalls.cpp

     // In -ffast-math, pow(x, 0.5) -> sqrt(x).
     if (CI->hasUnsafeAlgebra()) {
       IRBuilder<>::FastMathFlagGuard Guard(B);
       B.setFastMathFlags(CI->getFastMathFlags());
       return emitUnaryFloatFnCall(Op1, TLI->getName(LibFunc::sqrt), B,
                                   Callee->getAttributes());
     }
 
-    // Expand pow(x, 0.5) to (x == -infinity ? +infinity : fabs(sqrt(x))).
+    // In finite-only mode, pow(x, 0.5) -> fabs(sqrt(x))
+    Value *Sqrt = emitUnaryFloatFnCall(Op1, "sqrt", B, Callee->getAttributes());
+    Value *FAbs =
+        emitUnaryFloatFnCall(Sqrt, "fabs", B, Callee->getAttributes());
+    if (CI->hasNoNaNs() && CI->hasNoInfs())
+      return FAbs;
+
+    // Otherwise, expand pow(x, 0.5) to
+    //   (x == -infinity ? +infinity : fabs(sqrt(x))).
     // This is faster than calling pow, and still handles negative zero
     // and negative infinity correctly.
-    // TODO: In finite-only mode, this could be just fabs(sqrt(x)).
     Value *Inf = ConstantFP::getInfinity(CI->getType());
     Value *NegInf = ConstantFP::getInfinity(CI->getType(), true);
-    Value *Sqrt = emitUnaryFloatFnCall(Op1, "sqrt", B, Callee->getAttributes());
-    Value *FAbs =
-        emitUnaryFloatFnCall(Sqrt, "fabs", B, Callee->getAttributes());
     Value *FCmp = B.CreateFCmpOEQ(Op1, NegInf);
     Value *Sel = B.CreateSelect(FCmp, Inf, FAbs);
     return Sel;
   }
 
   if (Op2C->isExactlyValue(1.0)) // pow(x, 1.0) -> x
     return Op1;
   if (Op2C->isExactlyValue(2.0)) // pow(x, 2.0) -> x*x
     return B.CreateFMul(Op1, Op1, "pow2");
   if (Op2C->isExactlyValue(-1.0)) // pow(x, -1.0) -> 1.0/x
     return B.CreateFDiv(ConstantFP::get(CI->getType(), 1.0), Op1, "powrecip");
 

test/Transforms/InstCombine/pow-sqrt.ll

 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 define double @pow_half(double %x) {
   %pow = call fast double @llvm.pow.f64(double %x, double 5.000000e-01)
   ret double %pow
 }
 
 ; CHECK-LABEL: define double @pow_half(
 ; CHECK-NEXT:  %sqrt = call fast double @sqrt(double %x)
 ; CHECK-NEXT:  ret double %sqrt
 
 declare double @llvm.pow.f64(double, double)
 
+; In finite-only mode, we can transform pow(x, 0.5) -> fabs(sqrt(x))
+
+define double @pow_finite(double %x) {
+  %pow = call nnan ninf double @llvm.pow.f64(double %x, double 5.000000e-01)
+  ret double %pow
+}
+
+; CHECK-LABEL: define double @pow_finite(
+; CHECK-NEXT:  %sqrt = call double @sqrt(double %x)
+; CHECK-NEXT:  %fabs = call double @fabs(double %sqrt)
+; CHECK-NEXT:  ret double %fabs