This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Use __glibc_(un)likely instead __builtin_expect.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Mike Frysinger <vapier at gentoo dot org>
- Cc: libc-alpha at sourceware dot org, Siddhesh Poyarekar <siddhesh at redhat dot com>
- Date: Mon, 10 Feb 2014 15:38:42 +0100
- Subject: Re: [PATCH] Use __glibc_(un)likely instead __builtin_expect.
- Authentication-results: sourceware.org; auth=none
- References: <20131022220131 dot GA30971 at domone dot podge> <20131028071843 dot GH1633 at spoyarek dot pnq dot redhat dot com> <20131204094911 dot GA24810 at domone dot podge> <15487070 dot EJd1aTKqoS at vapier>
On Sat, Feb 08, 2014 at 09:42:57AM -0500, Mike Frysinger wrote:
> On Wednesday, December 04, 2013 10:49:11 OndÅej BÃlka wrote:
> > On Mon, Oct 28, 2013 at 12:48:43PM +0530, Siddhesh Poyarekar wrote:
> > > On Wed, Oct 23, 2013 at 12:01:31AM +0200, OndÅej BÃlka wrote:
> > > > Hi,
> > > >
> > > > Now I return to one of todo-list issues which is using
> > > > glibc-likely/unlikely.
> > > >
> > > > First comes a easy case which can be expressed as following script.
> > > >
> > > > cat $1 | sed -e "s/if (__builtin_expect (\(.*\), 0))/if
> > > > (__glibc_unlikely (\1))/" | sed -e "s/if (__builtin_expect (\(.*\),
> > > > 1))/if (__glibc_likely (\1))/">
> > > Based on Roland's comment, I did some automated verification of the
> > > patch. I found the following problems:
> > >
> > > 1. Changes in whitespace in macro definitions
> > > 2. Changes in whitespace in malloc routines
> > >
> > > Could you fix these and repost?
> >
> > I reposted patch but sourceware thinks its spam,
> >
> > To avoid that use following link:
> >
> > http://kam.mff.cuni.cz/~ondra/libc_likely.patch
>
> the ChangeLog entries need line wrapping
>
> i did a scan of the diff and it looked fine. i did notice some entries with
> redundant paren, but it's not entirely something you introduced:
> - else if (__builtin_expect ((a < 0.0), 1))
> + else if (__glibc_likely ((a < 0.0)))
>
> so if it compiled fine, LGTM
> -mike
Yes it compiled correctly. I ran a script again to fix midair collisions
which are:
diff --git a/csu/libc-start.c b/csu/libc-start.c
index 3b7092b..f8b14b7 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -231,13 +231,13 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
the standard file descriptors are not opened. We have to do this
only for statically linked applications since otherwise the dynamic
loader did the work already. */
- if (__builtin_expect (__libc_enable_secure, 0))
+ if (__glibc_unlikely (__libc_enable_secure))
__libc_check_standard_fds ();
#endif
/* Call the initializer of the program, if any. */
#ifdef SHARED
- if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS, 0))
+ if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS))
GLRO(dl_debug_printf) ("\ninitialize program: %s\n\n", argv[0]);
#endif
if (init)
diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index c18b888..727dcef 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -189,7 +189,7 @@ __libc_setup_tls (size_t tcbsize, size_t tcbalign)
#else
# error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
#endif
- if (__builtin_expect (lossage != NULL, 0))
+ if (__glibc_unlikely (lossage != NULL))
__libc_fatal (lossage);
/* Update the executable's link map with enough information to make
diff --git a/debug/stpncpy_chk.c b/debug/stpncpy_chk.c
index 17e6d95..4579af6 100644
--- a/debug/stpncpy_chk.c
+++ b/debug/stpncpy_chk.c
@@ -25,7 +25,7 @@
char *
__stpncpy_chk (char *dest, const char *src, size_t n, size_t destlen)
{
- if (__builtin_expect (destlen < n, 0))
+ if (__glibc_unlikely (destlen < n))
__chk_fail ();
return __stpncpy (dest, src, n);
diff --git a/debug/strncpy_chk.c b/debug/strncpy_chk.c
index 4c94ce5..5bd127e 100644
--- a/debug/strncpy_chk.c
+++ b/debug/strncpy_chk.c
@@ -26,7 +26,7 @@ __strncpy_chk (s1, s2, n, s1len)
size_t n;
size_t s1len;
{
- if (__builtin_expect (s1len < n, 0))
+ if (__glibc_unlikely (s1len < n))
__chk_fail ();
return strncpy (s1, s2, n);
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 6501ff2..e8893b8 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -2243,7 +2243,7 @@ _dl_map_object (struct link_map *loader, const char *name,
/* If the loader has the DF_1_NODEFLIB flag set we must not
use a cache entry from any of these directories. */
- if (__builtin_expect (l->l_flags_1 & DF_1_NODEFLIB, 0))
+ if (__glibc_unlikely (l->l_flags_1 & DF_1_NODEFLIB))
{
const char *dirp = system_dirs;
unsigned int cnt = 0;
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index d8cdb7e..57b9069 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -243,7 +243,7 @@ _dl_sysdep_start (void **start_argptr,
/* If this is a SUID program we make sure that FDs 0, 1, and 2 are
allocated. If necessary we are doing it ourself. If it is not
possible we stop the program. */
- if (__builtin_expect (INTUSE(__libc_enable_secure), 0))
+ if (__glibc_unlikely (INTUSE(__libc_enable_secure)))
__libc_check_standard_fds ();
(*dl_main) (phdr, phnum, &user_entry, GLRO(dl_auxv));
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index 9454d06..97444d8 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -339,7 +339,7 @@ _dl_allocate_tls_storage (void)
/* Allocate a correctly aligned chunk of memory. */
result = __libc_memalign (GL(dl_tls_static_align), size);
- if (__builtin_expect (result != NULL, 1))
+ if (__glibc_likely (result != NULL))
{
/* Allocate the DTV. */
void *allocated = result;
diff --git a/elf/rtld.c b/elf/rtld.c
index aa50cab..6aecb55 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2650,7 +2650,7 @@ process_envvars (enum mode *modep)
/* Extra security for SUID binaries. Remove all dangerous environment
variables. */
- if (__builtin_expect (INTUSE(__libc_enable_secure), 0))
+ if (__glibc_unlikely (INTUSE(__libc_enable_secure)))
{
static const char unsecure_envvars[] =
#ifdef EXTRA_UNSECURE_ENVVARS
diff --git a/iconvdata/iso-2022-jp-3.c b/iconvdata/iso-2022-jp-3.c
index b676aa1..fbca949 100644
--- a/iconvdata/iso-2022-jp-3.c
+++ b/iconvdata/iso-2022-jp-3.c
@@ -721,7 +721,7 @@ static const struct
{ \
if (set != JISX0201_Kana_set) \
{ \
- if (__builtin_expect (outptr + 3 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 3 > outend)) \
{ \
result = __GCONV_FULL_OUTPUT; \
break; \
diff --git a/iconvdata/iso-2022-jp.c b/iconvdata/iso-2022-jp.c
index 3428c32..d29c6e3 100644
--- a/iconvdata/iso-2022-jp.c
+++ b/iconvdata/iso-2022-jp.c
@@ -731,7 +731,7 @@ static const cvlist_t conversion_lists[4] =
{ \
if (set2 != ISO88591_set) \
{ \
- if (__builtin_expect (outptr + 3 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 3 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
@@ -781,7 +781,7 @@ static const cvlist_t conversion_lists[4] =
set2 = ISO88597_set; \
} \
\
- if (__builtin_expect (outptr + 3 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 3 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
@@ -806,7 +806,7 @@ static const cvlist_t conversion_lists[4] =
{ \
if (set != JISX0201_Roman_set) \
{ \
- if (__builtin_expect (outptr + 3 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 3 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
@@ -833,7 +833,7 @@ static const cvlist_t conversion_lists[4] =
{ \
if (set != JISX0208_1983_set) \
{ \
- if (__builtin_expect (outptr + 3 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 3 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
@@ -865,7 +865,7 @@ static const cvlist_t conversion_lists[4] =
{ \
if (set != JISX0212_set) \
{ \
- if (__builtin_expect (outptr + 4 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 4 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
@@ -899,7 +899,7 @@ static const cvlist_t conversion_lists[4] =
{ \
if (set != GB2312_set) \
{ \
- if (__builtin_expect (outptr + 3 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 3 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
@@ -932,7 +932,7 @@ static const cvlist_t conversion_lists[4] =
{ \
if (set != KSC5601_set) \
{ \
- if (__builtin_expect (outptr + 4 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 4 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
@@ -968,7 +968,7 @@ static const cvlist_t conversion_lists[4] =
{ \
if (set != JISX0201_Kana_set) \
{ \
- if (__builtin_expect (outptr + 3 > outend, 0)) \
+ if (__glibc_unlikely (outptr + 3 > outend)) \
{ \
res = __GCONV_FULL_OUTPUT; \
break; \
diff --git a/nptl/sysdeps/pthread/setxid.h b/nptl/sysdeps/pthread/setxid.h
index 76c88e0..4798735 100644
--- a/nptl/sysdeps/pthread/setxid.h
+++ b/nptl/sysdeps/pthread/setxid.h
@@ -32,7 +32,7 @@
# define INLINE_SETXID_SYSCALL(name, nr, args...) \
({ \
int __result; \
- if (__builtin_expect (__libc_pthread_functions_init, 0)) \
+ if (__glibc_unlikely (__libc_pthread_functions_init)) \
{ \
struct xid_command __cmd; \
__cmd.syscall_no = __NR_##name; \
diff --git a/nptl/sysdeps/unix/sysv/linux/fork.c b/nptl/sysdeps/unix/sysv/linux/fork.c
index 961fc8a..45ad36c 100644
--- a/nptl/sysdeps/unix/sysv/linux/fork.c
+++ b/nptl/sysdeps/unix/sysv/linux/fork.c
@@ -162,12 +162,12 @@ __libc_fork (void)
self->robust_head.futex_offset since we inherit the correct
value from the parent. */
# ifdef SHARED
- if (__builtin_expect (__libc_pthread_functions_init, 0))
+ if (__glibc_unlikely (__libc_pthread_functions_init))
PTHFCT_CALL (ptr_set_robust, (self));
# else
extern __typeof (__nptl_set_robust) __nptl_set_robust
__attribute__((weak));
- if (__builtin_expect (__nptl_set_robust != NULL, 0))
+ if (__glibc_unlikely (__nptl_set_robust != NULL))
__nptl_set_robust (self);
# endif
#endif
diff --git a/stdio-common/printf_fp.c b/stdio-common/printf_fp.c
index 9cd4b4b..884e515 100644
--- a/stdio-common/printf_fp.c
+++ b/stdio-common/printf_fp.c
@@ -896,7 +896,7 @@ ___printf_fp (FILE *fp,
}
size_t wbuffer_to_alloc = (2 + chars_needed) * sizeof (wchar_t);
buffer_malloced = ! __libc_use_alloca (wbuffer_to_alloc);
- if (__builtin_expect (buffer_malloced, 0))
+ if (__glibc_unlikely (buffer_malloced))
{
wbuffer = (wchar_t *) malloc (wbuffer_to_alloc);
if (wbuffer == NULL)
diff --git a/stdlib/putenv.c b/stdlib/putenv.c
index e6ad267..4aa2185 100644
--- a/stdlib/putenv.c
+++ b/stdlib/putenv.c
@@ -59,7 +59,7 @@ putenv (string)
char *name;
#ifdef _LIBC
int use_malloc = !__libc_use_alloca (name_end - string + 1);
- if (__builtin_expect (use_malloc, 0))
+ if (__glibc_unlikely (use_malloc))
{
name = strndup (string, name_end - string);
if (name == NULL)
diff --git a/stdlib/setenv.c b/stdlib/setenv.c
index e244e18..94c603a 100644
--- a/stdlib/setenv.c
+++ b/stdlib/setenv.c
@@ -170,7 +170,7 @@ __add_to_environ (name, value, combined, replace)
#ifdef USE_TSEARCH
char *new_value;
int use_alloca = __libc_use_alloca (varlen);
- if (__builtin_expect (use_alloca, 1))
+ if (__glibc_likely (use_alloca))
new_value = (char *) alloca (varlen);
else
{
diff --git a/sysdeps/powerpc/power7/fpu/s_logb.c b/sysdeps/powerpc/power7/fpu/s_logb.c
index 8e324d7..61d5729 100644
--- a/sysdeps/powerpc/power7/fpu/s_logb.c
+++ b/sysdeps/powerpc/power7/fpu/s_logb.c
@@ -35,7 +35,7 @@ __logb (double x)
{
double ret;
- if (__builtin_expect (x == 0.0, 0))
+ if (__glibc_unlikely (x == 0.0))
/* Raise FE_DIVBYZERO and return -HUGE_VAL[LF]. */
return -1.0 / __builtin_fabs (x);
@@ -47,10 +47,10 @@ __logb (double x)
: "f" (x), "f" (mask.d));
/* ret = (ret >> 52) - 1023.0; */
ret = (ret * two1div52) + two10m1;
- if (__builtin_expect (ret > -two10m1, 0))
+ if (__glibc_unlikely (ret > -two10m1))
/* Multiplication is used to set logb (+-INF) = INF. */
return (x * x);
- else if (__builtin_expect (ret == two10m1, 0))
+ else if (__glibc_unlikely (ret == two10m1))
{
/* POSIX specifies that denormal numbers are treated as
though they were normalized. */
diff --git a/sysdeps/powerpc/power7/fpu/s_logbf.c b/sysdeps/powerpc/power7/fpu/s_logbf.c
index bf68c0e..ba75c22 100644
--- a/sysdeps/powerpc/power7/fpu/s_logbf.c
+++ b/sysdeps/powerpc/power7/fpu/s_logbf.c
@@ -37,7 +37,7 @@ __logbf (float x)
/* VSX operation are all done internally as double. */
double ret;
- if (__builtin_expect (x == 0.0, 0))
+ if (__glibc_unlikely (x == 0.0))
/* Raise FE_DIVBYZERO and return -HUGE_VAL[LF]. */
return -1.0 / __builtin_fabsf (x);
@@ -49,7 +49,7 @@ __logbf (float x)
: "f" (x), "f" (mask.d));
/* ret = (ret >> 52) - 1023.0, since ret is double. */
ret = (ret * two1div52) + two10m1;
- if (__builtin_expect (ret > -two7m1, 0))
+ if (__glibc_unlikely (ret > -two7m1))
/* Multiplication is used to set logb (+-INF) = INF. */
return (x * x);
/* Since operations are done with double we don't need
diff --git a/sysdeps/powerpc/power7/fpu/s_logbl.c b/sysdeps/powerpc/power7/fpu/s_logbl.c
index 251e99d..661e3ca 100644
--- a/sysdeps/powerpc/power7/fpu/s_logbl.c
+++ b/sysdeps/powerpc/power7/fpu/s_logbl.c
@@ -38,7 +38,7 @@ __logbl (long double x)
double xh;
double ret;
- if (__builtin_expect (x == 0.0L, 0))
+ if (__glibc_unlikely (x == 0.0L))
/* Raise FE_DIVBYZERO and return -HUGE_VAL[LF]. */
return -1.0L / __builtin_fabsl (x);
@@ -51,10 +51,10 @@ __logbl (long double x)
: "f" (xh), "f" (mask.d));
/* ret = (ret >> 52) - 1023.0; */
ret = (ret * two1div52) + two10m1;
- if (__builtin_expect (ret > -two10m1, 0))
+ if (__glibc_unlikely (ret > -two10m1))
/* Multiplication is used to set logb (+-INF) = INF. */
return (xh * xh);
- else if (__builtin_expect (ret == two10m1, 0))
+ else if (__glibc_unlikely (ret == two10m1))
{
/* POSIX specifies that denormal number is treated as
though it were normalized. */
diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
index bc99183..35177c5 100644
--- a/sysdeps/powerpc/powerpc64/dl-machine.h
+++ b/sysdeps/powerpc/powerpc64/dl-machine.h
@@ -453,7 +453,7 @@ ppc64_local_entry_offset (struct link_map *map, lookup_t sym_map,
/* If the target function is an ifunc then the local entry offset is
for the resolver, not the final destination. */
- if (__builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC, 0))
+ if (__glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC))
return 0;
return PPC64_LOCAL_ENTRY_OFFSET (sym->st_other);