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

[Wilco Dijkstra <Wilco.Dijkstra@arm.com>]

Ondřej Bílka wrote:
> On Fri, Mar 09, 2018 at 09:47:09PM +0530, Siddhesh Poyarekar wrote:
>> On Friday 09 March 2018 09:16 PM, Wilco Dijkstra wrote:
>> > This patch removes 2nd of the 3 range reduction cases and defer to the final one.
>> > Input values above 2^27 are extremely rare, so this case doesn't need to as be
>> > optimized as smaller inputs.
>> 
>> Please elaborate on two more points:
>> 
>> - The basis for your claim that values above 2^27 are extremely rare,
>>   i.e. papers that you may have referred that conclude this and/or
>>   applications you may have profiled
>> 
> Main reason is that for these inputs accuracy doesn't make a sense.
> There is already millions bigger error caused by limited precision when
> rounding input to float.

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.

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.

Basically I can only see a reason for 3 levels of range reduction if the first range reduction
only handles a tiny range (say up to 10), the final range reduction is extremely slow and we
can find proof of an application relying on fast range reduction up to say 1000 or 1000000.

I think a 3-4x speedup of __branred should be feasible using a much simpler algorithm and
integer arithmetic. This will not only cover a much wider range but then we will also be sure
it works correctly!

Wilco