All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: Lenard Szolnoki <blog@lenardszolnoki.com>
Subject: [PATCH] target/i386: fix operand order for PDEP and PEXT
Date: Mon, 23 Nov 2020 08:14:26 -0500	[thread overview]
Message-ID: <20201123131426.2725276-1-pbonzini@redhat.com> (raw)

For PDEP and PEXT, the mask is provided in the memory (mod+r/m)
operand, and therefore is loaded in s->T0 by gen_ldst_modrm.
The source is provided in the second source operand (VEX.vvvv)
and therefore is loaded in s->T1.  Fix the order in which
they are passed to the helpers.

Reported-by: Lenard Szolnoki <blog@lenardszolnoki.com>
Analyzed-by: Lenard Szolnoki <blog@lenardszolnoki.com>
Fixes: https://bugs.launchpad.net/qemu/+bug/1605123
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/translate.c         |  8 +++----
 tests/tcg/i386/Makefile.target  |  4 ++++
 tests/tcg/i386/test-i386-bmi2.c | 40 +++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 tests/tcg/i386/test-i386-bmi2.c

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 4c57307e42..e8f5f5803a 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -3936,14 +3936,14 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
                 }
                 ot = mo_64_32(s->dflag);
                 gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
-                /* Note that by zero-extending the mask operand, we
+                /* Note that by zero-extending the source operand, we
                    automatically handle zero-extending the result.  */
                 if (ot == MO_64) {
                     tcg_gen_mov_tl(s->T1, cpu_regs[s->vex_v]);
                 } else {
                     tcg_gen_ext32u_tl(s->T1, cpu_regs[s->vex_v]);
                 }
-                gen_helper_pdep(cpu_regs[reg], s->T0, s->T1);
+                gen_helper_pdep(cpu_regs[reg], s->T1, s->T0);
                 break;
 
             case 0x2f5: /* pext Gy, By, Ey */
@@ -3954,14 +3954,14 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
                 }
                 ot = mo_64_32(s->dflag);
                 gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
-                /* Note that by zero-extending the mask operand, we
+                /* Note that by zero-extending the source operand, we
                    automatically handle zero-extending the result.  */
                 if (ot == MO_64) {
                     tcg_gen_mov_tl(s->T1, cpu_regs[s->vex_v]);
                 } else {
                     tcg_gen_ext32u_tl(s->T1, cpu_regs[s->vex_v]);
                 }
-                gen_helper_pext(cpu_regs[reg], s->T0, s->T1);
+                gen_helper_pext(cpu_regs[reg], s->T1, s->T0);
                 break;
 
             case 0x1f6: /* adcx Gy, Ey */
diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target
index a66232a67d..9f6f620944 100644
--- a/tests/tcg/i386/Makefile.target
+++ b/tests/tcg/i386/Makefile.target
@@ -18,6 +18,10 @@ test-i386-pcmpistri: CFLAGS += -msse4.2
 run-test-i386-pcmpistri: QEMU_OPTS += -cpu max
 run-plugin-test-i386-pcmpistri-%: QEMU_OPTS += -cpu max
 
+test-i386-bmi2: CFLAGS += -mbmi2
+run-test-i386-bmi2: QEMU_OPTS += -cpu max
+run-plugin-test-i386-bmi2-%: QEMU_OPTS += -cpu max
+
 #
 # hello-i386 is a barebones app
 #
diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c
new file mode 100644
index 0000000000..e7a93b88b4
--- /dev/null
+++ b/tests/tcg/i386/test-i386-bmi2.c
@@ -0,0 +1,40 @@
+/* See if various BMI2 instructions give expected results */
+#include <assert.h>
+#include <stdint.h>
+
+int main(int argc, char *argv[]) {
+    char hello[16];
+    uint64_t ehlo = 0x202020204f4c4845ull;
+    uint64_t mask = 0xa080800302020001ull;
+    uint64_t result64;
+    uint32_t result32;
+
+    /* 64 bits */
+    asm volatile ("pextq   %2, %1, %0" : "=r"(result64) : "r"(ehlo), "m"(mask));
+    assert(result64 == 133);
+
+    asm volatile ("pdepq   %2, %1, %0" : "=r"(result64) : "r"(result64), "m"(mask));
+    assert(result64 == (ehlo & mask));
+
+    asm volatile ("pextq   %2, %1, %0" : "=r"(result64) : "r"(-1ull), "m"(mask));
+    assert(result64 == 511); /* mask has 9 bits set */
+
+    asm volatile ("pdepq   %2, %1, %0" : "=r"(result64) : "r"(-1ull), "m"(mask));
+    assert(result64 == mask);
+
+    /* 32 bits */
+    asm volatile ("pextl   %2, %k1, %k0" : "=r"(result32) : "r"(ehlo), "m"(mask));
+    assert(result32 == 5);
+
+    asm volatile ("pdepl   %2, %k1, %k0" : "=r"(result32) : "r"(result32), "m"(mask));
+    assert(result32 == (uint32_t)(ehlo & mask));
+
+    asm volatile ("pextl   %2, %k1, %k0" : "=r"(result32) : "r"(-1ull), "m"(mask));
+    assert(result32 == 7); /* mask has 3 bits set */
+
+    asm volatile ("pdepl   %2, %k1, %k0" : "=r"(result32) : "r"(-1ull), "m"(mask));
+    assert(result32 == (uint32_t)mask);
+
+    return 0;
+}
+
-- 
2.26.2



             reply	other threads:[~2020-11-23 13:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-23 13:14 Paolo Bonzini [this message]
2020-11-23 13:21 ` [PATCH] target/i386: fix operand order for PDEP and PEXT no-reply
2020-11-24 17:54 ` Richard Henderson
2020-11-24 19:44   ` 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=20201123131426.2725276-1-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=blog@lenardszolnoki.com \
    --cc=qemu-devel@nongnu.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 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.