[PATCH] D41286: [WIP][InstCombine] Missed optimization in math expression: sin(x) / cos(x) => tan(x)

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1456
+  if (AllowReassociate) {
+    if (IntrinsicInst *Sin = dyn_cast<IntrinsicInst>(Op0)) {
+      if (IntrinsicInst *Cos = dyn_cast<IntrinsicInst>(Op1)) {
----------------
Use `match` here?


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1463
+          Type *Ty = I.getType();
+          Value *Tan = M->getOrInsertFunction("tan", Ty, Ty);
+          Builder.setFastMathFlags(I.getFastMathFlags());
----------------
For the name here...
 1. Predicate the transform on TLI->has(LibFunc_tan) (or tanf, tanl, depending on the type).
 2. Use TLI->getName(LibFunc_tan) (or tanf, tanl, depending on the type).


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1464
+          Value *Tan = M->getOrInsertFunction("tan", Ty, Ty);
+          Builder.setFastMathFlags(I.getFastMathFlags());
+          Value *TanCall = Builder.CreateCall(Tan, Cos->getArgOperand(0), "tan");
----------------
Needs
  BuilderTy::FastMathFlagGuard Guard(Builder);
above this.


https://reviews.llvm.org/D41286