From: Florian Krohm <florian@eich-krohm.de>
Date: Thu, 27 Dec 2012 00:59:43 +0000 (+0000)
Subject: s390: Do not waste a register when assigning a constant to a memory
X-Git-Tag: svn/VALGRIND_3_9_0^2~167
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4c9242cfbe8667e72db727cc067909d802b202e4;p=thirdparty%2Fvalgrind.git

s390: Do not waste a register when assigning a constant to a memory
location. If available, use MVHI and friends. If those are not available,
load the constant value into register r0 and store that. r0 is not visible
to register allocation and therefore using it does not increase register
pressure.
Remove S390_INSN_MZERO and replace it with S390_INSN_MIMM. Assigning zero
is just a special case..
Saves between 0.9% and 2.4% of insns as measured with the perf regression
bucket.


git-svn-id: svn://svn.valgrind.org/vex/trunk@2619
---

diff --git a/VEX/priv/host_s390_defs.c b/VEX/priv/host_s390_defs.c
index 3a822222ba..adea62e740 100644
--- a/VEX/priv/host_s390_defs.c
+++ b/VEX/priv/host_s390_defs.c
@@ -741,8 +741,8 @@ s390_insn_get_reg_usage(HRegUsage *u, const s390_insn *insn)
          addHRegUse(u, HRmRead, insn->variant.dfp_convert.op_lo); /* operand */
       break;
 
-   case S390_INSN_MZERO:
-      s390_amode_get_reg_usage(u, insn->variant.mzero.dst);
+   case S390_INSN_MIMM:
+      s390_amode_get_reg_usage(u, insn->variant.mimm.dst);
       break;
 
    case S390_INSN_MADD:
@@ -1026,8 +1026,8 @@ s390_insn_map_regs(HRegRemap *m, s390_insn *insn)
             lookupHRegRemap(m, insn->variant.dfp_convert.op_lo);
       break;
 
-   case S390_INSN_MZERO:
-      s390_amode_map_regs(m, insn->variant.mzero.dst);
+   case S390_INSN_MIMM:
+      s390_amode_map_regs(m, insn->variant.mimm.dst);
       break;
 
    case S390_INSN_MADD:
@@ -1309,6 +1309,32 @@ emit_S(UChar *p, UInt op, UChar b2, UShort d2)
 }
 
 
+static UChar *
+emit_SI(UChar *p, UInt op, UChar i2, UChar b1, UShort d1)
+{
+   ULong the_insn = op;
+
+   the_insn |= ((ULong)i2) << 16;
+   the_insn |= ((ULong)b1) << 12;
+   the_insn |= ((ULong)d1) << 0;
+
+   return emit_4bytes(p, the_insn);
+}
+
+
+static UChar *
+emit_SIL(UChar *p, ULong op, UChar b1, UShort d1, UShort i2)
+{
+   ULong the_insn = op;
+
+   the_insn |= ((ULong)b1) << 28;
+   the_insn |= ((ULong)d1) << 16;
+   the_insn |= ((ULong)i2) << 0;
+
+   return emit_6bytes(p, the_insn);
+}
+
+
 static UChar *
 emit_SIY(UChar *p, ULong op, UChar i2, UChar b1, UShort dl1, UChar dh1)
 {
@@ -2734,6 +2760,46 @@ s390_emit_MSGFI(UChar *p, UChar r1, UInt i2)
 }
 
 
