[OPTIM] GCC4's builtin_expect() is suboptimal

GCC4 is stupid (unbelievable news!).

When some code uses __builtin_expect(x != 0, 1), it really performs
the check of x != 0 then tests that the result is not zero! This is
a double check when only one was expected. Some performance drops
of 10% in the HTTP parser code have been observed due to this bug.

GCC 3.4 is fine though.

A solution consists in expecting that the tested value is 1. In
this case, it emits the correct code, but it's still not optimal
it seems. Finally the best solution is to ignore likely() and to
pray for the compiler to emit correct code. However, we still have
to fix unlikely() to remove the test there too, and to fix all
code which passed pointers overthere to pass integers instead.

include/common/ebtree.h

@@ -1,6 +1,6 @@
 /*
  * Elastic Binary Trees - generic macros and structures.
- * (C) 2002-2007 - Willy Tarreau <w@1wt.eu>
+ * (C) 2002-2008 - Willy Tarreau <w@1wt.eu>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -307,13 +307,23 @@ static inline int fls64(unsigned long long x)
  * this in inline functions, because the code reordering it causes very often
  * has a negative impact on the calling functions.
  */
-#if __GNUC__ < 3 && !defined(__builtin_expect)
+#if !defined(likely)
+#if __GNUC__ < 3
 #define __builtin_expect(x,y) (x)
-#endif
-
-#ifndef likely
+#define likely(x) (x)
+#define unlikely(x) (x)
+#elif __GNUC__ < 4
+/* gcc 3.x does the best job at this */
 #define likely(x) (__builtin_expect((x) != 0, 1))
 #define unlikely(x) (__builtin_expect((x) != 0, 0))
+#else
+/* GCC 4.x is stupid, it performs the comparison then compares it to 1,
+ * so we cheat in a dirty way to prevent it from doing this. This will
+ * only work with ints and booleans though.
+ */
+#define likely(x) (x)
+#define unlikely(x) (__builtin_expect((x), 0))
+#endif
 #endif
 
 /* Support passing function parameters in registers. For this, the

include/common/standard.h

@@ -2,7 +2,7 @@
   include/common/standard.h
   This files contains some general purpose functions and macros.
 
-  Copyright (C) 2000-2007 Willy Tarreau - w@1wt.eu
+  Copyright (C) 2000-2008 Willy Tarreau - w@1wt.eu
   
   This library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
@@ -46,13 +46,24 @@
  * helps the compiler produce the most compact critical paths, which is
  * generally better for the cache and to reduce the number of jumps.
  */
+#if !defined(likely)
 #if __GNUC__ < 3
 #define __builtin_expect(x,y) (x)
-#endif
-
+#define likely(x) (x)
+#define unlikely(x) (x)
+#elif __GNUC__ < 4
+/* gcc 3.x does the best job at this */
 #define likely(x) (__builtin_expect((x) != 0, 1))
 #define unlikely(x) (__builtin_expect((x) != 0, 0))
-
+#else
+/* GCC 4.x is stupid, it performs the comparison then compares it to 1,
+ * so we cheat in a dirty way to prevent it from doing this. This will
+ * only work with ints and booleans though.
+ */
+#define likely(x) (x)
+#define unlikely(x) (__builtin_expect((x), 0))
+#endif
+#endif
 
 /*
  * copies at most <size-1> chars from <src> to <dst>. Last char is always

include/proto/task.h

@@ -55,7 +55,7 @@ static inline struct task *__task_wakeup(struct task *t)
 	DLIST_ADD(run_queue, &t->qlist);
 	t->state = TASK_RUNNING;
 
-	if (likely(t->wq)) {
+	if (likely(t->wq != NULL)) {
 		tree_delete(t->wq);
 		t->wq = NULL;
 	}

src/backend.c

@@ -786,7 +786,7 @@ static struct server *fwrr_get_next_server(struct proxy *p)
 	fwrr_queue_srv(srv);
 
  requeue_servers:
-	if (unlikely(full)) {
+	if (unlikely(full != NULL)) {
 		if (switched) {
 			/* the tree has switched, requeue all extracted servers
 			 * into "init", because their place was lost, and only

src/ev_epoll.c

@@ -142,7 +142,7 @@ REGPRM2 static void alloc_chg_list(const int fd, int old_evt)
 {
 	struct fd_chg *ptr;
 
-	if (unlikely(chg_ptr[fd]))
+	if (unlikely(chg_ptr[fd] != NULL))
 		return;
 
 #if LIMIT_NUMBER_OF_CHANGES

src/task.c

@@ -68,7 +68,7 @@ struct task *task_queue(struct task *task)
 		task->qlist.p = NULL;
 	}
 
-	if (unlikely(task->wq)) {
+	if (unlikely(task->wq != NULL)) {
 		tree_delete(task->wq);
 		task->wq = NULL;
 	}