From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Alex Gramiak Newsgroups: gmane.emacs.devel Subject: Re: Using __builtin_expect (likely/unlikely macros) Date: Mon, 15 Apr 2019 18:16:02 -0600 Message-ID: <87h8aykdod.fsf@gmail.com> References: <87a7gst973.fsf@gmail.com> <875zrgt12q.fsf@gmail.com> <6919a4c8-df76-ea1e-34db-1fa62a360e5a@cs.ucla.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="177023"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) Cc: emacs-devel@gnu.org To: Paul Eggert Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Apr 16 02:16:21 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hGBls-000js4-Pd for ged-emacs-devel@m.gmane.org; Tue, 16 Apr 2019 02:16:21 +0200 Original-Received: from localhost ([127.0.0.1]:57130 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hGBlr-0004EP-FZ for ged-emacs-devel@m.gmane.org; Mon, 15 Apr 2019 20:16:19 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:60580) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hGBlk-0004EC-ET for emacs-devel@gnu.org; Mon, 15 Apr 2019 20:16:14 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hGBli-00021F-S8 for emacs-devel@gnu.org; Mon, 15 Apr 2019 20:16:12 -0400 Original-Received: from mail-pf1-x429.google.com ([2607:f8b0:4864:20::429]:42771) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hGBli-00020U-9b for emacs-devel@gnu.org; Mon, 15 Apr 2019 20:16:10 -0400 Original-Received: by mail-pf1-x429.google.com with SMTP id w25so9428382pfi.9 for ; Mon, 15 Apr 2019 17:16:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:in-reply-to:references:user-agent:date :message-id:mime-version; bh=srxLbPbfa1Zd7KxdXyJ+oaek/uR9gXWDhnW2frXPkZU=; b=c1NlpjQKcbUfV+CtkHBkiGs+xJhe7HcTtFqOFHOD2U9aS3DJjLMx0kl3SoBHxC5AOB FhaYaBHmhXQBpLQ764jix8bUDs5faEceZ57YS0PABlQv9LDx85mNSj1n6KKCIqnao4Qi EkejQqekcUgr2Hzdzft2vYltxIXBrpUQxRT9acK9OwEmJib0hcNXKPiUk6lN9BC69+sx 8ydhAjHe55OBgN4PE6cNTL7U31jO85wbl0oQUxdQX/rqSFcxJIt7W2IpU6H2aMfV9xAd RP6pu1AJPKOD4Mjh9XtJB2+tGhb8U922ephM9smh82GXqnL3hJuQLLKimbmboUrBXl3a RGxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references :user-agent:date:message-id:mime-version; bh=srxLbPbfa1Zd7KxdXyJ+oaek/uR9gXWDhnW2frXPkZU=; b=MJvx/zEBsSMM40xu495pafD8UvhpUeKLIo1zgGFcshrOF5c5TJX6/YXWgcNyASpQ3w AKpKTl7G6ziJP/8WLukkSWeS4zYwaCV0qxh7o/x1UvHlcqNAysf7CrE+wgIXZLKu671A HJz0dAHfGprjySS/6D319pIGMvucupMVkK1KR7H3AF0gfwhVU5lynkDca4w2elQVyMjy Glcn7q69++IjxQxlyAjQQnBPHp9dPfMxWuhKR8hRcikv39p8VHEojiTLXY3mXzNkeUmL kTbYVz5P4trl23WdHIPQAE/6QI3jT3OfvxOJNzpTOvR75dJckqZGm0Js1O/+Gi3bIKxq 7gQw== X-Gm-Message-State: APjAAAVnmH9qR8gSJEKa1Aped2XcYkuf4VwdgYUWeQ8ta4VArcW9JBOO LDzpNLdevgRdLttA26XA7VssbgoS X-Google-Smtp-Source: APXvYqyCHdrQ6HWALOpI75hqVYV2lSriby4V3AsJOp6GQl6elBvcHOgybNr/kdfqa7+xfv2PoHtOqw== X-Received: by 2002:a65:53cb:: with SMTP id z11mr71869940pgr.139.1555373768840; Mon, 15 Apr 2019 17:16:08 -0700 (PDT) Original-Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302]) by smtp.gmail.com with ESMTPSA id f5sm52560113pgo.75.2019.04.15.17.16.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 15 Apr 2019 17:16:07 -0700 (PDT) In-Reply-To: <6919a4c8-df76-ea1e-34db-1fa62a360e5a@cs.ucla.edu> (Paul Eggert's message of "Sun, 14 Apr 2019 21:41:02 -0700") X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::429 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:235498 Archived-At: --=-=-= Content-Type: text/plain Paul Eggert 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. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=builtin-expect.diff Content-Description: likely 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 . */ #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 . */ /* 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, '$'); --=-=-=--