[PATCH v1 0/8] s390x/tcg: Implement some z14 facilities

[PATCH v1 0/8] s390x/tcg: Implement some z14 facilities

[David Hildenbrand]

With this patchset, I can boot a kernel compiled for z14 (requiring
Miscellaneous-Instruction-Extensions Facility 2) (using -cpu max).

I am currently working on "vector-enhancements facility 1", I have most
stuff in place except:
- VECTOR FP (MAXIMUM|MINIMUM): needs some manual work to cover all types of
                               comparisons.
- VECTOR FP (NEGATIVE) MULTIPLY AND (ADD|SUBTRACT): needs float128_muladd()

Once we have that in place, we can convert the QEMU machine to a
stripped-down z14.

David Hildenbrand (8):
  s390x/tcg: Implement ADD HALFWORD (AGH)
  s390x/tcg: Implement SUBTRACT HALFWORD (SGH)
  s390x/tcg: Implement MULTIPLY (MG, MGRK)
  s390x/tcg: Implement MULTIPLY HALFWORD (MGH)
  s390x/tcg: Implement BRANCH INDIRECT ON CONDITION (BIC)
  s390x/tcg: Implement MULTIPLY SINGLE (MSC, MSGC, MSGRKC, MSRKC)
  s390x/tcg: We support Miscellaneous-Instruction-Extensions Facility 2
  s390x/tcg: Implement CIPHER MESSAGE WITH AUTHENTICATION (KMA)

 target/s390x/cc_helper.c    | 34 ++++++++++++++++++++++++++++
 target/s390x/gen-features.c |  2 ++
 target/s390x/helper.c       |  2 ++
 target/s390x/insn-data.def  | 12 ++++++++++
 target/s390x/internal.h     |  2 ++
 target/s390x/translate.c    | 45 +++++++++++++++++++++++++++++++++++++
 6 files changed, 97 insertions(+)

-- 
2.26.2

[PATCH v1 1/8] s390x/tcg: Implement ADD HALFWORD (AGH)

[David Hildenbrand]

Easy, just like ADD HALFWORD IMMEDIATE (AGHI).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def | 1 +
 target/s390x/translate.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index d79ae9e3f1..8dbeaf8c49 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -52,6 +52,7 @@
 /* ADD HALFWORD */
     C(0x4a00, AH,      RX_a,  Z,   r1, m2_16s, new, r1_32, add, adds32)
     C(0xe37a, AHY,     RXY_a, LD,  r1, m2_16s, new, r1_32, add, adds32)
