This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix up strtod_l.c for -O0
On 02/19/2013 11:13 AM, Jakub Jelinek wrote:
> On Tue, Feb 19, 2013 at 09:50:20AM -0500, Carlos O'Donell wrote:
>> * 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.
>
> Here it is as a macro without inline helpers.
>
> If I remove the
> Implemented as a macro, so that __builtin_constant_p works even at -O0.
> and following empty line in comment, so that line numbers match, then in
> the whole libc.so the difference is on x86_64 just:
> - 3fa9d: 40 3a 33 cmp (%rbx),%sil
> + 3fa9d: 40 38 33 cmp %sil,(%rbx)
> in a single spot. Of course with the added two lines there are various
> differences, because __assert_fail arguments differ (wasn't doing non-debug
> build).
>
> 2013-02-19 Jakub Jelinek <jakub@redhat.com>
>
> * stdlib/strtod_l.c (__mpn_lshift_1): Rewritten as function-like
> macro.
>
> diff --git a/stdlib/strtod_l.c b/stdlib/strtod_l.c
> index 5959354..47247b5 100644
> --- a/stdlib/strtod_l.c
> +++ b/stdlib/strtod_l.c
> @@ -444,28 +444,30 @@ str_to_mpn (const STRING_TYPE *str, int digcnt, mp_limb_t *n, mp_size_t *nsize,
> /* Shift {PTR, SIZE} COUNT bits to the left, and fill the vacated bits
> with the COUNT most significant bits of LIMB.
>
> - Tege doesn't like this function so I have to write it here myself. :)
> + Implemented as a macro, so that __builtin_constant_p works even at -O0.
> +
> + Tege doesn't like this macro so I have to write it here myself. :)
> --drepper */
> -static inline void
> -__attribute ((always_inline))
> -__mpn_lshift_1 (mp_limb_t *ptr, mp_size_t size, unsigned int count,
> - mp_limb_t limb)
> -{
> - if (__builtin_constant_p (count) && count == BITS_PER_MP_LIMB)
> - {
> - /* Optimize the case of shifting by exactly a word:
> - just copy words, with no actual bit-shifting. */
> - mp_size_t i;
> - for (i = size - 1; i > 0; --i)
> - ptr[i] = ptr[i - 1];
> - ptr[0] = limb;
> - }
> - else
> - {
> - (void) __mpn_lshift (ptr, ptr, size, count);
> - ptr[0] |= limb >> (BITS_PER_MP_LIMB - count);
> - }
> -}
> +#define __mpn_lshift_1(ptr, size, count, limb) \
> + do \
> + { \
> + mp_limb_t *__ptr = (ptr); \
> + if (__builtin_constant_p (count) && count == BITS_PER_MP_LIMB) \
> + { \
> + mp_size_t i; \
> + for (i = (size) - 1; i > 0; --i) \
> + __ptr[i] = __ptr[i - 1]; \
> + __ptr[0] = (limb); \
> + } \
> + else \
> + { \
> + /* We assume count > 0 && count < BITS_PER_MP_LIMB here. */ \
> + unsigned int __count = (count); \
> + (void) __mpn_lshift (__ptr, __ptr, size, __count); \
> + __ptr[0] |= (limb) >> (BITS_PER_MP_LIMB - __count); \
> + } \
> + } \
> + while (0)
>
>
> #define INTERNAL(x) INTERNAL1(x)
This looks perfect and I don't think anyone can argue against this.
Please check this in.
Cheers,
Carlos.