+static UChar *
+s390_emit_MVI(UChar *p, UChar i2, UChar b1, UShort d1)
+{
+   if (UNLIKELY(vex_traceflags & VEX_TRACE_ASM))
+      s390_disasm(ENC3(MNM, UDXB, INT), "mvi", b1, 0, d1, i2);
+
+   return emit_SI(p, 0x92000000, i2, b1, d1);
+}
+
+
+static UChar *
+s390_emit_MVHHI(UChar *p, UChar b1, UShort d1, UShort i2)
+{
+   if (UNLIKELY(vex_traceflags & VEX_TRACE_ASM))
+      s390_disasm(ENC3(MNM, UDXB, INT), "mvhhi", b1, 0, d1, i2);
+
+   return emit_SIL(p, 0xe54400000000ULL, b1, d1, i2);
+}
+
+
+static UChar *
+s390_emit_MVHI(UChar *p, UChar b1, UShort d1, UShort i2)
+{
+   if (UNLIKELY(vex_traceflags & VEX_TRACE_ASM))
+      s390_disasm(ENC3(MNM, UDXB, INT), "mvhi", b1, 0, d1, i2);
+
+   return emit_SIL(p, 0xe54c00000000ULL, b1, d1, i2);
+}
+
+
+static UChar *
+s390_emit_MVGHI(UChar *p, UChar b1, UShort d1, UShort i2)
+{
+   if (UNLIKELY(vex_traceflags & VEX_TRACE_ASM))
+      s390_disasm(ENC3(MNM, UDXB, INT), "mvghi", b1, 0, d1, i2);
+
+   return emit_SIL(p, 0xe54800000000ULL, b1, d1, i2);
+}
+
+
 static UChar *
 s390_emit_OR(UChar *p, UChar r1, UChar r2)
 {
@@ -5472,17 +5538,18 @@ s390_insn_mfence(void)
 
 
 s390_insn *
-s390_insn_mzero(UChar size, s390_amode *dst)
+s390_insn_mimm(UChar size, s390_amode *dst, ULong value)
 {
    s390_insn *insn = LibVEX_Alloc(sizeof(s390_insn));
 
-   /* This insn will be mapped to an XC so we can only allow base register
+   /* This insn will be mapped to insns that require base register
       plus 12-bit displacement */
    vassert(dst->tag == S390_AMODE_B12);
 
-   insn->tag  = S390_INSN_MZERO;
+   insn->tag  = S390_INSN_MIMM;
    insn->size = size;
-   insn->variant.mzero.dst = dst;
+   insn->variant.mimm.dst = dst;
+   insn->variant.mimm.value = value;
 
    return insn;
 }
@@ -6074,8 +6141,9 @@ s390_insn_as_string(const s390_insn *insn)
       s390_sprintf(buf, "%M", "v-mfence");
       return buf;   /* avoid printing "size = ..." which is meaningless */
 
-   case S390_INSN_MZERO:
-      s390_sprintf(buf, "%M %A", "v-mzero", insn->variant.mzero.dst);
+   case S390_INSN_MIMM:
+      s390_sprintf(buf, "%M %A,%I", "v-mimm", insn->variant.mimm.dst,
+                   insn->variant.mimm.value);
       break;
 
    case S390_INSN_MADD:
@@ -8305,11 +8373,42 @@ s390_insn_mfence_emit(UChar *buf, const s390_insn *insn)
 
 
 static UChar *
-s390_insn_mzero_emit(UChar *buf, const s390_insn *insn)
+s390_insn_mimm_emit(UChar *buf, const s390_insn *insn)
 {
-   s390_amode *am = insn->variant.mzero.dst;
+   s390_amode *am = insn->variant.mimm.dst;
+   ULong value = insn->variant.mimm.value;
 
-   return s390_emit_XC(buf, insn->size - 1, am->b, am->d, am->b, am->d);
+   if (value == 0) {
+      return s390_emit_XC(buf, insn->size - 1, am->b, am->d, am->b, am->d);
+   }
+
+   if (insn->size == 1) {
+      return s390_emit_MVI(buf, value & 0xFF, am->b, am->d);
+   }
+
+   if (s390_host_has_gie && ulong_fits_signed_16bit(value)) {
+      value &= 0xFFFF;
+      switch (insn->size) {
+      case 2: return s390_emit_MVHHI(buf, am->b, am->d, value);
+      case 4: return s390_emit_MVHI(buf,  am->b, am->d, value);
+      case 8: return s390_emit_MVGHI(buf, am->b, am->d, value);
+      }
+   } else {
+      // Load value to R0, then store.
+      switch (insn->size) {
+      case 2:
+         buf = s390_emit_LHI(buf, R0, value & 0xFFFF);
+         return s390_emit_STH(buf, R0, 0, am->b, am->d);
+      case 4:
+         buf = s390_emit_load_32imm(buf, R0, value);
+         return s390_emit_ST(buf, R0, 0, am->b, am->d);
+      case 8:
+         buf = s390_emit_load_64imm(buf, R0, value);
+         return s390_emit_STG(buf, R0, 0, am->b, DISP20(am->d));
+      }
+   }
+   
+   vpanic("s390_insn_mimm_emit");
 }
 
 
@@ -8912,8 +9011,8 @@ emit_S390Instr(Bool *is_profinc, UChar *buf, Int nbuf, s390_insn *insn,
       end = s390_insn_mfence_emit(buf, insn);
       break;
 
-   case S390_INSN_MZERO:
-      end = s390_insn_mzero_emit(buf, insn);
+   case S390_INSN_MIMM:
+      end = s390_insn_mimm_emit(buf, insn);
       break;
 
    case S390_INSN_MADD:
diff --git a/VEX/priv/host_s390_defs.h b/VEX/priv/host_s390_defs.h
index 4b795c4a80..56b8b126bb 100644
--- a/VEX/priv/host_s390_defs.h
+++ b/VEX/priv/host_s390_defs.h
@@ -144,7 +144,7 @@ typedef enum {
    S390_INSN_DFP_COMPARE,
    S390_INSN_DFP_CONVERT,
    S390_INSN_MFENCE,
-   S390_INSN_MZERO,   /* Assign zero to a memory location */
+   S390_INSN_MIMM,    /* Assign an immediate constant to a memory location */
    S390_INSN_MADD,    /* Add a value to a memory location */
    S390_INSN_SET_FPC_BFPRM, /* Set the bfp rounding mode in the FPC */
    S390_INSN_SET_FPC_DFPRM, /* Set the dfp rounding mode in the FPC */
@@ -463,7 +463,8 @@ typedef struct {
       /* Miscellaneous */
       struct {
          s390_amode      *dst;
-      } mzero;
+         ULong            value;  /* sign extended */
+      } mimm;
       struct {
          s390_amode      *dst;
          UChar            delta;
@@ -584,7 +585,7 @@ s390_insn *s390_insn_dfp128_convert_from(UChar size, s390_dfp_conv_t,
                                          HReg dst, HReg op_hi, HReg op_lo,
                                          s390_dfp_round_t);
 s390_insn *s390_insn_mfence(void);
-s390_insn *s390_insn_mzero(UChar size, s390_amode *dst);
+s390_insn *s390_insn_mimm(UChar size, s390_amode *dst, ULong value);
 s390_insn *s390_insn_madd(UChar size, s390_amode *dst, UChar delta,
                           ULong value);
 s390_insn *s390_insn_set_fpc_bfprm(UChar size, HReg mode);
diff --git a/VEX/priv/host_s390_isel.c b/VEX/priv/host_s390_isel.c
index 3027045049..ce81ceeb89 100644
--- a/VEX/priv/host_s390_isel.c
+++ b/VEX/priv/host_s390_isel.c
@@ -426,6 +426,24 @@ s390_expr_is_const_zero(IRExpr *expr)
 }
 
 
+/* Return the value of CON as a sign-exteded ULong value */
+static ULong
+get_const_value_as_ulong(const IRConst *con)
+{
+   Long value;
+
+   switch (con->tag) {
+   case Ico_U1:  value = con->Ico.U1;  return (ULong) ((value << 63) >> 63);
+   case Ico_U8:  value = con->Ico.U8;  return (ULong) ((value << 56) >> 56);
+   case Ico_U16: value = con->Ico.U16; return (ULong) ((value << 48) >> 48);
+   case Ico_U32: value = con->Ico.U32; return (ULong) ((value << 32) >> 32);
+   case Ico_U64: return con->Ico.U64;
+   default:
+      vpanic("get_const_value_as_ulong");
+   }
+}
+
+
 /* Call a helper (clean or dirty)
    Arguments must satisfy the following conditions:
 
@@ -2924,8 +2942,10 @@ s390_isel_stmt(ISelEnv *env, IRStmt *stmt)
       case Ity_I64:
          /* fixs390: We could check for INSN_MADD here. */
          if (am->tag == S390_AMODE_B12 &&
-             s390_expr_is_const_zero(stmt->Ist.Store.data)) {
-            addInstr(env, s390_insn_mzero(sizeofIRType(tyd), am));
+             stmt->Ist.Store.data->tag == Iex_Const) {
+            ULong value =
+               get_const_value_as_ulong(stmt->Ist.Store.data->Iex.Const.con);
+            addInstr(env, s390_insn_mimm(sizeofIRType(tyd), am, value));
             return;
          }
          src = s390_isel_int_expr(env, stmt->Ist.Store.data);
@@ -3000,13 +3020,6 @@ s390_isel_stmt(ISelEnv *env, IRStmt *stmt)
          return;
       }
 
-      /* guest register = 0 */
-      if (new_value == 0) {
-         am = s390_amode_for_guest_state(offset);
-         addInstr(env, s390_insn_mzero(sizeofIRType(tyd), am));
-         return;
-      }
-
       if (old_value_is_valid == False) goto not_special;
 
       /* If the new value is in the neighbourhood of the old value
@@ -3020,22 +3033,17 @@ s390_isel_stmt(ISelEnv *env, IRStmt *stmt)
          return;
       }
 
-      /* If the high word is the same it is sufficient to load the low word.
-         Use R0 as a scratch reg. */
+      /* If the high word is the same it is sufficient to load the low word. */
       if ((old_value >> 32) == (new_value >> 32)) {
-         HReg r0  = make_gpr(0);
-
          am = s390_amode_for_guest_state(offset + 4);
-         addInstr(env, s390_insn_load_immediate(4, r0,
-                                                new_value & 0xFFFFFFFF));
-         addInstr(env, s390_insn_store(4, am, r0));
+         addInstr(env, s390_insn_mimm(4, am, new_value & 0xFFFFFFFF));
          return;
       }
 
       /* No special case applies... fall through */
 
    not_special:
-      am = s390_amode_for_guest_state(stmt->Ist.Put.offset);
+      am = s390_amode_for_guest_state(offset);
 
       switch (tyd) {
       case Ity_I8:
@@ -3043,8 +3051,10 @@ s390_isel_stmt(ISelEnv *env, IRStmt *stmt)
       case Ity_I32:
       case Ity_I64:
          if (am->tag == S390_AMODE_B12 &&
-             s390_expr_is_const_zero(stmt->Ist.Put.data)) {
-            addInstr(env, s390_insn_mzero(sizeofIRType(tyd), am));
+             stmt->Ist.Put.data->tag == Iex_Const) {
+            ULong value =
+               get_const_value_as_ulong(stmt->Ist.Put.data->Iex.Const.con);
+            addInstr(env, s390_insn_mimm(sizeofIRType(tyd), am, value));
             return;
          }
          src = s390_isel_int_expr(env, stmt->Ist.Put.data);