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(-)
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 <eblake@redhat.com>
---
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
On 1/6/19 11:38 AM, Eric Blake wrote:
> +/*
> + * 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
What's wrong with always using typeof? This seems like it leaves potential odd
bugs affecting gcc-4.8.
> +#undef MIN
> +#define MIN(a, b) \
> + ({ \
> + QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
> + QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
If you're promoting the type, why don't you want to promote to the common type
between A and B? E.g.
__typeof((a) + (b)) _a = (a), _b = (b);
After all, that's what the result type of (p ? _a : _b) will be.
r~
On 1/6/19 2:32 AM, Richard Henderson wrote:
> On 1/6/19 11:38 AM, Eric Blake wrote:
>> +/*
>> + * 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
>
> What's wrong with always using typeof? This seems like it leaves potential odd
> bugs affecting gcc-4.8.
Always using typeof is an option, but gcc documents that __auto_type is
nicer than typeof:
> Using '__auto_type' instead of 'typeof' has two advantages:
>
> * Each argument to the macro appears only once in the expansion of
> the macro. This prevents the size of the macro expansion growing
> exponentially when calls to such macros are nested inside arguments
> of such macros.
>
> * If the argument to the macro has variably modified type, it is
> evaluated only once when using '__auto_type', but twice if 'typeof'
> is used.
We don't use variably modified types (at least, I don't think we do), so
the latter is moot (but WOULD be the spot where we are most likely to be
bitten on 4.8 compilers lacking __auto_type); the former point is a
minor speed win in favor of __auto_type.
>
>> +#undef MIN
>> +#define MIN(a, b) \
>> + ({ \
>> + QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
>> + QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
>
> If you're promoting the type, why don't you want to promote to the common type
> between A and B? E.g.
>
> __typeof((a) + (b)) _a = (a), _b = (b);
>
> After all, that's what the result type of (p ? _a : _b) will be.
That formulation should work as well, if anyone likes it better (but it
does NOT work with __auto_type).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
* Eric Blake (eblake@redhat.com) wrote:
> 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 <eblake@redhat.com>
>
> ---
> 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())
Why do these need to be separate macros? Can't you just put the
non-constant code in what you have as the 'builtin_unreachable' side of
the choose_expr:
#define DMIN(a,b) __builtin_choose_expr( \
__builtin_constant_p(a) && __builtin_constant_p(b), \
(a) < (b) ? (a) : (b), \
({ \
QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
_a < _b ? _a : _b; \
}))
Dave
> +#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
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 1/7/19 3:49 AM, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
>> 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(),
>> +#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())
>
> Why do these need to be separate macros? Can't you just put the
> non-constant code in what you have as the 'builtin_unreachable' side of
> the choose_expr:
>
> #define DMIN(a,b) __builtin_choose_expr( \
> __builtin_constant_p(a) && __builtin_constant_p(b), \
> (a) < (b) ? (a) : (b), \
> ({ \
> QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
> QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
> _a < _b ? _a : _b; \
> }))
Because it doesn't work - gcc treats ({}) as a syntax error inside
constant expressions, even in dead code (although 'info gcc' said that
might change in the future, we can't wait for that change). I also
tried it as documented here:
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00715.html
hence my mention in the commit message.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
* Eric Blake (eblake@redhat.com) wrote:
> On 1/7/19 3:49 AM, Dr. David Alan Gilbert wrote:
> > * Eric Blake (eblake@redhat.com) wrote:
> >> 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(),
>
>
> >> +#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())
> >
> > Why do these need to be separate macros? Can't you just put the
> > non-constant code in what you have as the 'builtin_unreachable' side of
> > the choose_expr:
> >
> > #define DMIN(a,b) __builtin_choose_expr( \
> > __builtin_constant_p(a) && __builtin_constant_p(b), \
> > (a) < (b) ? (a) : (b), \
> > ({ \
> > QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
> > QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
> > _a < _b ? _a : _b; \
> > }))
>
> Because it doesn't work - gcc treats ({}) as a syntax error inside
> constant expressions, even in dead code (although 'info gcc' said that
> might change in the future, we can't wait for that change). I also
> tried it as documented here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00715.html
> hence my mention in the commit message.
Ah, I didn't understand the context in your message; you say 'even in
the dead branch of a __builtin_choose_expr()' but the following works
for me (on f29 and rhel7):
#include <stdio.h>
# define QEMU_TYPEOF(a) typeof(a)
#define DMIN(a,b) __builtin_choose_expr( \
__builtin_constant_p(a) && __builtin_constant_p(b), \
(a) < (b) ? (a) : (b), \
({ \
QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
_a < _b ? _a : _b; \
}))
int main(int argc, char *argv[])
{
int anarray[DMIN(5, 10)];
int a=5;
int b=6;
fprintf(stderr, "sizeof(anarray)=%zd DMIN(a,b)=%d\n", sizeof(anarray), DMIN(a++,b++));
fprintf(stderr, "a=%d b=%d\n", a,b);
}
Dave
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 1/7/19 9:07 AM, Dr. David Alan Gilbert wrote:
>>>> 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(),
>> Because it doesn't work - gcc treats ({}) as a syntax error inside
>> constant expressions, even in dead code (although 'info gcc' said that
>> might change in the future, we can't wait for that change). I also
>> tried it as documented here:
>> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00715.html
>> hence my mention in the commit message.
>
> Ah, I didn't understand the context in your message; you say 'even in
> the dead branch of a __builtin_choose_expr()' but the following works
> for me (on f29 and rhel7):
>
> #include <stdio.h>
>
> # define QEMU_TYPEOF(a) typeof(a)
>
> #define DMIN(a,b) __builtin_choose_expr( \
> __builtin_constant_p(a) && __builtin_constant_p(b), \
> (a) < (b) ? (a) : (b), \
> ({ \
> QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
> QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
> _a < _b ? _a : _b; \
> }))
>
>
> int main(int argc, char *argv[])
> {
> int anarray[DMIN(5, 10)];
Not a constant context. As written, you have declared a variable-length
array, determined at runtime (even if the array is not actually
variable-length because you always provide the same length). Hoist the
declaration anarray outside of main() to see the difference. Or try:
struct foo {
int bar : DMIN(5, 10);
};
for another example of a constant context (again, outside of a function).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
* Eric Blake (eblake@redhat.com) wrote:
> On 1/7/19 9:07 AM, Dr. David Alan Gilbert wrote:
>
> >>>> 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(),
>
> >> Because it doesn't work - gcc treats ({}) as a syntax error inside
> >> constant expressions, even in dead code (although 'info gcc' said that
> >> might change in the future, we can't wait for that change). I also
> >> tried it as documented here:
> >> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00715.html
> >> hence my mention in the commit message.
> >
> > Ah, I didn't understand the context in your message; you say 'even in
> > the dead branch of a __builtin_choose_expr()' but the following works
> > for me (on f29 and rhel7):
> >
> > #include <stdio.h>
> >
> > # define QEMU_TYPEOF(a) typeof(a)
> >
> > #define DMIN(a,b) __builtin_choose_expr( \
> > __builtin_constant_p(a) && __builtin_constant_p(b), \
> > (a) < (b) ? (a) : (b), \
> > ({ \
> > QEMU_TYPEOF((a) + 0) _a = (a) + 0; \
> > QEMU_TYPEOF((b) + 0) _b = (b) + 0; \
> > _a < _b ? _a : _b; \
> > }))
> >
> >
> > int main(int argc, char *argv[])
> > {
> > int anarray[DMIN(5, 10)];
>
> Not a constant context. As written, you have declared a variable-length
> array, determined at runtime (even if the array is not actually
> variable-length because you always provide the same length). Hoist the
> declaration anarray outside of main() to see the difference. Or try:
>
> struct foo {
> int bar : DMIN(5, 10);
> };
>
> for another example of a constant context (again, outside of a function).
Ah OK, yes that does fail as you say. Fair enough.
Dave
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2024 Red Hat, Inc.