+    C(0xe338, AGH,     RXY_a, MIE2,r1, m2_16s, r1, 0, add, adds64)
 /* ADD HALFWORD IMMEDIATE */
     C(0xa70a, AHI,     RI_a,  Z,   r1, i2, new, r1_32, add, adds32)
     C(0xa70b, AGHI,    RI_a,  Z,   r1, i2, r1, 0, add, adds64)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a777343821..21d77b7e74 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6098,6 +6098,7 @@ enum DisasInsnEnum {
 #define FAC_AIS         S390_FEAT_ADAPTER_INT_SUPPRESSION
 #define FAC_V           S390_FEAT_VECTOR /* vector facility */
 #define FAC_VE          S390_FEAT_VECTOR_ENH /* vector enhancements facility 1 */
+#define FAC_MIE2        S390_FEAT_MISC_INSTRUCTION_EXT /* miscellaneous-instruction-extensions facility 2 */
 
 static const DisasInsn insn_info[] = {
 #include "insn-data.def"
-- 
2.26.2

[PATCH v1 2/8] s390x/tcg: Implement SUBTRACT HALFWORD (SGH)

[David Hildenbrand]

Easy to wire up.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 8dbeaf8c49..e851e9df5e 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -883,6 +883,7 @@
 /* SUBTRACT HALFWORD */
     C(0x4b00, SH,      RX_a,  Z,   r1, m2_16s, new, r1_32, sub, subs32)
     C(0xe37b, SHY,     RXY_a, LD,  r1, m2_16s, new, r1_32, sub, subs32)
+    C(0xe339, SGH,     RXY_a, MIE2,r1, m2_16s, r1, 0, sub, subs64)
 /* SUBTRACT HIGH */
     C(0xb9c9, SHHHR,   RRF_a, HW,  r2_sr32, r3_sr32, new, r1_32h, sub, subs32)
     C(0xb9d9, SHHLR,   RRF_a, HW,  r2_sr32, r3, new, r1_32h, sub, subs32)
-- 
2.26.2

[PATCH v1 3/8] s390x/tcg: Implement MULTIPLY (MG, MGRK)

[David Hildenbrand]

Multiply two signed 64bit values and store the 128bit result in r1 (0-63)
and r1 + 1 (64-127).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def |  2 ++
 target/s390x/translate.c   | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index e851e9df5e..2b4ad1530d 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -650,8 +650,10 @@
 
 /* MULTIPLY */
     C(0x1c00, MR,      RR_a,  Z,   r1p1_32s, r2_32s, new, r1_D32, mul, 0)
+    C(0xb9ec, MGRK,    RRF_a, MIE2,r3_o, r2_o, r1_P, 0, muls128, 0)
     C(0x5c00, M,       RX_a,  Z,   r1p1_32s, m2_32s, new, r1_D32, mul, 0)
     C(0xe35c, MFY,     RXY_a, GIE, r1p1_32s, m2_32s, new, r1_D32, mul, 0)
+    C(0xe384, MG,      RXY_a, MIE2,r1p1_o, m2_64, r1_P, 0, muls128, 0)
     F(0xb317, MEEBR,   RRE,   Z,   e1, e2, new, e1, meeb, 0, IF_BFP)
     F(0xb31c, MDBR,    RRE,   Z,   f1, f2, new, f1, mdb, 0, IF_BFP)
     F(0xb34c, MXBR,    RRE,   Z,   x2h, x2l, x1, x1, mxb, 0, IF_BFP)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 21d77b7e74..b536491892 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3518,6 +3518,12 @@ static DisasJumpType op_mul128(DisasContext *s, DisasOps *o)
     return DISAS_NEXT;
 }
 
+static DisasJumpType op_muls128(DisasContext *s, DisasOps *o)
+{
+    tcg_gen_muls2_i64(o->out2, o->out, o->in1, o->in2);
+    return DISAS_NEXT;
+}
+
 static DisasJumpType op_meeb(DisasContext *s, DisasOps *o)
 {
     gen_helper_meeb(o->out, cpu_env, o->in1, o->in2);
@@ -5542,6 +5548,13 @@ static void in1_r1p1(DisasContext *s, DisasOps *o)
 }
 #define SPEC_in1_r1p1 SPEC_r1_even
 
+static void in1_r1p1_o(DisasContext *s, DisasOps *o)
+{
+    o->in1 = regs[get_field(s, r1) + 1];
+    o->g_in1 = true;
+}
+#define SPEC_in1_r1p1_o SPEC_r1_even
+
 static void in1_r1p1_32s(DisasContext *s, DisasOps *o)
 {
     o->in1 = tcg_temp_new_i64();
-- 
2.26.2

[PATCH v1 4/8] s390x/tcg: Implement MULTIPLY HALFWORD (MGH)

[David Hildenbrand]

Just like MULTIPLY HALFWORD IMMEDIATE (MGHI), only the second operand
(signed 16 bit) comes from memory.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 2b4ad1530d..455efe73da 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -666,6 +666,7 @@
 /* MULTIPLY HALFWORD */
     C(0x4c00, MH,      RX_a,  Z,   r1_o, m2_16s, new, r1_32, mul, 0)
     C(0xe37c, MHY,     RXY_a, GIE, r1_o, m2_16s, new, r1_32, mul, 0)
+    C(0xe33c, MGH,     RXY_a, MIE2,r1_o, m2_16s, r1, 0, mul, 0)
 /* MULTIPLY HALFWORD IMMEDIATE */
     C(0xa70c, MHI,     RI_a,  Z,   r1_o, i2, new, r1_32, mul, 0)
     C(0xa70d, MGHI,    RI_a,  Z,   r1_o, i2, r1, 0, mul, 0)
-- 
2.26.2

[PATCH v1 5/8] s390x/tcg: Implement BRANCH INDIRECT ON CONDITION (BIC)

[David Hildenbrand]

Just like BRANCH ON CONDITION - however the address is read from memory
(always 8 bytes are read), we have to wrap the address manually. The
address is read using current CPU DAT/address-space controls, just like
ordinary data.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def | 2 ++
 target/s390x/translate.c   | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 455efe73da..dfb0ec067b 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -115,6 +115,8 @@
 /* BRANCH RELATIVE AND SAVE */
     C(0xa705, BRAS,    RI_b,  Z,   0, 0, r1, 0, basi, 0)
     C(0xc005, BRASL,   RIL_b, Z,   0, 0, r1, 0, basi, 0)
+/* BRANCH INDIRECT ON CONDITION */
+    C(0xe347, BIC,     RXY_b, MIE2,0, m2_64, 0, 0, bc, 0)
 /* BRANCH ON CONDITION */
     C(0x0700, BCR,     RR_b,  Z,   0, r2_nz, 0, 0, bc, 0)
     C(0x4700, BC,      RX_b,  Z,   0, a2, 0, 0, bc, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index b536491892..383edf7419 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1626,6 +1626,11 @@ static DisasJumpType op_bc(DisasContext *s, DisasOps *o)
         return DISAS_NEXT;
     }
 
+    /* For BIC the address came from memory, we need to wrap it again. */
+    if (s->fields.op2 == 0x47) {
+        gen_addi_and_wrap_i64(s, o->in2, o->in2, 0);
+    }
+
     disas_jcc(s, &c, m1);
     return help_branch(s, &c, is_imm, imm, o->in2);
 }
-- 
2.26.2

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

[David Hildenbrand]

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(+)

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);
-- 
2.26.2

