qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Richard Henderson <rth@twiddle.net>, qemu-devel@nongnu.org
Cc: dirty.ice.hu@gmail.com, f4bug@amsat.org,
	Gerd Hoffmann <kraxel@redhat.com>,
	Peter Crosthwaite <crosthwaite.peter@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2] osdep: Make MIN/MAX evaluate arguments only once
Date: Mon, 7 Jan 2019 08:22:16 -0600	[thread overview]
Message-ID: <80396f87-87d0-5ba9-3efc-405a8dce16d8@redhat.com> (raw)
In-Reply-To: <657598d6-0cea-91dc-a975-f716522a49b9@twiddle.net>

[-- Attachment #1: Type: text/plain, Size: 1985 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2019-01-07 14:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-06  1:38 [Qemu-devel] [PATCH v2] osdep: Make MIN/MAX evaluate arguments only once Eric Blake
2019-01-06  8:32 ` Richard Henderson
2019-01-07 14:22   ` Eric Blake [this message]
2019-01-07  9:49 ` Dr. David Alan Gilbert
2019-01-07 14:24   ` Eric Blake
2019-01-07 15:07     ` Dr. David Alan Gilbert
2019-01-07 16:16       ` Eric Blake
2019-01-07 16:24         ` Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=80396f87-87d0-5ba9-3efc-405a8dce16d8@redhat.com \
    --to=eblake@redhat.com \
    --cc=crosthwaite.peter@gmail.com \
    --cc=dgilbert@redhat.com \
    --cc=dirty.ice.hu@gmail.com \
    --cc=f4bug@amsat.org \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).