diff mbox

[21/26] target/s390x: implement PACK UNICODE

Message ID 20170525210508.4910-22-aurelien@aurel32.net
State New
Headers show

Commit Message

Aurelien Jarno May 25, 2017, 9:05 p.m. UTC 
Use a common helper with PACK ASCII as the differences are limited to
the stride of the source operand.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 30 +++++++++++++++++++++---------
 target/s390x/translate.c   | 16 ++++++++++++++++
 4 files changed, 40 insertions(+), 9 deletions(-)

Comments

Richard Henderson May 26, 2017, 4:35 p.m. UTC | #1 
On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> +        } else if (srclen > ssize) {
>               b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
> -            src--;
> -            srclen--;
> +            src -= ssize;
> +            srclen -= ssize;

Surely we need to use lduw in order to correctly read the big-endian 16-bit 
value, or bias src by +1 to read the low byte of same.


r~
Aurelien Jarno May 29, 2017, 11:22 a.m. UTC | #2 
On 2017-05-26 09:35, Richard Henderson wrote:
> On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
> > +        } else if (srclen > ssize) {
> >               b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
> > -            src--;
> > -            srclen--;
> > +            src -= ssize;
> > +            srclen -= ssize;
> 
> Surely we need to use lduw in order to correctly read the big-endian 16-bit
> value, or bias src by +1 to read the low byte of same.

The operands are read from right to left, therefore src is adjusted to
point to the last byte before the for loop:

+    /* The operands are processed from right to left.  */
+    src += srclen - 1;
+    dest += destlen - 1;

Given s390 is big-endian, it means src initially always point to the
low byte independently to the size of the value.
Richard Henderson May 30, 2017, 4:46 p.m. UTC | #3 
On 05/29/2017 04:22 AM, Aurelien Jarno wrote:
> On 2017-05-26 09:35, Richard Henderson wrote:
>> On 05/25/2017 02:05 PM, Aurelien Jarno wrote:
>>> +        } else if (srclen > ssize) {
>>>                b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
>>> -            src--;
>>> -            srclen--;
>>> +            src -= ssize;
>>> +            srclen -= ssize;
>>
>> Surely we need to use lduw in order to correctly read the big-endian 16-bit
>> value, or bias src by +1 to read the low byte of same.
> 
> The operands are read from right to left, therefore src is adjusted to
> point to the last byte before the for loop:
> 
> +    /* The operands are processed from right to left.  */
> +    src += srclen - 1;
> +    dest += destlen - 1;
> 
> Given s390 is big-endian, it means src initially always point to the
> low byte independently to the size of the value.
> 

Quite right.  I didn't consider the processing order.


r~
diff mbox

Patch

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 253e1e679c..1011df122d 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -84,6 +84,7 @@  DEF_HELPER_FLAGS_3(sqxb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
 DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
+DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(unpk, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(tr, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_4(tre, i64, env, i64, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index b6d2215d85..3335270182 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -663,6 +663,8 @@ 
     C(0xf200, PACK,    SS_a,  Z,   la1, a2, 0, 0, pack, 0)
 /* PACK ASCII */
     C(0xe900, PKA,     SS_f,  E2,  la1, a2, 0, 0, pka, 0)
+/* PACK UNICODE */
+    C(0xe100, PKU,     SS_f,  E2,  la1, a2, 0, 0, pku, 0)
 
 /* PREFETCH */
     /* Implemented as nops of course.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6d64497edc..ab34a309ce 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -959,10 +959,9 @@  void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
     }
 }
 
-void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
-                 uint32_t srclen)
+static inline void do_pkau(CPUS390XState *env, uint64_t dest, uint64_t src,
+                           uint32_t srclen, int ssize, uintptr_t ra)
 {
-    uintptr_t ra = GETPC();
     int i;
     /* The destination operand is always 16 bytes long.  */
     const int destlen = 16;
@@ -977,16 +976,16 @@  void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
         /* Start with a positive sign */
         if (i == 0) {
             b = 0xc;
-        } else if (srclen > 1) {
+        } else if (srclen > ssize) {
             b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
-            src--;
-            srclen--;
+            src -= ssize;
+            srclen -= ssize;
         }
 
-        if (srclen > 1) {
+        if (srclen > ssize) {
             b |= cpu_ldub_data_ra(env, src, ra) << 4;
-            src--;
-            srclen--;
+            src -= ssize;
+            srclen -= ssize;
         }
 
         cpu_stb_data_ra(env, dest, b, ra);
@@ -994,6 +993,19 @@  void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
     }
 }
 
+
+void HELPER(pka)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    do_pkau(env, dest, src, srclen, 1, GETPC());
+}
+
+void HELPER(pku)(CPUS390XState *env, uint64_t dest, uint64_t src,
+                 uint32_t srclen)
+{
+    do_pkau(env, dest, src, srclen, 2, GETPC());
+}
+
 void HELPER(unpk)(CPUS390XState *env, uint32_t len, uint64_t dest,
                   uint64_t src)
 {
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index c74a80bf60..a2a27c86ea 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3242,6 +3242,22 @@  static ExitStatus op_pka(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_pku(DisasContext *s, DisasOps *o)
+{
+    int l2 = get_field(s->fields, l2) + 1;
+    TCGv_i32 l;
+
+    /* The length must be even and should not exceed 64 bytes.  */
+    if ((l2 & 1) || (l2 > 64)) {
+        gen_program_exception(s, PGM_SPECIFICATION);
+        return EXIT_NORETURN;
+    }
+    l = tcg_const_i32(l2);
+    gen_helper_pku(cpu_env, o->addr1, o->in2, l);
+    tcg_temp_free_i32(l);
+    return NO_EXIT;
+}
+
 static ExitStatus op_popcnt(DisasContext *s, DisasOps *o)
 {
     gen_helper_popcnt(o->out, o->in2);