[InstCombine] Missed optimization in math expression: tan(a) * cos(a) == sin(a)

[Quolyk]

Summary

This patch enables folding tan(x) * cos(x) -> sin(x) under -ffast-math flag

[hfinkel]

lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

712

Please see my comments in D41389 about names.

715

Needs

BuilderTy::FastMathFlagGuard Guard(Builder);

above this.

[Quolyk]

As in D41389 I have tan calls that are never used and never erased.

[Quolyk]

This needs to be merged first, because it introduces m_LibFunc match, that would be useful for other optimizations.

[spatel]

Why doesn't tan have an LLVM intrinsic like sin/cos?

[Quolyk]

In D41283#974798, @spatel wrote:

Why doesn't tan have an LLVM intrinsic like sin/cos?

I don't know

[spatel]

In D41283#974808, @Quolyk wrote:

In D41283#974798, @spatel wrote:

Why doesn't tan have an LLVM intrinsic like sin/cos?

I don't know

Then we should find out. :)
I should've asked before D41286, but oops.
cc @efriedma in case he knows the history. If not, you should ask on llvm-dev. Maybe it's just not as common to see tan in code, but if we're going to do more trig transforms, I think it would make sense to have uniformity of trig intrinsics, so it's easier to do these folds. Also, if the vectorizers recognize trig functions, it would make it easier for them if we have more intrinsics for these functions?

[hfinkel]

In D41283#974832, @spatel wrote:

In D41283#974808, @Quolyk wrote:

In D41283#974798, @spatel wrote:

Why doesn't tan have an LLVM intrinsic like sin/cos?

I don't know

Then we should find out. :)
I should've asked before D41286, but oops.
cc @efriedma in case he knows the history. If not, you should ask on llvm-dev. Maybe it's just not as common to see tan in code, but if we're going to do more trig transforms, I think it would make sense to have uniformity of trig intrinsics, so it's easier to do these folds. Also, if the vectorizers recognize trig functions, it would make it easier for them if we have more intrinsics for these functions?

I agree. In the past, AFAIK, we've added the intrinsics on an as-needed basis.

[Quolyk]

I'll try introduce llvm.tan intrinsic then in different patch

include/llvm/IR/PatternMatch.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_PATTERNMATCH_H
 #define LLVM_IR_PATTERNMATCH_H
 
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Operator.h"

 }
 
 template <typename Opnd0, typename Opnd1>
 inline typename m_Intrinsic_Ty<Opnd0, Opnd1>::Ty m_FMax(const Opnd0 &Op0,
                                                         const Opnd1 &Op1) {
   return m_Intrinsic<Intrinsic::maxnum>(Op0, Op1);
 }
 
+/// \brief LibFunc matchers.
+struct LibFunc_match {
+  LibFunc F;
+  TargetLibraryInfo TLI;
+  
+  LibFunc_match(LibFunc Func, TargetLibraryInfo TargetLI) 
+      : F(Func), TLI(TargetLI) {}
+  
+  template <typename OpTy> bool match(OpTy *V) {
+    if (const auto *CI = dyn_cast<CallInst>(V))
+      if (const auto *CalledF = CI->getCalledFunction())
+        if (TLI.has(F))
+          return TLI.getName(F) == CalledF->getName();
+    return false;
+  }
+};
+
+/// LibFunc matches are combinations of Name matchers, and argument
+/// matchers.
+template <typename T0 = void, typename T1 = void, typename T2 = void,
+          typename T3 = void, typename T4 = void, typename T5 = void,
+          typename T6 = void, typename T7 = void, typename T8 = void,
+          typename T9 = void, typename T10 = void>
+struct m_LibFunc_Ty;
+template <typename T0> struct m_LibFunc_Ty<T0> {
+  using Ty = match_combine_and<LibFunc_match, Argument_match<T0>>;
+};
+template <typename T0, typename T1> struct m_LibFunc_Ty<T0, T1> {
+  using Ty =
+      match_combine_and<typename m_LibFunc_Ty<T0>::Ty,
+                        Argument_match<T1>>;
+};
+template <typename T0, typename T1, typename T2>
+struct m_LibFunc_Ty<T0, T1, T2> {
+  using Ty =
+      match_combine_and<typename m_LibFunc_Ty<T0, T1>::Ty,
+                        Argument_match<T2>>;
+};
+template <typename T0, typename T1, typename T2, typename T3>
+struct m_LibFunc_Ty<T0, T1, T2, T3> {
+  using Ty =
+      match_combine_and<typename m_LibFunc_Ty<T0, T1, T2>::Ty,
+                        Argument_match<T3>>;
+};
+
+/// \brief Match LibFunc calls like this:
+/// m_LibFunc<LibFunc_tan>(m_Value(X))
+template <LibFunc F> 
+inline LibFunc_match m_LibFunc(TargetLibraryInfo TLI) {
+  return LibFunc_match(F, TLI);
+}
+
+template <LibFunc F, typename T0>
+inline typename m_LibFunc_Ty<T0>::Ty 
+m_LibFunc(const TargetLibraryInfo TLI, const T0 &Op0) {
+  return m_CombineAnd(m_LibFunc<F>(TLI), m_Argument<0>(Op0));
+}
+
+template <LibFunc F, typename T0, typename T1>
+inline typename m_LibFunc_Ty<T0, T1>::Ty 
+m_LibFunc(const TargetLibraryInfo TLI, const T0 &Op0, const T0 &Op1) {
+  return m_CombineAnd(m_LibFunc<F>(TLI, Op0), m_Argument<1>(Op1));
+}
+
+template <LibFunc F, typename T0, typename T1, typename T2>
+inline typename m_LibFunc_Ty<T0, T1, T2>::Ty 
+m_LibFunc(const TargetLibraryInfo TLI,
+          const T0 &Op0,
+          const T0 &Op1,
+          const T0 &Op2) {
+  return m_CombineAnd(m_LibFunc<F>(TLI, Op0, Op1), m_Argument<2>(Op2));
+}
+
+template <LibFunc F, typename T0, typename T1, typename T2, typename T3>
+inline typename m_LibFunc_Ty<T0, T1, T2, T3>::Ty 
+m_LibFunc(const TargetLibraryInfo TLI,
+          const T0 &Op0,
+          const T0 &Op1,
+          const T0 &Op2,
+          const T0 &Op3) {
+  return m_CombineAnd(m_LibFunc<F>(TLI, Op0, Op1, Op2), m_Argument<3>(Op3));
+}
+
 template <typename Opnd_t> struct Signum_match {
   Opnd_t Val;
   Signum_match(const Opnd_t &V) : Val(V) {}
 
   template <typename OpTy> bool match(OpTy *V) {
     unsigned TypeSize = V->getType()->getScalarSizeInBits();
     if (TypeSize == 0)
       return false;

lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Op0)) {
                                                           II->getOperand(0),
                                                           I.getName());
         FMulVal->copyFastMathFlags(&I);
         return FMulVal;
       }
     }
   }
 
