Convert __builtin_expect to __glibc_(un)likely throughout elf/.

Message ID 20180602220831.25016-1-zackw@panix.com
State Superseded
Headers

Commit Message

Zack Weinberg June 2, 2018, 10:08 p.m. UTC
  In the previous version of the patch to use exclusively _nocancel
I/O in ld.so, I didn't change a use of __builtin_expect to
__glibc_(un)likely that happened to be right next to the code I
was actually working on; Andreas asked why not, and I said that
there were a bunch more uses of __builtin_expect in that file and
I would convert them all in a separate patch.  This is that patch,
converting all of elf/.  It may have textual dependencies on the
_nocancel patch but it is logically independent,

I would appreciate someone checking over this patch very carefully
indeed.  In a whole bunch of places, __builtin_expect was used in a
way that did not map mechanically to the newer convention.  The test
suite caught one mistake; I'm not confident I didn't make any more.

Writing this patch gave me a deeper appreciation for why we _want_ to
switch to __glibc_(un)likely, but also for how difficult it can be.
I now think that people should _not_ be asked to change existing uses
of __builtin_expect in a patch whose purpose is something else.

	* elf/dl-addr.c, elf/dl-cache.c, elf/dl-close.c, elf/dl-deps.c
	* elf/dl-environ.c, elf/dl-error-skeleton.c, elf/dl-fini.c
	* elf/dl-fptr.c, elf/dl-init.c, elf/dl-libc.c, elf/dl-minimal.c
	* elf/dl-open.c, elf/dl-reloc.c, elf/dl-runtime.c
	* elf/dl-sysdep.c, elf/dl-tls.c, elf/dl-version.c, elf/ldconfig.c
	* elf/rtld.c: Replace __builtin_expect with __glibc_likely or
	__glibc_unlikely, as appropriate, throughout.
---
 elf/dl-addr.c           |  2 +-
 elf/dl-cache.c          |  2 +-
 elf/dl-close.c          |  8 ++++----
 elf/dl-deps.c           | 14 +++++++-------
 elf/dl-environ.c        |  2 +-
 elf/dl-error-skeleton.c | 10 +++++-----
 elf/dl-fini.c           |  6 +++---
 elf/dl-fptr.c           |  4 ++--
 elf/dl-init.c           |  6 +++---
 elf/dl-libc.c           |  2 +-
 elf/dl-minimal.c        |  2 +-
 elf/dl-open.c           |  4 ++--
 elf/dl-reloc.c          | 11 +++++------
 elf/dl-runtime.c        | 14 +++++++-------
 elf/dl-sysdep.c         |  6 +++---
 elf/dl-tls.c            |  2 +-
 elf/dl-version.c        | 11 +++++------
 elf/ldconfig.c          |  4 ++--
 elf/rtld.c              | 19 +++++++++----------
 19 files changed, 63 insertions(+), 66 deletions(-)
  

Comments

Joseph Myers June 4, 2018, 2:36 p.m. UTC | #1
On Sat, 2 Jun 2018, Zack Weinberg wrote:

> I would appreciate someone checking over this patch very carefully
> indeed.  In a whole bunch of places, __builtin_expect was used in a
> way that did not map mechanically to the newer convention.  The test
> suite caught one mistake; I'm not confident I didn't make any more.

I think this means there is a case for splitting this patch up into a part 
that doesn't change installed stripped shared libraries (on one platform - 
so at least the sense of comparisons etc. is unchanged on that platform 
after the patch, even if likely/unlikely could be wrong sometimes without 
being detected by such a comparison of installed stripped shared 
libraries), and a part that does (presumably because the mapping isn't 
purely mechanical so compiler IR changes enough to result in different 
code generated), with the latter needing more careful review that no 
semantics have changed.
  
Zack Weinberg June 4, 2018, 3:33 p.m. UTC | #2
On Mon, Jun 4, 2018 at 10:36 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sat, 2 Jun 2018, Zack Weinberg wrote:
>
>> I would appreciate someone checking over this patch very carefully
>> indeed.  In a whole bunch of places, __builtin_expect was used in a
>> way that did not map mechanically to the newer convention.  The test
>> suite caught one mistake; I'm not confident I didn't make any more.
>
> I think this means there is a case for splitting this patch up into a part
> that doesn't change installed stripped shared libraries (on one platform -
> so at least the sense of comparisons etc. is unchanged on that platform
> after the patch, even if likely/unlikely could be wrong sometimes without
> being detected by such a comparison of installed stripped shared
> libraries), and a part that does (presumably because the mapping isn't
> purely mechanical so compiler IR changes enough to result in different
> code generated), with the latter needing more careful review that no
> semantics have changed.