[PATCH v1 7/8] s390x/tcg: We support Miscellaneous-Instruction-Extensions Facility 2

[David Hildenbrand]

We implement all relevant instructions.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/gen-features.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 8ddeebc544..1736b85ab0 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -716,6 +716,7 @@ static uint16_t qemu_MAX[] = {
     S390_FEAT_MSA_EXT_5,
     /* features introduced after the z13 */
     S390_FEAT_INSTRUCTION_EXEC_PROT,
+    S390_FEAT_MISC_INSTRUCTION_EXT,
 };
 
 /****** END FEATURE DEFS ******/
-- 
2.26.2

[PATCH v1 8/8] s390x/tcg: Implement CIPHER MESSAGE WITH AUTHENTICATION (KMA)

[David Hildenbrand]

As with the other crypto functions, we only implement subcode 0 (query)
and no actual encryption/decryption. We now implement S390_FEAT_MSA_EXT_8.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/gen-features.c | 1 +
 target/s390x/insn-data.def  | 1 +
 target/s390x/translate.c    | 7 +++++++
 3 files changed, 9 insertions(+)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 1736b85ab0..8de18f7f41 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -717,6 +717,7 @@ static uint16_t qemu_MAX[] = {
     /* features introduced after the z13 */
     S390_FEAT_INSTRUCTION_EXEC_PROT,
     S390_FEAT_MISC_INSTRUCTION_EXT,
+    S390_FEAT_MSA_EXT_8,
 };
 
 /****** END FEATURE DEFS ******/
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index bcd424e9ae..1ffdc20d59 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -979,6 +979,7 @@
     D(0xb92d, KMCTR,   RRF_b, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMCTR)
     D(0xb92e, KM,      RRE,   MSA,  0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KM)
     D(0xb92f, KMC,     RRE,   MSA,  0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMC)