+  // tan(a) * cos(a) -> sin(a)
+  if (AllowReassociate) {
+    Value *Tan = nullptr;
+    Value *Cos = nullptr;
+    bool matchCos = match(Op0, m_Intrinsic<Intrinsic::cos>(m_Value(Cos)));
+    bool matchTan = match(Op1, m_LibFunc<LibFunc_tan>(TLI, m_Value(Tan))) ||
+                    match(Op1, m_LibFunc<LibFunc_tanf>(TLI, m_Value(Tan))) ||
+                    match(Op1, m_LibFunc<LibFunc_tanl>(TLI, m_Value(Tan)));
+    if (matchCos && matchTan && (Tan == Cos)) {
+      BuilderTy::FastMathFlagGuard Guard(Builder);
+      Builder.setFastMathFlags(I.getFastMathFlags());
+      Value *Sin = Intrinsic::getDeclaration(I.getModule(),
+                                             Intrinsic::sin, I.getType());
+      Value *SinCall = Builder.CreateCall(Sin, Cos);
+      return replaceInstUsesWith(I, SinCall);
+    }
+  }
+
   // Under unsafe algebra do:
   // X * log2(0.5*Y) = X*log2(Y) - X
   if (AllowReassociate) {
     Value *OpX = nullptr;
     Value *OpY = nullptr;
     IntrinsicInst *Log2;
     detectLog2OfHalf(Op0, OpY, Log2);
     if (OpY) {

test/Transforms/InstCombine/fmul.ll

 ; CHECK: call float @llvm.fabs.f32(float %y)
 ; CHECK: %mul = fmul float %x.fabs, %y.fabs
 define float @fabs_x_fabs(float %x, float %y) {
   %x.fabs = call float @llvm.fabs.f32(float %x)
   %y.fabs = call float @llvm.fabs.f32(float %y)
   %mul = fmul float %x.fabs, %y.fabs
   ret float %mul
 }
+
+; CHECK-LABEL @tan_x_cos(
+; CHECK: call fast double @llvm.sin.f64(double %a)
+define double @tan_x_cos(double %a) {
+  %call = call double @tan(double %a)
+  %1 = call double @llvm.cos.f64(double %a)
+  %mul = fmul double %1, %call
+  ret double %mul
+}
+
+; CHECK-LABEL @tan_x_cos_fast(
+; CHECK: call fast double @llvm.sin.f64(double %a)
+define double @tan_x_cos_fast(double %a) {
+  %call = call fast double @tan(double %a)
+  %1 = call fast double @llvm.cos.f64(double %a)
+  %mul = fmul fast double %1, %call
+  ret double %mul
+}
+
+; CHECK-LABEL @tanf_x_cosf_fast(
+; CHECK: call fast float @llvm.sin.f32(float %a)
+define float @tanf_x_cosf_fast(float %a) {
+  %call = call fast float @tanf(float %a)
+  %1 = call fast float @llvm.cos.f32(float %a)
+  %mul = fmul fast float %1, %call
+  ret float %mul
+}
+
+; CHECK-LABEL @tanl_x_cosl_fast(
+; CHECK: call fast fp128 @llvm.sin.fp128(fp128 %a)
+define fp128 @tanl_x_cosl_fast(fp128 %a) {
+  %call = call fast fp128 @tanl(fp128 %a)
+  %1 = call fast fp128 @llvm.cos.fp128(fp128 %a)
+  %mul = fmul fast fp128 %1, %call
+  ret fp128 %mul
+}
+
+declare double @llvm.sin.f64(double)
+declare float @llvm.sin.f32(float)
+declare fp128 @llvm.sin.fp128(fp128)
+
+declare double @llvm.cos.f64(double)
+declare float @llvm.cos.f32(float)
+declare fp128 @llvm.cos.fp128(fp128)
+
+declare double @tan(double)
+declare float @tanf(float)
+declare fp128 @tanl(fp128)