Re: Using __builtin_expect (likely/unlikely macros)

[Alex Gramiak]

Paul Eggert <address@hidden> writes:

> Alex Gramiak wrote:
>> Though I don't think that these macros, used sparingly around code such
>> as emacs_abort, would make it harder to read/write/maintain.
>
> These macro calls would not help near calls to emacs_abort, as it should 
> already
> be obvious to a careful human reader that the jump to emacs_abort is the road
> less traveled. (That's also obvious to GCC, since emacs_abort is _Noreturn.)

To human readers, yes, but from what I can tell, GCC is mixed on this.
When applying the following diff that surrounds emacs_abort in
bytecode.c and xdisp.c, the assembly in both applied/unapplied is the
same for bytecode.c, but not for xdisp.c (even without the LIKELY
cases). I tested using gcc -O2 -S.

Also, I tried putting a LIKELY in lisp_h_CHECK_TYPE (seems like a nice
place for one) and that yielded different assembly for fns.c, even
though wrong_type_argument is _Noreturn.

diff --git a/src/bytecode.c b/src/bytecode.c
index 40977799bf..ce1a7bd254 100644
--- a/src/bytecode.c
+++ b/src/bytecode.c
@@ -39,6 +39,7 @@ along with GNU Emacs.  If not, see 
<https://www.gnu.org/licenses/>.  */
 #ifndef BYTE_CODE_SAFE
 # define BYTE_CODE_SAFE false
 #endif
+#define unlikely(expr) (__builtin_expect (expr, 0))
 
 /* Define BYTE_CODE_METER to generate a byte-op usage histogram.  */
 /* #define BYTE_CODE_METER */
@@ -404,7 +405,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, 
Lisp_Object maxdepth,
       int op;
       enum handlertype type;
 
-      if (BYTE_CODE_SAFE && ! (stack_base <= top && top < stack_lim))
+      if (unlikely (BYTE_CODE_SAFE && ! (stack_base <= top && top < 
stack_lim)))
        emacs_abort ();
 
 #ifdef BYTE_CODE_METER
@@ -664,9 +665,9 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, 
Lisp_Object maxdepth,
        op_branch:
          op -= pc - bytestr_data;
        op_relative_branch:
-         if (BYTE_CODE_SAFE
+         if (unlikely (BYTE_CODE_SAFE
              && ! (bytestr_data - pc <= op
-                   && op < bytestr_data + bytestr_length - pc))
+                   && op < bytestr_data + bytestr_length - pc)))
            emacs_abort ();
          quitcounter += op < 0;
          if (!quitcounter)
@@ -1397,7 +1398,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, 
Lisp_Object maxdepth,
               number of cases is less, which uses a simple vector for linear
               search as the jump table.  */
             Lisp_Object jmp_table = POP;
-           if (BYTE_CODE_SAFE && !HASH_TABLE_P (jmp_table))
+           if (unlikely (BYTE_CODE_SAFE && !HASH_TABLE_P (jmp_table)))
               emacs_abort ();
             Lisp_Object v1 = POP;
             ptrdiff_t i;
@@ -1426,7 +1427,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, 
Lisp_Object maxdepth,
            if (i >= 0)
              {
                Lisp_Object val = HASH_VALUE (h, i);
-               if (BYTE_CODE_SAFE && !FIXNUMP (val))
+               if (unlikely (BYTE_CODE_SAFE && !FIXNUMP (val)))
                  emacs_abort ();
                op = XFIXNUM (val);
                goto op_branch;
@@ -1436,8 +1437,8 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, 
Lisp_Object maxdepth,
 
        CASE_DEFAULT
        CASE (Bconstant):
-         if (BYTE_CODE_SAFE
-             && ! (Bconstant <= op && op < Bconstant + const_length))
+         if (unlikely (BYTE_CODE_SAFE
+                        && ! (Bconstant <= op && op < Bconstant + 
const_length)))
            emacs_abort ();
          PUSH (vectorp[op - Bconstant]);
          NEXT;
diff --git a/src/xdisp.c b/src/xdisp.c
index a88fc698b8..9872f69cb0 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -344,7 +344,8 @@ along with GNU Emacs.  If not, see 
<https://www.gnu.org/licenses/>.  */
 
 /* Holds the list (error).  */
 static Lisp_Object list_of_error;
-
+#define UNLIKELY(expr) (__builtin_expect (expr, 0))
+#define LIKELY(expr) (__builtin_expect (expr, 1))
 #ifdef HAVE_WINDOW_SYSTEM
 
 /* Test if overflow newline into fringe.  Called with iterator IT
@@ -8322,7 +8323,7 @@ compute_stop_pos_backwards (struct it *it)
       reseat_1 (it, pos, false);
       compute_stop_pos (it);
       /* We must advance forward, right?  */
-      if (it->stop_charpos <= charpos)
+      if (UNLIKELY (it->stop_charpos <= charpos))
        emacs_abort ();
     }
   while (charpos > BEGV && it->stop_charpos >= it->end_charpos);