I do not have time to do that in this release cycle.  I think my scanf
patches are much more important and I intend to devote what little
time I do have for glibc hacking to them.

zw
  
Zack Weinberg June 12, 2018, 2:05 p.m. UTC | #3
On Mon, Jun 4, 2018 at 11:33 AM, Zack Weinberg <zackw@panix.com> wrote:
> On Mon, Jun 4, 2018 at 10:36 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Sat, 2 Jun 2018, Zack Weinberg wrote:
>>
>>> I would appreciate someone checking over this patch very carefully
>>> indeed.  In a whole bunch of places, __builtin_expect was used in a
>>> way that did not map mechanically to the newer convention.  The test
>>> suite caught one mistake; I'm not confident I didn't make any more.
>>
>> I think this means there is a case for splitting this patch up into a part
>> that doesn't change installed stripped shared libraries (on one platform -
>> so at least the sense of comparisons etc. is unchanged on that platform
>> after the patch, even if likely/unlikely could be wrong sometimes without
>> being detected by such a comparison of installed stripped shared
>> libraries), and a part that does (presumably because the mapping isn't
>> purely mechanical so compiler IR changes enough to result in different
>> code generated), with the latter needing more careful review that no
>> semantics have changed.
>
> I do not have time to do that in this release cycle.  I think my scanf
> patches are much more important and I intend to devote what little
> time I do have for glibc hacking to them.

I have pushed this patch as the branch
zack/elf-builtin-expect-conversion.  I will not be working on it any
more for the foreseeable future but anyone is welcome to pick up the
ball.

zw
  

Patch

diff --git a/elf/dl-addr.c b/elf/dl-addr.c
index e6c7d02094..0b3070a7ea 100644
--- a/elf/dl-addr.c
+++ b/elf/dl-addr.c
@@ -31,7 +31,7 @@  determine_info (const ElfW(Addr) addr, struct link_map *match, Dl_info *info,
   info->dli_fbase = (void *) match->l_map_start;
 
   /* If this is the main program the information is incomplete.  */
-  if (__builtin_expect (match->l_name[0], 'a') == '\0'
+  if (__glibc_unlikely (match->l_name[0] == '\0')
       && match->l_type == lt_executable)
     info->dli_fname = _dl_argv[0];
 
diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index 6ee5153ff9..3cba2e463e 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -289,7 +289,7 @@  _dl_load_cache_lookup (const char *name)
     }
 
   /* Print our result if wanted.  */
-  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_LIBS, 0)
+  if (__glibc_unlikely ((GLRO(dl_debug_mask) & DL_DEBUG_LIBS) != 0)
       && best != NULL)
     _dl_debug_printf ("  trying file=%s\n", best);
 
diff --git a/elf/dl-close.c b/elf/dl-close.c
index ecd6729704..4e44bdd930 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -271,8 +271,8 @@  _dl_close_worker (struct link_map *map, bool force)
 	  if (imap->l_init_called)
 	    {
 	      /* When debugging print a message first.  */
-	      if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS,
-				    0))
+	      if (__glibc_unlikely ((GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS)
+				    != 0))
 		_dl_debug_printf ("\ncalling fini: %s [%lu]\n\n",
 				  imap->l_name, nsid);
 
@@ -782,7 +782,7 @@  _dl_close_worker (struct link_map *map, bool force)
     }
 #endif
 
