Re: [PATCH 2/6] Remove slow paths from sin/cos

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On Saturday 10 March 2018 12:36 AM, Wilco Dijkstra wrote:
> Exactly, it's rare for sin/cos input to be > 10, largest I've ever seen was around 1000. 
> There is a case to be made for making wildly out or range inputs an error or just return 0.0
> rather than using insanely accurate range reduction (the result is going to be equally wrong
> either way), but that's a different discussion.

To be clear, I do not object to the patchset, it is the right way
forward.  I would like to see clearer documentation as to why we decided
to drop this path in this patch and what the impact of that is.  In that
sense, it would be nice to have a better rationale in the commit log
than "I've never seen inputs that big".

> Fast range reduction of up to 2^27 is a huge overkill, we could get a significant speedup by
> reducing this range.
>
> The 2nd level range reduction (2^27 till 2^48) is so pointless that there is absolutely no
> reason keep it. There is about a 2.3x slowdown in this range due to __branred being
> braindead slow.

Thanks, all this should be part of the commit log.  However, from what I
understand, this particular patch will result in a regression (since
IIRC reduce_and_compute is slower) but the following patch will fix that
and improve things.

Siddhesh