[PATCH 1/8] Add GCC_LIKELY and GCC_UNLIKELY

[Alexander Monakov]

On Thu, 14 Jun 2018, David Malcolm wrote:

> The idea is to later use these macros to mark the
>   if (dump_enabled_p ())
> parts of the compiler as cold, in the hope of helping non-PGO builds
> of gcc.

I think a cleaner way to achieve that would be to mark a function called
under the predicate with ATTRIBUTE_COLD: if you have

    if (dump_enabled_p ())
      {
        ...
        dump_something ();
	...
      }

then declaring dump_something with __attribute__((cold)) informs the
compiler that the branch leading to it is unlikely. It also moves the
function body to the "cold" text region, but that may be acceptable.

Or, perhaps even simpler, you can

    #define dump_enabled_p() __builtin_expect (dump_enabled_p (), 0)

> 	* system.h (GCC_LIKELY, GCC_UNLIKELY): New macros, adapted from
> 	libgfortran.h.

I like the idea of these macros, but can their spelling use lowercase please,
like the Linux kernel, Glibc (and even libgfortran.h) spell their variants.

> +/* The following macros can be used to annotate conditions which are likely or
> +   unlikely to be true.  Avoid using them when a condition is only slightly
> +   more likely/less unlikely than average to avoid the performance penalties of
> +   branch misprediction. In addition, as __builtin_expect overrides the compiler
> +   heuristic, do not use in conditions where one of the branches ends with a
> +   call to a function with __attribute__((noreturn)): the compiler internal
> +   heuristic will mark this branch as much less likely as GCC_UNLIKELY() would
> +   do.  */

I realize this is copied verbatim from libgfortran.h, but I think the comment
is not accurately worded and should not be duplicated. I'd suggest simply

/* Shorthands for informing compiler's static branch prediction.  */

> +#define GCC_LIKELY(X)       __builtin_expect(!!(X), 1)
> +#define GCC_UNLIKELY(X)     __builtin_expect(!!(X), 0)

The '!!'s are not necessary here, as far as I can tell.

Alexander