+    D(0xb929, KMA,     RRF_b, MSA8, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMA)
     D(0xb93c, PPNO,    RRE,   MSA5, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_PPNO)
     D(0xb93e, KIMD,    RRE,   MSA,  0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KIMD)
     D(0xb93f, KLMD,    RRE,   MSA,  0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KLMD)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index c90cb37aae..df549e59e9 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2715,6 +2715,12 @@ static DisasJumpType op_msa(DisasContext *s, DisasOps *o)
     TCGv_i32 t_r1, t_r2, t_r3, type;
 
     switch (s->insn->data) {
+    case S390_FEAT_TYPE_KMA:
+        if (r3 == r1 || r3 == r2) {
+            gen_program_exception(s, PGM_SPECIFICATION);
+            return DISAS_NORETURN;
+        }
+        /* FALL THROUGH */
     case S390_FEAT_TYPE_KMCTR:
         if (r3 & 1 || !r3) {
             gen_program_exception(s, PGM_SPECIFICATION);
@@ -6130,6 +6136,7 @@ enum DisasInsnEnum {
 #define FAC_MSA3        S390_FEAT_MSA_EXT_3 /* msa-extension-3 facility */
 #define FAC_MSA4        S390_FEAT_MSA_EXT_4 /* msa-extension-4 facility */
 #define FAC_MSA5        S390_FEAT_MSA_EXT_5 /* msa-extension-5 facility */
+#define FAC_MSA8        S390_FEAT_MSA_EXT_8 /* msa-extension-8 facility */
 #define FAC_ECT         S390_FEAT_EXTRACT_CPU_TIME
 #define FAC_PCI         S390_FEAT_ZPCI /* z/PCI facility */
 #define FAC_AIS         S390_FEAT_ADAPTER_INT_SUPPRESSION
-- 
2.26.2

Re: [PATCH v1 0/8] s390x/tcg: Implement some z14 facilities

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200922103129.12824-1-david@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200922103129.12824-1-david@redhat.com
Subject: [PATCH v1 0/8] s390x/tcg: Implement some z14 facilities

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
4a710cb s390x/tcg: Implement CIPHER MESSAGE WITH AUTHENTICATION (KMA)
67c8e40 s390x/tcg: We support Miscellaneous-Instruction-Extensions Facility 2
e867c7b s390x/tcg: Implement MULTIPLY SINGLE (MSC, MSGC, MSGRKC, MSRKC)
2af9b83 s390x/tcg: Implement BRANCH INDIRECT ON CONDITION (BIC)
656cf89 s390x/tcg: Implement MULTIPLY HALFWORD (MGH)
6262d2e s390x/tcg: Implement MULTIPLY (MG, MGRK)
3aba4e2 s390x/tcg: Implement SUBTRACT HALFWORD (SGH)
c69a385 s390x/tcg: Implement ADD HALFWORD (AGH)

=== OUTPUT BEGIN ===
1/8 Checking commit c69a385d838c (s390x/tcg: Implement ADD HALFWORD (AGH))
ERROR: line over 90 characters
#32: FILE: target/s390x/translate.c:6101:
+#define FAC_MIE2        S390_FEAT_MISC_INSTRUCTION_EXT /* miscellaneous-instruction-extensions facility 2 */

total: 1 errors, 0 warnings, 14 lines checked

Patch 1/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/8 Checking commit 3aba4e25e456 (s390x/tcg: Implement SUBTRACT HALFWORD (SGH))
3/8 Checking commit 6262d2edc76d (s390x/tcg: Implement MULTIPLY (MG, MGRK))
4/8 Checking commit 656cf89d2355 (s390x/tcg: Implement MULTIPLY HALFWORD (MGH))
5/8 Checking commit 2af9b83b61be (s390x/tcg: Implement BRANCH INDIRECT ON CONDITION (BIC))
6/8 Checking commit e867c7b42400 (s390x/tcg: Implement MULTIPLY SINGLE (MSC, MSGC, MSGRKC, MSRKC))
7/8 Checking commit 67c8e4024fd0 (s390x/tcg: We support Miscellaneous-Instruction-Extensions Facility 2)
8/8 Checking commit 4a710cb58f52 (s390x/tcg: Implement CIPHER MESSAGE WITH AUTHENTICATION (KMA))
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200922103129.12824-1-david@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

[Richard Henderson]

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~

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

[David Hildenbrand]

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

Re: [PATCH v1 1/8] s390x/tcg: Implement ADD HALFWORD (AGH)

[Thomas Huth]

On 22/09/2020 12.31, David Hildenbrand wrote:
> Easy, just like ADD HALFWORD IMMEDIATE (AGHI).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def | 1 +
>  target/s390x/translate.c   | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index d79ae9e3f1..8dbeaf8c49 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -52,6 +52,7 @@
>  /* ADD HALFWORD */
>      C(0x4a00, AH,      RX_a,  Z,   r1, m2_16s, new, r1_32, add, adds32)
>      C(0xe37a, AHY,     RXY_a, LD,  r1, m2_16s, new, r1_32, add, adds32)
> +    C(0xe338, AGH,     RXY_a, MIE2,r1, m2_16s, r1, 0, add, adds64)
>  /* ADD HALFWORD IMMEDIATE */
>      C(0xa70a, AHI,     RI_a,  Z,   r1, i2, new, r1_32, add, adds32)
>      C(0xa70b, AGHI,    RI_a,  Z,   r1, i2, r1, 0, add, adds64)
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index a777343821..21d77b7e74 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -6098,6 +6098,7 @@ enum DisasInsnEnum {
>  #define FAC_AIS         S390_FEAT_ADAPTER_INT_SUPPRESSION
>  #define FAC_V           S390_FEAT_VECTOR /* vector facility */
>  #define FAC_VE          S390_FEAT_VECTOR_ENH /* vector enhancements facility 1 */
> +#define FAC_MIE2        S390_FEAT_MISC_INSTRUCTION_EXT /* miscellaneous-instruction-extensions facility 2 */
>  
>  static const DisasInsn insn_info[] = {
>  #include "insn-data.def"
> 

Looks right to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 2/8] s390x/tcg: Implement SUBTRACT HALFWORD (SGH)

[Thomas Huth]

On 22/09/2020 12.31, David Hildenbrand wrote:
> Easy to wire up.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 8dbeaf8c49..e851e9df5e 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -883,6 +883,7 @@
>  /* SUBTRACT HALFWORD */
>      C(0x4b00, SH,      RX_a,  Z,   r1, m2_16s, new, r1_32, sub, subs32)
>      C(0xe37b, SHY,     RXY_a, LD,  r1, m2_16s, new, r1_32, sub, subs32)
> +    C(0xe339, SGH,     RXY_a, MIE2,r1, m2_16s, r1, 0, sub, subs64)
>  /* SUBTRACT HIGH */
>      C(0xb9c9, SHHHR,   RRF_a, HW,  r2_sr32, r3_sr32, new, r1_32h, sub, subs32)
>      C(0xb9d9, SHHLR,   RRF_a, HW,  r2_sr32, r3, new, r1_32h, sub, subs32)
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 1/8] s390x/tcg: Implement ADD HALFWORD (AGH)

[Thomas Huth]

On 22/09/2020 12.31, David Hildenbrand wrote:
> Easy, just like ADD HALFWORD IMMEDIATE (AGHI).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def | 1 +
>  target/s390x/translate.c   | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index d79ae9e3f1..8dbeaf8c49 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -52,6 +52,7 @@
>  /* ADD HALFWORD */
>      C(0x4a00, AH,      RX_a,  Z,   r1, m2_16s, new, r1_32, add, adds32)
>      C(0xe37a, AHY,     RXY_a, LD,  r1, m2_16s, new, r1_32, add, adds32)
> +    C(0xe338, AGH,     RXY_a, MIE2,r1, m2_16s, r1, 0, add, adds64)
>  /* ADD HALFWORD IMMEDIATE */
>      C(0xa70a, AHI,     RI_a,  Z,   r1, i2, new, r1_32, add, adds32)
>      C(0xa70b, AGHI,    RI_a,  Z,   r1, i2, r1, 0, add, adds64)
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index a777343821..21d77b7e74 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -6098,6 +6098,7 @@ enum DisasInsnEnum {
>  #define FAC_AIS         S390_FEAT_ADAPTER_INT_SUPPRESSION
>  #define FAC_V           S390_FEAT_VECTOR /* vector facility */
>  #define FAC_VE          S390_FEAT_VECTOR_ENH /* vector enhancements facility 1 */
> +#define FAC_MIE2        S390_FEAT_MISC_INSTRUCTION_EXT /* miscellaneous-instruction-extensions facility 2 */

Maybe you should use "S390_FEAT_MISC_INSTRUCTION_EXT2" (i.e. with 2 at
the end) to avoid that it gets confused with the first MIE ?

 Thomas

Re: [PATCH v1 1/8] s390x/tcg: Implement ADD HALFWORD (AGH)

[David Hildenbrand]

On 23.09.20 06:53, Thomas Huth wrote:
> On 22/09/2020 12.31, David Hildenbrand wrote:
>> Easy, just like ADD HALFWORD IMMEDIATE (AGHI).
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  target/s390x/insn-data.def | 1 +
>>  target/s390x/translate.c   | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
>> index d79ae9e3f1..8dbeaf8c49 100644
>> --- a/target/s390x/insn-data.def
>> +++ b/target/s390x/insn-data.def
>> @@ -52,6 +52,7 @@
>>  /* ADD HALFWORD */
>>      C(0x4a00, AH,      RX_a,  Z,   r1, m2_16s, new, r1_32, add, adds32)
>>      C(0xe37a, AHY,     RXY_a, LD,  r1, m2_16s, new, r1_32, add, adds32)
>> +    C(0xe338, AGH,     RXY_a, MIE2,r1, m2_16s, r1, 0, add, adds64)
>>  /* ADD HALFWORD IMMEDIATE */
>>      C(0xa70a, AHI,     RI_a,  Z,   r1, i2, new, r1_32, add, adds32)
>>      C(0xa70b, AGHI,    RI_a,  Z,   r1, i2, r1, 0, add, adds64)
>> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
>> index a777343821..21d77b7e74 100644
>> --- a/target/s390x/translate.c
>> +++ b/target/s390x/translate.c
>> @@ -6098,6 +6098,7 @@ enum DisasInsnEnum {
>>  #define FAC_AIS         S390_FEAT_ADAPTER_INT_SUPPRESSION
>>  #define FAC_V           S390_FEAT_VECTOR /* vector facility */
>>  #define FAC_VE          S390_FEAT_VECTOR_ENH /* vector enhancements facility 1 */
>> +#define FAC_MIE2        S390_FEAT_MISC_INSTRUCTION_EXT /* miscellaneous-instruction-extensions facility 2 */
> 
> Maybe you should use "S390_FEAT_MISC_INSTRUCTION_EXT2" (i.e. with 2 at
> the end) to avoid that it gets confused with the first MIE ?

Yes, renaming S390_FEAT_MISC_INSTRUCTION_EXT ->
S390_FEAT_MISC_INSTRUCTION_EXT2 makes sense.

> 
>  Thomas
> 


-- 
Thanks,

David / dhildenb

Re: [PATCH v1 1/8] s390x/tcg: Implement ADD HALFWORD (AGH)

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> Easy, just like ADD HALFWORD IMMEDIATE (AGHI).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def | 1 +
>  target/s390x/translate.c   | 1 +
>  2 files changed, 2 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v1 2/8] s390x/tcg: Implement SUBTRACT HALFWORD (SGH)

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> Easy to wire up.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v1 3/8] s390x/tcg: Implement MULTIPLY (MG, MGRK)

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> Multiply two signed 64bit values and store the 128bit result in r1 (0-63)
> and r1 + 1 (64-127).
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def |  2 ++
>  target/s390x/translate.c   | 13 +++++++++++++
>  2 files changed, 15 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v1 4/8] s390x/tcg: Implement MULTIPLY HALFWORD (MGH)

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> Just like MULTIPLY HALFWORD IMMEDIATE (MGHI), only the second operand
> (signed 16 bit) comes from memory.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v1 5/8] s390x/tcg: Implement BRANCH INDIRECT ON CONDITION (BIC)

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> +/* BRANCH INDIRECT ON CONDITION */
> +    C(0xe347, BIC,     RXY_b, MIE2,0, m2_64, 0, 0, bc, 0)
>  /* BRANCH ON CONDITION */
>      C(0x0700, BCR,     RR_b,  Z,   0, r2_nz, 0, 0, bc, 0)
>      C(0x4700, BC,      RX_b,  Z,   0, a2, 0, 0, bc, 0)
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index b536491892..383edf7419 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -1626,6 +1626,11 @@ static DisasJumpType op_bc(DisasContext *s, DisasOps *o)
>          return DISAS_NEXT;
>      }
>  
> +    /* For BIC the address came from memory, we need to wrap it again. */
> +    if (s->fields.op2 == 0x47) {
> +        gen_addi_and_wrap_i64(s, o->in2, o->in2, 0);
> +    }

