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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:1472
 
+  // sin(a) / cos(a) -> tan(a)
+  if (AllowReassociate) {
----------------
davide wrote:
> spatel wrote:
> > Should we also handle:
> >   cos(a) / sin(a) -> 1 / tan(a)
> > ?
> Please wait for @scanon opinion before implementing every possible 10th grade trigonometrical identity.
Do you have some specific numerical concern here? As you've noted, this is a well-known math transform.

We can make cos(a) / sin(a) a 'TODO' if you think we should use a different transform.
https://stackoverflow.com/questions/3738384/stable-cotangent




https://reviews.llvm.org/D41286