Re: [hackers] [libgrapheme] Mark likely branches || Laslo Hunhold
On Wed, 5 Jan 2022 02:24:01 +0600 NRK wrote: Dear NRK, > Answering my own question: because it fails if `__has_builtin` is not > defined. I was expecting the 2nd expression wouldn't get evaluated at > all. Should probably take some time and learn more about the > pre-processor sometimes. yes exactly. If you use normal non-function-like-macros that don't exist, it works out, as they are simply replaced with 0 in such an expression. At least GCC, from what I know of, always evaluates all macros in each expression, and it seems to be undefined in the standard if you do that beforehand. It's different for function-like-macros: If they do not exist, it throws an error instead of replacing it with 0, which can easily be confirmed by trying to compile a file test.c containing #if defined (idontexist) && idontexist(test) #endif yielding $ cc -o test test.c test.c:1:29: error: function-like macro 'idontexist' is not defined #if defined (idontexist) && idontexist(test) ^ 1 error generated. $ This is also probably a good reason to always use nested ifdefs instead of ifs, as using if-constructs leads to such surprises given the macro-logic-operators don't seem to behave like the ones in the language itself. Using ifdef forces you to only evaluate one condition per line. With best regards Laslo
Re: [hackers] [libgrapheme] Mark likely branches || Laslo Hunhold
NRK wrote: > On Wed, Jan 05, 2022 at 01:56:26AM +0600, NRK wrote: > > Just curious, why not use: > > > > #if defined(__has_builtin) && __has_builtin(__builtin_expect) > > #define likely(expr) __builtin_expect(!!(expr), 1) > > #define unlikely(expr) __builtin_expect(!!(expr), 0) > > #else > > #define likely(expr) (expr) > > #define unlikely(expr) (expr) > > #endif > > Answering my own question: because it fails if `__has_builtin` is not > defined. I was expecting the 2nd expression wouldn't get evaluated at > all. Should probably take some time and learn more about the > pre-processor sometimes. Same for me! I didn't know that the C pre-processor doesn't short-circuit ... Thanks for answering your own question! As you can see, it's useful for others as well :) Cheers, Silvan
Re: [hackers] [libgrapheme] Mark likely branches || Laslo Hunhold
On Wed, Jan 05, 2022 at 01:56:26AM +0600, NRK wrote: > Just curious, why not use: > > #if defined(__has_builtin) && __has_builtin(__builtin_expect) > #define likely(expr) __builtin_expect(!!(expr), 1) > #define unlikely(expr) __builtin_expect(!!(expr), 0) > #else > #define likely(expr) (expr) > #define unlikely(expr) (expr) > #endif Answering my own question: because it fails if `__has_builtin` is not defined. I was expecting the 2nd expression wouldn't get evaluated at all. Should probably take some time and learn more about the pre-processor sometimes. - NRK
Re: [hackers] [libgrapheme] Mark likely branches || Laslo Hunhold
> +#ifdef __has_builtin > + #if __has_builtin(__builtin_expect) > + #define likely(expr) __builtin_expect(!!(expr), 1) > + #define unlikely(expr) __builtin_expect(!!(expr), 0) > + #else > + #define likely(expr) (expr) > + #define unlikely(expr) (expr) > + #endif > +#else > + #define likely(expr) (expr) > + #define unlikely(expr) (expr) > +#endif Just curious, why not use: #if defined(__has_builtin) && __has_builtin(__builtin_expect) #define likely(expr) __builtin_expect(!!(expr), 1) #define unlikely(expr) __builtin_expect(!!(expr), 0) #else #define likely(expr) (expr) #define unlikely(expr) (expr) #endif - NRK
[hackers] [libgrapheme] Mark likely branches || Laslo Hunhold
commit e917e53a05de7cab2591d6b2fa3f2ce5ece89bcf
Author: Laslo Hunhold
AuthorDate: Tue Jan 4 18:56:38 2022 +0100
Commit: Laslo Hunhold
CommitDate: Tue Jan 4 18:56:38 2022 +0100
Mark likely branches
The likely() and unlikely() macros (known from the Linux kernel) in
src/util.h are defined in a portable way.
This addition brings a performance improvment of around 6%, which also
shows how well-optimized grapheme_is_character_break() already is. One
break check for two codepoints now takes only around 10µs on average on
my machine, which is insanely fast when you consider the complexity of
the algorithm behind it.
Signed-off-by: Laslo Hunhold
diff --git a/src/character.c b/src/character.c
index 80a9a79..27e1cf0 100644
--- a/src/character.c
+++ b/src/character.c
@@ -105,10 +105,10 @@ static const uint_least16_t dont_break_gb12_13[2 *
NUM_BREAK_PROPS] = {
static enum break_property
get_break_prop(uint_least32_t cp)
{
- if (cp > 0x10) {
- return BREAK_PROP_OTHER;
- } else {
+ if (likely(cp <= 0x10)) {
return prop[minor[major[cp >> 8] + (cp & 0xff)]].break_property;
+ } else {
+ return BREAK_PROP_OTHER;
}
}
@@ -118,11 +118,11 @@ grapheme_is_character_break(uint_least32_t cp0,
uint_least32_t cp1, GRAPHEME_STA
enum break_property cp0_prop, cp1_prop;
bool notbreak = false;
- if (state) {
- if (!state->prop_set) {
- cp0_prop = get_break_prop(cp0);
- } else {
+ if (likely(state)) {
+ if (likely(state->prop_set)) {
cp0_prop = state->prop;
+ } else {
+ cp0_prop = get_break_prop(cp0);
}
cp1_prop = get_break_prop(cp1);
@@ -153,7 +153,7 @@ grapheme_is_character_break(uint_least32_t cp0,
uint_least32_t cp1, GRAPHEME_STA
UINT16_C(1 << cp1_prop));
/* update or reset flags (when we have a break) */
- if (!notbreak) {
+ if (likely(!notbreak)) {
state->gb11_flag = state->gb12_13_flag = false;
}
} else {
diff --git a/src/util.h b/src/util.h
index 049af2f..b61a026 100644
--- a/src/util.h
+++ b/src/util.h
@@ -10,4 +10,19 @@
#define LEN(x) (sizeof(x) / sizeof(*(x)))
+#undef likely
+#undef unlikely
+#ifdef __has_builtin
+ #if __has_builtin(__builtin_expect)
+ #define likely(expr) __builtin_expect(!!(expr), 1)
+ #define unlikely(expr) __builtin_expect(!!(expr), 0)
+ #else
+ #define likely(expr) (expr)
+ #define unlikely(expr) (expr)
+ #endif
+#else
+ #define likely(expr) (expr)
+ #define unlikely(expr) (expr)
+#endif
+
#endif /* UTIL_H */
