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] __builtin_expect cleanup for iconvdata/*.c


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.


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