@@ -8371,7 +8372,7 @@ handle_stop_backwards (struct it *it, ptrdiff_t charpos)
        it->current.string_pos = string_pos (charpos, it->string);
       compute_stop_pos (it);
       /* We must advance forward, right?  */
-      if (it->stop_charpos <= it->prev_stop)
+      if (UNLIKELY (it->stop_charpos <= it->prev_stop))
        emacs_abort ();
       charpos = it->stop_charpos;
     }
@@ -11443,7 +11444,7 @@ pop_message_unwind (void)
 void
 check_message_stack (void)
 {
-  if (!NILP (Vmessage_stack))
+  if (UNLIKELY (!NILP (Vmessage_stack)))
     emacs_abort ();
 }
 
@@ -15495,7 +15496,7 @@ set_cursor_from_row (struct window *w, struct glyph_row 
*row,
       /* Need to compute x that corresponds to GLYPH.  */
       for (g = row->glyphs[TEXT_AREA], x = row->x; g < glyph; g++)
        {
-         if (g >= row->glyphs[TEXT_AREA] + row->used[TEXT_AREA])
+         if (UNLIKELY (g >= row->glyphs[TEXT_AREA] + row->used[TEXT_AREA]))
            emacs_abort ();
          x += g->pixel_width;
        }
@@ -16827,9 +16828,9 @@ redisplay_window (Lisp_Object window, bool 
just_this_one_p)
 
   /* Some sanity checks.  */
   CHECK_WINDOW_END (w);
-  if (Z == Z_BYTE && CHARPOS (opoint) != BYTEPOS (opoint))
+  if (UNLIKELY (Z == Z_BYTE && CHARPOS (opoint) != BYTEPOS (opoint)))
     emacs_abort ();
-  if (BYTEPOS (opoint) < CHARPOS (opoint))
+  if (UNLIKELY (BYTEPOS (opoint) < CHARPOS (opoint)))
     emacs_abort ();
 
   if (mode_line_update_needed (w))
@@ -19318,9 +19319,9 @@ try_window_id (struct window *w)
       adjust_window_ends (w, last_text_row, false);
       eassert (w->window_end_bytepos >= 0);
     }
-  else if (first_unchanged_at_end_row == NULL
-          && last_text_row == NULL
-          && last_text_row_at_end == NULL)
+  else if (LIKELY (first_unchanged_at_end_row == NULL
+                   && last_text_row == NULL
+                   && last_text_row_at_end == NULL))
     {
       /* Displayed to end of window, but no line containing text was
         displayed.  Lines were deleted at the end of the window.  */
@@ -21015,7 +21016,7 @@ find_row_edges (struct it *it, struct glyph_row *row,
           which puts the iterator at the beginning of the next line, in
           the logical order. */
        row->maxpos = it->current.pos;
-      else if (max_pos == min_pos && it->method != GET_FROM_BUFFER)
+      else if (LIKELY (max_pos == min_pos && it->method != GET_FROM_BUFFER))
        /* A line that is entirely from a string/image/stretch...  */
        row->maxpos = row->minpos;
       else
@@ -25210,7 +25211,7 @@ display_string (const char *string, Lisp_Object 
lisp_string, Lisp_Object face_st
                }
              break;
            }
-         else if (x + glyph->pixel_width >= it->first_visible_x)
+         else if (LIKELY (x + glyph->pixel_width >= it->first_visible_x))
            {
              /* Glyph is at least partially visible.  */
              ++it->hpos;
@@ -27847,7 +27848,7 @@ produce_special_glyphs (struct it *it, enum 
display_element_type what)
          spec_glyph_lookup_face (XWINDOW (it->window), &glyph);
        }
     }
-  else if (what == IT_TRUNCATION)
+  else if (LIKELY (what == IT_TRUNCATION))
     {
       /* Truncation glyph.  */
       SET_GLYPH_FROM_CHAR (glyph, '$');