[LibCallSimplifier] use instruction-level fast-math-flags to transform sqrt calls

[spatel]

Summary

This is a continuation of adding FMF to call instructions:
http://reviews.llvm.org/rL255555

The intent of the patch is to preserve the current behavior of the transform except that we use the sqrt instruction's 'fast' attribute as a trigger rather than the function-level attribute.

But this raises a possible bug noted by the new FIXME comment.

I think that in order to do this transform:
sqrt((x * x) * y) ---> fabs(x) * sqrt(y)

...we need all of the sqrt, the first fmul, and the second fmul to be 'fast'. If any of those ops is strict, we should bail out. Thoughts?

[davide]

This looks fine to me but I'd love to hear Steve's opinion on it. I'm looking forward to see other instructions fixed (pow & friends =))

lib/Transforms/Utils/SimplifyLibCalls.cpp

1425 ↗

(On Diff #44153)

I think you're correct that if any of the ops is strict we should bail out, and your code is fine. Have you checked by any chance what GCC does? Steven Canon might have some more opinions on this.

1442 ↗

(On Diff #44153)

Spurious blank linke.

[davide]

lib/Transforms/Utils/SimplifyLibCalls.cpp

1442 ↗

(On Diff #44153)

Oh I think this one makes sense but please commit the style fix separately.

[spatel]

lib/Transforms/Utils/SimplifyLibCalls.cpp

1425 ↗

(On Diff #44153)

Based on:
https://gcc.godbolt.org/

I don't think GCC has this optimization. It only matches the simpler case of "sqrt(x * x)".

1442 ↗

(On Diff #44153)

Will do.

Thanks for the review!

[scanon]

LGTM.

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

 
 Value *LibCallSimplifier::optimizeSqrt(CallInst *CI, IRBuilder<> &B) {
   Function *Callee = CI->getCalledFunction();
   
   Value *Ret = nullptr;
   if (TLI->has(LibFunc::sqrtf) && (Callee->getName() == "sqrt" ||
                                    Callee->getIntrinsicID() == Intrinsic::sqrt))
     Ret = optimizeUnaryDoubleFP(CI, B, true);
-  if (!canUseUnsafeFPMath(CI->getParent()->getParent()))
+
+  if (!CI->hasUnsafeAlgebra())
     return Ret;
 
   Instruction *I = dyn_cast<Instruction>(CI->getArgOperand(0));
   if (!I || I->getOpcode() != Instruction::FMul || !I->hasUnsafeAlgebra())
     return Ret;
 
   // We're looking for a repeated factor in a multiplication tree,
   // so we can do this fold: sqrt(x * x) -> fabs(x);
-  // or this fold: sqrt(x * x * y) -> fabs(x) * sqrt(y).
+  // or this fold: sqrt((x * x) * y) -> fabs(x) * sqrt(y).
   Value *Op0 = I->getOperand(0);
   Value *Op1 = I->getOperand(1);
   Value *RepeatOp = nullptr;
   Value *OtherOp = nullptr;
   if (Op0 == Op1) {
     // Simple match: the operands of the multiply are identical.
     RepeatOp = Op0;
   } else {
     // Look for a more complicated pattern: one of the operands is itself
     // a multiply, so search for a common factor in that multiply.
     // Note: We don't bother looking any deeper than this first level or for
     // variations of this pattern because instcombine's visitFMUL and/or the
     // reassociation pass should give us this form.
     Value *OtherMul0, *OtherMul1;
+    // FIXME: This multiply must be unsafe to allow this transform.
     if (match(Op0, m_FMul(m_Value(OtherMul0), m_Value(OtherMul1)))) {
       // Pattern: sqrt((x * y) * z)
       if (OtherMul0 == OtherMul1) {
         // Matched: sqrt((x * x) * z)
         RepeatOp = OtherMul0;
         OtherOp = Op1;
       }
     }
   }
   if (!RepeatOp)
     return Ret;
 
   // Fast math flags for any created instructions should match the sqrt
   // and multiply.
-  // FIXME: We're not checking the sqrt because it doesn't have
-  // fast-math-flags (see earlier comment).
   IRBuilder<>::FastMathFlagGuard Guard(B);
   B.SetFastMathFlags(I->getFastMathFlags());
   // If we found a repeated factor, hoist it out of the square root and
   // replace it with the fabs of that factor.
   Module *M = Callee->getParent();
   Type *ArgType = I->getType();
   Value *Fabs = Intrinsic::getDeclaration(M, Intrinsic::fabs, ArgType);
   Value *FabsCall = B.CreateCall(Fabs, RepeatOp, "fabs");

llvm/trunk/test/Transforms/InstCombine/fast-math.ll

 ; =========================================================================
 ;
 ;   Test-cases for square root
 ;
 ; =========================================================================
 
 ; A squared factor fed into a square root intrinsic should be hoisted out
 ; as a fabs() value.
-; We have to rely on a function-level attribute to enable this optimization
-; because intrinsics don't currently have access to IR-level fast-math
-; flags. If that changes, we can relax the requirement on all of these
-; tests to just specify 'fast' on the sqrt.
-
-attributes #0 = { "unsafe-fp-math" = "true" }
 
 declare double @llvm.sqrt.f64(double)
 
-define double @sqrt_intrinsic_arg_squared(double %x) #0 {
+define double @sqrt_intrinsic_arg_squared(double %x) {
   %mul = fmul fast double %x, %x
-  %sqrt = call double @llvm.sqrt.f64(double %mul)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_arg_squared(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: ret double %fabs
 }
 
 ; Check all 6 combinations of a 3-way multiplication tree where
 ; one factor is repeated.
 
-define double @sqrt_intrinsic_three_args1(double %x, double %y) #0 {
+define double @sqrt_intrinsic_three_args1(double %x, double %y) {
   %mul = fmul fast double %y, %x
   %mul2 = fmul fast double %mul, %x
-  %sqrt = call double @llvm.sqrt.f64(double %mul2)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul2)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_three_args1(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: %sqrt1 = call fast double @llvm.sqrt.f64(double %y)
 ; CHECK-NEXT: %1 = fmul fast double %fabs, %sqrt1
 ; CHECK-NEXT: ret double %1
 }
 
-define double @sqrt_intrinsic_three_args2(double %x, double %y) #0 {
+define double @sqrt_intrinsic_three_args2(double %x, double %y) {
   %mul = fmul fast double %x, %y
   %mul2 = fmul fast double %mul, %x
-  %sqrt = call double @llvm.sqrt.f64(double %mul2)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul2)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_three_args2(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: %sqrt1 = call fast double @llvm.sqrt.f64(double %y)
 ; CHECK-NEXT: %1 = fmul fast double %fabs, %sqrt1
 ; CHECK-NEXT: ret double %1
 }
 
-define double @sqrt_intrinsic_three_args3(double %x, double %y) #0 {
+define double @sqrt_intrinsic_three_args3(double %x, double %y) {
   %mul = fmul fast double %x, %x
   %mul2 = fmul fast double %mul, %y
-  %sqrt = call double @llvm.sqrt.f64(double %mul2)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul2)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_three_args3(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: %sqrt1 = call fast double @llvm.sqrt.f64(double %y)
 ; CHECK-NEXT: %1 = fmul fast double %fabs, %sqrt1
 ; CHECK-NEXT: ret double %1
 }
 
-define double @sqrt_intrinsic_three_args4(double %x, double %y) #0 {
+define double @sqrt_intrinsic_three_args4(double %x, double %y) {
   %mul = fmul fast double %y, %x
   %mul2 = fmul fast double %x, %mul
-  %sqrt = call double @llvm.sqrt.f64(double %mul2)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul2)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_three_args4(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: %sqrt1 = call fast double @llvm.sqrt.f64(double %y)
 ; CHECK-NEXT: %1 = fmul fast double %fabs, %sqrt1
 ; CHECK-NEXT: ret double %1
 }
 
-define double @sqrt_intrinsic_three_args5(double %x, double %y) #0 {
+define double @sqrt_intrinsic_three_args5(double %x, double %y) {
   %mul = fmul fast double %x, %y
   %mul2 = fmul fast double %x, %mul
-  %sqrt = call double @llvm.sqrt.f64(double %mul2)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul2)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_three_args5(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: %sqrt1 = call fast double @llvm.sqrt.f64(double %y)
 ; CHECK-NEXT: %1 = fmul fast double %fabs, %sqrt1
 ; CHECK-NEXT: ret double %1
 }
 
-define double @sqrt_intrinsic_three_args6(double %x, double %y) #0 {
+define double @sqrt_intrinsic_three_args6(double %x, double %y) {
   %mul = fmul fast double %x, %x
   %mul2 = fmul fast double %y, %mul
-  %sqrt = call double @llvm.sqrt.f64(double %mul2)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul2)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_three_args6(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: %sqrt1 = call fast double @llvm.sqrt.f64(double %y)
 ; CHECK-NEXT: %1 = fmul fast double %fabs, %sqrt1
 ; CHECK-NEXT: ret double %1
 }
 
-define double @sqrt_intrinsic_arg_4th(double %x) #0 {
+define double @sqrt_intrinsic_arg_4th(double %x) {
   %mul = fmul fast double %x, %x
   %mul2 = fmul fast double %mul, %mul
-  %sqrt = call double @llvm.sqrt.f64(double %mul2)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul2)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_arg_4th(
 ; CHECK-NEXT: %mul = fmul fast double %x, %x
 ; CHECK-NEXT: ret double %mul
 }
 
-define double @sqrt_intrinsic_arg_5th(double %x) #0 {
+define double @sqrt_intrinsic_arg_5th(double %x) {
   %mul = fmul fast double %x, %x
   %mul2 = fmul fast double %mul, %x
   %mul3 = fmul fast double %mul2, %mul
-  %sqrt = call double @llvm.sqrt.f64(double %mul3)
+  %sqrt = call fast double @llvm.sqrt.f64(double %mul3)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_intrinsic_arg_5th(
 ; CHECK-NEXT: %mul = fmul fast double %x, %x
 ; CHECK-NEXT: %sqrt1 = call fast double @llvm.sqrt.f64(double %x)
 ; CHECK-NEXT: %1 = fmul fast double %mul, %sqrt1
 ; CHECK-NEXT: ret double %1
 }
 
 ; Check that square root calls have the same behavior.
 
 declare float @sqrtf(float)
 declare double @sqrt(double)
 declare fp128 @sqrtl(fp128)
 
-define float @sqrt_call_squared_f32(float %x) #0 {
+define float @sqrt_call_squared_f32(float %x) {
   %mul = fmul fast float %x, %x
-  %sqrt = call float @sqrtf(float %mul)
+  %sqrt = call fast float @sqrtf(float %mul)
   ret float %sqrt
 
 ; CHECK-LABEL: sqrt_call_squared_f32(
 ; CHECK-NEXT: %fabs = call fast float @llvm.fabs.f32(float %x)
 ; CHECK-NEXT: ret float %fabs
 }
 
-define double @sqrt_call_squared_f64(double %x) #0 {
+define double @sqrt_call_squared_f64(double %x) {
   %mul = fmul fast double %x, %x
-  %sqrt = call double @sqrt(double %mul)
+  %sqrt = call fast double @sqrt(double %mul)
   ret double %sqrt
 
 ; CHECK-LABEL: sqrt_call_squared_f64(
 ; CHECK-NEXT: %fabs = call fast double @llvm.fabs.f64(double %x)
 ; CHECK-NEXT: ret double %fabs
 }
 
-define fp128 @sqrt_call_squared_f128(fp128 %x) #0 {
+define fp128 @sqrt_call_squared_f128(fp128 %x) {
   %mul = fmul fast fp128 %x, %x
-  %sqrt = call fp128 @sqrtl(fp128 %mul)
+  %sqrt = call fast fp128 @sqrtl(fp128 %mul)
   ret fp128 %sqrt
 
 ; CHECK-LABEL: sqrt_call_squared_f128(
 ; CHECK-NEXT: %fabs = call fast fp128 @llvm.fabs.f128(fp128 %x)
 ; CHECK-NEXT: ret fp128 %fabs
 }
 
 ; =========================================================================

llvm/trunk/test/Transforms/InstCombine/inline-intrinsic-assert.ll

 ; RUN: opt < %s -inline -instcombine -S | FileCheck %s
 
 ; PR22857: http://llvm.org/bugs/show_bug.cgi?id=22857
 ; The inliner should not add an edge to an intrinsic and
 ; then assert that it did not add an edge to an intrinsic!
 
-define float @foo(float %f1) #0 {
+define float @foo(float %f1) {
   %call = call float @bar(float %f1)
   ret float %call
 
 ; CHECK-LABEL: @foo(
 ; CHECK-NEXT: call fast float @llvm.fabs.f32
 ; CHECK-NEXT: ret float
 }
 
-define float @bar(float %f1) #0 {
+define float @bar(float %f1) {
   %call = call float @sqr(float %f1)
-  %call1 = call float @sqrtf(float %call) #0
+  %call1 = call fast float @sqrtf(float %call)
   ret float %call1
 }
 
-define float @sqr(float %f) #0 {
+define float @sqr(float %f) {
   %mul = fmul fast float %f, %f
   ret float %mul
 }
 
-declare float @sqrtf(float) #0
-
-attributes #0 = { "unsafe-fp-math"="true" }
+declare float @sqrtf(float)
 

llvm/trunk/test/Transforms/InstCombine/no_cgscc_assert.ll

 ; RUN: opt < %s -inline -instcombine -S | FileCheck %s
 
 ; PR21403: http://llvm.org/bugs/show_bug.cgi?id=21403
 ; When the call to sqrtf is replaced by an intrinsic call to fabs,
 ; it should not cause a problem in CGSCC. 
 
 define float @bar(float %f) #0 {
   %mul = fmul fast float %f, %f
-  %call1 = call float @sqrtf(float %mul) #0
+  %call1 = call fast float @sqrtf(float %mul)
   ret float %call1
 
 ; CHECK-LABEL: @bar(
 ; CHECK-NEXT: call fast float @llvm.fabs.f32
 ; CHECK-NEXT: ret float
 }
 
-declare float @sqrtf(float) #0
+declare float @sqrtf(float)
 
-attributes #0 = { readnone "unsafe-fp-math"="true" }