Re: [Qemu-devel] [RFC PATCH v4 75/75] target/i386: convert pmovmskb/movmskps/movmskpd helpers to gvec style

2019-08-21 Thread Richard Henderson 
On 8/21/19 10:29 AM, Jan Bobek wrote:
> +for (intptr_t i = 0; i * sizeof(uint8_t) < oprsz; ++i) {
> +const uint8_t t = a->B(i) & (1 << 7);
> +ret |= i < 8 ? t >> (7 - i) : t << (i - 7);

You can avoid this variable shift by doing

  uint32_t t = a->B(i) >> 7;
  ret |= t << i;

> +uint64_t glue(helper_pmovmskbq, SUFFIX)(Reg *a, uint32_t desc)
> +{
> +return glue(helper_pmovmskbd, SUFFIX)(a, desc);
>  }
...
> +DEF_GEN_INSN2_GVEC(vpmovmskb, Gd, Uqq, sd1_ool, XMM_OPRSZ, XMM_MAXSZ, 
> pmovmskbd_xmm)
> +DEF_GEN_INSN2_GVEC(vpmovmskb, Gq, Uqq, sq1_ool, XMM_OPRSZ, XMM_MAXSZ, 
> pmovmskbq_xmm)

What is the difference between these two?

Given that we aren't attempting avx512, uint32_t is sufficient for all of the
bytes of a YMM register.

I have a feeling that some of this should simply use target_ulong, so that a
direct assignment to the general register can be done without extra extensions
within the generated code.


r~

[Qemu-devel] [RFC PATCH v4 75/75] target/i386: convert pmovmskb/movmskps/movmskpd helpers to gvec style

2019-08-21 Thread Jan Bobek 
Make these helpers suitable for use with tcg_gen_gvec_* functions.

Signed-off-by: Jan Bobek 
---
 target/i386/ops_sse.h|  74 ++--
 target/i386/ops_sse_header.h |   9 ++-
 target/i386/translate.c  | 132 ++-
 3 files changed, 65 insertions(+), 150 deletions(-)

diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index 2e50d91a25..82562c9473 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -1169,52 +1169,56 @@ void helper_comisd(CPUX86State *env, Reg *d, Reg *s)
 CC_SRC = comis_eflags[ret + 1];
 }
 
-uint32_t helper_movmskps(CPUX86State *env, Reg *s)
+uint32_t helper_movmskpsd(Reg *a, uint32_t desc)
 {
-int b0, b1, b2, b3;
+const intptr_t oprsz = simd_oprsz(desc);
 
-b0 = s->ZMM_L(0) >> 31;
-b1 = s->ZMM_L(1) >> 31;
-b2 = s->ZMM_L(2) >> 31;
-b3 = s->ZMM_L(3) >> 31;
-return b0 | (b1 << 1) | (b2 << 2) | (b3 << 3);
+uint32_t ret = 0;
+for (intptr_t i = 0; i * sizeof(uint32_t) < oprsz; ++i) {
+const uint32_t t = a->ZMM_L(i) & (1UL << 31);
+ret |= t >> (31 - i);
+}
+return ret;
 }
 
-uint32_t helper_movmskpd(CPUX86State *env, Reg *s)
+uint64_t helper_movmskpsq(Reg *a, uint32_t desc)
 {
-int b0, b1;
+return helper_movmskpsd(a, desc);
+}
+
+uint32_t helper_movmskpdd(Reg *a, uint32_t desc)
+{
+const intptr_t oprsz = simd_oprsz(desc);
 
-b0 = s->ZMM_L(1) >> 31;
-b1 = s->ZMM_L(3) >> 31;
-return b0 | (b1 << 1);
+uint32_t ret = 0;
+for (intptr_t i = 0; i * sizeof(uint64_t) < oprsz; ++i) {
+const uint64_t t = a->ZMM_Q(i) & (1ULL << 63);
+ret |= t >> (63 - i);
+}
+return ret;
 }
 
+uint64_t helper_movmskpdq(Reg *a, uint32_t desc)
+{
+return helper_movmskpdd(a, desc);
+}
 #endif
 
