qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Emilio G. Cota" <cota@braap.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "QEMU Developers" <qemu-devel@nongnu.org>,
	"Laurent Desnogues" <laurent.desnogues@gmail.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Richard Henderson" <richard.henderson@linaro.org>
Subject: Re: [Qemu-devel] [PATCH] softfloat: enforce softfloat if the host's FMA is broken
Date: Tue, 25 Dec 2018 01:55:39 -0500	[thread overview]
Message-ID: <20181225065539.GA12687@flamenco> (raw)
In-Reply-To: <CAFEAcA8Bh4q8UdYznLF2E7wZR2p-bF7Zfog90dzeZOc4AfMrMQ@mail.gmail.com>

On Mon, Dec 24, 2018 at 21:38:52 +0000, Peter Maydell wrote:
> On Mon, 24 Dec 2018 at 19:16, Emilio G. Cota <cota@braap.org> wrote:
> > The added branch to the FMA ops is marked as unlikely and therefore
> > its impact on performance (measured with fp-bench) is within noise range
> > when measured on an Intel(R) Xeon(R) Gold 6142 CPU @ 2.60GHz.
> 
> > +        /* check whether avx is usable */
> > +        if (c & bit_OSXSAVE) {
> > +            int bv;
> > +
> > +            __asm("xgetbv" : "=a"(bv), "=d"(d) : "c"(0));
> > +            if ((bv & 6) == 6) {
> > +                has_avx = c & bit_AVX;
> > +            }
> > +        }
> 
> I note that the code in tcg/i386/tcg-target.inc.c
> that does something similar has the note:
>             /* The xgetbv instruction is not available to older versions of
>              * the assembler, so we encode the instruction manually.
>              */
>  -- does that concern apply here?

Yes.

> This will also be the fourth piece of code that has an
> inline xgetbv asm insn to try to query a property of the
> CPU (the other 3 are target/i386/hvf/x86_cpuid.c, util/bufferiszero.c
> and tcg/i386/tcg-target.inc.c), which strongly suggests that we
> should abstract this out a bit better. That also might help
> avoid bugs which exist in one version but not the other:
> see for instance https://bugs.launchpad.net/bugs/1758819

Agreed.

I'll be away until mid-January starting tomorrow, so I'll
send a v2 with the cpuid bits removed, which aren't needed
to fix the bug that Laurent hit.

We can later on address cpuid consolidation.

Thanks,

		Emilio

      reply	other threads:[~2018-12-25  6:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-24 19:15 [Qemu-devel] [PATCH] softfloat: enforce softfloat if the host's FMA is broken Emilio G. Cota
2018-12-24 21:38 ` Peter Maydell
2018-12-25  6:55   ` Emilio G. Cota [this message]

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=20181225065539.GA12687@flamenco \
    --to=cota@braap.org \
    --cc=alex.bennee@linaro.org \
    --cc=laurent.desnogues@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /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).