[Qemu-devel] [PATCH v2] osdep: Make MIN/MAX evaluate arguments only once

[Eric Blake]

Add the macro QEMU_TYPEOF() to access __auto_type in new enough
compilers, while falling back to typeof on older compilers (the
fallback doesn't handle variable length arrays, but we don't use
those; it also expands to more text).

Then use that macro to make MIN/MAX only evaluate their argument
once; this uses type promotion (by adding to 0) to work around
the fact that typeof(bitfield) won't compile.  However, we are
unable to work around gcc refusing to compile ({}) in a constant
context, even when only used in the dead branch of a
__builtin_choose_expr(), so we have to provide a second macro
pair MIN_CONST and MAX_CONST for use when both arguments are
known to be compile-time constants and where the result must
also be usable in constant contexts.

Fix the resulting callsites that compute a compile-time constant
min or max to use the new macros.

Signed-off-by: Eric Blake <address@hidden>

---
v2: Force use of our smart macros, and make macro handle bitfields,
constant expressions, and older compilers [Zoltan]
---
 hw/usb/hcd-xhci.h       |  2 +-
 include/exec/cpu-defs.h |  2 +-
 include/qemu/compiler.h | 10 +++++++++
 include/qemu/osdep.h    | 46 ++++++++++++++++++++++++++++++++++-------
 migration/qemu-file.c   |  2 +-
 5 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index fc36a4c787c..10beee9a7b1 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -210,7 +210,7 @@ struct XHCIState {
     uint32_t dcbaap_high;
     uint32_t config;

-    USBPort  uports[MAX(MAXPORTS_2, MAXPORTS_3)];
+    USBPort  uports[MAX_CONST(MAXPORTS_2, MAXPORTS_3)];
     XHCIPort ports[MAXPORTS];
     XHCISlot slots[MAXSLOTS];
     uint32_t numports;
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 6a60f94a41d..5b2fd46f687 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -90,7 +90,7 @@ typedef uint64_t target_ulong;
  * of tlb_table inside env (which is non-trivial but not huge).
  */
 #define CPU_TLB_BITS                                             \
-    MIN(8,                                                       \
+    MIN_CONST(8,                                                 \
         TCG_TARGET_TLB_DISPLACEMENT_BITS - CPU_TLB_ENTRY_BITS -  \
         (NB_MMU_MODES <= 1 ? 0 :                                 \
          NB_MMU_MODES <= 2 ? 1 :                                 \
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 261842beae2..3bf6f68a6b0 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -191,4 +191,14 @@
 #define QEMU_GENERIC9(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC8(x, 
__VA_ARGS__))
 #define QEMU_GENERIC10(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC9(x, 
__VA_ARGS__))

+/*
+ * Automatic type deduction, to be used as:
+ * QEMU_TYPEOF(expr) name = expr;
+ */
+#if QEMU_GNUC_PREREQ(4, 9)
+# define QEMU_TYPEOF(a) __auto_type
+#else
+# define QEMU_TYPEOF(a) typeof(a)
+#endif
+
 #endif /* COMPILER_H */
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 3bf48bcdec0..821ce627f73 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -232,18 +232,48 @@ extern int daemon(int, int);
 #define SIZE_MAX ((size_t)-1)
 #endif

-#ifndef MIN
-#define MIN(a, b) (((a) < (b)) ? (a) : (b))
-#endif
-#ifndef MAX
-#define MAX(a, b) (((a) > (b)) ? (a) : (b))
-#endif
+/*
+ * Two variations of MIN/MAX macros. The first is for runtime use, and
+ * evaluates arguments only once (so it is safe even with side
+ * effects), but will not work in constant contexts (such as array
+ * size declarations).  The second is for compile-time use, where
+ * evaluating arguments twice is safe because the result is going to
+ * be constant anyway.
+ */
+#undef MIN
+#define MIN(a, b)                            \
+    ({                                       \
+        QEMU_TYPEOF((a) + 0) _a = (a) + 0;   \
+        QEMU_TYPEOF((b) + 0) _b = (b) + 0;   \
+        _a < _b ? _a : _b;                   \
+    })
+#define MIN_CONST(a, b)                                         \
+    __builtin_choose_expr(                                      \
+        __builtin_constant_p(a) && __builtin_constant_p(b),     \
+        (a) < (b) ? (a) : (b),                                  \
+        __builtin_unreachable())
+#undef MAX
+#define MAX(a, b)                            \
+    ({                                       \
+        QEMU_TYPEOF((a) + 0) _a = (a) + 0;   \
+        QEMU_TYPEOF((b) + 0) _b = (b) + 0;   \
+        _a > _b ? _a : _b;                   \
+    })
+#define MAX_CONST(a, b)                                         \
+    __builtin_choose_expr(                                      \
+        __builtin_constant_p(a) && __builtin_constant_p(b),     \
+        (a) > (b) ? (a) : (b),                                  \
+        __builtin_unreachable())

 /* Minimum function that returns zero only iff both values are zero.
  * Intended for use with unsigned values only. */
 #ifndef MIN_NON_ZERO
-#define MIN_NON_ZERO(a, b) ((a) == 0 ? (b) : \
-                                ((b) == 0 ? (a) : (MIN(a, b))))
+#define MIN_NON_ZERO(a, b)                              \
+    ({                                                  \
+        QEMU_TYPEOF((a) + 0) _a = (a) + 0;              \
+        QEMU_TYPEOF((b) + 0) _b = (b) + 0;              \
+        _a == 0 ? _b : (_b == 0 || _b > _a) ? _a : _b;  \
+    })
 #endif

 /* Round number down to multiple */
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 977b9ae07c1..9746cbec0f3 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -31,7 +31,7 @@
 #include "trace.h"

 #define IO_BUF_SIZE 32768
-#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
+#define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)

 struct QEMUFile {
     const QEMUFileOps *ops;
-- 
2.20.1