Re: [PATCH] Do not provide inlines for GCC build-in functions

[Andreas Jaeger <aj at suse dot de>]

On Friday, May 18, 2012 06:12:25 Carlos O'Donell wrote:
> On Tue, May 15, 2012 at 3:36 PM, Andreas Jaeger <aj@suse.de> wrote:
> > As mentioned previously
> > (http://sourceware.org/ml/libc-alpha/2012-05/msg00106.html), GCC 3.4
> > introduced builtin functions and thus the inlines are not needed at
> > all.
> > 
> > Patch was tested on Linux/x86-64 and I compared libc.so and libm.so
> > and found no difference in the assembly.
> > 
> > Looking at math/test-idouble I found a couple of places where GCC used
> > now the builtins - instead of a function call like previously. So, GCC
> > does a better job than the inlines.
> > 
> > Ok to commit?
> > 
> > Andreas
> > 
> > 2012-05-15  Andreas Jaeger  <aj@suse.de>
> > 
> >        * sysdeps/x86_64/fpu/bits/mathinline.h (lrintf): Make inline
> >        function only available for GCCs before 3.4 since GCC 3.4
> >        introduced a builtin.
> >        (lrint): Likewise.
> >        (llrintf): Likewise.
> >        (llrint): Likewise.
> >        (fmaxf): Likewise.
> >        (fmax): Likewise.
> >        (fminf): Likewise.
> >        (fmin): Likewise.
> >        (rint): Likewise.
> >        (rintf): Likewise.
> >        (nearbyint): Likewise.
> >        (nearbyintf): Likewise.
> >        (ceil): Likewise.
> >        (ceilf): Likewise.
> >        (floor): Likewise.
> >        (floorf): Likewise.
> > 
> > diff --git a/sysdeps/x86_64/fpu/bits/mathinline.h
> > b/sysdeps/x86_64/fpu/bits/mathinline.h index 49a199b..310fcad 100644
> > --- a/sysdeps/x86_64/fpu/bits/mathinline.h
> > +++ b/sysdeps/x86_64/fpu/bits/mathinline.h
> > @@ -68,7 +68,10 @@ __END_NAMESPACE_C99
> >  #endif
> > 
> > 
> > -#if __GNUC_PREREQ (2, 8) && !defined __NO_MATH_INLINES && defined
> > __OPTIMIZE__ +#if __GNUC_PREREQ (2, 8) && !__GNUC_PREREQ (3, 4) \
> > +    && !defined __NO_MATH_INLINES && defined __OPTIMIZE__
> > +/* GCC 3.4 introduced builtins for all functions below, so
> > +   there's no need to define any of these inline functions.  */
> > 
> >  # ifdef __USE_ISOC99
> >  __BEGIN_NAMESPACE_C99
> 
> Looks good to me.
> 
> I double checked that GCC 3.3 doesn't have the builtins and GCC 3.4
> does.
> 
> I find it a bit lame that we go from a math inline to a builtin behind
> the users back, but this seems to be common practice, and I don't see
> a better solution.

As Uros commented in gcc.gnu.org/bugzilla/show_bug.cgi?id=53190 "As a side 
note, mathinline.h really needs some serious TLC, there are many
functions that are much better implemented with gcc builtin functions, not 
to
mention that x87 asm in 32bit case interferes *badly* with -mfpmath=sse.
"

GCC can do a better job optimization the call if a builtin exists, so let's 
use it.

I'll commit this now,

Thanks for the review,
Andreas
-- 
 Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
  SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
   GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
    GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126