Front page | perl.perl5.porters |
Postings from March 2013
Re: use of LIKELY() and UNLIKELY() branch predictors
Thread Previous
|
Thread Next
From:
Nicholas Clark
Date:
March 6, 2013 15:55
Subject:
Re: use of LIKELY() and UNLIKELY() branch predictors
Message ID:
20130306155523.GO3729@plum.flirble.org
On Thu, Jan 31, 2013 at 01:51:15PM +0100, Steffen Mueller wrote:
> On 01/31/2013 11:45 AM, Zefram wrote:
^^^^^^^^^^
your mailer seems to be on the wrong side of the pond :-)
> > By the way, LIKELY() and UNLIKELY() should probably include an implicit
> > cBOOL in their expansions. As it is now, LIKELY(ptr) is saying that we
> > expect the pointer to have the value 1, not merely that we expect it to
> > be non-null.
>
> Not in the branch either.
I pushed this to smoke-me/nicholas/likely-cBOOL:
commit c0b8e78149fde4b5e8c0319f943946a178b3109e
Author: Nicholas Clark <nick@ccl4.org>
Date: Wed Mar 6 16:27:08 2013 +0100
Change LIKELY() and UNLIKELY() to cast their first argument to a bool.
This avoids problems such as pointers being compared with the literal value
Suggested by Zefram.
Note that this is technically a change of behaviour, as the macro EXPECT(),
which they are both both wrappers for, returns its value, so they will now
be returning TRUE or FALSE, rather than the actual value passed to them.
However, all uses in the core and on CPAN are only within if() statements, s
this should not make any difference in practice.
diff --git a/perl.h b/perl.h
index 4b020c7..a7b8251 100644
--- a/perl.h
+++ b/perl.h
@@ -3329,8 +3329,8 @@ typedef pthread_key_t perl_key;
#else
# define EXPECT(expr,val) (expr)
#endif
-#define LIKELY(cond) EXPECT(cond,1)
-#define UNLIKELY(cond) EXPECT(cond,0)
+#define LIKELY(cond) EXPECT(cBOOL(cond),TRUE)
+#define UNLIKELY(cond) EXPECT(cBOOL(cond),FALSE)
#ifdef HAS_BUILTIN_CHOOSE_EXPR
/* placeholder */
#endif
While grovelling on CPAN for uses of LIKELY and UNLIKELY, I found this code
in bundled code from CityHash:
#if !defined(LIKELY)
#if HAVE_BUILTIN_EXPECT
#define LIKELY(x) (__builtin_expect(!!(x), 1))
#else
#define LIKELY(x) (x)
#endif
#endif
Which I think is actually a potential bug, isn't it?
In that __builtin_expect() is documented as returning its first argument,
hence a no-op replacement for it has to return the same value, or it's not
actually a replacement.
> So far, my trivial and meaningless benchmarks have not uncovered any
> effect of the hints on linux/gcc4.7.2/x84_64. I did not look at the
> generated code.
I looked at the code generated by "gcc (Debian 4.6.3-14) 4.6.3" on ARM.
At -O2, it is generating different code. Rather too much to figure out, but
as best I could tell from the first diff hunk, it's inverting the sense of
branches. I don't know how much branch prediction current ARMs have
(15 years ago they had none), so this is going to make a difference of a
cycle per branch avoided. Which isn't going to be much, but is in the right
direction *provided* the annotation is more correct than the compiler's
guesswork. Which is probably going to be true for the easy to reason cases.
Nicholas Clark
Thread Previous
|
Thread Next