[Qemu-devel] [PATCH v1 28/41] s390x/tcg: Implement VECTOR ELEMENT ROTATE AND INSERT UNDER MASK

David Hildenbrand posted 41 patches 6 years, 10 months ago
Maintainers: Richard Henderson <rth@twiddle.net>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v1 28/41] s390x/tcg: Implement VECTOR ELEMENT ROTATE AND INSERT UNDER MASK
Posted by David Hildenbrand 6 years, 10 months ago
Use the new vector expansion for GVecGen3i. In the ool helpers, reuse
the rotation funvtions introduced with VECTOR ELEMENT ROTATE LEFT
LOGICAL.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/helper.h           |  2 ++
 target/s390x/insn-data.def      |  2 ++
 target/s390x/translate_vx.inc.c | 53 +++++++++++++++++++++++++++++++++
 target/s390x/vec_int_helper.c   | 19 ++++++++++++
 4 files changed, 76 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index f0efaf9cd5..bfde7e3cc6 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -206,6 +206,8 @@ DEF_HELPER_FLAGS_4(gvec_verllv8, TCG_CALL_NO_RWG, void, ptr, cptr, cptr, i32)
 DEF_HELPER_FLAGS_4(gvec_verllv16, TCG_CALL_NO_RWG, void, ptr, cptr, cptr, i32)
 DEF_HELPER_FLAGS_4(gvec_verll8, TCG_CALL_NO_RWG, void, ptr, cptr, i64, i32)
 DEF_HELPER_FLAGS_4(gvec_verll16, TCG_CALL_NO_RWG, void, ptr, cptr, i64, i32)
+DEF_HELPER_FLAGS_4(gvec_verim8, TCG_CALL_NO_RWG, void, ptr, cptr, cptr, i32)
+DEF_HELPER_FLAGS_4(gvec_verim16, TCG_CALL_NO_RWG, void, ptr, cptr, cptr, i32)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index e765c15941..59c323a796 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -1149,6 +1149,8 @@
 /* VECTOR ELEMENT ROTATE LEFT LOGICAL */
     F(0xe773, VERLLV,  VRR_c, V,   0, 0, 0, 0, verllv, 0, IF_VEC)
     F(0xe733, VERLL,   VRS_a, V,   la2, 0, 0, 0, verll, 0, IF_VEC)
+/* VECTOR ELEMENT ROTATE AND INSERT UNDER MASK */
+    F(0xe772, VERIM,   VRI_d, V,   0, 0, 0, 0, verim, 0, IF_VEC)
 
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index 92c14174da..a6169b9827 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -197,6 +197,9 @@ static void get_vec_element_ptr_i64(TCGv_ptr ptr, uint8_t reg, TCGv_i64 enr,
 #define gen_gvec_3_ptr(v1, v2, v3, ptr, data, fn) \
     tcg_gen_gvec_3_ptr(vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
                        vec_full_reg_offset(v3), ptr, 16, 16, data, fn)
+#define gen_gvec_3i(v1, v2, v3, c, gen) \
+    tcg_gen_gvec_3i(vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
+                    vec_full_reg_offset(v3), c, 16, 16, gen)
 #define gen_gvec_4(v1, v2, v3, v4, gen) \
     tcg_gen_gvec_4(vec_full_reg_offset(v1), vec_full_reg_offset(v2), \
                    vec_full_reg_offset(v3), vec_full_reg_offset(v4), \
@@ -1903,3 +1906,53 @@ static DisasJumpType op_verll(DisasContext *s, DisasOps *o)
                 &g[es]);
     return DISAS_NEXT;
 }
+
+static void gen_rim_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b, int32_t c)
+{
+    TCGv_i32 t0 = tcg_temp_new_i32();
+    TCGv_i32 t1 = tcg_temp_new_i32();
+
+    tcg_gen_andc_i32(t0, a, b);
+    tcg_gen_rotli_i32(t1, a, c & 31);
+    tcg_gen_and_i32(t1, t1, b);
+    tcg_gen_or_i32(d, t0, t1);
+
+    tcg_temp_free_i32(t0);
+    tcg_temp_free_i32(t1);
+}
+
+static void gen_rim_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b, int64_t c)
+{
+    TCGv_i64 t0 = tcg_temp_new_i64();
+    TCGv_i64 t1 = tcg_temp_new_i64();
+
+    tcg_gen_andc_i64(t0, a, b);
+    tcg_gen_rotli_i64(t1, a, c & 63);
+    tcg_gen_and_i64(t1, t1, b);
+    tcg_gen_or_i64(d, t0, t1);
+
+    tcg_temp_free_i64(t0);
+    tcg_temp_free_i64(t1);
+}
+
+static DisasJumpType op_verim(DisasContext *s, DisasOps *o)
+{
+    const uint8_t es = get_field(s->fields, m5);
+    const uint8_t i4 = get_field(s->fields, i4) &
+                       (NUM_VEC_ELEMENT_BITS(es) - 1);
+    static const GVecGen3i g[4] = {
+        { .fno = gen_helper_gvec_verim8, },
+        { .fno = gen_helper_gvec_verim16, },
+        { .fni4 = gen_rim_i32, },
+        { .fni8 = gen_rim_i64, },
+    };
+
+    if (es > ES_64) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return DISAS_NORETURN;
+    }
+
+    gen_gvec_3i(get_field(s->fields, v1), get_field(s->fields, v2),
+                get_field(s->fields, v3), i4, &g[es]);
+    return DISAS_NEXT;
+}
diff --git a/target/s390x/vec_int_helper.c b/target/s390x/vec_int_helper.c
index ed67fa73fb..6dc31003b9 100644
--- a/target/s390x/vec_int_helper.c
+++ b/target/s390x/vec_int_helper.c
@@ -14,6 +14,7 @@
 #include "cpu.h"
 #include "vec.h"
 #include "exec/helper-proto.h"
