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

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

In https://reviews.llvm.org/D41286#970342, @spatel wrote:

> > If you have a numerical explanation of why this patch can go in, I'll be happy to accept,
>
> I'm not sure what you're looking for. Brute force sin/cos is equal to tan for every possible value?


I don't think this is feasible (for 64-bit values). `-ffast-math` is a little fun and complicated, e.g.

  log(pow(x, y)) -> y*log(x)

which seems perfectly fine on paper, for `x = -1, y = 4`

  log(pow(-1, 4)) -> 0
  4*log(-1) -> NaN.

(courtesy of Steven)

My point is that reasoning about seemingly innocuous algebraic simplification turns out to be harder than expected, and therefore we should make a conscious choice on whether implement these after careful numerical analysis.


https://reviews.llvm.org/D41286