qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/11] tcg updates
@ 2014-09-22 20:57 Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 01/11] tcg-sparc: Support addsub2_i64 Richard Henderson
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Three pending tcg patch sets: sparc backend updates, aarch64 backend
updates, and the tcg typechecking patch set from last week.

Please pull.


r~


The following changes since commit 07e2863d0271ac6c05206d8ce9e4f4c39b25d3ea:

  exec.c: fix setting 1-byte-long watchpoints (2014-09-19 17:42:16 +0100)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/tcg-next-20140722

for you to fetch changes up to fdacaf6ea0dea5719b30eb0906d03fa1f4c59c68:

  tcg: Always enable TCGv type checking (2014-09-22 12:31:13 -0700)

----------------------------------------------------------------
tcg changes for 2014-07-22

----------------------------------------------------------------
Richard Henderson (11):
      tcg-sparc: Support addsub2_i64
      tcg-sparc: Use ADDXC in addsub2_i64
      tcg-sparc: Fix setcond_i32 uninitialized value
      tcg-sparc: Use ADDXC in setcond_i64
      tcg-sparc: Rename ADDX/SUBX insns
      tcg-sparc: Use UMULXHI instruction
      tcg: Compress TCGLabelQemuLdst
      tcg: Move TCG_TYPE_COUNT out of enum TCGType
      tcg-aarch64: Use 32-bit loads for qemu_ld_i32
      qemu/compiler: Define QEMU_ARTIFICIAL
      tcg: Always enable TCGv type checking

 disas/sparc.c            |  34 +++++--------
 include/elf.h            |  37 +++++++++++---
 include/qemu/compiler.h  |   6 +++
 tcg/aarch64/tcg-target.c |  29 ++++++-----
 tcg/sparc/tcg-target.c   | 129 ++++++++++++++++++++++++++++++++++++++++++-----
 tcg/sparc/tcg-target.h   |  12 +++--
 tcg/tcg-be-ldst.h        |  20 +++++---
 tcg/tcg.h                |  92 +++++++++++++--------------------
 8 files changed, 238 insertions(+), 121 deletions(-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 01/11] tcg-sparc: Support addsub2_i64
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 02/11] tcg-sparc: Use ADDXC in addsub2_i64 Richard Henderson
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/sparc/tcg-target.c | 72 +++++++++++++++++++++++++++++++++++++++++++++-----
 tcg/sparc/tcg-target.h |  4 +--
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 40f2ec1..21981d8 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -719,9 +719,9 @@ static void tcg_out_setcond_i64(TCGContext *s, TCGCond cond, TCGReg ret,
     }
 }
 
-static void tcg_out_addsub2(TCGContext *s, TCGReg rl, TCGReg rh,
-                            TCGReg al, TCGReg ah, int32_t bl, int blconst,
-                            int32_t bh, int bhconst, int opl, int oph)
+static void tcg_out_addsub2_i32(TCGContext *s, TCGReg rl, TCGReg rh,
+                                TCGReg al, TCGReg ah, int32_t bl, int blconst,
+                                int32_t bh, int bhconst, int opl, int oph)
 {
     TCGReg tmp = TCG_REG_T1;
 
@@ -735,6 +735,51 @@ static void tcg_out_addsub2(TCGContext *s, TCGReg rl, TCGReg rh,
     tcg_out_mov(s, TCG_TYPE_I32, rl, tmp);
 }
 
+static void tcg_out_addsub2_i64(TCGContext *s, TCGReg rl, TCGReg rh,
+                                TCGReg al, TCGReg ah, int32_t bl, int blconst,
+                                int32_t bh, int bhconst, bool is_sub)
+{
+    TCGReg tmp = TCG_REG_T1;
+
+    /* Note that the low parts are fully consumed before tmp is set.  */
+    if (rl != ah && (bhconst || rl != bh)) {
+        tmp = rl;
+    }
+
+    tcg_out_arithc(s, tmp, al, bl, blconst, is_sub ? ARITH_SUBCC : ARITH_ADDCC);
+
+    /* Note that ADDX/SUBX take the carry-in from %icc, the 32-bit carry,
+       while we want %xcc, the 64-bit carry.  */
+    /* ??? There is a 2011 VIS3 ADDXC insn that does take a 64-bit carry.  */
+
+    if (bh == TCG_REG_G0) {
+	/* If we have a zero, we can perform the operation in two insns,
+           with the arithmetic first, and a conditional move into place.  */
+	if (rh == ah) {
+            tcg_out_arithi(s, TCG_REG_T2, ah, 1,
+			   is_sub ? ARITH_SUB : ARITH_ADD);
+            tcg_out_movcc(s, TCG_COND_LTU, MOVCC_XCC, rh, TCG_REG_T2, 0);
+	} else {
+            tcg_out_arithi(s, rh, ah, 1, is_sub ? ARITH_SUB : ARITH_ADD);
+	    tcg_out_movcc(s, TCG_COND_GEU, MOVCC_XCC, rh, ah, 0);
+	}
+    } else {
+        /* Otherwise adjust BH as if there is carry into T2 ... */
+        if (bhconst) {
+            tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_T2, bh + (is_sub ? -1 : 1));
+        } else {
+            tcg_out_arithi(s, TCG_REG_T2, bh, 1,
+                           is_sub ? ARITH_SUB : ARITH_ADD);
+        }
+        /* ... smoosh T2 back to original BH if carry is clear ... */
+        tcg_out_movcc(s, TCG_COND_GEU, MOVCC_XCC, TCG_REG_T2, bh, bhconst);
+	/* ... and finally perform the arithmetic with the new operand.  */
+        tcg_out_arith(s, rh, ah, TCG_REG_T2, is_sub ? ARITH_SUB : ARITH_ADD);
+    }
+
+    tcg_out_mov(s, TCG_TYPE_I64, rl, tmp);
+}
+
 static void tcg_out_call_nodelay(TCGContext *s, tcg_insn_unit *dest)
 {
     ptrdiff_t disp = tcg_pcrel_diff(s, dest);
@@ -1264,12 +1309,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
         break;
 
     case INDEX_op_add2_i32:
-        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], const_args[4],
-                        args[5], const_args[5], ARITH_ADDCC, ARITH_ADDX);
+        tcg_out_addsub2_i32(s, args[0], args[1], args[2], args[3],
+                            args[4], const_args[4], args[5], const_args[5],
+                            ARITH_ADDCC, ARITH_ADDX);
         break;
     case INDEX_op_sub2_i32:
-        tcg_out_addsub2(s, a0, a1, a2, args[3], args[4], const_args[4],
-                        args[5], const_args[5], ARITH_SUBCC, ARITH_SUBX);
+        tcg_out_addsub2_i32(s, args[0], args[1], args[2], args[3],
+                            args[4], const_args[4], args[5], const_args[5],
+                            ARITH_SUBCC, ARITH_SUBX);
         break;
     case INDEX_op_mulu2_i32:
         c = ARITH_UMUL;
@@ -1351,6 +1398,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
     case INDEX_op_movcond_i64:
         tcg_out_movcond_i64(s, args[5], a0, a1, a2, c2, args[3], const_args[3]);
         break;
+    case INDEX_op_add2_i64:
+        tcg_out_addsub2_i64(s, args[0], args[1], args[2], args[3], args[4],
+                            const_args[4], args[5], const_args[5], false);
+        break;
+    case INDEX_op_sub2_i64:
+        tcg_out_addsub2_i64(s, args[0], args[1], args[2], args[3], args[4],
+                            const_args[4], args[5], const_args[5], true);
+        break;
 
     gen_arith:
         tcg_out_arithc(s, a0, a1, a2, c2, c);