-  if (__builtin_expect (ns->_ns_loaded == NULL, 0)
+  if (__glibc_unlikely (ns->_ns_loaded == NULL)
       && nsid == GL(dl_nns) - 1)
     do
       --GL(dl_nns);
@@ -833,7 +833,7 @@  _dl_close (void *_map)
      should be a detectable case and given that dlclose should be threadsafe
      we need this to be a reliable detection.
      This is bug 20990. */
-  if (__builtin_expect (map->l_direct_opencount, 1) == 0)
+  if (__glibc_unlikely (map->l_direct_opencount == 0))
     {
       __rtld_lock_unlock_recursive (GL(dl_load_lock));
       _dl_signal_error (0, map->l_name, NULL, N_("shared object not open"));
diff --git a/elf/dl-deps.c b/elf/dl-deps.c
index c975fcffd7..e4eac687b9 100644
--- a/elf/dl-deps.c
+++ b/elf/dl-deps.c
@@ -236,7 +236,7 @@  _dl_map_object_deps (struct link_map *map,
 	  orig = runp;
 
 	  for (d = l->l_ld; d->d_tag != DT_NULL; ++d)
-	    if (__builtin_expect (d->d_tag, DT_NEEDED) == DT_NEEDED)
+	    if (__glibc_likely (d->d_tag == DT_NEEDED))
 	      {
 		/* Map in the needed object.  */
 		struct link_map *dep;
@@ -291,8 +291,8 @@  _dl_map_object_deps (struct link_map *map,
 		args.name = name;
 
 		/* Say that we are about to load an auxiliary library.  */
-		if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_LIBS,
-				      0))
+		if (__glibc_unlikely ((GLRO(dl_debug_mask) & DL_DEBUG_LIBS)
+				      != 0))
 		  _dl_debug_printf ("load auxiliary object=%s"
 				    " requested by file=%s\n",
 				    name,
@@ -484,7 +484,7 @@  _dl_map_object_deps (struct link_map *map,
 
   for (nlist = 0, runp = known; runp; runp = runp->next)
     {
-      if (__builtin_expect (trace_mode, 0) && runp->map->l_faked)
+      if (__glibc_unlikely (trace_mode != 0) && runp->map->l_faked)
 	/* This can happen when we trace the loading.  */
 	--map->l_searchlist.r_nlist;
       else
@@ -495,7 +495,7 @@  _dl_map_object_deps (struct link_map *map,
       runp->map->l_reserved = 0;
     }
 
-  if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_PRELINK, 0) != 0
+  if (__glibc_unlikely ((GLRO(dl_debug_mask) & DL_DEBUG_PRELINK) != 0)
       && map == GL(dl_ns)[LM_ID_BASE]._ns_loaded)
     {
       /* If we are to compute conflicts, we have to build local scope
@@ -525,8 +525,8 @@  Filters not supported with LD_TRACE_PRELINKING"));
 	  for (j = 0; j < cnt; j++)
 	    {
 	      l_initfini[j]->l_reserved = 0;
-	      if (j && __builtin_expect (l_initfini[j]->l_info[DT_SYMBOLIC]
-					 != NULL, 0))
+	      if (j && __glibc_unlikely (l_initfini[j]->l_info[DT_SYMBOLIC]
+					 != NULL))
 		l->l_symbolic_in_local_scope = true;
 	    }
 
diff --git a/elf/dl-environ.c b/elf/dl-environ.c
index b20045b870..1c35344d42 100644
--- a/elf/dl-environ.c
+++ b/elf/dl-environ.c
@@ -31,7 +31,7 @@  _dl_next_ld_env_entry (char ***position)
 
   while (*current != NULL)
     {
-      if (__builtin_expect ((*current)[0] == 'L', 0)
+      if (__glibc_unlikely ((*current)[0] == 'L')
 	  && (*current)[1] == 'D' && (*current)[2] == '_')
 	{
 	  result = &(*current)[3];
diff --git a/elf/dl-error-skeleton.c b/elf/dl-error-skeleton.c
index d5f418ab18..ca80c790d2 100644
--- a/elf/dl-error-skeleton.c
+++ b/elf/dl-error-skeleton.c
@@ -131,8 +131,8 @@  void
 _dl_signal_cexception (int errcode, struct dl_exception *exception,
 		       const char *occasion)
 {
-  if (__builtin_expect (GLRO(dl_debug_mask)
-			& ~(DL_DEBUG_STATISTICS|DL_DEBUG_PRELINK), 0))
+  if (__glibc_unlikely ((GLRO(dl_debug_mask)
+			 & ~(DL_DEBUG_STATISTICS|DL_DEBUG_PRELINK)) != 0))
     _dl_debug_printf ("%s: error: %s: %s (%s)\n",
 		      exception->objname, occasion,
 		      exception->errstring, receiver ? "continued" : "fatal");
@@ -152,8 +152,8 @@  void
 _dl_signal_cerror (int errcode, const char *objname, const char *occation,
 		   const char *errstring)
 {
-  if (__builtin_expect (GLRO(dl_debug_mask)
-			& ~(DL_DEBUG_STATISTICS|DL_DEBUG_PRELINK), 0))
+  if (__glibc_unlikely ((GLRO(dl_debug_mask)
+			 & ~(DL_DEBUG_STATISTICS|DL_DEBUG_PRELINK)) != 0))
     _dl_debug_printf ("%s: error: %s: %s (%s)\n", objname, occation,
 		      errstring, receiver ? "continued" : "fatal");
 
@@ -191,7 +191,7 @@  _dl_catch_exception (struct dl_exception *exception,
   catch_hook = &c;
 
   /* Do not save the signal mask.  */
-  if (__builtin_expect (__sigsetjmp (c.env, 0), 0) == 0)
+  if (__glibc_likely (__sigsetjmp (c.env, 0) == 0))
     {
       (*operate) (args);
       catch_hook = old;
diff --git a/elf/dl-fini.c b/elf/dl-fini.c
index 3cfc262400..28d9704559 100644
--- a/elf/dl-fini.c
+++ b/elf/dl-fini.c
@@ -120,8 +120,8 @@  _dl_fini (void)
 		      || l->l_info[DT_FINI] != NULL)
 		    {
 		      /* When debugging print a message first.  */
-		      if (__builtin_expect (GLRO(dl_debug_mask)
-					    & DL_DEBUG_IMPCALLS, 0))
+		      if (__glibc_unlikely ((GLRO(dl_debug_mask)
+                                             & DL_DEBUG_IMPCALLS) != 0))
 			_dl_debug_printf ("\ncalling fini: %s [%lu]\n\n",
 					  DSO_FILENAME (l->l_name),
 					  ns);
@@ -146,7 +146,7 @@  _dl_fini (void)
 
 #ifdef SHARED
 		  /* Auditing checkpoint: another object closed.  */
-		  if (!do_audit && __builtin_expect (GLRO(dl_naudit) > 0, 0))
+		  if (!do_audit && __glibc_unlikely (GLRO(dl_naudit) > 0))
 		    {
 		      struct audit_ifaces *afct = GLRO(dl_audit);
 		      for (unsigned int cnt = 0; cnt < GLRO(dl_naudit); ++cnt)
diff --git a/elf/dl-fptr.c b/elf/dl-fptr.c
index 027c5029eb..a7701410f9 100644
--- a/elf/dl-fptr.c
+++ b/elf/dl-fptr.c
@@ -227,8 +227,8 @@  _dl_make_fptr (struct link_map *map, const ElfW(Sym) *sym,
       ElfW(Addr) fdesc
 	= make_fdesc (ip, map->l_info[DT_PLTGOT]->d_un.d_ptr);
 
-      if (__builtin_expect (COMPARE_AND_SWAP (&ftab[symidx], (ElfW(Addr)) NULL,
-					      fdesc), 1))
+      if (__glibc_likely (COMPARE_AND_SWAP (&ftab[symidx], (ElfW(Addr)) NULL,
+                                            fdesc) == 1))
 	{
 	  /* Noone has updated the entry and the new function
 	     descriptor has been installed.  */
diff --git a/elf/dl-init.c b/elf/dl-init.c
index 3e72fa3013..50c5b57986 100644
--- a/elf/dl-init.c
+++ b/elf/dl-init.c
@@ -36,13 +36,13 @@  call_init (struct link_map *l, int argc, char **argv, char **env)
   l->l_init_called = 1;
 
   /* Check for object which constructors we do not run here.  */
-  if (__builtin_expect (l->l_name[0], 'a') == '\0'
+  if (__glibc_unlikely (l->l_name[0] == '\0')
       && l->l_type == lt_executable)
     return;
 
   /* Are there any constructors?  */
   if (l->l_info[DT_INIT] == NULL
-      && __builtin_expect (l->l_info[DT_INIT_ARRAY] == NULL, 1))
+      && __glibc_likely (l->l_info[DT_INIT_ARRAY] == NULL))
     return;
 
   /* Print a debug message if wanted.  */
@@ -88,7 +88,7 @@  _dl_init (struct link_map *main_map, int argc, char **argv, char **env)
     }
 
   /* Don't do anything if there is no preinit array.  */
-  if (__builtin_expect (preinit_array != NULL, 0)
+  if (__glibc_unlikely (preinit_array != NULL)
       && preinit_array_size != NULL
       && (i = preinit_array_size->d_un.d_val / sizeof (ElfW(Addr))) > 0)
     {
diff --git a/elf/dl-libc.c b/elf/dl-libc.c
index fc01f5514d..3183846ca4 100644
--- a/elf/dl-libc.c
+++ b/elf/dl-libc.c
@@ -359,7 +359,7 @@  libc_freeres_fn (free_mem)
 	  l->l_initfini = NULL;
 	}
 
-      if (__builtin_expect (GL(dl_ns)[ns]._ns_global_scope_alloc, 0) != 0
+      if (__glibc_unlikely (GL(dl_ns)[ns]._ns_global_scope_alloc != 0)
 	  && (GL(dl_ns)[ns]._ns_main_searchlist->r_nlist
 	      // XXX Check whether we need NS-specific initial_searchlist
 	      == GLRO(dl_initial_searchlist).r_nlist))
diff --git a/elf/dl-minimal.c b/elf/dl-minimal.c
index 25ceded6fe..ea511282a3 100644
--- a/elf/dl-minimal.c
+++ b/elf/dl-minimal.c
@@ -96,7 +96,7 @@  calloc (size_t nmemb, size_t size)
   size_t bytes = nmemb * size;
 
 #define HALF_SIZE_T (((size_t) 1) << (8 * sizeof (size_t) / 2))
-  if (__builtin_expect ((nmemb | size) >= HALF_SIZE_T, 0)
+  if (__glibc_unlikely ((nmemb | size) >= HALF_SIZE_T)
       && size != 0 && bytes / size != nmemb)
     return NULL;
 
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 9dde4acfbc..ba4d97a5eb 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -442,7 +442,7 @@  dl_open_worker (void *a)
 	 therefore is not yet initialized.  */
       else if (! imap->l_init_called
 	       /* Only if the module defines thread local data.  */
-	       && __builtin_expect (imap->l_tls_blocksize > 0, 0))
+	       && __glibc_unlikely (imap->l_tls_blocksize > 0))
 	{
 	  /* Now that we know the object is loaded successfully add
 	     modules containing TLS data to the slot info table.  We
@@ -463,7 +463,7 @@  dl_open_worker (void *a)
     }
 
   /* Bump the generation number if necessary.  */
-  if (any_tls && __builtin_expect (++GL(dl_tls_generation) == 0, 0))
+  if (any_tls && __glibc_unlikely (++GL(dl_tls_generation) == 0))
     _dl_fatal_printf (N_("\
 TLS generation counter wrapped!  Please report this."));
 
diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
index 053916eeae..2126fad265 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -95,8 +95,7 @@  _dl_try_allocate_static_tls (struct link_map *map)
   if (map->l_real->l_relocated)
     {
 #ifdef SHARED
-      if (__builtin_expect (THREAD_DTV()[0].counter != GL(dl_tls_generation),
-			    0))
+      if (__glibc_unlikely (THREAD_DTV()[0].counter != GL(dl_tls_generation)))
 	/* Update the slot information data for at least the generation of
 	   the DSO we are allocating data for.  */
 	(void) _dl_update_slotinfo (map->l_tls_modid);
@@ -174,7 +173,7 @@  _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
      do not do this if we are profiling, of course.  */
   // XXX Correct for auditing?
   if (!consider_profiling
-      && __builtin_expect (l->l_info[DT_BIND_NOW] != NULL, 0))
+      && __glibc_unlikely (l->l_info[DT_BIND_NOW] != NULL))
     lazy = 0;
 
   if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_RELOC))
@@ -234,7 +233,7 @@  _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
 #define RESOLVE_MAP(ref, version, r_type) \
     ((ELFW(ST_BIND) ((*ref)->st_info) != STB_LOCAL			      \
       && __glibc_likely (!dl_symbol_visibility_binds_local_p (*ref)))	      \
-     ? ((__builtin_expect ((*ref) == l->l_lookup_cache.sym, 0)		      \
+     ? ((__glibc_unlikely ((*ref) == l->l_lookup_cache.sym)		      \
 	 && elf_machine_type_class (r_type) == l->l_lookup_cache.type_class)  \
 	? (bump_num_cache_relocations (),				      \
 	   (*ref) = l->l_lookup_cache.ret,				      \
@@ -285,7 +284,7 @@  _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
   l->l_relocated = 1;
 
   /* Undo the segment protection changes.  */
-  while (__builtin_expect (textrels != NULL, 0))
+  while (__glibc_unlikely (textrels != NULL))
     {
       if (__mprotect (textrels->start, textrels->len, textrels->prot) < 0)
 	{
@@ -344,7 +343,7 @@  _dl_reloc_bad_type (struct link_map *map, unsigned int type, int plt)
 
   cp = __stpcpy (msgbuf, msg[plt]);
 #if __ELF_NATIVE_CLASS == 64
-  if (__builtin_expect(type > 0xff, 0))
+  if (__glibc_unlikely (type > 0xff))
     {
       *cp++ = DIGIT (type >> 28);
       *cp++ = DIGIT (type >> 24);
diff --git a/elf/dl-runtime.c b/elf/dl-runtime.c
index 63bbc89776..17d691ceb9 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -81,7 +81,7 @@  _dl_fixup (
 
    /* Look up the target symbol.  If the normal lookup rules are not
       used don't look in the global scope.  */
-  if (__builtin_expect (ELFW(ST_VISIBILITY) (sym->st_other), 0) == 0)
+  if (__glibc_likely (ELFW(ST_VISIBILITY) (sym->st_other) == 0))
     {
       const struct r_found_version *version = NULL;
 
@@ -138,7 +138,7 @@  _dl_fixup (
   value = elf_machine_plt_value (l, reloc, value);
 
   if (sym != NULL
-      && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC, 0))
+      && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC))
     value = elf_ifunc_invoke (DL_FIXUP_VALUE_ADDR (value));
 
   /* Finally, fix up the plt itself.  */
@@ -204,7 +204,7 @@  _dl_profile_fixup (
 
       /* Look up the target symbol.  If the symbol is marked STV_PROTECTED
 	 don't look in the global scope.  */
-      if (__builtin_expect (ELFW(ST_VISIBILITY) (refsym->st_other), 0) == 0)
+      if (__glibc_likely (ELFW(ST_VISIBILITY) (refsym->st_other) == 0))
 	{
 	  const struct r_found_version *version = NULL;
 
@@ -243,8 +243,8 @@  _dl_profile_fixup (
 				       SYMBOL_ADDRESS (result, defsym, false));
 
 	  if (defsym != NULL
-	      && __builtin_expect (ELFW(ST_TYPE) (defsym->st_info)
-				   == STT_GNU_IFUNC, 0))
+	      && __glibc_unlikely (ELFW(ST_TYPE) (defsym->st_info)
+				   == STT_GNU_IFUNC))
 	    value = elf_ifunc_invoke (DL_FIXUP_VALUE_ADDR (value));
 	}
       else
@@ -253,8 +253,8 @@  _dl_profile_fixup (
 	     address) is also known.  */
 	  value = DL_FIXUP_MAKE_VALUE (l, SYMBOL_ADDRESS (l, refsym, true));
 
-	  if (__builtin_expect (ELFW(ST_TYPE) (refsym->st_info)
-				== STT_GNU_IFUNC, 0))
+	  if (__glibc_unlikely (ELFW(ST_TYPE) (refsym->st_info)
+				== STT_GNU_IFUNC))
 	    value = elf_ifunc_invoke (DL_FIXUP_VALUE_ADDR (value));
 
 	  result = l;
diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c
index 998c5d52bc..1a2a043d88 100644
--- a/elf/dl-sysdep.c
+++ b/elf/dl-sysdep.c
@@ -247,7 +247,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 (__libc_enable_secure, 0))
+  if (__glibc_unlikely (__libc_enable_secure))
     __libc_check_standard_fds ();
 
   (*dl_main) (phdr, phnum, &user_entry, GLRO(dl_auxv));
@@ -332,10 +332,10 @@  _dl_show_auxv (void)
 	{
 	  const char *val = (char *) av->a_un.a_val;
 
-	  if (__builtin_expect (auxvars[idx].form, dec) == dec)
+	  if (__glibc_likely (auxvars[idx].form == dec))
 	    val = _itoa ((unsigned long int) av->a_un.a_val,
 			 buf + sizeof buf - 1, 10, 0);
-	  else if (__builtin_expect (auxvars[idx].form, hex) == hex)
+	  else if (__glibc_likely (auxvars[idx].form == hex))
 	    val = _itoa ((unsigned long int) av->a_un.a_val,
 			 buf + sizeof buf - 1, 16, 0);
 
diff --git a/elf/dl-tls.c b/elf/dl-tls.c
index c87caf13d6..9dd27f3cf8 100644
--- a/elf/dl-tls.c
+++ b/elf/dl-tls.c
@@ -48,7 +48,7 @@  _dl_next_tls_modid (void)
 {
   size_t result;
 
-  if (__builtin_expect (GL(dl_tls_dtv_gaps), false))
+  if (__glibc_unlikely (GL(dl_tls_dtv_gaps) != 0))
     {
       size_t disp = 0;
       struct dtv_slotinfo_list *runp = GL(dl_tls_dtv_slotinfo_list);
diff --git a/elf/dl-version.c b/elf/dl-version.c
index 7860cc817e..a08af6c746 100644
--- a/elf/dl-version.c
+++ b/elf/dl-version.c
@@ -92,7 +92,7 @@  checking for version `%s' in file %s [%lu] required by file %s [%lu]\n",
     {
       /* Currently the version number of the definition entry is 1.
 	 Make sure all we see is this version.  */
-      if (__builtin_expect (def->vd_version, 1) != 1)
+      if (__glibc_unlikely (def->vd_version != 1))
 	{
 	  char buf[20];
 	  buf[sizeof (buf) - 1] = '\0';
@@ -111,8 +111,7 @@  checking for version `%s' in file %s [%lu] required by file %s [%lu]\n",
 	  ElfW(Verdaux) *aux = (ElfW(Verdaux) *) ((char *) def + def->vd_aux);
 
 	  /* To be safe, compare the string as well.  */
-	  if (__builtin_expect (strcmp (string, strtab + aux->vda_name), 0)
-	      == 0)
+	  if (__glibc_likely (strcmp (string, strtab + aux->vda_name) == 0))
 	    /* Bingo!  */
 	    return 0;
 	}
@@ -182,7 +181,7 @@  _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
 
       /* Currently the version number of the needed entry is 1.
 	 Make sure all we see is this version.  */
-      if (__builtin_expect (ent->vn_version, 1) != 1)
+      if (__glibc_unlikely (ent->vn_version != 1))
 	{
 	  char buf[20];
 	  buf[sizeof (buf) - 1] = '\0';
@@ -206,8 +205,8 @@  _dl_check_map_versions (struct link_map *map, int verbose, int trace_mode)
 
 	  /* Make sure this is no stub we created because of a missing
 	     dependency.  */
-	  if (__builtin_expect (! trace_mode, 1)
-	      || ! __builtin_expect (needed->l_faked, 0))
+	  if (__glibc_likely (! trace_mode)
+	      || __glibc_likely (! needed->l_faked))
 	    {
 	      /* NEEDED is the map for the file we need.  Now look for the
 		 dependency symbols.  */
diff --git a/elf/ldconfig.c b/elf/ldconfig.c
index fbdd814edf..deb72956b7 100644
--- a/elf/ldconfig.c
+++ b/elf/ldconfig.c
@@ -820,7 +820,7 @@  search_dir (const struct dir_entry *entry)
 	  new_entry->next = NULL;
 	  if (!is_link
 	      && direntry->d_type != DT_UNKNOWN
-	      && __builtin_expect (lstat64 (real_file_name, &lstat_buf), 0))
+	      && __glibc_unlikely (lstat64 (real_file_name, &lstat_buf) != 0))
 	    {
 	      error (0, errno, _("Cannot lstat %s"), file_name);
 	      free (new_entry->path);
@@ -852,7 +852,7 @@  search_dir (const struct dir_entry *entry)
       /* Call lstat64 if not done yet.  */
       if (!is_link
 	  && direntry->d_type != DT_UNKNOWN
-	  && __builtin_expect (lstat64 (real_file_name, &lstat_buf), 0))
+	  && __glibc_unlikely (lstat64 (real_file_name, &lstat_buf) != 0))
 	{
 	  error (0, errno, _("Cannot lstat %s"), file_name);
 	  continue;
diff --git a/elf/rtld.c b/elf/rtld.c
index 8c732adb68..98ba82d15c 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1030,7 +1030,7 @@  of this helper program; chances are you did not intend to run this program.\n\
 	    break;
 	  }
 
-      if (__builtin_expect (mode, normal) == verify)
+      if (__glibc_unlikely (mode == verify))
 	{
 	  const char *objname;
 	  const char *err_str = NULL;
@@ -1060,7 +1060,7 @@  of this helper program; chances are you did not intend to run this program.\n\
       /* Now the map for the main executable is available.  */
       main_map = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
 
-      if (__builtin_expect (mode, normal) == normal
+      if (__glibc_likely (mode == normal)
 	  && GL(dl_rtld_map).l_info[DT_SONAME] != NULL
 	  && main_map->l_info[DT_SONAME] != NULL
 	  && strcmp ((const char *) D_PTR (&GL(dl_rtld_map), l_info[DT_STRTAB])
@@ -1291,7 +1291,7 @@  of this helper program; chances are you did not intend to run this program.\n\
       _dl_setup_hash (main_map);
     }
 
-  if (__builtin_expect (mode, normal) == verify)
+  if (__glibc_unlikely (mode == verify))
     {
       /* We were called just to verify that this is a dynamic
 	 executable using us as the program interpreter.  Exit with an
@@ -1749,7 +1749,7 @@  ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
       rtld_multiple_ref = true;
 
       GL(dl_rtld_map).l_prev = main_map->l_searchlist.r_list[i - 1];
-      if (__builtin_expect (mode, normal) == normal)
+      if (__glibc_likely (mode == normal))
 	{
 	  GL(dl_rtld_map).l_next = (i + 1 < main_map->l_searchlist.r_nlist
 				    ? main_map->l_searchlist.r_list[i + 1]
@@ -1803,7 +1803,7 @@  ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
        earlier.  */
     security_init ();
 
-  if (__builtin_expect (mode, normal) != normal)
+  if (__glibc_unlikely (mode != normal))
     {
       /* We were run just to list the shared libraries.  It is
 	 important that we do this before real relocation, because the
@@ -1905,7 +1905,7 @@  ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
 			  (size_t) l->l_map_start);
 	}
 
-      if (__builtin_expect (mode, trace) != trace)
+      if (__glibc_unlikely (mode != trace))
 	for (i = 1; i < (unsigned int) _dl_argc; ++i)
 	  {
 	    const ElfW(Sym) *ref = NULL;
@@ -2034,8 +2034,7 @@  ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
     }
 
   if (main_map->l_info[ADDRIDX (DT_GNU_LIBLIST)]
-      && ! __builtin_expect (GLRO(dl_profile) != NULL, 0)
-      && ! __builtin_expect (GLRO(dl_dynamic_weak), 0))
+      && __glibc_likely (GLRO(dl_profile) == NULL && GLRO(dl_dynamic_weak) == 0))
     {
       ElfW(Lib) *liblist, *liblistend;
       struct link_map **r_list, **r_listend, *l;
@@ -2176,7 +2175,7 @@  ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
 	     implementation in ld.so.  */
 	  struct libname_list *lnp = l->l_libname->next;
 
-	  while (__builtin_expect (lnp != NULL, 0))
+	  while (__glibc_unlikely (lnp != NULL))
 	    {
 	      lnp->dont_free = 1;
 	      lnp = lnp->next;
@@ -2630,7 +2629,7 @@  process_envvars (enum mode *modep)
 
   /* Extra security for SUID binaries.  Remove all dangerous environment
      variables.  */
-  if (__builtin_expect (__libc_enable_secure, 0))
+  if (__glibc_unlikely (__libc_enable_secure))
     {
       static const char unsecure_envvars[] =
 #ifdef EXTRA_UNSECURE_ENVVARS