All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pranith Kumar <bobby.prani@gmail.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, "Emilio G. Cota" <cota@braap.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Richard Henderson" <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 2/3] atomics: Use __atomic_*_n() variant primitives
Date: Mon, 29 Aug 2016 11:38:45 -0400	[thread overview]
Message-ID: <87twe37fga.fsf@gmail.com> (raw)
In-Reply-To: <c6dc5a5a-76db-9239-6068-f0ea111cf1f3@redhat.com>


Paolo Bonzini writes:

> On 24/08/2016 22:44, Pranith Kumar wrote:
>> Use the __atomic_*_n() primitives which take the value as argument. It
>> is not necessary to store the value locally before calling the
>> primitive, hence saving us a stack store and load.
>
> If you do this, you might as well do it for __atomic_load and
> __atomic_compare_exchange.  However, you'd still need typeof_strip_qual
> for __atomic_compare_exchange's "_old" local variable.

OK, I will update the patch doing the same for load and compare exchange.

>
> The question is, does this actually cause a change in generated code?  I
> would be very surprised if it did, but then it's perfectly possible to
> have a bug in GCC.

It looks like it does. I tested atomic_load() with previous and the
atomic_load_n() version and it generates the following code:

current:
  mov    0x200c4a(%rip),%eax        # 601030 <value>
  mov    %eax,-0x4(%rsp)
  mov    -0x4(%rsp),%eax

atomic_load_n():
  mov    0x200c4a(%rip),%eax        # 601030 <value>

So looks like it's worth it.

Thanks,
-- 
Pranith

  reply	other threads:[~2016-08-29 15:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-24 20:44 [Qemu-devel] [PATCH 1/3] atomics: Test __STDC_VERSION__ for C11 compat Pranith Kumar
2016-08-24 20:44 ` [Qemu-devel] [PATCH 2/3] atomics: Use __atomic_*_n() variant primitives Pranith Kumar
2016-08-29 10:25   ` Paolo Bonzini
2016-08-29 15:38     ` Pranith Kumar [this message]
2016-08-24 20:44 ` [Qemu-devel] [PATCH 3/3] atomics: Remove redundant barrier()'s Pranith Kumar
2016-08-29 10:11 ` [Qemu-devel] [PATCH 1/3] atomics: Test __STDC_VERSION__ for C11 compat Paolo Bonzini

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=87twe37fga.fsf@gmail.com \
    --to=bobby.prani@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=cota@braap.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.