@@ -1449,6 +1504,9 @@ static const TCGTargetOpDef sparc_op_defs[] = {
     { INDEX_op_setcond_i64, { "R", "RZ", "RJ" } },
     { INDEX_op_movcond_i64, { "R", "RZ", "RJ", "RI", "0" } },
 
+    { INDEX_op_add2_i64, { "R", "R", "RZ", "RZ", "RJ", "RI" } },
+    { INDEX_op_sub2_i64, { "R", "R", "RZ", "RZ", "RJ", "RI" } },
+
     { INDEX_op_qemu_ld_i32, { "r", "A" } },
     { INDEX_op_qemu_ld_i64, { "R", "A" } },
     { INDEX_op_qemu_st_i32, { "sZ", "A" } },
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index 089f976..a44d34f 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -133,8 +133,8 @@ typedef enum {
 #define TCG_TARGET_HAS_nor_i64          0
 #define TCG_TARGET_HAS_deposit_i64      0
 #define TCG_TARGET_HAS_movcond_i64      1
-#define TCG_TARGET_HAS_add2_i64         0
-#define TCG_TARGET_HAS_sub2_i64         0
+#define TCG_TARGET_HAS_add2_i64         1
+#define TCG_TARGET_HAS_sub2_i64         1
 #define TCG_TARGET_HAS_mulu2_i64        0
 #define TCG_TARGET_HAS_muls2_i64        0
 #define TCG_TARGET_HAS_muluh_i64        0
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 02/11] tcg-sparc: Use ADDXC in addsub2_i64
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 01/11] tcg-sparc: Support addsub2_i64 Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 03/11] tcg-sparc: Fix setcond_i32 uninitialized value Richard Henderson
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

On T4 and newer Sparc chips we have an add-with-carry insn
that takes its input from %xcc instead of %icc.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 disas/sparc.c          |  3 +++
 include/elf.h          | 37 +++++++++++++++++++++++++++++--------
 tcg/sparc/tcg-target.c | 28 +++++++++++++++++++++++-----
 tcg/sparc/tcg-target.h |  6 ++++++
 4 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/disas/sparc.c b/disas/sparc.c
index 8eb22e6..092e1b6 100644
--- a/disas/sparc.c
+++ b/disas/sparc.c
@@ -2042,6 +2042,9 @@ IMPDEP ("impdep2", 0x37),
 
 #undef IMPDEP
 
+{ "addxc", F3F(2, 0x36, 0x011), F3F(~2, ~0x36, ~0x011), "1,2,d", 0, v9b },
+{ "addxccc", F3F(2, 0x36, 0x013), F3F(~2, ~0x36, ~0x013), "1,2,d", 0, v9b },
+
 };
 
 static const int sparc_num_opcodes = ((sizeof sparc_opcodes)/(sizeof sparc_opcodes[0]));
