s390x: Support And1/Or1, improve handling of Int1 expressions

This provides an instruction selector for Int1-expressions that supports
And1 and Or1.  This implementation tries to keep values in registers as
much as possible, to avoid too many conversions from a Boolean value to a
condition code or vice versa.  To this end, the new function
s390_isel_int1_expr() is added, which handles bit-typed expressions that
are supposed to end up in a register.

Also change the representation of Int1 values in registers and always
sign-extend them to 64 bits.

diff --git a/VEX/priv/host_s390_defs.c b/VEX/priv/host_s390_defs.c
index de267078e17e6ad0201f52b0a45ff1344f82e42e..1cd469637e418c00d501f173fc0412ae7a7fe040 100644 (file)
--- a/VEX/priv/host_s390_defs.c
+++ b/VEX/priv/host_s390_defs.c
@@ -9376,25 +9376,24 @@ s390_insn_cc2bool_emit(UChar *buf, const s390_insn *insn)
    UChar r1 = hregNumber(insn->variant.cc2bool.dst);
    s390_cc_t cond = insn->variant.cc2bool.cond;
 
-   /* Make the destination register be 1 or 0, depending on whether
+   /* Make the destination register be -1 or 0, depending on whether
       the relevant condition holds. A 64-bit value is computed. */
    if (cond == S390_CC_ALWAYS)
-      return s390_emit_LGHI(buf, r1, 1);  /* r1 = 1 */
+      return s390_emit_LGHI(buf, r1, -1);  /* r1 = -1 */
 
    /* If LOCGHI is available, use it. */
    if (s390_host_has_lsc2) {
-      /* Clear r1, then load immediate 1 on condition. */
+      /* Clear r1, then load immediate -1 on condition. */
       buf = s390_emit_LGHI(buf, r1, 0);
       if (cond != S390_CC_NEVER)
-         buf = s390_emit_LOCGHI(buf, r1, 1, cond);
+         buf = s390_emit_LOCGHI(buf, r1, -1, cond);
       return buf;
    }
 
    buf = s390_emit_load_cc(buf, r1);                 /* r1 = cc */
    buf = s390_emit_LGHI(buf, R0, cond);              /* r0 = mask */
-   buf = s390_emit_SLLG(buf, r1, R0, r1, DISP20(0)); /* r1 = mask << cc */
-   buf = s390_emit_SRLG(buf, r1, r1, 0,  DISP20(3)); /* r1 = r1 >> 3 */
-   buf = s390_emit_NILL(buf, r1, 1);                 /* r1 = r1 & 0x1 */
+   buf = s390_emit_SLLG(buf, r1, R0, r1, DISP20(60)); /* r1 = mask << (cc+60) */
+   buf = s390_emit_SRAG(buf, r1, r1, 0,  DISP20(63)); /* r1 = r1 >> 63 */
 
    return buf;
 }

diff --git a/VEX/priv/host_s390_isel.c b/VEX/priv/host_s390_isel.c
index 00b4aee8182877137c21111ca3a454f45b1639f0..4d0cf1721b7f969a8f7b68d879af9fabcf7561f5 100644 (file)
--- a/VEX/priv/host_s390_isel.c
+++ b/VEX/priv/host_s390_isel.c
@@ -127,6 +127,7 @@ static HReg          s390_isel_int_expr(ISelEnv *, IRExpr *);
 static s390_amode   *s390_isel_amode(ISelEnv *, IRExpr *);
 static s390_amode   *s390_isel_amode_b12_b20(ISelEnv *, IRExpr *);
 static s390_cc_t     s390_isel_cc(ISelEnv *, IRExpr *);
+static HReg          s390_isel_int1_expr(ISelEnv *env, IRExpr *expr, HReg dst);
 static s390_opnd_RMI s390_isel_int_expr_RMI(ISelEnv *, IRExpr *);
 static void          s390_isel_int128_expr(HReg *, HReg *, ISelEnv *, IRExpr *);
 static HReg          s390_isel_float_expr(ISelEnv *, IRExpr *);
