This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix up strtod_l.c for -O0


On 02/19/2013 09:27 AM, Carlos O'Donell wrote:
> On 02/19/2013 08:18 AM, Jakub Jelinek wrote:
>> On Tue, Feb 19, 2013 at 08:02:06AM -0500, Carlos O'Donell wrote:
>>> No, we should fix this here.
>>>
>>> I think a macro and two helper functions, as you have done below, is
>>> a good solution and keeps the code from bit-rotting.
>>>
>>> No inlines looks like it would be worse code.
>>
>> Why?  macro vs. inline function should be generally the same code quality if
>> the inline function is inlined, or perhaps macro could be slightly better if
>> early optimizations before inlining would make a difference.
>> But as it is always_inline, it generally should be inlined during early
>> inlining, so the difference should be minimal.
> 
> The macro makes the code available earlier for optimization, that's
> all, and I agree that the difference is likely minimal. Thus all things
> considered I prefer a macro.
> 
> Is there any reason not to use a macro?

Just to clarify:

* The number one goal is to avoid any serious performance regressions.

* The number two goal is avoid divergence between copied sources.

* I'm OK with a macro and two inline helper functions as implemented
  in your patch. I'm happy to see that your testing with libquadmath
  shows that this likely produces no measurable performance difference
  in the generated code. Before committing this patch I'd like you to 
  verify that is also the case for glibc.

* I have a slight preference for making the entire function into a macro.
  However, I have no evidence that this would generate higher performing
  code in this case.

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]