diff --git a/include/elf.h b/include/elf.h
index 70107f0..a516584 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -473,14 +473,35 @@ typedef struct {
 #define PPC_FEATURE_TRUE_LE             0x00000002
 #define PPC_FEATURE_PPC_LE              0x00000001
 
-/* Bits present in AT_HWCAP, primarily for Sparc32.  */
-
-#define HWCAP_SPARC_FLUSH       1    /* CPU supports flush instruction. */
-#define HWCAP_SPARC_STBAR       2
-#define HWCAP_SPARC_SWAP        4
-#define HWCAP_SPARC_MULDIV      8
-#define HWCAP_SPARC_V9		16
-#define HWCAP_SPARC_ULTRA3	32
+/* Bits present in AT_HWCAP for Sparc.  */
+
+#define HWCAP_SPARC_FLUSH               0x00000001
+#define HWCAP_SPARC_STBAR               0x00000002
+#define HWCAP_SPARC_SWAP                0x00000004
+#define HWCAP_SPARC_MULDIV              0x00000008
+#define HWCAP_SPARC_V9                  0x00000010
+#define HWCAP_SPARC_ULTRA3              0x00000020
+#define HWCAP_SPARC_BLKINIT             0x00000040
+#define HWCAP_SPARC_N2                  0x00000080
+#define HWCAP_SPARC_MUL32               0x00000100
+#define HWCAP_SPARC_DIV32               0x00000200
+#define HWCAP_SPARC_FSMULD              0x00000400
+#define HWCAP_SPARC_V8PLUS              0x00000800
+#define HWCAP_SPARC_POPC                0x00001000
+#define HWCAP_SPARC_VIS                 0x00002000
+#define HWCAP_SPARC_VIS2                0x00004000
+#define HWCAP_SPARC_ASI_BLK_INIT        0x00008000
+#define HWCAP_SPARC_FMAF                0x00010000
+#define HWCAP_SPARC_VIS3                0x00020000
+#define HWCAP_SPARC_HPC                 0x00040000
+#define HWCAP_SPARC_RANDOM              0x00080000
+#define HWCAP_SPARC_TRANS               0x00100000
+#define HWCAP_SPARC_FJFMAU              0x00200000
+#define HWCAP_SPARC_IMA                 0x00400000
+#define HWCAP_SPARC_ASI_CACHE_SPARING   0x00800000
+#define HWCAP_SPARC_PAUSE               0x01000000
+#define HWCAP_SPARC_CBCOND              0x02000000
+#define HWCAP_SPARC_CRYPTO              0x04000000
 
 /* Bits present in AT_HWCAP for s390.  */
 
diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 21981d8..08ca482 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -209,6 +209,8 @@ static const int tcg_target_call_oarg_regs[] = {
 #define ARITH_MOVCC (INSN_OP(2) | INSN_OP3(0x2c))
 #define ARITH_MOVR (INSN_OP(2) | INSN_OP3(0x2f))
 
+#define ARITH_ADDXC (INSN_OP(2) | INSN_OP3(0x36) | INSN_OPF(0x11))
+
 #define SHIFT_SLL  (INSN_OP(2) | INSN_OP3(0x25))
 #define SHIFT_SRL  (INSN_OP(2) | INSN_OP3(0x26))
 #define SHIFT_SRA  (INSN_OP(2) | INSN_OP3(0x27))
@@ -262,6 +264,10 @@ static const int tcg_target_call_oarg_regs[] = {
 #define STW_LE     (STWA  | INSN_ASI(ASI_PRIMARY_LITTLE))
 #define STX_LE     (STXA  | INSN_ASI(ASI_PRIMARY_LITTLE))
 
+#ifndef use_vis3_instructions
+bool use_vis3_instructions;
+#endif
+
 static inline int check_fit_i64(int64_t val, unsigned int bits)
 {
     return val == sextract64(val, 0, bits);
@@ -748,11 +754,14 @@ static void tcg_out_addsub2_i64(TCGContext *s, TCGReg rl, TCGReg rh,
 
     tcg_out_arithc(s, tmp, al, bl, blconst, is_sub ? ARITH_SUBCC : ARITH_ADDCC);
 
-    /* Note that ADDX/SUBX take the carry-in from %icc, the 32-bit carry,
-       while we want %xcc, the 64-bit carry.  */
-    /* ??? There is a 2011 VIS3 ADDXC insn that does take a 64-bit carry.  */
-
-    if (bh == TCG_REG_G0) {
+    if (use_vis3_instructions && !is_sub) {
+        /* Note that ADDXC doesn't accept immediates.  */
+        if (bhconst && bh != 0) {
+           tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_T2, bh);
+           bh = TCG_REG_T2;
+        }
+        tcg_out_arith(s, rh, ah, bh, ARITH_ADDXC);
+    } else if (bh == TCG_REG_G0) {
 	/* If we have a zero, we can perform the operation in two insns,
            with the arithmetic first, and a conditional move into place.  */
 	if (rh == ah) {
@@ -1517,6 +1526,15 @@ static const TCGTargetOpDef sparc_op_defs[] = {
 
 static void tcg_target_init(TCGContext *s)
 {
+    /* Only probe for the platform and capabilities if we havn't already
+       determined maximum values at compile time.  */
+#ifndef use_vis3_instructions
+    {
+        unsigned long hwcap = qemu_getauxval(AT_HWCAP);
+        use_vis3_instructions = (hwcap & HWCAP_SPARC_VIS3) != 0;
+    }
+#endif
+
     tcg_regset_set32(tcg_target_available_regs[TCG_TYPE_I32], 0, 0xffffffff);
     tcg_regset_set32(tcg_target_available_regs[TCG_TYPE_I64], 0, ALL_64);
 
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index a44d34f..099b308 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -85,6 +85,12 @@ typedef enum {
 #define TCG_TARGET_EXTEND_ARGS 1
 #endif
 
+#if defined(__VIS__) && __VIS__ >= 0x300
+#define use_vis3_instructions  1
+#else
+extern bool use_vis3_instructions;
+#endif
+
 /* optional instructions */
 #define TCG_TARGET_HAS_div_i32		1
 #define TCG_TARGET_HAS_rem_i32		0
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 03/11] tcg-sparc: Fix setcond_i32 uninitialized value
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 01/11] tcg-sparc: Support addsub2_i64 Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 02/11] tcg-sparc: Use ADDXC in addsub2_i64 Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 04/11] tcg-sparc: Use ADDXC in setcond_i64 Richard Henderson
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

We failed to swap c1 and c2 correctly for NE c2 == 0.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/sparc/tcg-target.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 08ca482..3b232d6 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -674,9 +674,12 @@ static void tcg_out_setcond_i32(TCGContext *s, TCGCond cond, TCGReg ret,
     case TCG_COND_NE:
         /* For equality, we can transform to inequality vs zero.  */
         if (c2 != 0) {
-            tcg_out_arithc(s, ret, c1, c2, c2const, ARITH_XOR);
+            tcg_out_arithc(s, TCG_REG_T1, c1, c2, c2const, ARITH_XOR);
+            c2 = TCG_REG_T1;
+        } else {
+            c2 = c1;
         }
-        c1 = TCG_REG_G0, c2 = ret, c2const = 0;
+        c1 = TCG_REG_G0, c2const = 0;
         cond = (cond == TCG_COND_EQ ? TCG_COND_GEU : TCG_COND_LTU);
 	break;
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 04/11] tcg-sparc: Use ADDXC in setcond_i64
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (2 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 03/11] tcg-sparc: Fix setcond_i32 uninitialized value Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 05/11] tcg-sparc: Rename ADDX/SUBX insns Richard Henderson
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Similar to the ADDC tricks we use in setcond_i32.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/sparc/tcg-target.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 3b232d6..d0bd08c 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -716,6 +716,23 @@ static void tcg_out_setcond_i32(TCGContext *s, TCGCond cond, TCGReg ret,
 static void tcg_out_setcond_i64(TCGContext *s, TCGCond cond, TCGReg ret,
                                 TCGReg c1, int32_t c2, int c2const)
 {
+    if (use_vis3_instructions) {
+        switch (cond) {
+        case TCG_COND_NE:
+            if (c2 != 0) {
+                break;
+            }
+            c2 = c1, c2const = 0, c1 = TCG_REG_G0;
+            /* FALLTHRU */
+        case TCG_COND_LTU:
+            tcg_out_cmp(s, c1, c2, c2const);
+            tcg_out_arith(s, ret, TCG_REG_G0, TCG_REG_G0, ARITH_ADDXC);
+            return;
+        default:
+            break;
+        }
+    }
+
     /* For 64-bit signed comparisons vs zero, we can avoid the compare
        if the input does not overlap the output.  */
     if (c2 == 0 && !is_unsigned_cond(cond) && c1 != ret) {
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 05/11] tcg-sparc: Rename ADDX/SUBX insns
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (3 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 04/11] tcg-sparc: Use ADDXC in setcond_i64 Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 06/11] tcg-sparc: Use UMULXHI instruction Richard Henderson
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The pre-v9 ADDX/SUBX insns were renamed ADDC/SUBC for v9.
Standardizing on the v9 name makes things less confusing.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 disas/sparc.c          | 32 +++++++++++---------------------
 tcg/sparc/tcg-target.c | 14 +++++++-------
 2 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/disas/sparc.c b/disas/sparc.c
index 092e1b6..22ceac3 100644
--- a/disas/sparc.c
+++ b/disas/sparc.c
@@ -1175,15 +1175,11 @@ static const struct sparc_opcode sparc_opcodes[] = {
 { "subcc",      F3(2, 0x14, 0), F3(~2, ~0x14, ~0)|ASI(~0),      "1,2,d", 0, v6 },
 { "subcc",      F3(2, 0x14, 1), F3(~2, ~0x14, ~1),              "1,i,d", 0, v6 },
 
-{ "subx",       F3(2, 0x0c, 0), F3(~2, ~0x0c, ~0)|ASI(~0),      "1,2,d", 0, v6notv9 },
-{ "subx",       F3(2, 0x0c, 1), F3(~2, ~0x0c, ~1),              "1,i,d", 0, v6notv9 },
-{ "subc",       F3(2, 0x0c, 0), F3(~2, ~0x0c, ~0)|ASI(~0),      "1,2,d", 0, v9 },
-{ "subc",       F3(2, 0x0c, 1), F3(~2, ~0x0c, ~1),              "1,i,d", 0, v9 },
+{ "subc",       F3(2, 0x0c, 0), F3(~2, ~0x0c, ~0)|ASI(~0),      "1,2,d", 0, v6 },
+{ "subc",       F3(2, 0x0c, 1), F3(~2, ~0x0c, ~1),              "1,i,d", 0, v6 },
 
-{ "subxcc",     F3(2, 0x1c, 0), F3(~2, ~0x1c, ~0)|ASI(~0),      "1,2,d", 0, v6notv9 },
-{ "subxcc",     F3(2, 0x1c, 1), F3(~2, ~0x1c, ~1),              "1,i,d", 0, v6notv9 },
-{ "subccc",     F3(2, 0x1c, 0), F3(~2, ~0x1c, ~0)|ASI(~0),      "1,2,d", 0, v9 },
-{ "subccc",     F3(2, 0x1c, 1), F3(~2, ~0x1c, ~1),              "1,i,d", 0, v9 },
+{ "subccc",     F3(2, 0x1c, 0), F3(~2, ~0x1c, ~0)|ASI(~0),      "1,2,d", 0, v6 },
+{ "subccc",     F3(2, 0x1c, 1), F3(~2, ~0x1c, ~1),              "1,i,d", 0, v6 },
 
 { "and",        F3(2, 0x01, 0), F3(~2, ~0x01, ~0)|ASI(~0),      "1,2,d", 0, v6 },
 { "and",        F3(2, 0x01, 1), F3(~2, ~0x01, ~1),              "1,i,d", 0, v6 },
@@ -1215,19 +1211,13 @@ static const struct sparc_opcode sparc_opcodes[] = {
 { "addcc",      F3(2, 0x10, 1), F3(~2, ~0x10, ~1),              "1,i,d", 0, v6 },
 { "addcc",      F3(2, 0x10, 1), F3(~2, ~0x10, ~1),              "i,1,d", 0, v6 },
 
-{ "addx",       F3(2, 0x08, 0), F3(~2, ~0x08, ~0)|ASI(~0),      "1,2,d", 0, v6notv9 },
-{ "addx",       F3(2, 0x08, 1), F3(~2, ~0x08, ~1),              "1,i,d", 0, v6notv9 },
-{ "addx",       F3(2, 0x08, 1), F3(~2, ~0x08, ~1),              "i,1,d", 0, v6notv9 },
-{ "addc",       F3(2, 0x08, 0), F3(~2, ~0x08, ~0)|ASI(~0),      "1,2,d", 0, v9 },
-{ "addc",       F3(2, 0x08, 1), F3(~2, ~0x08, ~1),              "1,i,d", 0, v9 },
-{ "addc",       F3(2, 0x08, 1), F3(~2, ~0x08, ~1),              "i,1,d", 0, v9 },
-
-{ "addxcc",     F3(2, 0x18, 0), F3(~2, ~0x18, ~0)|ASI(~0),      "1,2,d", 0, v6notv9 },
-{ "addxcc",     F3(2, 0x18, 1), F3(~2, ~0x18, ~1),              "1,i,d", 0, v6notv9 },
-{ "addxcc",     F3(2, 0x18, 1), F3(~2, ~0x18, ~1),              "i,1,d", 0, v6notv9 },
-{ "addccc",     F3(2, 0x18, 0), F3(~2, ~0x18, ~0)|ASI(~0),      "1,2,d", 0, v9 },
-{ "addccc",     F3(2, 0x18, 1), F3(~2, ~0x18, ~1),              "1,i,d", 0, v9 },
-{ "addccc",     F3(2, 0x18, 1), F3(~2, ~0x18, ~1),              "i,1,d", 0, v9 },
+{ "addc",       F3(2, 0x08, 0), F3(~2, ~0x08, ~0)|ASI(~0),      "1,2,d", 0, v6 },
+{ "addc",       F3(2, 0x08, 1), F3(~2, ~0x08, ~1),              "1,i,d", 0, v6 },
+{ "addc",       F3(2, 0x08, 1), F3(~2, ~0x08, ~1),              "i,1,d", 0, v6 },
+
+{ "addccc",     F3(2, 0x18, 0), F3(~2, ~0x18, ~0)|ASI(~0),      "1,2,d", 0, v6 },
+{ "addccc",     F3(2, 0x18, 1), F3(~2, ~0x18, ~1),              "1,i,d", 0, v6 },
+{ "addccc",     F3(2, 0x18, 1), F3(~2, ~0x18, ~1),              "i,1,d", 0, v6 },
 
 { "smul",       F3(2, 0x0b, 0), F3(~2, ~0x0b, ~0)|ASI(~0),      "1,2,d", 0, v8 },
 { "smul",       F3(2, 0x0b, 1), F3(~2, ~0x0b, ~1),              "1,i,d", 0, v8 },
diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index d0bd08c..0a8c26a 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -197,8 +197,8 @@ static const int tcg_target_call_oarg_regs[] = {
 #define ARITH_XOR  (INSN_OP(2) | INSN_OP3(0x03))
 #define ARITH_SUB  (INSN_OP(2) | INSN_OP3(0x04))
 #define ARITH_SUBCC (INSN_OP(2) | INSN_OP3(0x14))
-#define ARITH_ADDX (INSN_OP(2) | INSN_OP3(0x08))
-#define ARITH_SUBX (INSN_OP(2) | INSN_OP3(0x0c))
+#define ARITH_ADDC (INSN_OP(2) | INSN_OP3(0x08))
+#define ARITH_SUBC (INSN_OP(2) | INSN_OP3(0x0c))
 #define ARITH_UMUL (INSN_OP(2) | INSN_OP3(0x0a))
 #define ARITH_SMUL (INSN_OP(2) | INSN_OP3(0x0b))
 #define ARITH_UDIV (INSN_OP(2) | INSN_OP3(0x0e))
@@ -663,7 +663,7 @@ static void tcg_out_movcond_i64(TCGContext *s, TCGCond cond, TCGReg ret,
 static void tcg_out_setcond_i32(TCGContext *s, TCGCond cond, TCGReg ret,
                                 TCGReg c1, int32_t c2, int c2const)
 {
-    /* For 32-bit comparisons, we can play games with ADDX/SUBX.  */
+    /* For 32-bit comparisons, we can play games with ADDC/SUBC.  */
     switch (cond) {
     case TCG_COND_LTU:
     case TCG_COND_GEU:
@@ -707,9 +707,9 @@ static void tcg_out_setcond_i32(TCGContext *s, TCGCond cond, TCGReg ret,
 
     tcg_out_cmp(s, c1, c2, c2const);
     if (cond == TCG_COND_LTU) {
-        tcg_out_arithi(s, ret, TCG_REG_G0, 0, ARITH_ADDX);
+        tcg_out_arithi(s, ret, TCG_REG_G0, 0, ARITH_ADDC);
     } else {
-        tcg_out_arithi(s, ret, TCG_REG_G0, -1, ARITH_SUBX);
+        tcg_out_arithi(s, ret, TCG_REG_G0, -1, ARITH_SUBC);
     }
 }
 
@@ -1340,12 +1340,12 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
     case INDEX_op_add2_i32:
         tcg_out_addsub2_i32(s, args[0], args[1], args[2], args[3],
                             args[4], const_args[4], args[5], const_args[5],
-                            ARITH_ADDCC, ARITH_ADDX);
+                            ARITH_ADDCC, ARITH_ADDC);
         break;
     case INDEX_op_sub2_i32:
         tcg_out_addsub2_i32(s, args[0], args[1], args[2], args[3],
                             args[4], const_args[4], args[5], const_args[5],
-                            ARITH_SUBCC, ARITH_SUBX);
+                            ARITH_SUBCC, ARITH_SUBC);
         break;
     case INDEX_op_mulu2_i32:
         c = ARITH_UMUL;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 06/11] tcg-sparc: Use UMULXHI instruction
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (4 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 05/11] tcg-sparc: Rename ADDX/SUBX insns Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst Richard Henderson
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 disas/sparc.c          | 1 +
 tcg/sparc/tcg-target.c | 5 +++++
 tcg/sparc/tcg-target.h | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/disas/sparc.c b/disas/sparc.c
index 22ceac3..8e755d1 100644
--- a/disas/sparc.c
+++ b/disas/sparc.c
@@ -2034,6 +2034,7 @@ IMPDEP ("impdep2", 0x37),
 
 { "addxc", F3F(2, 0x36, 0x011), F3F(~2, ~0x36, ~0x011), "1,2,d", 0, v9b },
 { "addxccc", F3F(2, 0x36, 0x013), F3F(~2, ~0x36, ~0x013), "1,2,d", 0, v9b },
+{ "umulxhi", F3F(2, 0x36, 0x016), F3F(~2, ~0x36, ~0x016), "1,2,d", 0, v9b },
 
 };
 
diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 0a8c26a..0c4b028 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -210,6 +210,7 @@ static const int tcg_target_call_oarg_regs[] = {
 #define ARITH_MOVR (INSN_OP(2) | INSN_OP3(0x2f))
 
 #define ARITH_ADDXC (INSN_OP(2) | INSN_OP3(0x36) | INSN_OPF(0x11))
+#define ARITH_UMULXHI (INSN_OP(2) | INSN_OP3(0x36) | INSN_OPF(0x16))
 
 #define SHIFT_SLL  (INSN_OP(2) | INSN_OP3(0x25))
 #define SHIFT_SRL  (INSN_OP(2) | INSN_OP3(0x26))
@@ -1435,6 +1436,9 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
         tcg_out_addsub2_i64(s, args[0], args[1], args[2], args[3], args[4],
                             const_args[4], args[5], const_args[5], true);
         break;
+    case INDEX_op_muluh_i64:
+        tcg_out_arith(s, args[0], args[1], args[2], ARITH_UMULXHI);
+        break;
 
     gen_arith:
         tcg_out_arithc(s, a0, a1, a2, c2, c);
@@ -1535,6 +1539,7 @@ static const TCGTargetOpDef sparc_op_defs[] = {
 
     { INDEX_op_add2_i64, { "R", "R", "RZ", "RZ", "RJ", "RI" } },
     { INDEX_op_sub2_i64, { "R", "R", "RZ", "RZ", "RJ", "RI" } },
+    { INDEX_op_muluh_i64, { "R", "RZ", "RZ" } },
 
     { INDEX_op_qemu_ld_i32, { "r", "A" } },
     { INDEX_op_qemu_ld_i64, { "R", "A" } },
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index 099b308..0c4c8af 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -143,7 +143,7 @@ extern bool use_vis3_instructions;
 #define TCG_TARGET_HAS_sub2_i64         1
 #define TCG_TARGET_HAS_mulu2_i64        0
 #define TCG_TARGET_HAS_muls2_i64        0
-#define TCG_TARGET_HAS_muluh_i64        0
+#define TCG_TARGET_HAS_muluh_i64        use_vis3_instructions
 #define TCG_TARGET_HAS_mulsh_i64        0
 
 #define TCG_AREG0 TCG_REG_I0
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (5 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 06/11] tcg-sparc: Use UMULXHI instruction Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 22:19   ` Paolo Bonzini
  2014-09-22 20:57 ` [Qemu-devel] [PULL 08/11] tcg: Move TCG_TYPE_COUNT out of enum TCGType Richard Henderson
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Use 1 32-bit word instead of 6.

Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg-be-ldst.h | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tcg/tcg-be-ldst.h b/tcg/tcg-be-ldst.h
index 49b3de6..904eeda 100644
--- a/tcg/tcg-be-ldst.h
+++ b/tcg/tcg-be-ldst.h
@@ -23,14 +23,19 @@
 #ifdef CONFIG_SOFTMMU
 #define TCG_MAX_QEMU_LDST       640
 
+QEMU_BUILD_BUG_ON(TCG_TARGET_NB_REGS > 32);
+QEMU_BUILD_BUG_ON(NB_MMU_MODES > 8);
+
 typedef struct TCGLabelQemuLdst {
-    bool is_ld:1;           /* qemu_ld: true, qemu_st: false */
-    TCGMemOp opc:4;
-    TCGReg addrlo_reg;      /* reg index for low word of guest virtual addr */
-    TCGReg addrhi_reg;      /* reg index for high word of guest virtual addr */
-    TCGReg datalo_reg;      /* reg index for low word to be loaded or stored */
-    TCGReg datahi_reg;      /* reg index for high word to be loaded or stored */
-    int mem_index;          /* soft MMU memory index */
+    TCGMemOp opc : 4;
+    bool is_ld : 1;         /* qemu_ld: true, qemu_st: false */
+    TCGReg addrlo_reg : 5;  /* reg index for low word of guest virtual addr */
+    TCGReg addrhi_reg : 5;  /* reg index for high word of guest virtual addr */
+    TCGReg datalo_reg : 5;  /* reg index for low word to be loaded or stored */
+    TCGReg datahi_reg : 5;  /* reg index for high word to be loaded or stored */
+    unsigned mem_index : 3; /* soft MMU memory index */
+    /* 4 bits unused in 32-bit word */
+
     tcg_insn_unit *raddr;   /* gen code addr of the next IR of qemu_ld/st IR */
     tcg_insn_unit *label_ptr[2]; /* label pointers to be updated */
 } TCGLabelQemuLdst;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 08/11] tcg: Move TCG_TYPE_COUNT out of enum TCGType
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (6 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32 Richard Henderson
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Having the count inside the enumeration makes gcc believe that we
want to be able to store values 0-2, and thus a 1-bit bitfield cannot
hold the enumeration.  With the count as a define external to the enum,
gcc correctly sees that we only care about values 0-1.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tcg/tcg.h b/tcg/tcg.h
index 997a704..fcc50bd 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -194,7 +194,6 @@ typedef struct TCGPool {
 typedef enum TCGType {
     TCG_TYPE_I32,
     TCG_TYPE_I64,
-    TCG_TYPE_COUNT, /* number of different types */
 
     /* An alias for the size of the host register.  */
 #if TCG_TARGET_REG_BITS == 32
@@ -218,6 +217,8 @@ typedef enum TCGType {
 #endif
 } TCGType;
 
+#define TCG_TYPE_COUNT 2
+
 /* Constants for qemu_ld and qemu_st for the Memory Operation field.  */
 typedef enum TCGMemOp {
     MO_8     = 0,
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (7 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 08/11] tcg: Move TCG_TYPE_COUNT out of enum TCGType Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-24  8:20   ` Claudio Fontana
  2014-09-22 20:57 ` [Qemu-devel] [PULL 10/11] qemu/compiler: Define QEMU_ARTIFICIAL Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 11/11] tcg: Always enable TCGv type checking Richard Henderson
  10 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The "old" qemu_ld opcode did not specify the size of the result,
and so we had to assume full register width.  With the new opcodes,
we can narrow the result.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/aarch64/tcg-target.c | 29 ++++++++++++++++-------------
 tcg/tcg-be-ldst.h        |  1 +
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
index 56dae66..5017cfe 100644
--- a/tcg/aarch64/tcg-target.c
+++ b/tcg/aarch64/tcg-target.c
@@ -1007,7 +1007,7 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *lb)
     tcg_out_adr(s, TCG_REG_X3, lb->raddr);
     tcg_out_call(s, qemu_ld_helpers[opc & ~MO_SIGN]);
     if (opc & MO_SIGN) {
-        tcg_out_sxt(s, TCG_TYPE_I64, size, lb->datalo_reg, TCG_REG_X0);
+        tcg_out_sxt(s, lb->type, size, lb->datalo_reg, TCG_REG_X0);
     } else {
         tcg_out_mov(s, size == MO_64, lb->datalo_reg, TCG_REG_X0);
     }
@@ -1032,14 +1032,15 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *lb)
 }
 
 static void add_qemu_ldst_label(TCGContext *s, bool is_ld, TCGMemOp opc,
-                                TCGReg data_reg, TCGReg addr_reg,
+                                TCGType type, TCGReg data_reg, TCGReg addr_reg,
                                 int mem_index, tcg_insn_unit *raddr,
                                 tcg_insn_unit *label_ptr)
 {
     TCGLabelQemuLdst *label = new_ldst_label(s);
 
-    label->is_ld = is_ld;
     label->opc = opc;
+    label->is_ld = is_ld;
+    label->type = type;
     label->datalo_reg = data_reg;
     label->addrlo_reg = addr_reg;
     label->mem_index = mem_index;
@@ -1108,7 +1109,7 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp s_bits,
 
 #endif /* CONFIG_SOFTMMU */
 
-static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
+static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop, TCGType type,
                                    TCGReg data_r, TCGReg addr_r, TCGReg off_r)
 {
     const TCGMemOp bswap = memop & MO_BSWAP;
@@ -1118,7 +1119,8 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
         tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, off_r);
         break;
     case MO_SB:
-        tcg_out_ldst_r(s, I3312_LDRSBX, data_r, addr_r, off_r);
+        tcg_out_ldst_r(s, type ? I3312_LDRSBX : I3312_LDRSBW,
+                       data_r, addr_r, off_r);
         break;
     case MO_UW:
         tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, off_r);
@@ -1130,9 +1132,10 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
         if (bswap) {
             tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, off_r);
             tcg_out_rev16(s, data_r, data_r);
-            tcg_out_sxt(s, TCG_TYPE_I64, MO_16, data_r, data_r);
+            tcg_out_sxt(s, type, MO_16, data_r, data_r);
         } else {
-            tcg_out_ldst_r(s, I3312_LDRSHX, data_r, addr_r, off_r);
+            tcg_out_ldst_r(s, type ? I3312_LDRSHX : I3312_LDRSHW,
+                           data_r, addr_r, off_r);
         }
         break;
     case MO_UL:
@@ -1197,18 +1200,18 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGMemOp memop,
 }
 
 static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
-                            TCGMemOp memop, int mem_index)
+                            TCGMemOp memop, TCGType type, int mem_index)
 {
 #ifdef CONFIG_SOFTMMU
     TCGMemOp s_bits = memop & MO_SIZE;
     tcg_insn_unit *label_ptr;
 
     tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 1);
-    tcg_out_qemu_ld_direct(s, memop, data_reg, addr_reg, TCG_REG_X1);
-    add_qemu_ldst_label(s, true, memop, data_reg, addr_reg,
+    tcg_out_qemu_ld_direct(s, memop, type, data_reg, addr_reg, TCG_REG_X1);
+    add_qemu_ldst_label(s, true, memop, type, data_reg, addr_reg,
                         mem_index, s->code_ptr, label_ptr);
 #else /* !CONFIG_SOFTMMU */
-    tcg_out_qemu_ld_direct(s, memop, data_reg, addr_reg,
+    tcg_out_qemu_ld_direct(s, memop, type, data_reg, addr_reg,
                            GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR);
 #endif /* CONFIG_SOFTMMU */
 }
@@ -1222,7 +1225,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
 
     tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 0);
     tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg, TCG_REG_X1);
-    add_qemu_ldst_label(s, false, memop, data_reg, addr_reg,
+    add_qemu_ldst_label(s, false, memop, s_bits == MO_64, data_reg, addr_reg,
                         mem_index, s->code_ptr, label_ptr);
 #else /* !CONFIG_SOFTMMU */
     tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg,
@@ -1515,7 +1518,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
 
     case INDEX_op_qemu_ld_i32:
     case INDEX_op_qemu_ld_i64:
-        tcg_out_qemu_ld(s, a0, a1, a2, args[3]);
+        tcg_out_qemu_ld(s, a0, a1, a2, ext, args[3]);
         break;
     case INDEX_op_qemu_st_i32:
     case INDEX_op_qemu_st_i64:
diff --git a/tcg/tcg-be-ldst.h b/tcg/tcg-be-ldst.h
index 904eeda..825de14 100644
--- a/tcg/tcg-be-ldst.h
+++ b/tcg/tcg-be-ldst.h
@@ -29,6 +29,7 @@ QEMU_BUILD_BUG_ON(NB_MMU_MODES > 8);
 typedef struct TCGLabelQemuLdst {
     TCGMemOp opc : 4;
     bool is_ld : 1;         /* qemu_ld: true, qemu_st: false */
+    TCGType type : 1;       /* result type of a load */
     TCGReg addrlo_reg : 5;  /* reg index for low word of guest virtual addr */
     TCGReg addrhi_reg : 5;  /* reg index for high word of guest virtual addr */
     TCGReg datalo_reg : 5;  /* reg index for low word to be loaded or stored */
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 10/11] qemu/compiler: Define QEMU_ARTIFICIAL
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (8 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32 Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  2014-09-22 20:57 ` [Qemu-devel] [PULL 11/11] tcg: Always enable TCGv type checking Richard Henderson
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The combination of always_inline + artificial allows tiny inline
functions to be written that do not interfere with debugging.
In particular, gdb will not step into an artificial function.

The always_inline attribute was introduced in gcc 4.2,
and the artificial attribute was introduced in gcc 4.3.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 include/qemu/compiler.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 155b358..ac7c4c4 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -24,6 +24,12 @@
 #define QEMU_WARN_UNUSED_RESULT
 #endif
 
+#if QEMU_GNUC_PREREQ(4, 3)
+#define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
+#else
+#define QEMU_ARTIFICIAL
+#endif
+
 #if defined(_WIN32)
 # define QEMU_PACKED __attribute__((gcc_struct, packed))
 #else
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PULL 11/11] tcg: Always enable TCGv type checking
  2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
                   ` (9 preceding siblings ...)
  2014-09-22 20:57 ` [Qemu-devel] [PULL 10/11] qemu/compiler: Define QEMU_ARTIFICIAL Richard Henderson
@ 2014-09-22 20:57 ` Richard Henderson
  10 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-22 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

Instead of using structures, which imply some amount of overhead
on certain ABIs, use pointer types.

This actually reduces the size of the binaries vs a NON-debug
build on ppc64 and x86_64, due to a reduction in the number of
sign-extension insns.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.h | 89 ++++++++++++++++++++++++---------------------------------------
 1 file changed, 34 insertions(+), 55 deletions(-)

diff --git a/tcg/tcg.h b/tcg/tcg.h
index fcc50bd..a2fe1a9 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -275,75 +275,54 @@ typedef enum TCGMemOp {
 
 typedef tcg_target_ulong TCGArg;
 
-/* Define a type and accessor macros for variables.  Using a struct is
-   nice because it gives some level of type safely.  Ideally the compiler
-   be able to see through all this.  However in practice this is not true,
-   especially on targets with braindamaged ABIs (e.g. i386).
-   We use plain int by default to avoid this runtime overhead.
-   Users of tcg_gen_* don't need to know about any of this, and should
-   treat TCGv as an opaque type.
+/* Define a type and accessor macros for variables.  Using pointer types
+   is nice because it gives some level of type safety.  Converting to and
+   from intptr_t rather than int reduces the number of sign-extension
+   instructions that get implied on 64-bit hosts.  Users of tcg_gen_* don't
+   need to know about any of this, and should treat TCGv as an opaque type.
    In addition we do typechecking for different types of variables.  TCGv_i32
    and TCGv_i64 are 32/64-bit variables respectively.  TCGv and TCGv_ptr
-   are aliases for target_ulong and host pointer sized values respectively.
- */
+   are aliases for target_ulong and host pointer sized values respectively.  */
 
-#ifdef CONFIG_DEBUG_TCG
-#define DEBUG_TCGV 1
-#endif
+typedef struct TCGv_i32_d *TCGv_i32;
+typedef struct TCGv_i64_d *TCGv_i64;
+typedef struct TCGv_ptr_d *TCGv_ptr;
 
-#ifdef DEBUG_TCGV
+static inline TCGv_i32 QEMU_ARTIFICIAL MAKE_TCGV_I32(intptr_t i)
+{
+    return (TCGv_i32)i;
+}
 
-typedef struct
+static inline TCGv_i64 QEMU_ARTIFICIAL MAKE_TCGV_I64(intptr_t i)
 {
-    int i32;
-} TCGv_i32;
+    return (TCGv_i64)i;
+}
 
-typedef struct
+static inline TCGv_ptr QEMU_ARTIFICIAL MAKE_TCGV_PTR(intptr_t i)
 {
-    int i64;
-} TCGv_i64;
-
-typedef struct {
-    int iptr;
-} TCGv_ptr;
-
-#define MAKE_TCGV_I32(i) __extension__                  \
-    ({ TCGv_i32 make_tcgv_tmp = {i}; make_tcgv_tmp;})
-#define MAKE_TCGV_I64(i) __extension__                  \
-    ({ TCGv_i64 make_tcgv_tmp = {i}; make_tcgv_tmp;})
-#define MAKE_TCGV_PTR(i) __extension__                  \
-    ({ TCGv_ptr make_tcgv_tmp = {i}; make_tcgv_tmp; })
-#define GET_TCGV_I32(t) ((t).i32)
-#define GET_TCGV_I64(t) ((t).i64)
-#define GET_TCGV_PTR(t) ((t).iptr)
-#if TCG_TARGET_REG_BITS == 32
-#define TCGV_LOW(t) MAKE_TCGV_I32(GET_TCGV_I64(t))
-#define TCGV_HIGH(t) MAKE_TCGV_I32(GET_TCGV_I64(t) + 1)
-#endif
+    return (TCGv_ptr)i;
+}
+
+static inline intptr_t QEMU_ARTIFICIAL GET_TCGV_I32(TCGv_i32 t)
+{
+    return (intptr_t)t;
+}
 
-#else /* !DEBUG_TCGV */
+static inline intptr_t QEMU_ARTIFICIAL GET_TCGV_I64(TCGv_i64 t)
+{
+    return (intptr_t)t;
+}
 
-typedef int TCGv_i32;
-typedef int TCGv_i64;
-#if TCG_TARGET_REG_BITS == 32
-#define TCGv_ptr TCGv_i32
-#else
-#define TCGv_ptr TCGv_i64
-#endif
-#define MAKE_TCGV_I32(x) (x)
-#define MAKE_TCGV_I64(x) (x)
-#define MAKE_TCGV_PTR(x) (x)
-#define GET_TCGV_I32(t) (t)
-#define GET_TCGV_I64(t) (t)
-#define GET_TCGV_PTR(t) (t)
+static inline intptr_t QEMU_ARTIFICIAL GET_TCGV_PTR(TCGv_ptr t)
+{
+    return (intptr_t)t;
+}
 
 #if TCG_TARGET_REG_BITS == 32
-#define TCGV_LOW(t) (t)
-#define TCGV_HIGH(t) ((t) + 1)
+#define TCGV_LOW(t) MAKE_TCGV_I32(GET_TCGV_I64(t))
+#define TCGV_HIGH(t) MAKE_TCGV_I32(GET_TCGV_I64(t) + 1)
 #endif
 
-#endif /* DEBUG_TCGV */
-
 #define TCGV_EQUAL_I32(a, b) (GET_TCGV_I32(a) == GET_TCGV_I32(b))
 #define TCGV_EQUAL_I64(a, b) (GET_TCGV_I64(a) == GET_TCGV_I64(b))
 #define TCGV_EQUAL_PTR(a, b) (GET_TCGV_PTR(a) == GET_TCGV_PTR(b))
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst
  2014-09-22 20:57 ` [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst Richard Henderson
@ 2014-09-22 22:19   ` Paolo Bonzini
  2014-09-23 17:48     ` Peter Maydell
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2014-09-22 22:19 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: peter.maydell

Il 22/09/2014 22:57, Richard Henderson ha scritto:
> +QEMU_BUILD_BUG_ON(TCG_TARGET_NB_REGS > 32);
> +QEMU_BUILD_BUG_ON(NB_MMU_MODES > 8);
> +
>  typedef struct TCGLabelQemuLdst {
> -    bool is_ld:1;           /* qemu_ld: true, qemu_st: false */
> -    TCGMemOp opc:4;
> -    TCGReg addrlo_reg;      /* reg index for low word of guest virtual addr */
> -    TCGReg addrhi_reg;      /* reg index for high word of guest virtual addr */
> -    TCGReg datalo_reg;      /* reg index for low word to be loaded or stored */
> -    TCGReg datahi_reg;      /* reg index for high word to be loaded or stored */
> -    int mem_index;          /* soft MMU memory index */
> +    TCGMemOp opc : 4;
> +    bool is_ld : 1;         /* qemu_ld: true, qemu_st: false */
> +    TCGReg addrlo_reg : 5;  /* reg index for low word of guest virtual addr */
> +    TCGReg addrhi_reg : 5;  /* reg index for high word of guest virtual addr */
> +    TCGReg datalo_reg : 5;  /* reg index for low word to be loaded or stored */
> +    TCGReg datahi_reg : 5;  /* reg index for high word to be loaded or stored */
> +    unsigned mem_index : 3; /* soft MMU memory index */
> +    /* 4 bits unused in 32-bit word */

Why?  Are there more than 10 or so loads in the typical tb?

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst
  2014-09-22 22:19   ` Paolo Bonzini
@ 2014-09-23 17:48     ` Peter Maydell
  2014-09-23 18:42       ` Paolo Bonzini
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2014-09-23 17:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers, Richard Henderson

On 22 September 2014 23:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 22/09/2014 22:57, Richard Henderson ha scritto:
>> +QEMU_BUILD_BUG_ON(TCG_TARGET_NB_REGS > 32);
>> +QEMU_BUILD_BUG_ON(NB_MMU_MODES > 8);
>> +
>>  typedef struct TCGLabelQemuLdst {
>> -    bool is_ld:1;           /* qemu_ld: true, qemu_st: false */
>> -    TCGMemOp opc:4;
>> -    TCGReg addrlo_reg;      /* reg index for low word of guest virtual addr */
>> -    TCGReg addrhi_reg;      /* reg index for high word of guest virtual addr */
>> -    TCGReg datalo_reg;      /* reg index for low word to be loaded or stored */
>> -    TCGReg datahi_reg;      /* reg index for high word to be loaded or stored */
>> -    int mem_index;          /* soft MMU memory index */
>> +    TCGMemOp opc : 4;
>> +    bool is_ld : 1;         /* qemu_ld: true, qemu_st: false */
>> +    TCGReg addrlo_reg : 5;  /* reg index for low word of guest virtual addr */
>> +    TCGReg addrhi_reg : 5;  /* reg index for high word of guest virtual addr */
>> +    TCGReg datalo_reg : 5;  /* reg index for low word to be loaded or stored */
>> +    TCGReg datahi_reg : 5;  /* reg index for high word to be loaded or stored */
>> +    unsigned mem_index : 3; /* soft MMU memory index */
>> +    /* 4 bits unused in 32-bit word */
>
> Why?  Are there more than 10 or so loads in the typical tb?

For clarity, does this comment amount to a request for
me not to apply this pullreq?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst
  2014-09-23 17:48     ` Peter Maydell
@ 2014-09-23 18:42       ` Paolo Bonzini
  2014-09-23 18:46         ` Richard Henderson
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2014-09-23 18:42 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, Richard Henderson

Il 23/09/2014 19:48, Peter Maydell ha scritto:
> On 22 September 2014 23:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 22/09/2014 22:57, Richard Henderson ha scritto:
>>> +QEMU_BUILD_BUG_ON(TCG_TARGET_NB_REGS > 32);
>>> +QEMU_BUILD_BUG_ON(NB_MMU_MODES > 8);
>>> +
>>>  typedef struct TCGLabelQemuLdst {
>>> -    bool is_ld:1;           /* qemu_ld: true, qemu_st: false */
>>> -    TCGMemOp opc:4;
>>> -    TCGReg addrlo_reg;      /* reg index for low word of guest virtual addr */
>>> -    TCGReg addrhi_reg;      /* reg index for high word of guest virtual addr */
>>> -    TCGReg datalo_reg;      /* reg index for low word to be loaded or stored */
>>> -    TCGReg datahi_reg;      /* reg index for high word to be loaded or stored */
>>> -    int mem_index;          /* soft MMU memory index */
>>> +    TCGMemOp opc : 4;
>>> +    bool is_ld : 1;         /* qemu_ld: true, qemu_st: false */
>>> +    TCGReg addrlo_reg : 5;  /* reg index for low word of guest virtual addr */
>>> +    TCGReg addrhi_reg : 5;  /* reg index for high word of guest virtual addr */
>>> +    TCGReg datalo_reg : 5;  /* reg index for low word to be loaded or stored */
>>> +    TCGReg datahi_reg : 5;  /* reg index for high word to be loaded or stored */
>>> +    unsigned mem_index : 3; /* soft MMU memory index */
>>> +    /* 4 bits unused in 32-bit word */
>>
>> Why?  Are there more than 10 or so loads in the typical tb?
> 
> For clarity, does this comment amount to a request for
> me not to apply this pullreq?

No, it just caught my eye because it'll conflict with the PPC patches
that have NB_MMU_MODES == 12.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst
  2014-09-23 18:42       ` Paolo Bonzini
@ 2014-09-23 18:46         ` Richard Henderson
  0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-09-23 18:46 UTC (permalink / raw)
  To: Paolo Bonzini, Peter Maydell; +Cc: QEMU Developers

On 09/23/2014 11:42 AM, Paolo Bonzini wrote:
> No, it just caught my eye because it'll conflict with the PPC patches
> that have NB_MMU_MODES == 12.

Ah, right, so it will.

Peter, please don't pull.  I'll drop that patch for now.


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32
  2014-09-22 20:57 ` [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32 Richard Henderson
@ 2014-09-24  8:20   ` Claudio Fontana
  2014-09-24 15:19     ` Richard Henderson
  0 siblings, 1 reply; 19+ messages in thread
From: Claudio Fontana @ 2014-09-24  8:20 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: peter.maydell

As I mentioned before, I just have one nit with this,
functionally it is fine (and I tested it with multiple targets, so you can add my

Tested-by: Claudio Fontana <claudio.fontana@huawei.com>

I describe my nit below:

On 22.09.2014 22:57, Richard Henderson wrote:
> The "old" qemu_ld opcode did not specify the size of the result,
> and so we had to assume full register width.  With the new opcodes,
> we can narrow the result.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/aarch64/tcg-target.c | 29 ++++++++++++++++-------------
>  tcg/tcg-be-ldst.h        |  1 +
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 56dae66..5017cfe 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -1007,7 +1007,7 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *lb)
>      tcg_out_adr(s, TCG_REG_X3, lb->raddr);
>      tcg_out_call(s, qemu_ld_helpers[opc & ~MO_SIGN]);
>      if (opc & MO_SIGN) {
> -        tcg_out_sxt(s, TCG_TYPE_I64, size, lb->datalo_reg, TCG_REG_X0);
> +        tcg_out_sxt(s, lb->type, size, lb->datalo_reg, TCG_REG_X0);
>      } else {
>          tcg_out_mov(s, size == MO_64, lb->datalo_reg, TCG_REG_X0);
>      }
> @@ -1032,14 +1032,15 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *lb)
>  }
>  
>  static void add_qemu_ldst_label(TCGContext *s, bool is_ld, TCGMemOp opc,
> -                                TCGReg data_reg, TCGReg addr_reg,
> +                                TCGType type, TCGReg data_reg, TCGReg addr_reg,
>                                  int mem_index, tcg_insn_unit *raddr,
>                                  tcg_insn_unit *label_ptr)
>  {
>      TCGLabelQemuLdst *label = new_ldst_label(s);
>  
> -    label->is_ld = is_ld;
>      label->opc = opc;
> +    label->is_ld = is_ld;
> +    label->type = type;
>      label->datalo_reg = data_reg;
>      label->addrlo_reg = addr_reg;
>      label->mem_index = mem_index;
> @@ -1108,7 +1109,7 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp s_bits,
>  
>  #endif /* CONFIG_SOFTMMU */
>  
> -static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
> +static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop, TCGType type,
>                                     TCGReg data_r, TCGReg addr_r, TCGReg off_r)
>  {
>      const TCGMemOp bswap = memop & MO_BSWAP;
> @@ -1118,7 +1119,8 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
>          tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, off_r);
>          break;
>      case MO_SB:
> -        tcg_out_ldst_r(s, I3312_LDRSBX, data_r, addr_r, off_r);
> +        tcg_out_ldst_r(s, type ? I3312_LDRSBX : I3312_LDRSBW,
> +                       data_r, addr_r, off_r);


since we are using the enum type TCGType, why do we check type as "type ?"

I would have expected the conditional to be something like

type == TCG_TYPE_I32 ? I3312_LDRSBW : I3312_LDRSBX

It's pretty obvious what is happening but it might spare someone a lookup into the header file
to test that type 0 is indeed TCG_TYPE_I32.

Ciao,

Claudio

>          break;
>      case MO_UW:
>          tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, off_r);
> @@ -1130,9 +1132,10 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
>          if (bswap) {
>              tcg_out_ldst_r(s, I3312_LDRH, data_r, addr_r, off_r);
>              tcg_out_rev16(s, data_r, data_r);
> -            tcg_out_sxt(s, TCG_TYPE_I64, MO_16, data_r, data_r);
> +            tcg_out_sxt(s, type, MO_16, data_r, data_r);
>          } else {
> -            tcg_out_ldst_r(s, I3312_LDRSHX, data_r, addr_r, off_r);
> +            tcg_out_ldst_r(s, type ? I3312_LDRSHX : I3312_LDRSHW,
> +                           data_r, addr_r, off_r);
>          }
>          break;
>      case MO_UL:
> @@ -1197,18 +1200,18 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGMemOp memop,
>  }
>  
>  static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
> -                            TCGMemOp memop, int mem_index)
> +                            TCGMemOp memop, TCGType type, int mem_index)
>  {
>  #ifdef CONFIG_SOFTMMU
>      TCGMemOp s_bits = memop & MO_SIZE;
>      tcg_insn_unit *label_ptr;
>  
>      tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 1);
> -    tcg_out_qemu_ld_direct(s, memop, data_reg, addr_reg, TCG_REG_X1);
> -    add_qemu_ldst_label(s, true, memop, data_reg, addr_reg,
> +    tcg_out_qemu_ld_direct(s, memop, type, data_reg, addr_reg, TCG_REG_X1);
> +    add_qemu_ldst_label(s, true, memop, type, data_reg, addr_reg,
>                          mem_index, s->code_ptr, label_ptr);
>  #else /* !CONFIG_SOFTMMU */
> -    tcg_out_qemu_ld_direct(s, memop, data_reg, addr_reg,
> +    tcg_out_qemu_ld_direct(s, memop, type, data_reg, addr_reg,
>                             GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR);
>  #endif /* CONFIG_SOFTMMU */
>  }
> @@ -1222,7 +1225,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
>  
>      tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 0);
>      tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg, TCG_REG_X1);
> -    add_qemu_ldst_label(s, false, memop, data_reg, addr_reg,
> +    add_qemu_ldst_label(s, false, memop, s_bits == MO_64, data_reg, addr_reg,
>                          mem_index, s->code_ptr, label_ptr);
>  #else /* !CONFIG_SOFTMMU */
>      tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg,
> @@ -1515,7 +1518,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
>  
>      case INDEX_op_qemu_ld_i32:
>      case INDEX_op_qemu_ld_i64:
> -        tcg_out_qemu_ld(s, a0, a1, a2, args[3]);
> +        tcg_out_qemu_ld(s, a0, a1, a2, ext, args[3]);
>          break;
>      case INDEX_op_qemu_st_i32:
>      case INDEX_op_qemu_st_i64:
> diff --git a/tcg/tcg-be-ldst.h b/tcg/tcg-be-ldst.h
> index 904eeda..825de14 100644
> --- a/tcg/tcg-be-ldst.h
> +++ b/tcg/tcg-be-ldst.h
> @@ -29,6 +29,7 @@ QEMU_BUILD_BUG_ON(NB_MMU_MODES > 8);
>  typedef struct TCGLabelQemuLdst {
>      TCGMemOp opc : 4;
>      bool is_ld : 1;         /* qemu_ld: true, qemu_st: false */
> +    TCGType type : 1;       /* result type of a load */
>      TCGReg addrlo_reg : 5;  /* reg index for low word of guest virtual addr */
>      TCGReg addrhi_reg : 5;  /* reg index for high word of guest virtual addr */
>      TCGReg datalo_reg : 5;  /* reg index for low word to be loaded or stored */
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32
  2014-09-24  8:20   ` Claudio Fontana
@ 2014-09-24 15:19     ` Richard Henderson
  2014-09-25  8:03       ` Claudio Fontana
  0 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2014-09-24 15:19 UTC (permalink / raw)
  To: Claudio Fontana, qemu-devel; +Cc: peter.maydell

On 09/24/2014 01:20 AM, Claudio Fontana wrote:
>> @@ -1118,7 +1119,8 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
>>          tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, off_r);
>>          break;
>>      case MO_SB:
>> -        tcg_out_ldst_r(s, I3312_LDRSBX, data_r, addr_r, off_r);
>> +        tcg_out_ldst_r(s, type ? I3312_LDRSBX : I3312_LDRSBW,
>> +                       data_r, addr_r, off_r);
> 
> 
> since we are using the enum type TCGType, why do we check type as "type ?"
> 
> I would have expected the conditional to be something like
> 
> type == TCG_TYPE_I32 ? I3312_LDRSBW : I3312_LDRSBX
> 
> It's pretty obvious what is happening but it might spare someone a lookup into the header file
> to test that type 0 is indeed TCG_TYPE_I32.

We assert the boolean-ish nature of TCGType at the start of the file, and use
it for the "ext" variable throughout.  Would it help if the variable weren't
named "type"?


r~

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32
  2014-09-24 15:19     ` Richard Henderson
@ 2014-09-25  8:03       ` Claudio Fontana
  0 siblings, 0 replies; 19+ messages in thread
From: Claudio Fontana @ 2014-09-25  8:03 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: peter.maydell

On 24.09.2014 17:19, Richard Henderson wrote:
> On 09/24/2014 01:20 AM, Claudio Fontana wrote:
>>> @@ -1118,7 +1119,8 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGMemOp memop,
>>>          tcg_out_ldst_r(s, I3312_LDRB, data_r, addr_r, off_r);
>>>          break;
>>>      case MO_SB:
>>> -        tcg_out_ldst_r(s, I3312_LDRSBX, data_r, addr_r, off_r);
>>> +        tcg_out_ldst_r(s, type ? I3312_LDRSBX : I3312_LDRSBW,
>>> +                       data_r, addr_r, off_r);
>>
>>
>> since we are using the enum type TCGType, why do we check type as "type ?"
>>
>> I would have expected the conditional to be something like
>>
>> type == TCG_TYPE_I32 ? I3312_LDRSBW : I3312_LDRSBX
>>
>> It's pretty obvious what is happening but it might spare someone a lookup into the header file
>> to test that type 0 is indeed TCG_TYPE_I32.
> 
> We assert the boolean-ish nature of TCGType at the start of the file, and use
> it for the "ext" variable throughout.  Would it help if the variable weren't
> named "type"?
> 
> 
> r~

I could not find a comment describing that in tcg.h, did I miss it in the patch?

I find it difficult to come up with a better name for TCGType, I wonder what that could be..

but what about not caring about the boolish nature of TCGType and instead
check it explicitly? I understand that there are multiple enumeration constants
with the same value, but if the names of the first two enumeration constants are
general enough, that should be understandable, no?

A comment before TCGType could explain this, and then we could have values like

TCG_TYPE_32 = 0,
TCG_TYPE_64,

/* .. all the other aliases */

Which is basically what we have now isn't it? TCG_TYPE_I32 and TCG_TYPE_I64 are
general enough I think, so why aren't we checking those constants explicitly?

Ciao,

Claudio

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-09-25  8:04 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-22 20:57 [Qemu-devel] [PULL 00/11] tcg updates Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 01/11] tcg-sparc: Support addsub2_i64 Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 02/11] tcg-sparc: Use ADDXC in addsub2_i64 Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 03/11] tcg-sparc: Fix setcond_i32 uninitialized value Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 04/11] tcg-sparc: Use ADDXC in setcond_i64 Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 05/11] tcg-sparc: Rename ADDX/SUBX insns Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 06/11] tcg-sparc: Use UMULXHI instruction Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 07/11] tcg: Compress TCGLabelQemuLdst Richard Henderson
2014-09-22 22:19   ` Paolo Bonzini
2014-09-23 17:48     ` Peter Maydell
2014-09-23 18:42       ` Paolo Bonzini
2014-09-23 18:46         ` Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 08/11] tcg: Move TCG_TYPE_COUNT out of enum TCGType Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 09/11] tcg-aarch64: Use 32-bit loads for qemu_ld_i32 Richard Henderson
2014-09-24  8:20   ` Claudio Fontana
2014-09-24 15:19     ` Richard Henderson
2014-09-25  8:03       ` Claudio Fontana
2014-09-22 20:57 ` [Qemu-devel] [PULL 10/11] qemu/compiler: Define QEMU_ARTIFICIAL Richard Henderson
2014-09-22 20:57 ` [Qemu-devel] [PULL 11/11] tcg: Always enable TCGv type checking Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).