This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] __builtin_expect cleanup for iconvdata/*.c
- From: Kalle Olavi Niemitalo <kon at iki dot fi>
- To: libc-alpha <libc-alpha at sourceware dot org>
- Date: Mon, 08 Sep 2014 23:11:45 +0300
- Subject: Re: [PATCH] __builtin_expect cleanup for iconvdata/*.c
- Authentication-results: sourceware.org; auth=none
- References: <540E06B0 dot 50406 at redhat dot com>
Florian Weimer <fweimer@redhat.com> writes:
> --- a/iconvdata/8bit-generic.c
> +++ b/iconvdata/8bit-generic.c
> @@ -68,7 +68,7 @@
> { \
> uint32_t ch = get32 (inptr); \
> \
> - if (__builtin_expect (ch >= sizeof (from_ucs4) / sizeof (from_ucs4[0]), 0)\
> + if (__glibc_unlikely (ch >= sizeof (from_ucs4) / sizeof (from_ucs4[0])) \
> || (__builtin_expect (from_ucs4[ch], '\1') == '\0' && ch != 0)) \
Why keep the second __builtin_expect?
> diff --git a/iconvdata/gbk.c b/iconvdata/gbk.c
> index b1a7719..e27312e 100644
> --- a/iconvdata/gbk.c
> +++ b/iconvdata/gbk.c
> @@ -13171,7 +13171,7 @@ static const char __gbk_from_ucs4_tab12[][2] =
> /* All second bytes of a multibyte character must be >= 0x40, and \
> the __gbk_to_ucs table only covers the range up to 0xfe 0xa0. */ \
> if (__builtin_expect (ch2 < 0x40, 0) \
> - || (__builtin_expect (ch, 0x81) == 0xfe && ch2 > 0xa0)) \
> + || (__glibc_unlikely (ch == 0xfe && ch2 > 0xa0))) \
And the first one here. Also, the ch2 > 0xa0 test wasn't inside
__builtin_expect originally, but I don't know what the
probabilities are.
In 0001-Manual-part-of-iconvdata-__builtin_expect-cleanup.patch,
I didn't find any mistakes where an originally likely test would
have turned unlikely or vice versa.