[SimplifyLibCalls] Add a new transform: pow(exp(x), y) -> exp(x*y)

[davide]

Summary

Hi David, this should save a function call. The code passes basic testing. This is my first dive into SimplifyLibCalls, I may miss some details, please bear with me.

[majnemer]

I'm not sure what the implications of this transform are with respect to correctness. My naïve understanding is that this could produce different results because it removes a step where rounding was introduced. I'd feel more comfortable with this if @scanon, @resistor and/or @escha could take a look.

lib/Transforms/Utils/SimplifyLibCalls.cpp

1096 ↗

(On Diff #38342)

Please use auto *OpC here.

1102 ↗

(On Diff #38342)

Any reason in particular why we must limit this to isDoubleTy?

1103–1105 ↗

(On Diff #38342)

Is this clang-format'd?

1105 ↗

(On Diff #38342)

Wouldn't TLI->getName(LibFunc::exp) be equivalent to FuncName ?

[davide]

Hi, thanks for the review. I addressed the comments in this patch.
That said, I also share your concerns about correctness, that's mainly why I asked for review. On a second thought, maybe this should be enabled only under -ffast-math ? Anyway, looking forward to hear feedback from other reviewers.

[scanon]

As suggested by David, this should be fast-math only. It's roughly equivalent to re-association of multiplication. Besides rounding differences, this changes overflow and underflow behavior quite dramatically. Consider x = 1000, y = 0.001. pow(exp(x), y) = pow(inf, 0.001) = inf, whereas exp(x*y) = exp(1).

Also, this should really be generalized to apply to exp2 and exp10 as well as exp.

[davide]

Addressed Stephen's comments. Thank you for the explanation.
I added also some tests to ensure it's handled correctly with and without fast-math. I included a case for exp2, but not exp10 as it doesn't seem to be available on FreeBSD (GNU extension). I think we can leave that for a subsequent patch.

[davide]

For completeness, this is the code we emit with the patch (only under fast-math):

00000000004007b0 <mypow>:

4007b0:       f2 0f 59 c1             mulsd  %xmm1,%xmm0
4007b4:       e9 87 fd ff ff          jmpq   400540 <exp2@plt>
4007b9:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

and this was the code we emit without:

00000000004007f0 <mypow>:

4007f0:       50                      push   %rax
4007f1:       f2 0f 11 0c 24          movsd  %xmm1,(%rsp)
4007f6:       e8 75 fd ff ff          callq  400570 <exp2@plt>
4007fb:       f2 0f 10 0c 24          movsd  (%rsp),%xmm1
400800:       58                      pop    %rax
400801:       e9 7a fd ff ff          jmpq   400580 <pow@plt>
400806:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
40080d:       00 00 00

[scanon]

Seems reasonable to me. One of the owners should sign off on it.

[davide]

David, as InstCombine owner -- do you have any objections to get this in (now that Stephen reviewed it)?

[majnemer]

LGTM once the tests have been cleaned up.

test/Transforms/InstCombine/pow-exp2.ll

3–14 ↗

(On Diff #38752)

Please simplify this test case, you don't need the alloca/store/load sequence. I believe the following should work:

define double @mypow(double %x, double %y) #0 {
entry:
  %call = call double @exp2(double %x) #2
  %pow = call double @llvm.pow.f64(double %call, double %y)
  ret double %pow
}

10 ↗

(On Diff #38752)

You don't define attribute #2, please remove this reference to it.

17 ↗

(On Diff #38752)

You don't define attribute #2, please remove it.

20–26 ↗

(On Diff #38752)

Please move these check directives immediately after the function.

21 ↗

(On Diff #38752)

I would use the following:

; CHECK-LABEL: define double @mypow(

22 ↗

(On Diff #38752)

This check line is a little superfluous.

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

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());
   }
 
+  // pow(exp(x), y) -> exp(x*y)
+  // pow(exp2(x), y) -> exp2(x * y)
+  // We enable these only under fast-math. Besides rounding
+  // differences the transformation changes overflow and
+  // underflow behavior quite dramatically.
+  // Example: x = 1000, y = 0.001.
+  // pow(exp(x), y) = pow(inf, 0.001) = inf, whereas exp(x*y) = exp(1).
+  if (canUseUnsafeFPMath(CI->getParent()->getParent())) {
+    if (auto *OpC = dyn_cast<CallInst>(Op1)) {
+      IRBuilder<>::FastMathFlagGuard Guard(B);
+      FastMathFlags FMF;
+      FMF.setUnsafeAlgebra();
+      B.SetFastMathFlags(FMF);
+
+      LibFunc::Func Func;
+      Function *Callee = OpC->getCalledFunction();
+      StringRef FuncName = Callee->getName();
+
+      if (TLI->getLibFunc(FuncName, Func) && TLI->has(Func) &&
+          (Func == LibFunc::exp || Func == LibFunc::exp2))
+        return EmitUnaryFloatFnCall(
+            B.CreateFMul(OpC->getArgOperand(0), Op2, "mul"), FuncName, B,
+            Callee->getAttributes());
+    }
+  }
+
   ConstantFP *Op2C = dyn_cast<ConstantFP>(Op2);
   if (!Op2C)
     return Ret;
 
   if (Op2C->getValueAPF().isZero()) // pow(x, 0.0) -> 1.0
     return ConstantFP::get(CI->getType(), 1.0);
 
   if (Op2C->isExactlyValue(0.5) &&

llvm/trunk/test/Transforms/InstCombine/pow-exp-nofastmath.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+define double @mypow(double %x, double %y) #0 {
+entry:
+  %call = call double @exp(double %x)
+  %pow = call double @llvm.pow.f64(double %call, double %y)
+  ret double %pow
+}
+
+; CHECK-LABEL: define double @mypow(
+; CHECK:   %call = call double @exp(double %x)
+; CHECK:   %pow = call double @llvm.pow.f64(double %call, double %y)
+; CHECK:   ret double %pow
+; CHECK: }
+
+declare double @exp(double) #1
+declare double @llvm.pow.f64(double, double)

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

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+define double @mypow(double %x, double %y) #0 {
+entry:
+  %call = call double @exp(double %x)
+  %pow = call double @llvm.pow.f64(double %call, double %y)
+  ret double %pow
+}
+
+; CHECK-LABEL: define double @mypow(
+; CHECK:   %mul = fmul fast double %x, %y
+; CHECK:   %exp = call double @exp(double %mul) #0
+; CHECK:   ret double %exp
+; CHECK: }
+
+declare double @exp(double) #1
+declare double @llvm.pow.f64(double, double)
+attributes #0 = { "unsafe-fp-math"="true" }
+attributes #1 = { "unsafe-fp-math"="true" }

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

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+define double @mypow(double %x, double %y) #0 {
+entry:
+  %call = call double @exp2(double %x)
+  %pow = call double @llvm.pow.f64(double %call, double %y)
+  ret double %pow
+}
+
+; CHECK-LABEL: define double @mypow(
+; CHECK:   %mul = fmul fast double %x, %y
+; CHECK:   %exp2 = call double @exp2(double %mul) #0
+; CHECK:   ret double %exp2
+; CHECK: }
+
+declare double @exp2(double) #1
+declare double @llvm.pow.f64(double, double)
+attributes #0 = { "unsafe-fp-math"="true" }
+attributes #1 = { "unsafe-fp-math"="true" }