From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49761 invoked by alias); 9 Mar 2018 19:31:07 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 49752 invoked by uid 89); 9 Mar 2018 19:31:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy= X-HELO: homiemail-a82.g.dreamhost.com Subject: Re: [PATCH 2/6] Remove slow paths from sin/cos To: Wilco Dijkstra , =?UTF-8?B?T25kxZllaiBCw61sa2E=?= Cc: "libc-alpha@sourceware.org" , nd References: <8065ca77-ec8f-9925-7e9c-52266cd1e4c6@gotplt.org> <20180309181902.GA26430@domone> From: Siddhesh Poyarekar Message-ID: <41b81cbf-b27a-07c2-1945-4c201327f162@gotplt.org> Date: Fri, 09 Mar 2018 19:31:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=iso-8859-2 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2018-03/txt/msg00254.txt.bz2 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 w= as around 1000.=20 > 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 sig= nificant 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 __bra= nred 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