Re: Using __builtin_expect (likely/unlikely macros)

[Alex Gramiak <agrambot@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1114 bytes --]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Konstantin Kharlamov wrote:
>> I was told that e.g. "cold" attribute can sometimes produce unbearably slow
>> code https://gcc.gnu.org/ml/gcc-help/2019-01/msg00035.html
>
> Although cold functions can be slow, it appears that overall it's a win for
> Emacs to mark _Noreturn error function declarations as cold: on my platform,
> 'make compile-always' ran about 1.3% faster. So I installed the attached patch
> into master.

Thanks. What do you think about marking a few bytecode cases as
cold/unused? I've attached a diff below that does that. Does it make a
difference for you on your setup? It seems to slow Emacs down a slight
bit for me, but I was hoping you might know why it would do so. Since
the newly cold attributes should be unused, is this perhaps a GCC bug?

> This patch also adds a convenience macro AVOID for the now-common pattern
> '_Noreturn ATTRIBUTE_COLD void'.

I'm not sure about the name. If I wasn't part of this discussion I might
have thought AVOID meant that one should avoid usage of the procedure in
new code. Not a big deal, of course.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: cold attributes --]
[-- Type: text/x-patch, Size: 2159 bytes --]

diff --git a/src/bytecode.c b/src/bytecode.c
index 40977799bf..17c4716fe0 100644
--- a/src/bytecode.c
+++ b/src/bytecode.c
@@ -656,6 +656,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
 	CASE (Bunbind_all):	/* Obsolete.  Never used.  */
 	  /* To unbind back to the beginning of this frame.  Not used yet,
 	     but will be needed for tail-recursion elimination.  */
+          __attribute__((cold, unused));
 	  unbind_to (count, Qnil);
 	  NEXT;
 
@@ -749,6 +750,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
 	  NEXT;
 
 	CASE (Bsave_window_excursion): /* Obsolete since 24.1.  */
+          ATTRIBUTE_COLD;
 	  {
 	    ptrdiff_t count1 = SPECPDL_INDEX ();
 	    record_unwind_protect (restore_window_configuration,
@@ -808,6 +810,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
 	  }
 
 	CASE (Bcondition_case):		/* Obsolete since 24.4.  */
+          ATTRIBUTE_COLD;
 	  {
 	    Lisp_Object handlers = POP, body = POP;
 	    TOP = internal_lisp_condition_case (TOP, body, handlers);
@@ -815,12 +818,14 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
 	  }
 
 	CASE (Btemp_output_buffer_setup): /* Obsolete since 24.1.  */
+          ATTRIBUTE_COLD;
 	  CHECK_STRING (TOP);
 	  temp_output_buffer_setup (SSDATA (TOP));
 	  TOP = Vstandard_output;
 	  NEXT;
 
 	CASE (Btemp_output_buffer_show): /* Obsolete since 24.1.  */
+          ATTRIBUTE_COLD;
 	  {
 	    Lisp_Object v1 = POP;
 	    temp_output_buffer_show (TOP);
@@ -1138,6 +1143,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
 	  NEXT;
 
 	CASE (Binteractive_p):	/* Obsolete since 24.1.  */
+          ATTRIBUTE_COLD;
 	  PUSH (call0 (intern ("interactive-p")));
 	  NEXT;
 
@@ -1342,6 +1348,7 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
 	  /* Actually this is Bstack_ref with offset 0, but we use Bdup
 	     for that instead.  */
 	  /* CASE (Bstack_ref): */
+          ATTRIBUTE_COLD;
 	  error ("Invalid byte opcode: op=%d, ptr=%"pD"d",
 		 op, pc - 1 - bytestr_data);