-uint32_t glue(helper_pmovmskb, SUFFIX)(CPUX86State *env, Reg *s)
+uint32_t glue(helper_pmovmskbd, SUFFIX)(Reg *a, uint32_t desc)
 {
-uint32_t val;
-
-val = 0;
-val |= (s->B(0) >> 7);
-val |= (s->B(1) >> 6) & 0x02;
-val |= (s->B(2) >> 5) & 0x04;
-val |= (s->B(3) >> 4) & 0x08;
-val |= (s->B(4) >> 3) & 0x10;
-val |= (s->B(5) >> 2) & 0x20;
-val |= (s->B(6) >> 1) & 0x40;
-val |= (s->B(7)) & 0x80;
-#if SHIFT == 1
-val |= (s->B(8) << 1) & 0x0100;
-val |= (s->B(9) << 2) & 0x0200;
-val |= (s->B(10) << 3) & 0x0400;
-val |= (s->B(11) << 4) & 0x0800;
-val |= (s->B(12) << 5) & 0x1000;
-val |= (s->B(13) << 6) & 0x2000;
-val |= (s->B(14) << 7) & 0x4000;
-val |= (s->B(15) << 8) & 0x8000;
-#endif
-return val;
+const intptr_t oprsz = simd_oprsz(desc);
+
+uint32_t ret = 0;
+for (intptr_t i = 0; i * sizeof(uint8_t) < oprsz; ++i) {
+const uint8_t t = a->B(i) & (1 << 7);
+ret |= i < 8 ? t >> (7 - i) : t << (i - 7);
+}
+return ret;
+}
+
+uint64_t glue(helper_pmovmskbq, SUFFIX)(Reg *a, uint32_t desc)
+{
+return glue(helper_pmovmskbd, SUFFIX)(a, desc);
 }
 
 void glue(helper_packsswb, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
diff --git a/target/i386/ops_sse_header.h b/target/i386/ops_sse_header.h
index 207d41e248..59ac1f28e3 100644
--- a/target/i386/ops_sse_header.h
+++ b/target/i386/ops_sse_header.h
@@ -178,11 +178,14 @@ DEF_HELPER_3(ucomiss, void, env, Reg, Reg)
 DEF_HELPER_3(comiss, void, env, Reg, Reg)
 DEF_HELPER_3(ucomisd, void, env, Reg, Reg)
 DEF_HELPER_3(comisd, void, env, Reg, Reg)
-DEF_HELPER_2(movmskps, i32, env, Reg)
-DEF_HELPER_2(movmskpd, i32, env, Reg)
+DEF_HELPER_2(movmskpsd, i32, Reg, i32)
+DEF_HELPER_2(movmskpsq, i64, Reg, i32)
+DEF_HELPER_2(movmskpdd, i32, Reg, i32)
+DEF_HELPER_2(movmskpdq, i64, Reg, i32)
 #endif
 
-DEF_HELPER_2(glue(pmovmskb, SUFFIX), i32, env, Reg)
+DEF_HELPER_2(glue(pmovmskbd, SUFFIX), i32, Reg, i32)
+DEF_HELPER_2(glue(pmovmskbq, SUFFIX), i64, Reg, i32)
 DEF_HELPER_3(glue(packsswb, SUFFIX), void, env, Reg, Reg)
 DEF_HELPER_3(glue(packuswb, SUFFIX), void, env, Reg, Reg)
 DEF_HELPER_3(glue(packssdw, SUFFIX), void, env, Reg, Reg)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index bb4120a848..8f891b6e47 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -3339,20 +3339,6 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b)
 goto illegal_op;
 }
 break;
-case 0x050: /* movmskps */
-rm = (modrm & 7) | REX_B(s);
-tcg_gen_addi_ptr(s->ptr0, cpu_env,
- offsetof(CPUX86State,xmm_regs[rm]));
-gen_helper_movmskps(s->tmp2_i32, cpu_env, s->ptr0);
-tcg_gen_extu_i32_tl(cpu_regs[reg], s->tmp2_i32);
-break;
-case 0x150: /* movmskpd */
-rm = (modrm & 7) | REX_B(s);
-tcg_gen_addi_ptr(s->ptr0, cpu_env,
- offsetof(CPUX86State,xmm_regs[rm]));
-gen_helper_movmskpd(s->tmp2_i32, cpu_env, s->ptr0);
-tcg_gen_extu_i32_tl(cpu_regs[reg], s->tmp2_i32);
-break;