[SimplifyLibCalls] Add a new transform: tan(atan(x)) -> x

[davide]

Summary

To put things into context, I did some basic research trying to understand which lib calls transformation gcc does but llvm does not under -ffast-math, and started writing code to reduce the difference. Probably I should've explained this in my previous pow() patch, but, anyway.

This is the second one of a series of local patches I'll try to submit over the next weeks. It may at this point considered a WIP (still need to add more comprehensive tests, among others). I'm not completely sure if we should remove the call to atan (I wasn't confident on how to do that) or let subsequent passes deal with that. Ideas? Thanks!

[majnemer]

lib/Transforms/Utils/SimplifyLibCalls.cpp

1382–1385

No new instructions are created, why is this present?

1392–1394

The parens around Func == LibFunc::atan don't seem necessary. Also, I'd flip the order of the comparisons so that we are comparing Func first.

test/Transforms/InstCombine/tan.ll

21

Attribute #0 and #1 are identical, I'd switch all users over to #0.

[scanon]

I'd like to see this made generic so it applies to any 1-to-1 libm function whose inverse is available, since the same pattern applies for logN(expN(x)), asinh(sinh(x)), atanh(tanh(x)), etc ...

[davide]

Addressed David's comments.

[davide]

In D14302#280441, @scanon wrote:

I'd like to see this made generic so it applies to any 1-to-1 libm function whose inverse is available, since the same pattern applies for logN(expN(x)), asinh(sinh(x)), atanh(tanh(x)), etc ...

I'll prepare a generalized version of this patch. Thanks.

[davide]

Upload correct version of the patch.

[majnemer]

lib/Transforms/Utils/SimplifyLibCalls.cpp

1381

I think it would be nicer to do an early return:

auto *OpC = dyn_cast<CallInst>(Op1);
if (!OpC)
  return Ret;

1382

Is the guard needed?

[davide]

Updated after second round of comments. Thank you for your review, David.

[davide]

Also, forgot to mention. I would like to get this as-is (given it works) and then generalize it as next step, unless David/Stephen have objections.

[majnemer]

LGTM

test/Transforms/InstCombine/tan.ll

11–14

You just need a single check statement after the CHECK-LABEL directive, ; CHECK: ret float %x

include/llvm/Transforms/Utils/SimplifyLibCalls.h

private:
   Value *optimizeBinaryDoubleFP(CallInst *CI, IRBuilder<> &B);
   Value *optimizeCos(CallInst *CI, IRBuilder<> &B);
   Value *optimizePow(CallInst *CI, IRBuilder<> &B);
   Value *optimizeExp2(CallInst *CI, IRBuilder<> &B);
   Value *optimizeFabs(CallInst *CI, IRBuilder<> &B);
   Value *optimizeFMinFMax(CallInst *CI, IRBuilder<> &B);
   Value *optimizeSqrt(CallInst *CI, IRBuilder<> &B);
   Value *optimizeSinCosPi(CallInst *CI, IRBuilder<> &B);
+  Value *optimizeTan(CallInst *CI, IRBuilder<> &B);
 
   // Integer Library Call Optimizations
   Value *optimizeFFS(CallInst *CI, IRBuilder<> &B);
   Value *optimizeAbs(CallInst *CI, IRBuilder<> &B);
   Value *optimizeIsDigit(CallInst *CI, IRBuilder<> &B);
   Value *optimizeIsAscii(CallInst *CI, IRBuilder<> &B);
   Value *optimizeToAscii(CallInst *CI, IRBuilder<> &B);
 

lib/Transforms/Utils/SimplifyLibCalls.cpp

         }
         return FabsCall;
       }
     }
   }
   return Ret;
 }
 
