[llvm] r340784 - [PATCH] [InstCombine] Fix issue in the simplification of pow() with nested exp{, 2}()

[Evandro Menezes via llvm-commits]

Author: evandro
Date: Mon Aug 27 15:11:15 2018
New Revision: 340784

URL: http://llvm.org/viewvc/llvm-project?rev=340784&view=rev
Log:
[PATCH] [InstCombine] Fix issue in the simplification of pow() with nested exp{,2}()

Fix the issue of duplicating the call to `exp{,2}()` when it's nested in
`pow()`, as exposed by rL340462.

Differential revision: https://reviews.llvm.org/D51194

Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
    llvm/trunk/test/Transforms/InstCombine/pow-exp.ll

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp?rev=340784&r1=340783&r2=340784&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp Mon Aug 27 15:11:15 2018
@@ -1194,23 +1194,39 @@ Value *LibCallSimplifier::replacePowWith
 
   // pow(exp(x), y) -> exp(x * y)
   // pow(exp2(x), y) -> exp2(x * y)
-  // Only with fully relaxed math semantics, since, besides rounding
-  // differences, the transformation changes overflow and underflow behavior
-  // quite dramatically.
-  // For example:
+  // If exp{,2}() is used only once, it is better to fold two transcendental
+  // math functions into one.  If used again, exp{,2}() would still have to be
+  // called with the original argument, then keep both original transcendental
+  // functions.  However, this transformation is only safe with fully relaxed
+  // math semantics, since, besides rounding differences, it changes overflow
+  // and underflow behavior quite dramatically.  For example:
   //   pow(exp(1000), 0.001) = pow(inf, 0.001) = inf
   // Whereas:
   //   exp(1000 * 0.001) = exp(1)
   // TODO: Loosen the requirement for fully relaxed math semantics.
   // TODO: Handle exp10() when more targets have it available.
   CallInst *BaseFn = dyn_cast<CallInst>(Base);
-  if (BaseFn && BaseFn->isFast() && Pow->isFast()) {
+  if (BaseFn && BaseFn->hasOneUse() && BaseFn->isFast() && Pow->isFast()) {
     LibFunc LibFn;
+
     Function *CalleeFn = BaseFn->getCalledFunction();
     if (CalleeFn && TLI->getLibFunc(CalleeFn->getName(), LibFn) &&
         (LibFn == LibFunc_exp || LibFn == LibFunc_exp2) && TLI->has(LibFn)) {
+      Value *ExpFn;
+
+      // Create new exp{,2}() with the product as its argument.
       Value *FMul = B.CreateFMul(BaseFn->getArgOperand(0), Expo, "mul");
-      return emitUnaryFloatFnCall(FMul, CalleeFn->getName(), B, Attrs);
+      ExpFn = emitUnaryFloatFnCall(FMul, CalleeFn->getName(), B,
+                                   BaseFn->getAttributes());
+
+      // Since the new exp{,2}() is different from the original one, dead code
+      // elimination cannot be trusted to remove it, since it may have side
+      // effects (e.g., errno).  When the only consumer for the original
+      // exp{,2}() is pow(), then it has to be explicitly erased.
+      BaseFn->replaceAllUsesWith(ExpFn);
+      BaseFn->eraseFromParent();
+
+      return ExpFn;
     }
   }
 

Modified: llvm/trunk/test/Transforms/InstCombine/pow-exp.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/pow-exp.ll?rev=340784&r1=340783&r2=340784&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/pow-exp.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/pow-exp.ll Mon Aug 27 15:11:15 2018
@@ -108,10 +108,8 @@ define double @pow_exp2(double %x, doubl
   ret double %pow
 }
 
-; FIXME: Should not result in two calls to exp2().
 define double @pow_exp2_libcall(double %x, double %y) {
 ; CHECK-LABEL: @pow_exp2_libcall(
-; CHECK-NEXT:    [[CALL:%.*]] = call fast double @exp2(double [[X:%.*]])
 ; CHECK-NEXT:    [[MUL:%.*]] = fmul fast double [[X]], [[Y:%.*]]
 ; CHECK-NEXT:    [[EXP2:%.*]] = call fast double @exp2(double [[MUL]])
 ; CHECK-NEXT:    ret double [[EXP2]]
@@ -179,6 +177,32 @@ define fp128 @pow_exp10l(fp128 %x, fp128
   ret fp128 %pow
 }
 
+define float @reuse_fast(float %x, float %y, float * %p) {
+; CHECK-LABEL: @reuse_fast(
+; CHECK-NEXT:    [[EXP:%.*]] = call fast float @expf(float [[X:%.*]])
+; CHECK-NEXT:    [[POW:%.*]] = call fast float @powf(float [[EXP]], float [[Y:%.*]])
+; CHECK-NEXT:    store float [[EXP]], float* [[P:%.*]], align 4
+; CHECK-NEXT:    ret float [[POW]]
+;
+  %exp = call fast float @expf(float %x)
+  %pow = call fast float @powf(float %exp, float %y)
+  store float %exp, float *%p, align 4
+  ret float %pow
+}
+
+define fp128 @reuse_libcall(fp128 %x, fp128 %y, fp128 * %p) {
+; CHECK-LABEL: @reuse_libcall(
+; CHECK-NEXT:    [[EXP:%.*]] = call fp128 @expl(fp128 [[X:%.*]])
+; CHECK-NEXT:    [[POW:%.*]] = call fp128 @powl(fp128 [[EXP]], fp128 [[Y:%.*]])
+; CHECK-NEXT:    store fp128 [[EXP]], fp128* [[P:%.*]], align 16
+; CHECK-NEXT:    ret fp128 [[POW]]
+;
+  %exp = call fp128 @expl(fp128 %x)
+  %pow = call fp128 @powl(fp128 %exp, fp128 %y)
+  store fp128 %exp, fp128 *%p, align 16
+  ret fp128 %pow
+}
+
 define double @function_pointer(double ()* %fptr, double %p1) {
 ; CHECK-LABEL: @function_pointer(
 ; CHECK-NEXT:    [[CALL1:%.*]] = call fast double [[FPTR:%.*]]()