From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Using __builtin_expect (likely/unlikely macros) Date: Tue, 16 Apr 2019 18:23:08 +0300 Message-ID: <83bm16gejn.fsf@gnu.org> References: <87a7gst973.fsf@gmail.com> <875zrgt12q.fsf@gmail.com> <6919a4c8-df76-ea1e-34db-1fa62a360e5a@cs.ucla.edu> <87h8aykdod.fsf@gmail.com> <83tveyhe41.fsf@gnu.org> <871s22jyyo.fsf@gmail.com> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="127900"; mail-complaints-to="usenet@blaine.gmane.org" Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org To: Alex Gramiak Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Apr 16 17:23:57 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 1hGPwC-000X5E-Mg for ged-emacs-devel@m.gmane.org; Tue, 16 Apr 2019 17:23:56 +0200 Original-Received: from localhost ([127.0.0.1]:38345 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hGPwB-0007Im-LB for ged-emacs-devel@m.gmane.org; Tue, 16 Apr 2019 11:23:55 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:38430) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hGPvW-0006t6-Ia for emacs-devel@gnu.org; Tue, 16 Apr 2019 11:23:15 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:50657) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hGPvW-0004wO-An; Tue, 16 Apr 2019 11:23:14 -0400 Original-Received: from [176.228.60.248] (port=3485 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hGPvV-0006Ze-1f; Tue, 16 Apr 2019 11:23:13 -0400 In-reply-to: <871s22jyyo.fsf@gmail.com> (message from Alex Gramiak on Mon, 15 Apr 2019 23:33:51 -0600) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] 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:235523 Archived-At: > From: Alex Gramiak > Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org > Date: Mon, 15 Apr 2019 23:33:51 -0600 > > Eli Zaretskii writes: > > > I'm not sure whether you put LIKELY and UNLIKELY somewhat randomly, > > just for testing, or did you really think each place is > > likely/unlikely as in the change, but at least some places in xdisp.c > > are wrong: they use LIKELY where UNLIKELY is more appropriate, abd > > vice versa. > > Out of curiosity, which places would that be? I only put UNLIKELY calls > around the checks for emacs_abort (which I presume to be unlikely), and > the LIKELY calls are of the form: > > else if (LIKELY ()) > { > ... > } > else > emacs_abort (); OK, it's possible that I don't understand the exact semantics of __builtin_expect in these situations. The problem is that you used LIKELY like this: if (A) do_A; else if (B) do_B; else if (C) do_C; else if (LIKELY (D)) do_D; else cant_happen (); Essentially, the above is a moral equivalent of a 'switch' with the 'default' case aborting because it "cannot happen". In such code, the order of the clauses doesn't necessarily tell anything about their likelihood; up front, they all are equally "likely". So using LIKELY only in the last one sends a wrong signal: that last condition is neither more nor less likely than all the others. Actually, in some cases it might be _less_ likely than the preceding ones, because if I knew that some of these conditions happens much more frequently, I'd test it first. Now, it's possible that the effect of __builtin_expect doesn't care about this issue. The GCC manual doesn't help to figure out whether this is the case, because it only talks about a simple case of a single 'if' clause, and doesn't tell any details about what GCC is allowed to do when it sees __builtin_expect. But just by looking at how the code looks, I immediately raised a brow.