+Value *LibCallSimplifier::optimizeTan(CallInst *CI, IRBuilder<> &B) {
+  Function *Callee = CI->getCalledFunction();
+  Value *Ret = nullptr;
+  if (UnsafeFPShrink && Callee->getName() == "tan" && TLI->has(LibFunc::tanf))
+    Ret = optimizeUnaryDoubleFP(CI, B, true);
+  FunctionType *FT = Callee->getFunctionType();
+
+  // Just make sure this has 1 argument of FP type, which matches the
+  // result type.
+  if (FT->getNumParams() != 1 || FT->getReturnType() != FT->getParamType(0) ||
+      !FT->getParamType(0)->isFloatingPointTy())
+    return Ret;
+
+  if (!canUseUnsafeFPMath(CI->getParent()->getParent()))
+    return Ret;
+  Value *Op1 = CI->getArgOperand(0);
+  auto *OpC = dyn_cast<CallInst>(Op1);
+  if (!OpC)
+    return Ret;
+
+  // tan(atan(x)) -> x
+  // tanf(atanf(x)) -> x
+  // tanl(atanl(x)) -> x
+  LibFunc::Func Func;
+  Function *F = OpC->getCalledFunction();
+  StringRef FuncName = F->getName();
+  if (TLI->getLibFunc(FuncName, Func) && TLI->has(Func) &&
+      ((Func == LibFunc::atan && Callee->getName() == "tan") ||
+       (Func == LibFunc::atanf && Callee->getName() == "tanf") ||
+       (Func == LibFunc::atanl && Callee->getName() == "tanl")))
+    Ret = OpC->getArgOperand(0);
+  return Ret;
+}
+
 static bool isTrigLibCall(CallInst *CI);
 static void insertSinCosCall(IRBuilder<> &B, Function *OrigCallee, Value *Arg,
                              bool UseFloat, Value *&Sin, Value *&Cos,
                              Value *&SinCos);
 
 Value *LibCallSimplifier::optimizeSinCosPi(CallInst *CI, IRBuilder<> &B) {
 
   // Make sure the prototype is as expected, otherwise the rest of the

     case LibFunc::fprintf:
       return optimizeFPrintF(CI, Builder);
     case LibFunc::fwrite:
       return optimizeFWrite(CI, Builder);
     case LibFunc::fputs:
       return optimizeFPuts(CI, Builder);
     case LibFunc::puts:
       return optimizePuts(CI, Builder);
+    case LibFunc::tan:
+    case LibFunc::tanf:
+    case LibFunc::tanl:
+      return optimizeTan(CI, Builder);
     case LibFunc::perror:
       return optimizeErrorReporting(CI, Builder);
     case LibFunc::vfprintf:
     case LibFunc::fiprintf:
       return optimizeErrorReporting(CI, Builder, 0);
     case LibFunc::fputc:
       return optimizeErrorReporting(CI, Builder, 1);
     case LibFunc::ceil:

     case LibFunc::expm1:
     case LibFunc::log:
     case LibFunc::log10:
     case LibFunc::log1p:
     case LibFunc::log2:
     case LibFunc::logb:
     case LibFunc::sin:
     case LibFunc::sinh:
-    case LibFunc::tan:
     case LibFunc::tanh:
       if (UnsafeFPShrink && hasFloatVersion(FuncName))
         return optimizeUnaryDoubleFP(CI, Builder, true);
       return nullptr;
     case LibFunc::copysign:
       if (hasFloatVersion(FuncName))
         return optimizeBinaryDoubleFP(CI, Builder);
       return nullptr;

test/Transforms/InstCombine/tan-nofastmath.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+define float @mytan(float %x) {
+entry:
+  %call = call float @atanf(float %x)
+  %call1 = call float @tanf(float %call) 
+  ret float %call1
+}
+
+; CHECK-LABEL: define float @mytan(
+; CHECK:   %call = call float @atanf(float %x)
+; CHECK-NEXT:   %call1 = call float @tanf(float %call)
+; CHECK-NEXT:   ret float %call1
+; CHECK-NEXT: }
+
+declare float @tanf(float)
+declare float @atanf(float)

test/Transforms/InstCombine/tan.ll

+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+define float @mytan(float %x) #0 {
+entry:
+  %call = call float @atanf(float %x)
+  %call1 = call float @tanf(float %call)
+  ret float %call1
+}
+
+; CHECK-LABEL: define float @mytan(
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %call = call float @atanf(float %x)
+; CHECK-NEXT:   ret float %x
+; CHECK-NEXT: }
+
+declare float @tanf(float) #0
+declare float @atanf(float) #0
+attributes #0 = { "unsafe-fp-math"="true" }