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: Florian Weimer <fweimer at redhat dot com>
- To: Kalle Olavi Niemitalo <kon at iki dot fi>, libc-alpha <libc-alpha at sourceware dot org>
- Date: Mon, 08 Sep 2014 22:43:25 +0200
- Subject: Re: [PATCH] __builtin_expect cleanup for iconvdata/*.c
- Authentication-results: sourceware.org; auth=none
- References: <540E06B0 dot 50406 at redhat dot com> <87wq9d50u6 dot fsf at Niukka dot kon dot iki dot fi>
On 09/08/2014 10:11 PM, Kalle Olavi Niemitalo wrote:
Why keep the second __builtin_expect?
> And the first one here.
The first patch only contains the changes which could not reasonably be
automated with a short script.
- || (__builtin_expect (ch, 0x81) == 0xfe && ch2 > 0xa0)) \
+ || (__glibc_unlikely (ch == 0xfe && ch2 > 0xa0))) \
Also, the ch2 > 0xa0 test wasn't inside
__builtin_expect originally, but I don't know what the
probabilities are.
Yes, I should probably undo this change. I made it deliberately because
I thought that the intent was to mark any reason to enter the error
condition as unlikely, but then I'd have to wrap the entire if condition
for consistency.
Note that is quite unlikely that current GCC uses __builtin_expect hints
in the same way as it did when this code was initially written, and as
I've seen during my binary diffing, the whole mechanism is quite brittle
for such complex conditions.
--
Florian Weimer / Red Hat Product Security