>
> >> Uses the algorithm:
> >> tan(x) = sin(x) / sqrt(1-sin^2(x))
> >>
> >> An alternative is:
> >> tan(x) = sin(x) / cos(x)
> >>
> >> Which produces more verbose bitcode and longer assembly.
> >
> > this is weird. both EG and SI have both sin and cos instructions. Is the
> > input normalization code so bad that we are better of doing MUL+SUB+SQRT
> > instead?
>
> Those are only useful for native_sin / native_cos. For the standard
> function, they are far from precise enough. The current (float) sin
> implementation should be correct, though native_sin right now is still
> defined to just be the regular sin function instead of the LLVM
> intrinsicoh I didn't know the hw implementaion was so imprecise. In that case it
makes sense. Although I wonder why it ended up needing twice as many
instructions. it looks to me that sin and cos don't differ in more than
4 operations, so CSE should have eliminated most of it.
either way it's not going to be more efficient than this patch.LGTM
Thanks for the review. I'm guessing that the CSE isn't recognizing the pattern correctly... And then doing the sqrt, sub, and mul ends up being cheap in comparison to another sin or cos operation
--Aaron