I'm not keen on this sort of per-opcode checks.

I'd prefer to add an in2_m2_64w() helper that performs the load and then wraps.


r~

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

[Richard Henderson]

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~

Re: [PATCH v1 7/8] s390x/tcg: We support Miscellaneous-Instruction-Extensions Facility 2

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> We implement all relevant instructions.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/gen-features.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v1 8/8] s390x/tcg: Implement CIPHER MESSAGE WITH AUTHENTICATION (KMA)

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> As with the other crypto functions, we only implement subcode 0 (query)
> and no actual encryption/decryption. We now implement S390_FEAT_MSA_EXT_8.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/gen-features.c | 1 +
>  target/s390x/insn-data.def  | 1 +
>  target/s390x/translate.c    | 7 +++++++
>  3 files changed, 9 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v1 0/8] s390x/tcg: Implement some z14 facilities

[Richard Henderson]

On 9/22/20 3:31 AM, David Hildenbrand wrote:
> With this patchset, I can boot a kernel compiled for z14 (requiring
> Miscellaneous-Instruction-Extensions Facility 2) (using -cpu max).

Excellent.

Looks like it wouldn't take too much additional work for z15, if you're of a
mind while we're at it.

Message-Security-Assist 9:
	KDSA to ignore, like KMA.

Miscellaneous-Instruction-Extensions 3:
	A bunch of logical operations, all of which
	already have tcg primitives.

	SELECT, which can re-use op_loc with different inputs.

	MOVE RIGHT TO LEFT, which can't reuse do_helper_mvc
	directly, but could be a copy with trivial changes.

	Trivial changes to op_popcnt.