@@ -1763,9 +1764,7 @@ s390_isel_int_expr_wrk(ISelEnv *env, IRExpr *expr)
 
       /* Expressions whose argument is 1-bit wide */
       if (typeOfIRExpr(env->type_env, arg) == Ity_I1) {
-         s390_cc_t cond = s390_isel_cc(env, arg);
-         dst = newVRegI(env);     /* Result goes into a new register */
-         addInstr(env, s390_insn_cc2bool(dst, cond));
+         dst = s390_isel_int1_expr(env, arg, INVALID_HREG);
 
          switch (unop) {
          case Iop_1Uto8:
@@ -1784,16 +1783,9 @@ s390_isel_int_expr_wrk(ISelEnv *env, IRExpr *expr)
          case Iop_1Sto8:
          case Iop_1Sto16:
          case Iop_1Sto32:
-            shift.variant.imm = 31;
-            addInstr(env, s390_insn_alu(4, S390_ALU_LSH,  dst, shift));
-            addInstr(env, s390_insn_alu(4, S390_ALU_RSHA, dst, shift));
-            break;
-
          case Iop_1Sto64:
-            shift.variant.imm = 63;
-            addInstr(env, s390_insn_alu(8, S390_ALU_LSH,  dst, shift));
-            addInstr(env, s390_insn_alu(8, S390_ALU_RSHA, dst, shift));
-            break;
+            /* Int1 values are already sign-extended to 64 bits. */
+            return dst;
 
          default:
             goto irreducible;
@@ -3466,7 +3458,8 @@ s390_isel_dfp_expr(ISelEnv *env, IRExpr *expr)
 /*--- ISEL: Condition Code                              ---*/
 /*---------------------------------------------------------*/
 
-/* This function handles all operators that produce a 1-bit result */
+/* Handle all operators that produce a 1-bit result.  This function is mutually
+   recursive with s390_isel_int1_expr. */
 static s390_cc_t
 s390_isel_cc(ISelEnv *env, IRExpr *cond)
 {
@@ -3554,45 +3547,14 @@ s390_isel_cc(ISelEnv *env, IRExpr *cond)
    if (cond->tag == Iex_Binop) {
       IRExpr *arg1 = cond->Iex.Binop.arg1;
       IRExpr *arg2 = cond->Iex.Binop.arg2;
-      HReg reg1, reg2;
-
-      /* sewardj 2019Nov30: This will be needed when chasing through conditional
-         branches in guest_generic_bb_to_IR.c is enabled on s390x.
-         Unfortunately that is currently disabled on s390x as it causes
-         mysterious segfaults and also exposes some unhandled Iex_ITE cases in
-         this instruction selector.  The following Iop_And1/Iop_Or1 cases are
-         also needed when enabled.  The code below is *believed* to be correct,
-         and has been lightly tested, but it is #if 0'd until such time as we
-         need it. */
-#     if 0
-      /* FIXME: We could (and probably should) do a lot better here, by using
-         the iselCondCode_C/_R scheme used in the amd64 insn selector. */
+
+      /* And1 / Or1 */
       if (cond->Iex.Binop.op == Iop_And1 || cond->Iex.Binop.op == Iop_Or1) {
-         /* In short: force both operands into registers, AND or OR them, mask
-            off all but the lowest bit, then convert the result back into a
-            condition code. */
-         const s390_opnd_RMI one = s390_opnd_imm(1);
-
-         HReg x_as_64 = newVRegI(env);
-         s390_cc_t cc_x = s390_isel_cc(env, arg1);
-         addInstr(env, s390_insn_cc2bool(x_as_64, cc_x));
-         addInstr(env, s390_insn_alu(8, S390_ALU_AND, x_as_64, one));
-
-         HReg y_as_64 = newVRegI(env);
-         s390_cc_t cc_y = s390_isel_cc(env, arg2);
-         addInstr(env, s390_insn_cc2bool(y_as_64, cc_y));
-         addInstr(env, s390_insn_alu(8, S390_ALU_AND, y_as_64, one));
-
-         s390_alu_t opkind
-            = cond->Iex.Binop.op == Iop_And1 ? S390_ALU_AND : S390_ALU_OR;
-         addInstr(env, s390_insn_alu(/*size=*/8,
-                                     opkind, x_as_64, s390_opnd_reg(y_as_64)));
-
-         addInstr(env, s390_insn_alu(/*size=*/8, S390_ALU_AND, x_as_64, one));
-         addInstr(env, s390_insn_test(/*size=*/8, s390_opnd_reg(x_as_64)));
+         /* Perform the calculation in registers, but ignore the resulting
+            value.  Instead, assume that the condition code is set. */
+         (void) s390_isel_int1_expr(env, cond, INVALID_HREG);
          return S390_CC_NE;
       }
-#     endif /* 0 */
 
       // |sizeofIRType| asserts on Ity_I1, so we can't do it until after we're
       // sure that Iop_And1 and Iop_Or1 can't make it this far.
@@ -3627,14 +3589,12 @@ s390_isel_cc(ISelEnv *env, IRExpr *cond)
          goto do_compare_ze;
 
       do_compare_ze: {
-            s390_opnd_RMI op1, op2;
-
-            op1  = s390_isel_int_expr_RMI(env, arg1);
-            reg1 = newVRegI(env);
+            s390_opnd_RMI op1 = s390_isel_int_expr_RMI(env, arg1);
+            HReg reg1 = newVRegI(env);
             addInstr(env, s390_insn_unop(4, op, reg1, op1));
 
-            op2  = s390_isel_int_expr_RMI(env, arg2);
-            reg2 = newVRegI(env);
+            s390_opnd_RMI op2 = s390_isel_int_expr_RMI(env, arg2);
+            HReg reg2 = newVRegI(env);
             addInstr(env, s390_insn_unop(4, op, reg2, op2));  /* zero extend */
 
             op2 = s390_opnd_reg(reg2);
@@ -3713,6 +3673,51 @@ s390_isel_cc(ISelEnv *env, IRExpr *cond)
    vpanic("s390_isel_cc: unexpected operator");
 }
 
+/*---------------------------------------------------------*/
+/*--- ISEL: Bit-typed expression                        ---*/
+/*---------------------------------------------------------*/
+
+/* Handle Int1-typed expressions that are supposed to end up in a register.
+   Place the result into the given target register 'dst', or, if INVALID_HREG is
+   specified, create a new virtual integer register if necessary.  Keep Int1
+   values in registers always sign-extended to 64 bits.  This function is
+   mutually recursive with s390_isel_cc. */
+static HReg
+s390_isel_int1_expr(ISelEnv *env, IRExpr *expr, HReg dst)
+{
+   vassert(expr);
+   vassert(typeOfIRExpr(env->type_env, expr) == Ity_I1);
+
+   /* Variable. */
+   if (expr->tag == Iex_RdTmp) {
+      HReg res = lookupIRTemp(env, expr->Iex.RdTmp.tmp);
+      if (hregIsInvalid(dst)) {
+         return res;
+      }
+      addInstr(env, s390_insn_move(8, dst, res));
+      return dst;
+   }
+
+   HReg res = hregIsInvalid(dst) ? newVRegI(env) : dst;
+
+   /* And1 / Or1 */
+   if (expr->tag == Iex_Binop
+       && (expr->Iex.Binop.op == Iop_And1 || expr->Iex.Binop.op == Iop_Or1)) {
+      HReg reg1 = s390_isel_int1_expr(env, expr->Iex.Binop.arg1, res);
+      HReg reg2 = s390_isel_int1_expr(env, expr->Iex.Binop.arg2, INVALID_HREG);
+      s390_alu_t opkind
+         = expr->Iex.Binop.op == Iop_And1 ? S390_ALU_AND : S390_ALU_OR;
+
+      /* Ensure that the condition code is set; s390_isel_cc relies on it. */
+      addInstr(env, s390_insn_alu(8, opkind, reg1, s390_opnd_reg(reg2)));
+      return reg1;
+   }
+
+   /* Else, call s390_isel_cc and force the value into a register. */
+   addInstr(env, s390_insn_cc2bool(res, s390_isel_cc(env, expr)));
+   return res;
+}
+
 
 /*---------------------------------------------------------*/
 /*--- ISEL: Vector expressions (128 bit)                ---*/
@@ -4988,9 +4993,8 @@ no_memcpy_put:
          break;
 
       case Ity_I1: {
-         s390_cc_t cond = s390_isel_cc(env, stmt->Ist.WrTmp.data);
          dst = lookupIRTemp(env, tmp);
-         addInstr(env, s390_insn_cc2bool(dst, cond));
+         dst = s390_isel_int1_expr(env, stmt->Ist.WrTmp.data, dst);
          return;
       }