All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.