Vector-Enhancements 2:
	Mostly load/store byte-reversed.
	New vector shift insns.
	FP/Int conversions.
	Vector string search.


r~

Re: [PATCH v1 5/8] s390x/tcg: Implement BRANCH INDIRECT ON CONDITION (BIC)

[David Hildenbrand]

On 25.09.20 23:45, Richard Henderson wrote:
> On 9/22/20 3:31 AM, David Hildenbrand wrote:
>> +/* BRANCH INDIRECT ON CONDITION */
>> +    C(0xe347, BIC,     RXY_b, MIE2,0, m2_64, 0, 0, bc, 0)
>>  /* BRANCH ON CONDITION */
>>      C(0x0700, BCR,     RR_b,  Z,   0, r2_nz, 0, 0, bc, 0)
>>      C(0x4700, BC,      RX_b,  Z,   0, a2, 0, 0, bc, 0)
>> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
>> index b536491892..383edf7419 100644
>> --- a/target/s390x/translate.c
>> +++ b/target/s390x/translate.c
>> @@ -1626,6 +1626,11 @@ static DisasJumpType op_bc(DisasContext *s, DisasOps *o)
>>          return DISAS_NEXT;
>>      }
>>  
>> +    /* For BIC the address came from memory, we need to wrap it again. */
>> +    if (s->fields.op2 == 0x47) {
>> +        gen_addi_and_wrap_i64(s, o->in2, o->in2, 0);
>> +    }
> 
> I'm not keen on this sort of per-opcode checks.
> 
> I'd prefer to add an in2_m2_64w() helper that performs the load and then wraps.

Makes sense, thanks!


-- 
Thanks,

David / dhildenb

Re: [PATCH v1 0/8] s390x/tcg: Implement some z14 facilities

[David Hildenbrand]

On 26.09.20 00:43, Richard Henderson wrote:
> On 9/22/20 3:31 AM, David Hildenbrand wrote:
>> With this patchset, I can boot a kernel compiled for z14 (requiring
>> Miscellaneous-Instruction-Extensions Facility 2) (using -cpu max).
> 
> Excellent.
> 
> Looks like it wouldn't take too much additional work for z15, if you're of a
> mind while we're at it.

Yes, on my todo list (however might take some time until I get to it).
I'm focusing on z14 right now because next-gen distributions will most
probably require at least z14 hw, so we're prepared for that.

I'm still wrapping my head around how to best implement the crazy VECTOR
FP MINIMUM / MAXIMUM stuff in the least ugly way (expressing all cases
via if/else/switch is just horribly error-prone).


-- 
Thanks,

David / dhildenb

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

[David Hildenbrand]

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