From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43204 invoked by alias); 6 Sep 2017 20:34:16 -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 41579 invoked by uid 89); 6 Sep 2017 20:34:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=Hx-languages-length:2356, H*u:6.3, H*UA:6.3, HContent-Transfer-Encoding:8bit X-HELO: userp1040.oracle.com Subject: Re: [PATCH] improves exp() and expf() performance on Sparc. To: Joseph Myers Cc: libc-alpha@sourceware.org References: <1504306749-46787-1-git-send-email-patrick.mcgehearty@oracle.com> From: Patrick McGehearty Message-ID: Date: Wed, 06 Sep 2017 20:34:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2017-09/txt/msg00275.txt.bz2 On 9/1/2017 6:13 PM, Joseph Myers wrote: > You're defining ifuncs for exp and expf (rather than __ieee754_exp, > __exp_finite etc. as on x86_64). But you're not doing anything to stop > the w_exp_compat / w_expf_compat wrappers from being built that also > define exp and expf, so I don't see how that can work without ending up > with multiple definitions of exp and expf; I'd expect you to need to > override the wrappers with empty files in such a case of a function > implementation with all the error handling integrated. The sysdeps/ieee754/dbl-64/w_exp_compat.c declares __exp (double x) and then adds: hidden_def (__exp) weak_alias (__exp, exp) I believe the weak_alias in w_exp_compat.c is overriden by the sparc_libm_ifunc in e_exp-generic.c.  At least, I am not seeing any link time errors about double exp declarations and I am seeing the new code being executed (as proved by the speed and accuracy changes). > > I'm also concerned that you have local matherr handling which is not > compatible with all the cases in __kernel_standard (which are not well > tested). If you need to have your own integrated error handling for > performance reasons, matherr handling should be bug-compatible with the > existing code, for both overflow and underflow. (Or define a new symbol > version, make the existing exp and expf into compat symbols for SPARC and > then your new version only needs to handle errno setting, not matherr.) > As for error handling, I believe the extra level of indirection on return from exp provided by the sysdeps/ieee754/dbl-64/w_exp_compat.c routine is an anti-performance design. Every normal return from e_exp requires an extra level of indirection and several tests/branches that should be bypassed unless an error is detected. The e_exp code already checks for underflow/overflow/NAN/etc. Those unnecessary tests for the non-exceptional cases add overhead for the most frequent uses of exp(). On the other hand, once an error occurs that triggers__set_err_exp, extra overhead is not a significant performance issue. I will add a call __kernel_standard in that routine when _LIB_VERSION is not _IEEE_ or _POSIX_. As you point out, that will provide more consistent error handling behavior. I'll make that change in my next exp patch submission. - patrick