Re: [Chicken-hackers] Add branch prediction for C_demand checks [was: Re: [PATCH] Statically determine if argvector can be reused]

[Dan Leslie]

Just a guess, but considering that the !! forces the value to resolve to a
valid bool, either 0 or 1, they ran into an issue where the input to their
macros wasn't just the outcome of Boolean operators? IE, perhaps they were
seeing:

    if (likely(SOME_FUNKY_MACRO))

And that SOME_FUNKY_MACRO could be any long value greater than or equal to
zero; or it could expand to a Boolean operation.

-Dan

-----Original Message-----
From: Chicken-hackers
[mailto:address@hidden On Behalf Of
Peter Bex
Sent: December 27, 2016 9:15 AM
To: chicken-hackers <address@hidden>
Subject: [Chicken-hackers] Add branch prediction for C_demand checks [was:
Re: [PATCH] Statically determine if argvector can be reused]

On Thu, Dec 15, 2016 at 11:55:01PM +0100, Peter Bex wrote:
> Hi all,
> 
> I've been playing around a little bit with "perf" and Valgrind's 
> cachegrind tool, and I noticed that the number of branch prediction 
> misses can be reduced if the argvector reusing check can be hardcoded 
> for cases where we know the size of the calling function's argvector.
> 
> Here's a simple patch to make this happen (works on both the master 
> and chicken-5 branches).

And here's another one, that adds C_likely() and C_unlikely() macros, a la
the Linux kernel's likely() and unlikely().  These are simple wrappers for
__builtin_expect() which tell the compiler which branches in a conditional
expression are likely to be taken.

These are now used in generated code, when we do a C_demand() check to
allocate memory.  This is noticably faster in some benchmarks, and can bring
down compilation time a bit as well.  Attached also are benchmark results
for unpatched CHICKEN 5, the previous patch which adds static argvector
checks and results for both this patch and the static argvector.

Question: Should this be:

# define C_unlikely(x)             __builtin_expect((x), 0)

or, like Linux:

# define C_unlikely(x)             __builtin_expect(!!(x), 0)

The latter seems to me like it would add more instructions due to the double
negation, and the exact value of the expression in an if() is never
important anyway, as long as it's zero or nonzero.  Right?

Cheers,
Peter