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

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: Tue, 09 Sep 2014 01:14:00 +0200
Subject: Re: [PATCH] __builtin_expect cleanup for iconvdata/*.c
Authentication-results: sourceware.org; auth=none
References: <540E06B0 dot 50406 at redhat dot com> <87sik14ydx dot fsf at Niukka dot kon dot iki dot fi>

On 09/08/2014 11:04 PM, Kalle Olavi Niemitalo wrote:

Florian Weimer <fweimer@redhat.com> writes:

--- a/iconvdata/iso-2022-cn.c
+++ b/iconvdata/iso-2022-cn.c
@@ -126,22 +126,22 @@ enum
        STANDARD_FROM_LOOP_ERR_HANDLER (1);				      \
  									      \
      /* Recognize escape sequences.  */					      \
-    if (__builtin_expect (ch, 0) == ESC)				      \
+    if (__glibc_unlikely (ch) == ESC)					      \


Shouldn't that be __glibc_unlikely (ch == ESC)?
I guess it makes no difference in practice, but it looks wrong.
There are other changes like that, for example:

-	if (__builtin_expect (found, 1) != __UNKNOWN_10646_CHAR)	      \
+	if (__glibc_likely (found) != __UNKNOWN_10646_CHAR)		      \

Right, these changes are technically correct (they are un-expanding themacro definitions), but they are even uglier than the original code.I'll work on a regenerated patch with additional heuristics to cover this.


--
Florian Weimer / Red Hat Product Security

References:
- [PATCH] __builtin_expect cleanup for iconvdata/*.c
  - From: Florian Weimer
- Re: [PATCH] __builtin_expect cleanup for iconvdata/*.c
  - From: Kalle Olavi Niemitalo

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