+#include "tcg/tcg-gvec-desc.h"
 
 /*
  * Add two 128 bit vectors, returning the carry.
@@ -580,3 +581,21 @@ void HELPER(gvec_verll##BITS)(void *v1, const void *v2, uint64_t count,        \
 }
 DEF_VERLL(8)
 DEF_VERLL(16)
+
+#define DEF_VERIM(BITS)                                                        \
+void HELPER(gvec_verim##BITS)(void *v1, const void *v2, const void *v3,        \
+                              uint32_t desc)                                   \
+{                                                                              \
+    const uint8_t count = simd_data(desc);                                     \
+    int i;                                                                     \
+                                                                               \
+    for (i = 0; i < (128 / BITS); i++) {                                       \
+        const uint##BITS##_t a = s390_vec_read_element##BITS(v2, i);           \
+        const uint##BITS##_t mask = s390_vec_read_element##BITS(v3, i);        \
+        const uint##BITS##_t d = (a & ~mask) | (rotl##BITS(a, count) & mask);  \
+                                                                               \
+        s390_vec_write_element##BITS(v1, i, d);                                \
+    }                                                                          \
+}
+DEF_VERIM(8)
+DEF_VERIM(16)
-- 
2.20.1


Re: [Qemu-devel] [PATCH v1 28/41] s390x/tcg: Implement VECTOR ELEMENT ROTATE AND INSERT UNDER MASK
Posted by Richard Henderson 6 years, 10 months ago
On 4/11/19 12:08 AM, David Hildenbrand wrote:
> +static void gen_rim_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b, int32_t c)
> +{
> +    TCGv_i32 t0 = tcg_temp_new_i32();
> +    TCGv_i32 t1 = tcg_temp_new_i32();
> +
> +    tcg_gen_andc_i32(t0, a, b);
> +    tcg_gen_rotli_i32(t1, a, c & 31);
> +    tcg_gen_and_i32(t1, t1, b);
> +    tcg_gen_or_i32(d, t0, t1);

The ANDC and ROTL look to be in the wrong order.

"For each bit in the third operand (b) that is one,
the corresponding bit *of the rotated elements* in
the second operand replaces the corresponding bit in
the first operand".

I think you need

    tcg_gen_rotli_i32(a, a, c & 31);
    tcg_gen_and_i32(a, a, b);
    tcg_gen_andc_i32(d, d, b);
    tcg_gen_or_i32(d, d, a);

with

  { .fni4 = gen_rim_32, .load_dest = true },

> +     const uint##BITS##_t a = s390_vec_read_element##BITS(v2, i);           \
> +     const uint##BITS##_t mask = s390_vec_read_element##BITS(v3, i);        \
> +     const uint##BITS##_t d = (a & ~mask) | (rotl##BITS(a, count) & mask);  \

Again, this seems to be missing the insert into "the first operand", i.e.
loading from v1 as well.


r~

Re: [Qemu-devel] [PATCH v1 28/41] s390x/tcg: Implement VECTOR ELEMENT ROTATE AND INSERT UNDER MASK
Posted by David Hildenbrand 6 years, 9 months ago
On 13.04.19 02:29, Richard Henderson wrote:
> On 4/11/19 12:08 AM, David Hildenbrand wrote:
>> +static void gen_rim_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b, int32_t c)
>> +{
>> +    TCGv_i32 t0 = tcg_temp_new_i32();
>> +    TCGv_i32 t1 = tcg_temp_new_i32();
>> +
>> +    tcg_gen_andc_i32(t0, a, b);
>> +    tcg_gen_rotli_i32(t1, a, c & 31);
>> +    tcg_gen_and_i32(t1, t1, b);
>> +    tcg_gen_or_i32(d, t0, t1);
> 
> The ANDC and ROTL look to be in the wrong order.
> 
> "For each bit in the third operand (b) that is one,
> the corresponding bit *of the rotated elements* in
> the second operand replaces the corresponding bit in
> the first operand".
> 
> I think you need
> 
>     tcg_gen_rotli_i32(a, a, c & 31);
>     tcg_gen_and_i32(a, a, b);
>     tcg_gen_andc_i32(d, d, b);
>     tcg_gen_or_i32(d, d, a);
> 
> with
> 
>   { .fni4 = gen_rim_32, .load_dest = true },
> 
>> +     const uint##BITS##_t a = s390_vec_read_element##BITS(v2, i);           \
>> +     const uint##BITS##_t mask = s390_vec_read_element##BITS(v3, i);        \
>> +     const uint##BITS##_t d = (a & ~mask) | (rotl##BITS(a, count) & mask);  \
> 
> Again, this seems to be missing the insert into "the first operand", i.e.
> loading from v1 as well.

Yes indeed, I misinterpreted/misread the PoP. Nice catch! (as usual,
excellent review)

> 
> 
> r~
> 


-- 

Thanks,

David / dhildenb