diff mbox series

[v1,6/8] s390x/tcg: Implement MULTIPLY SINGLE (MSC, MSGC, MSGRKC, MSRKC)

Message ID 20200922103129.12824-7-david@redhat.com
State New
Headers show
Series s390x/tcg: Implement some z14 facilities | expand

Commit Message

David Hildenbrand Sept. 22, 2020, 10:31 a.m. UTC
We need new CC handling, determining the CC based on the intermediate
result (64bit for MSC and MSRKC, 128bit for MSGC and MSGRKC).

We want to store out2 ("low") after muls128 to r1, so add
"wout_out2_r1".

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cc_helper.c   | 34 ++++++++++++++++++++++++++++++++++
 target/s390x/helper.c      |  2 ++
 target/s390x/insn-data.def |  4 ++++
 target/s390x/internal.h    |  2 ++
 target/s390x/translate.c   | 19 +++++++++++++++++++
 5 files changed, 61 insertions(+)

Comments

Richard Henderson Sept. 22, 2020, 4:14 p.m. UTC | #1
On 9/22/20 3:31 AM, David Hildenbrand wrote:
> @@ -512,6 +544,8 @@ static uint32_t do_calc_cc(CPUS390XState *env, uint32_t cc_op,

>      case CC_OP_COMP_32:

>          r =  cc_calc_comp_32(dst);

>          break;

> +    case CC_OP_MULS_32:

> +        r = cc_calc_muls_32(dst);

>  

>      case CC_OP_ICM:


Missing break.


r~
David Hildenbrand Sept. 22, 2020, 4:19 p.m. UTC | #2
On 22.09.20 18:14, Richard Henderson wrote:
> On 9/22/20 3:31 AM, David Hildenbrand wrote:

>> @@ -512,6 +544,8 @@ static uint32_t do_calc_cc(CPUS390XState *env, uint32_t cc_op,

>>      case CC_OP_COMP_32:

>>          r =  cc_calc_comp_32(dst);

>>          break;

>> +    case CC_OP_MULS_32:

>> +        r = cc_calc_muls_32(dst);

>>  

>>      case CC_OP_ICM:

> 

> Missing break.


Argh, thanks!


-- 
Thanks,

David / dhildenb
Richard Henderson Sept. 25, 2020, 10:06 p.m. UTC | #3
On 9/22/20 3:31 AM, David Hildenbrand wrote:
> +static uint32_t cc_calc_muls_32(int64_t res)

> +{

> +    /* Arithmetic shift with sign extension so we can compare against -1ull. */

> +    const uint64_t tmp = res >> 31;

> +

> +    if (!res) {

> +        return 0;

> +    } else if (!(!tmp || tmp == -1ull)) {


Comparing signed vs unsigned.  Use -1 without suffix.

> +static uint64_t cc_calc_muls_64(int64_t res_high, uint64_t res_low)

> +{

> +    const uint8_t tmp = res_low >> 63;

> +

> +    if (!res_high && !res_low) {

> +        return 0;

> +    } else if (!(!res_high && !tmp) || !(res_high == -1ull && tmp)) {


This simplifies to res_high + tmp != 0.

Probably better to keep tmp as uint64_t; otherwise we're likely to have an
unnecessary zero-extension from uint8_t to uint64_t.
Or, drop 'tmp' altogether and use

  if (res_high + (res_low >> 63) != 0)

Otherwise, looks good.


r~
David Hildenbrand Sept. 28, 2020, 12:13 p.m. UTC | #4
On 26.09.20 00:06, Richard Henderson wrote:
> On 9/22/20 3:31 AM, David Hildenbrand wrote:

>> +static uint32_t cc_calc_muls_32(int64_t res)

>> +{

>> +    /* Arithmetic shift with sign extension so we can compare against -1ull. */

>> +    const uint64_t tmp = res >> 31;

>> +

>> +    if (!res) {

>> +        return 0;

>> +    } else if (!(!tmp || tmp == -1ull)) {

> 

> Comparing signed vs unsigned.  Use -1 without suffix.


tmp is also uint64_t - but I can change that to int64_t.

(and condense to "tmp && tmp != -1")

> 

>> +static uint64_t cc_calc_muls_64(int64_t res_high, uint64_t res_low)

>> +{

>> +    const uint8_t tmp = res_low >> 63;

>> +

>> +    if (!res_high && !res_low) {

>> +        return 0;

>> +    } else if (!(!res_high && !tmp) || !(res_high == -1ull && tmp)) {

> 

> This simplifies to res_high + tmp != 0.


Yeah, after messing up one time I decided to phrase it just as stated in
the PoP.

> 

> Probably better to keep tmp as uint64_t; otherwise we're likely to have an

> unnecessary zero-extension from uint8_t to uint64_t.

> Or, drop 'tmp' altogether and use

> 

>   if (res_high + (res_low >> 63) != 0)


Thanks, I'll go with that.

> 

> Otherwise, looks good.

> 

> 

> r~

> 



-- 
Thanks,

David / dhildenb
diff mbox series

Patch

diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index 44731e4a85..62074648e6 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -417,6 +417,35 @@  static uint32_t cc_calc_vc(uint64_t low, uint64_t high)
     }
 }
 
+static uint32_t cc_calc_muls_32(int64_t res)
+{
+    /* Arithmetic shift with sign extension so we can compare against -1ull. */
+    const uint64_t tmp = res >> 31;
+
+    if (!res) {
+        return 0;
+    } else if (!(!tmp || tmp == -1ull)) {
+        return 3;
+    } else if (res < 0) {
+        return 1;
+    }
+    return 2;
+}
+
+static uint64_t cc_calc_muls_64(int64_t res_high, uint64_t res_low)
+{
+    const uint8_t tmp = res_low >> 63;
+
+    if (!res_high && !res_low) {
+        return 0;
+    } else if (!(!res_high && !tmp) || !(res_high == -1ull && tmp)) {
+        return 3;
+    } else if (res_high < 0) {
+        return 1;
+    }
+    return 2;
+}
+
 static uint32_t do_calc_cc(CPUS390XState *env, uint32_t cc_op,
                                   uint64_t src, uint64_t dst, uint64_t vr)
 {
@@ -484,6 +513,9 @@  static uint32_t do_calc_cc(CPUS390XState *env, uint32_t cc_op,
     case CC_OP_COMP_64:
         r =  cc_calc_comp_64(dst);
         break;
+    case CC_OP_MULS_64:
+        r = cc_calc_muls_64(src, dst);
+        break;
 
     case CC_OP_ADD_32:
         r =  cc_calc_add_32(src, dst, vr);
@@ -512,6 +544,8 @@  static uint32_t do_calc_cc(CPUS390XState *env, uint32_t cc_op,
     case CC_OP_COMP_32:
         r =  cc_calc_comp_32(dst);
         break;
+    case CC_OP_MULS_32:
+        r = cc_calc_muls_32(dst);
 
     case CC_OP_ICM:
         r =  cc_calc_icm(src, dst);
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 9257d388ba..b877690845 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -430,6 +430,8 @@  const char *cc_name(enum cc_op cc_op)
         [CC_OP_FLOGR]     = "CC_OP_FLOGR",
         [CC_OP_LCBB]      = "CC_OP_LCBB",
         [CC_OP_VC]        = "CC_OP_VC",
+        [CC_OP_MULS_32]   = "CC_OP_MULS_32",
+        [CC_OP_MULS_64]   = "CC_OP_MULS_64",
     };
 
     return cc_names[cc_op];
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index dfb0ec067b..bcd424e9ae 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -679,11 +679,15 @@ 
     C(0xe386, MLG,     RXY_a, Z,   r1p1, m2_64, r1_P, 0, mul128, 0)
 /* MULTIPLY SINGLE */
     C(0xb252, MSR,     RRE,   Z,   r1_o, r2_o, new, r1_32, mul, 0)
+    C(0xb9fd, MSRKC,   RRF_a, MIE2,r3_32s, r2_32s, new, r1_32, mul, muls32)
     C(0x7100, MS,      RX_a,  Z,   r1_o, m2_32s, new, r1_32, mul, 0)
     C(0xe351, MSY,     RXY_a, LD,  r1_o, m2_32s, new, r1_32, mul, 0)
+    C(0xe353, MSC,     RXY_a, MIE2,r1_32s, m2_32s, new, r1_32, mul, muls32)
     C(0xb90c, MSGR,    RRE,   Z,   r1_o, r2_o, r1, 0, mul, 0)
+    C(0xb9ed, MSGRKC,  RRF_a, MIE2,r3_o, r2_o, new_P, out2_r1, muls128, muls64)
     C(0xb91c, MSGFR,   RRE,   Z,   r1_o, r2_32s, r1, 0, mul, 0)
     C(0xe30c, MSG,     RXY_a, Z,   r1_o, m2_64, r1, 0, mul, 0)
+    C(0xe383, MSGC,    RXY_a, MIE2,r1_o, m2_64, new_P, out2_r1, muls128, muls64)
     C(0xe31c, MSGF,    RXY_a, Z,   r1_o, m2_32s, r1, 0, mul, 0)
 /* MULTIPLY SINGLE IMMEDIATE */
     C(0xc201, MSFI,    RIL_a, GIE, r1_o, i2, new, r1_32, mul, 0)
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index b1e0ebf67f..c5d32237ea 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -175,6 +175,7 @@  enum cc_op {
     CC_OP_SUBB_64,              /* overflow on unsigned sub-borrow (64bit) */
     CC_OP_ABS_64,               /* sign eval on abs (64bit) */
     CC_OP_NABS_64,              /* sign eval on nabs (64bit) */
+    CC_OP_MULS_64,              /* overflow on signed multiply (64bit) */
 
     CC_OP_ADD_32,               /* overflow on add (32bit) */
     CC_OP_ADDU_32,              /* overflow on unsigned add (32bit) */
@@ -184,6 +185,7 @@  enum cc_op {
     CC_OP_SUBB_32,              /* overflow on unsigned sub-borrow (32bit) */
     CC_OP_ABS_32,               /* sign eval on abs (64bit) */
     CC_OP_NABS_32,              /* sign eval on nabs (64bit) */
+    CC_OP_MULS_32,              /* overflow on signed multiply (32bit) */
 
     CC_OP_COMP_32,              /* complement */
     CC_OP_COMP_64,              /* complement */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 383edf7419..c90cb37aae 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -646,6 +646,7 @@  static void gen_op_calc_cc(DisasContext *s)
     case CC_OP_NZ_F64:
     case CC_OP_FLOGR:
     case CC_OP_LCBB:
+    case CC_OP_MULS_32:
         /* 1 argument */
         gen_helper_calc_cc(cc_op, cpu_env, local_cc_op, dummy, cc_dst, dummy);
         break;
@@ -660,6 +661,7 @@  static void gen_op_calc_cc(DisasContext *s)
     case CC_OP_SLA_64:
     case CC_OP_NZ_F128:
     case CC_OP_VC:
+    case CC_OP_MULS_64:
         /* 2 arguments */
         gen_helper_calc_cc(cc_op, cpu_env, local_cc_op, cc_src, cc_dst, dummy);
         break;
@@ -5294,6 +5296,17 @@  static void cout_tm64(DisasContext *s, DisasOps *o)
     gen_op_update2_cc_i64(s, CC_OP_TM_64, o->in1, o->in2);
 }
 
+static void cout_muls32(DisasContext *s, DisasOps *o)
+{
+    gen_op_update1_cc_i64(s, CC_OP_MULS_32, o->out);
+}
+
+static void cout_muls64(DisasContext *s, DisasOps *o)
+{
+    /* out contains "high" part, out2 contains "low" part of 128 bit result */
+    gen_op_update2_cc_i64(s, CC_OP_MULS_64, o->out, o->out2);
+}
+
 /* ====================================================================== */
 /* The "PREParation" generators.  These initialize the DisasOps.OUT fields
    with the TCG register to which we will write.  Used in combination with
@@ -5349,6 +5362,12 @@  static void wout_r1(DisasContext *s, DisasOps *o)
 }
 #define SPEC_wout_r1 0
 
+static void wout_out2_r1(DisasContext *s, DisasOps *o)
+{
+    store_reg(get_field(s, r1), o->out2);
+}
+#define SPEC_wout_out2_r1 0
+
 static void wout_r1_8(DisasContext *s, DisasOps *o)
 {
     int r1 = get_field(s, r1);