All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr
@ 2014-03-29  0:27 Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps Richard Henderson
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

Here's where I think we should go with the cleanup that Peter started.

I've only converted a couple of the backends as examples.  It's not 100%
mechanical, since one has to be aware of the change in the semantics of
pointer arithmetic (e.g. s->code_ptr - s->code_buf).

Taking Sparc as an example, the before code size of tcg.o is 0xad60, and
the after code size is 0xa200.  A savings of a bit less than 3k.

Thoughts?


r~


Peter Maydell (3):
  exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps
  tcg: Avoid stores to unaligned addresses
  tcg: Avoid undefined behaviour patching code at unaligned addresses

Richard Henderson (5):
  tcg: Define tcg_itype for code pointers
  tcg-ppc64: Define TCG_TARGET_ITYPE_SIZE
  tcg-ppc: Define TCG_TARGET_ITYPE_SIZE
  tcg-aarch64: Define TCG_TARGET_ITYPE_SIZE
  tcg-sparc: Define TCG_TARGET_ITYPE_SIZE

 include/exec/exec-all.h  |  4 +--
 tcg/aarch64/tcg-target.c | 38 +++++++++------------
 tcg/aarch64/tcg-target.h |  1 +
 tcg/i386/tcg-target.c    | 12 +++----
 tcg/i386/tcg-target.h    |  1 +
 tcg/ppc/tcg-target.c     | 55 +++++++++++++++---------------
 tcg/ppc/tcg-target.h     |  1 +
 tcg/ppc64/tcg-target.c   | 46 +++++++++++++------------
 tcg/ppc64/tcg-target.h   |  1 +
 tcg/sparc/tcg-target.c   | 57 +++++++++++++++----------------
 tcg/sparc/tcg-target.h   |  2 +-
 tcg/tcg-be-ldst.h        |  4 +--
 tcg/tcg.c                | 87 ++++++++++++++++++++++++++++++++++++++----------
 tcg/tcg.h                | 37 +++++++++++++++-----
 translate-all.c          |  6 ++--
 15 files changed, 209 insertions(+), 143 deletions(-)

-- 
1.9.0

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

* [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-04-01 12:09   ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefinedbehaviour patching x86 jumpss Alex Bennée
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 2/8] tcg: Avoid stores to unaligned addresses Richard Henderson
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

From: Peter Maydell <peter.maydell@linaro.org>

The code which patches x86 jump instructions assumes it can do an
unaligned write of a uint32_t. This is actually safe on x86, but it's
still undefined behaviour. We have infrastructure for doing efficient
unaligned accesses which doesn't engage in undefined behaviour, so
use it.

This is technically fractionally less efficient, at least with gcc 4.6;
instead of one instruction:
 7b2:   89 3e                   mov    %edi,(%rsi)
we get an extra spurious store to the stack slot:
 7b2:   89 7c 24 64             mov    %edi,0x64(%rsp)
 7b6:   89 3e                   mov    %edi,(%rsi)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 include/exec/exec-all.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index f9ac332..1c49a21 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -229,7 +229,7 @@ void ppc_tb_set_jmp_target(unsigned long jmp_addr, unsigned long addr);
 static inline void tb_set_jmp_target1(uintptr_t jmp_addr, uintptr_t addr)
 {
     /* patch the branch destination */
-    *(uint32_t *)jmp_addr = addr - (jmp_addr + 4);
+    stl_p((void*)jmp_addr, addr - (jmp_addr + 4));
     /* no need to flush icache explicitly */
 }
 #elif defined(__aarch64__)
-- 
1.9.0

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

* [Qemu-devel] [PATCH 2/8] tcg: Avoid stores to unaligned addresses
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-04-01 12:12   ` Alex Bennée
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patching code at " Richard Henderson
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

From: Peter Maydell <peter.maydell@linaro.org>

Avoid stores to unaligned addresses in TCG code generation, by using the
usual memcpy() approach. (Using bswap.h would drag a lot of QEMU baggage
into TCG, so it's simpler just to do direct memcpy() here.)

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

diff --git a/tcg/tcg.c b/tcg/tcg.c
index f1e0763..60f06c5 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -125,21 +125,21 @@ static inline void tcg_out8(TCGContext *s, uint8_t v)
 static inline void tcg_out16(TCGContext *s, uint16_t v)
 {
     uint8_t *p = s->code_ptr;
-    *(uint16_t *)p = v;
+    memcpy(p, &v, sizeof(v));
     s->code_ptr = p + 2;
 }
 
 static inline void tcg_out32(TCGContext *s, uint32_t v)
 {
     uint8_t *p = s->code_ptr;
-    *(uint32_t *)p = v;
+    memcpy(p, &v, sizeof(v));
     s->code_ptr = p + 4;
 }
 
 static inline void tcg_out64(TCGContext *s, uint64_t v)
 {
     uint8_t *p = s->code_ptr;
-    *(uint64_t *)p = v;
+    memcpy(p, &v, sizeof(v));
     s->code_ptr = p + 8;
 }
 
-- 
1.9.0

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

* [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patching code at unaligned addresses
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 2/8] tcg: Avoid stores to unaligned addresses Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-04-01 12:13   ` [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patchingcode at unaligned addressess Alex Bennée
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 4/8] tcg: Define tcg_itype for code pointers Richard Henderson
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

From: Peter Maydell <peter.maydell@linaro.org>

To avoid C undefined behaviour when patching generated code,
provide wrappers tcg_patch8/16/32/64 which use the usual memcpy
trick, and use them in the i386 backend.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/i386/tcg-target.c | 12 ++++++------
 tcg/tcg.c             | 20 ++++++++++++++++++++
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index f832282..a0f9dff 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -151,14 +151,14 @@ static void patch_reloc(uint8_t *code_ptr, int type,
         if (value != (int32_t)value) {
             tcg_abort();
         }
-        *(uint32_t *)code_ptr = value;
+        tcg_patch32(code_ptr, value);
         break;
     case R_386_PC8:
         value -= (uintptr_t)code_ptr;
         if (value != (int8_t)value) {
             tcg_abort();
         }
-        *(uint8_t *)code_ptr = value;
+        tcg_patch8(code_ptr, value);
         break;
     default:
         tcg_abort();
@@ -1276,9 +1276,9 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
     uint8_t **label_ptr = &l->label_ptr[0];
 
     /* resolve label address */
-    *(uint32_t *)label_ptr[0] = (uint32_t)(s->code_ptr - label_ptr[0] - 4);
+    tcg_patch32(label_ptr[0], s->code_ptr - label_ptr[0] - 4);
     if (TARGET_LONG_BITS > TCG_TARGET_REG_BITS) {
-        *(uint32_t *)label_ptr[1] = (uint32_t)(s->code_ptr - label_ptr[1] - 4);
+        tcg_patch32(label_ptr[1], s->code_ptr - label_ptr[1] - 4);
     }
 
     if (TCG_TARGET_REG_BITS == 32) {
@@ -1360,9 +1360,9 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
     TCGReg retaddr;
 
     /* resolve label address */
-    *(uint32_t *)label_ptr[0] = (uint32_t)(s->code_ptr - label_ptr[0] - 4);
+    tcg_patch32(label_ptr[0], s->code_ptr - label_ptr[0] - 4);
     if (TARGET_LONG_BITS > TCG_TARGET_REG_BITS) {
-        *(uint32_t *)label_ptr[1] = (uint32_t)(s->code_ptr - label_ptr[1] - 4);
+        tcg_patch32(label_ptr[1], s->code_ptr - label_ptr[1] - 4);
     }
 
     if (TCG_TARGET_REG_BITS == 32) {
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 60f06c5..727112d 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -143,6 +143,26 @@ static inline void tcg_out64(TCGContext *s, uint64_t v)
     s->code_ptr = p + 8;
 }
 
+static inline void tcg_patch8(uint8_t *p, uint8_t v)
+{
+    memcpy(p, &v, sizeof(v));
+}
+
+static inline void tcg_patch16(uint8_t *p, uint16_t v)
+{
+    memcpy(p, &v, sizeof(v));
+}
+
+static inline void tcg_patch32(uint8_t *p, uint32_t v)
+{
+    memcpy(p, &v, sizeof(v));
+}
+
+static inline void tcg_patch64(uint8_t *p, uint64_t v)
+{
+    memcpy(p, &v, sizeof(v));
+}
+
 /* label relocation processing */
 
 static void tcg_out_reloc(TCGContext *s, uint8_t *code_ptr, int type,
-- 
1.9.0

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

* [Qemu-devel] [PATCH 4/8] tcg: Define tcg_itype for code pointers
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
                   ` (2 preceding siblings ...)
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patching code at " Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 5/8] tcg-ppc64: Define TCG_TARGET_ITYPE_SIZE Richard Henderson
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

To be defined by the tcg backend based on the elemental unit of the ISA.
During the transition, allow TCG_TARGET_ITYPE_SIZE to be undefined, which
allows us to default tcg_itype to the current uint8_t.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 include/exec/exec-all.h |  2 +-
 tcg/i386/tcg-target.h   |  1 +
 tcg/tcg-be-ldst.h       |  4 +--
 tcg/tcg.c               | 89 +++++++++++++++++++++++++++++++++----------------
 tcg/tcg.h               | 37 +++++++++++++++-----
 translate-all.c         |  6 ++--
 6 files changed, 95 insertions(+), 44 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 1c49a21..0766e24 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -145,7 +145,7 @@ struct TranslationBlock {
 #define CF_COUNT_MASK  0x7fff
 #define CF_LAST_IO     0x8000 /* Last insn may be an IO access.  */
 
-    uint8_t *tc_ptr;    /* pointer to the translated code */
+    void *tc_ptr;    /* pointer to the translated code */
     /* next matching tb for physical address. */
     struct TranslationBlock *phys_hash_next;
     /* first and second physical page containing code. The lower bit
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index bdf2222..c6d13d2 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -25,6 +25,7 @@
 #define TCG_TARGET_I386 1
 
 #undef TCG_TARGET_WORDS_BIGENDIAN
+#define TCG_TARGET_ITYPE_SIZE 1
 
 #ifdef __x86_64__
 # define TCG_TARGET_REG_BITS  64
diff --git a/tcg/tcg-be-ldst.h b/tcg/tcg-be-ldst.h
index 284db0c..94a4472 100644
--- a/tcg/tcg-be-ldst.h
+++ b/tcg/tcg-be-ldst.h
@@ -31,8 +31,8 @@ typedef struct TCGLabelQemuLdst {
     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 */
-    uint8_t *raddr;         /* gen code addr of the next IR of qemu_ld/st IR */
-    uint8_t *label_ptr[2];  /* label pointers to be updated */
+    tcg_itype *raddr;       /* gen code addr of the next IR of qemu_ld/st IR */
+    tcg_itype *label_ptr[2]; /* label pointers to be updated */
 } TCGLabelQemuLdst;
 
 typedef struct TCGBackendData {
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 727112d..8500859 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -65,7 +65,7 @@
 /* Forward declarations for functions declared in tcg-target.c and used here. */
 static void tcg_target_init(TCGContext *s);
 static void tcg_target_qemu_prologue(TCGContext *s);
-static void patch_reloc(uint8_t *code_ptr, int type, 
+static void patch_reloc(tcg_itype *code_ptr, int type, 
                         intptr_t value, intptr_t addend);
 
 /* The CIE and FDE header definitions will be common to all hosts.  */
@@ -117,55 +117,85 @@ const size_t tcg_op_defs_max = ARRAY_SIZE(tcg_op_defs);
 static TCGRegSet tcg_target_available_regs[2];
 static TCGRegSet tcg_target_call_clobber_regs;
 
+#if TCG_TARGET_ITYPE_SIZE == 1
 static inline void tcg_out8(TCGContext *s, uint8_t v)
 {
     *s->code_ptr++ = v;
 }
 
-static inline void tcg_out16(TCGContext *s, uint16_t v)
+static inline void tcg_patch8(tcg_itype *p, uint8_t v)
 {
-    uint8_t *p = s->code_ptr;
-    memcpy(p, &v, sizeof(v));
-    s->code_ptr = p + 2;
+    *p = v;
 }
+#endif
 
-static inline void tcg_out32(TCGContext *s, uint32_t v)
+#if TCG_TARGET_ITYPE_SIZE <= 2
+static inline void tcg_out16(TCGContext *s, uint16_t v)
 {
-    uint8_t *p = s->code_ptr;
-    memcpy(p, &v, sizeof(v));
-    s->code_ptr = p + 4;
+    if (TCG_TARGET_ITYPE_SIZE == 2) {
+        *s->code_ptr++ = v;
+    } else {
+        tcg_itype *p = s->code_ptr;
+        memcpy(p, &v, sizeof(v));
+        s->code_ptr = p + (2 / TCG_TARGET_ITYPE_SIZE);
+    }
 }
 
-static inline void tcg_out64(TCGContext *s, uint64_t v)
+static inline void tcg_patch16(tcg_itype *p, uint16_t v)
 {
-    uint8_t *p = s->code_ptr;
-    memcpy(p, &v, sizeof(v));
-    s->code_ptr = p + 8;
+    if (TCG_TARGET_ITYPE_SIZE == 2) {
+        *p = v;
+    } else {
+        memcpy(p, &v, sizeof(v));
+    }
 }
+#endif
 
-static inline void tcg_patch8(uint8_t *p, uint8_t v)
+#if TCG_TARGET_ITYPE_SIZE <= 4
+static inline void tcg_out32(TCGContext *s, uint32_t v)
 {
-    memcpy(p, &v, sizeof(v));
+    if (TCG_TARGET_ITYPE_SIZE == 4) {
+        *s->code_ptr++ = v;
+    } else {
+        tcg_itype *p = s->code_ptr;
+        memcpy(p, &v, sizeof(v));
+        s->code_ptr = p + (4 / TCG_TARGET_ITYPE_SIZE);
+    }
 }
 
-static inline void tcg_patch16(uint8_t *p, uint16_t v)
+static inline void tcg_patch32(tcg_itype *p, uint32_t v)
 {
-    memcpy(p, &v, sizeof(v));
+    if (TCG_TARGET_ITYPE_SIZE) {
+        *p = v;
+    } else {
+        memcpy(p, &v, sizeof(v));
+    }
 }
+#endif
 
-static inline void tcg_patch32(uint8_t *p, uint32_t v)
+static inline void tcg_out64(TCGContext *s, uint64_t v)
 {
-    memcpy(p, &v, sizeof(v));
+    if (TCG_TARGET_ITYPE_SIZE == 8) {
+        *s->code_ptr++ = v;
+    } else {
+        tcg_itype *p = s->code_ptr;
+        memcpy(p, &v, sizeof(v));
+        s->code_ptr = p + (8 / TCG_TARGET_ITYPE_SIZE);
+    }
 }
 
-static inline void tcg_patch64(uint8_t *p, uint64_t v)
+static inline void tcg_patch64(tcg_itype *p, uint64_t v)
 {
-    memcpy(p, &v, sizeof(v));
+    if (TCG_TARGET_ITYPE_SIZE == 8) {
+        *p = v;
+    } else {
+        memcpy(p, &v, sizeof(v));
+    }
 }
 
 /* label relocation processing */
 
-static void tcg_out_reloc(TCGContext *s, uint8_t *code_ptr, int type,
+static void tcg_out_reloc(TCGContext *s, tcg_itype *code_ptr, int type,
                           int label_index, intptr_t addend)
 {
     TCGLabel *l;
@@ -188,7 +218,7 @@ static void tcg_out_reloc(TCGContext *s, uint8_t *code_ptr, int type,
     }
 }
 
-static void tcg_out_label(TCGContext *s, int label_index, void *ptr)
+static void tcg_out_label(TCGContext *s, int label_index, tcg_itype *ptr)
 {
     TCGLabel *l;
     TCGRelocation *r;
@@ -359,7 +389,7 @@ void tcg_prologue_init(TCGContext *s)
 
 #ifdef DEBUG_DISAS
     if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) {
-        size_t size = s->code_ptr - s->code_buf;
+        size_t size = (uintptr_t)s->code_ptr - (uintptr_t)s->code_buf;
         qemu_log("PROLOGUE: [size=%zu]\n", size);
         log_disas(s->code_buf, size);
         qemu_log("\n");
@@ -2472,7 +2502,7 @@ static void dump_op_count(void)
 #endif
 
 
-static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
+static inline int tcg_gen_code_common(TCGContext *s, tcg_itype *gen_code_buf,
                                       long search_pc)
 {
     TCGOpcode opc;
@@ -2587,7 +2617,8 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
         }
         args += def->nb_args;
     next:
-        if (search_pc >= 0 && search_pc < s->code_ptr - gen_code_buf) {
+        if (search_pc >= 0
+            && search_pc < (intptr_t)s->code_ptr - (intptr_t)gen_code_buf) {
             return op_index;
         }
         op_index++;
@@ -2601,7 +2632,7 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf,
     return -1;
 }
 
-int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
+int tcg_gen_code(TCGContext *s, tcg_itype *gen_code_buf)
 {
 #ifdef CONFIG_PROFILER
     {
@@ -2622,14 +2653,14 @@ int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
     /* flush instruction cache */
     flush_icache_range((uintptr_t)gen_code_buf, (uintptr_t)s->code_ptr);
 
-    return s->code_ptr -  gen_code_buf;
+    return (uintptr_t)s->code_ptr - (uintptr_t)gen_code_buf;
 }
 
 /* Return the index of the micro operation such as the pc after is <
    offset bytes from the start of the TB.  The contents of gen_code_buf must
    not be changed, though writing the same values is ok.
    Return -1 if not found. */
-int tcg_gen_code_search_pc(TCGContext *s, uint8_t *gen_code_buf, long offset)
+int tcg_gen_code_search_pc(TCGContext *s, tcg_itype *gen_code_buf, long offset)
 {
     return tcg_gen_code_common(s, gen_code_buf, offset);
 }
diff --git a/tcg/tcg.h b/tcg/tcg.h
index f7efcb4..1aa0800 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -139,10 +139,26 @@ typedef enum TCGOpcode {
 #define tcg_regset_andnot(d, a, b) (d) = (a) & ~(b)
 #define tcg_regset_not(d, a) (d) = ~(a)
 
+#ifndef TCG_TARGET_ITYPE_SIZE
+#define TCG_TARGET_ITYPE_SIZE 1
+#endif
+#if TCG_TARGET_ITYPE_SIZE == 1
+typedef uint8_t tcg_itype;
+#elif TCG_TARGET_ITYPE_SIZE == 2
+typedef uint16_t tcg_itype;
+#elif TCG_TARGET_ITYPE_SIZE == 4
+typedef uint32_t tcg_itype;
+#elif TCG_TARGET_ITYPE_SIZE == 8
+typedef uint64_t tcg_itype;
+#else
+# error
+#endif
+
+
 typedef struct TCGRelocation {
     struct TCGRelocation *next;
     int type;
-    uint8_t *ptr;
+    tcg_itype *ptr;
     intptr_t addend;
 } TCGRelocation; 
 
@@ -457,7 +473,7 @@ struct TCGContext {
     int nb_temps;
 
     /* goto_tb support */
-    uint8_t *code_buf;
+    tcg_itype *code_buf;
     uintptr_t *tb_next;
     uint16_t *tb_next_offset;
     uint16_t *tb_jmp_offset; /* != NULL if USE_DIRECT_JUMP */
@@ -478,7 +494,7 @@ struct TCGContext {
     intptr_t frame_end;
     int frame_reg;
 
-    uint8_t *code_ptr;
+    tcg_itype *code_ptr;
     TCGTemp temps[TCG_MAX_TEMPS]; /* globals first, temps after */
     TCGTempSet free_temps[TCG_TYPE_COUNT * 2];
 
@@ -517,14 +533,17 @@ struct TCGContext {
     uint16_t gen_opc_icount[OPC_BUF_SIZE];
     uint8_t gen_opc_instr_start[OPC_BUF_SIZE];
 
-    /* Code generation */
+    /* Code generation.  Note that we specifically do not use tcg_itype
+       here, because there's too much arithmetic throughout that relies
+       on addition and subtraction working on bytes.  Rely on the GCC
+       extension that allows arithmetic on void*.  */
     int code_gen_max_blocks;
-    uint8_t *code_gen_prologue;
-    uint8_t *code_gen_buffer;
+    void *code_gen_prologue;
+    void *code_gen_buffer;
     size_t code_gen_buffer_size;
     /* threshold to flush the translated code buffer */
     size_t code_gen_buffer_max_size;
-    uint8_t *code_gen_ptr;
+    void *code_gen_ptr;
 
     TBContext tb_ctx;
 
@@ -559,8 +578,8 @@ void tcg_context_init(TCGContext *s);
 void tcg_prologue_init(TCGContext *s);
 void tcg_func_start(TCGContext *s);
 
-int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf);
-int tcg_gen_code_search_pc(TCGContext *s, uint8_t *gen_code_buf, long offset);
+int tcg_gen_code(TCGContext *s, tcg_itype *gen_code_buf);
+int tcg_gen_code_search_pc(TCGContext *s, tcg_itype *gen_code_buf, long offset);
 
 void tcg_set_frame(TCGContext *s, int reg, intptr_t start, intptr_t size);
 
diff --git a/translate-all.c b/translate-all.c
index f243c10..377d97c 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -143,7 +143,7 @@ void cpu_gen_init(void)
 int cpu_gen_code(CPUArchState *env, TranslationBlock *tb, int *gen_code_size_ptr)
 {
     TCGContext *s = &tcg_ctx;
-    uint8_t *gen_code_buf;
+    tcg_itype *gen_code_buf;
     int gen_code_size;
 #ifdef CONFIG_PROFILER
     int64_t ti;
@@ -235,7 +235,7 @@ static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
     s->tb_jmp_offset = NULL;
     s->tb_next = tb->tb_next;
 #endif
-    j = tcg_gen_code_search_pc(s, (uint8_t *)tc_ptr, searched_pc - tc_ptr);
+    j = tcg_gen_code_search_pc(s, (tcg_itype *)tc_ptr, searched_pc - tc_ptr);
     if (j < 0)
         return -1;
     /* now find start of instruction before */
@@ -944,7 +944,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 {
     CPUArchState *env = cpu->env_ptr;
     TranslationBlock *tb;
-    uint8_t *tc_ptr;
+    tcg_itype *tc_ptr;
     tb_page_addr_t phys_pc, phys_page2;
     target_ulong virt_page2;
     int code_gen_size;
-- 
1.9.0

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

* [Qemu-devel] [PATCH 5/8] tcg-ppc64: Define TCG_TARGET_ITYPE_SIZE
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
                   ` (3 preceding siblings ...)
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 4/8] tcg: Define tcg_itype for code pointers Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 6/8] tcg-ppc: " Richard Henderson
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/ppc64/tcg-target.c | 46 ++++++++++++++++++++++++----------------------
 tcg/ppc64/tcg-target.h |  1 +
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index 06e440f..0920cae 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c
@@ -31,7 +31,7 @@
 #define TCG_CT_CONST_ZERO 0x1000
 #define TCG_CT_CONST_MONE 0x2000
 
-static uint8_t *tb_ret_addr;
+static intptr_t tb_ret_addr;
 
 #if TARGET_LONG_BITS == 32
 #define LD_ADDR LWZ
@@ -178,10 +178,9 @@ static uint32_t reloc_pc24_val(void *pc, tcg_target_long target)
     return disp & 0x3fffffc;
 }
 
-static void reloc_pc24(void *pc, tcg_target_long target)
+static void reloc_pc24(tcg_itype *pc, tcg_target_long target)
 {
-    *(uint32_t *)pc = (*(uint32_t *)pc & ~0x3fffffc)
-        | reloc_pc24_val(pc, target);
+    *pc = (*pc & ~0x3fffffc) | reloc_pc24_val(pc, target);
 }
 
 static uint16_t reloc_pc14_val(void *pc, tcg_target_long target)
@@ -196,24 +195,24 @@ static uint16_t reloc_pc14_val(void *pc, tcg_target_long target)
     return disp & 0xfffc;
 }
 
-static void reloc_pc14(void *pc, tcg_target_long target)
+static void reloc_pc14(tcg_itype *pc, tcg_target_long target)
 {
-    *(uint32_t *)pc = (*(uint32_t *)pc & ~0xfffc) | reloc_pc14_val(pc, target);
+    *pc = (*pc & ~0xfffc) | reloc_pc14_val(pc, target);
 }
 
 static inline void tcg_out_b_noaddr(TCGContext *s, int insn)
 {
-    unsigned retrans = *(uint32_t *)s->code_ptr & 0x3fffffc;
+    unsigned retrans = *s->code_ptr & 0x3fffffc;
     tcg_out32(s, insn | retrans);
 }
 
 static inline void tcg_out_bc_noaddr(TCGContext *s, int insn)
 {
-    unsigned retrans = *(uint32_t *)s->code_ptr & 0xfffc;
+    unsigned retrans = *s->code_ptr & 0xfffc;
     tcg_out32(s, insn | retrans);
 }
 
-static void patch_reloc(uint8_t *code_ptr, int type,
+static void patch_reloc(tcg_itype *code_ptr, int type,
                         intptr_t value, intptr_t addend)
 {
     value += addend;
@@ -938,7 +937,7 @@ static TCGReg tcg_out_tlb_read(TCGContext *s, TCGMemOp s_bits, TCGReg addr_reg,
    helper code.  */
 static void add_qemu_ldst_label(TCGContext *s, bool is_ld, TCGMemOp opc,
                                 int data_reg, int addr_reg, int mem_index,
-                                uint8_t *raddr, uint8_t *label_ptr)
+                                tcg_itype *raddr, tcg_itype *label_ptr)
 {
     TCGLabelQemuLdst *label = new_ldst_label(s);
 
@@ -1009,7 +1008,7 @@ static void tcg_out_qemu_ld(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
     uint32_t insn;
     TCGMemOp s_bits = opc & MO_SIZE;
 #ifdef CONFIG_SOFTMMU
-    void *label_ptr;
+    tcg_itype *label_ptr;
 #endif
 
 #ifdef CONFIG_SOFTMMU
@@ -1055,7 +1054,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
     TCGReg rbase;
     uint32_t insn;
 #ifdef CONFIG_SOFTMMU
-    void *label_ptr;
+    tcg_itype *label_ptr;
 #endif
 
 #ifdef CONFIG_SOFTMMU
@@ -1115,7 +1114,8 @@ static void tcg_target_qemu_prologue(TCGContext *s)
 #ifndef __APPLE__
     /* First emit adhoc function descriptor */
     tcg_out64(s, (uint64_t)s->code_ptr + 24); /* entry point */
-    s->code_ptr += 16;          /* skip TOC and environment pointer */
+    tcg_out64(s, 0);                          /* toc */
+    tcg_out64(s, 0);                          /* environment pointer */
 #endif
 
     /* Prologue */
@@ -1139,7 +1139,7 @@ static void tcg_target_qemu_prologue(TCGContext *s)
     tcg_out32(s, BCCTR | BO_ALWAYS);
 
     /* Epilogue */
-    tb_ret_addr = s->code_ptr;
+    tb_ret_addr = (intptr_t)s->code_ptr;
 
     for (i = 0; i < ARRAY_SIZE(tcg_target_callee_save_regs); ++i) {
         tcg_out32(s, LD | TAI(tcg_target_callee_save_regs[i], TCG_REG_R1,
@@ -1470,14 +1470,14 @@ static void tcg_out_movcond(TCGContext *s, TCGType type, TCGCond cond,
     }
 }
 
-void ppc_tb_set_jmp_target(unsigned long jmp_addr, unsigned long addr)
+void ppc_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr)
 {
     TCGContext s;
-    unsigned long patch_size;
+    uintptr_t patch_size;
 
-    s.code_ptr = (uint8_t *) jmp_addr;
+    s.code_ptr = (tcg_itype *)jmp_addr;
     tcg_out_b(&s, 0, addr);
-    patch_size = s.code_ptr - (uint8_t *) jmp_addr;
+    patch_size = (uintptr_t)s.code_ptr - jmp_addr;
     flush_icache_range(jmp_addr, jmp_addr + patch_size);
 }
 
@@ -1490,18 +1490,20 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
     switch (opc) {
     case INDEX_op_exit_tb:
         tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_R3, args[0]);
-        tcg_out_b(s, 0, (tcg_target_long)tb_ret_addr);
+        tcg_out_b(s, 0, tb_ret_addr);
         break;
     case INDEX_op_goto_tb:
         if (s->tb_jmp_offset) {
             /* Direct jump method.  */
-            s->tb_jmp_offset[args[0]] = s->code_ptr - s->code_buf;
-            s->code_ptr += 28;
+            s->tb_jmp_offset[args[0]]
+                = (uintptr_t)s->code_ptr - (uintptr_t)s->code_buf;
+            s->code_ptr += 7;
         } else {
             /* Indirect jump method.  */
             tcg_abort();
         }
-        s->tb_next_offset[args[0]] = s->code_ptr - s->code_buf;
+        s->tb_next_offset[args[0]]
+            = (uintptr_t)s->code_ptr - (uintptr_t)s->code_buf;
         break;
     case INDEX_op_br:
         {
diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h
index 7ee50b6..8c55a20 100644
--- a/tcg/ppc64/tcg-target.h
+++ b/tcg/ppc64/tcg-target.h
@@ -26,6 +26,7 @@
 
 #define TCG_TARGET_WORDS_BIGENDIAN
 #define TCG_TARGET_NB_REGS 32
+#define TCG_TARGET_ITYPE_SIZE 4
 
 typedef enum {
     TCG_REG_R0 = 0,
-- 
1.9.0

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

* [Qemu-devel] [PATCH 6/8] tcg-ppc: Define TCG_TARGET_ITYPE_SIZE
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
                   ` (4 preceding siblings ...)
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 5/8] tcg-ppc64: Define TCG_TARGET_ITYPE_SIZE Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 7/8] tcg-aarch64: " Richard Henderson
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/ppc/tcg-target.c | 55 ++++++++++++++++++++++++++--------------------------
 tcg/ppc/tcg-target.h |  1 +
 2 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index dc2c2df..36f261b 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -24,7 +24,7 @@
 
 #include "tcg-be-ldst.h"
 
-static uint8_t *tb_ret_addr;
+static intptr_t tb_ret_addr;
 
 #if defined _CALL_DARWIN || defined __APPLE__
 #define TCG_TARGET_CALL_DARWIN
@@ -205,7 +205,7 @@ static void reloc_pc14 (void *pc, tcg_target_long target)
         | reloc_pc14_val (pc, target);
 }
 
-static void patch_reloc(uint8_t *code_ptr, int type,
+static void patch_reloc(tcg_itype *code_ptr, int type,
                         intptr_t value, intptr_t addend)
 {
     value += addend;
@@ -531,8 +531,8 @@ static void add_qemu_ldst_label (TCGContext *s,
                                  int addrlo_reg,
                                  int addrhi_reg,
                                  int mem_index,
-                                 uint8_t *raddr,
-                                 uint8_t *label_ptr)
+                                 tcg_itype *raddr,
+                                 tcg_itype *label_ptr)
 {
     TCGLabelQemuLdst *label = new_ldst_label(s);
 
@@ -582,14 +582,14 @@ static void *st_trampolines[16];
 
 static void tcg_out_tlb_check(TCGContext *s, TCGReg r0, TCGReg r1, TCGReg r2,
                               TCGReg addrlo, TCGReg addrhi, TCGMemOp s_bits,
-                              int mem_index, int is_load, uint8_t **label_ptr)
+                              int mem_index, int is_load, tcg_itype **label_ptr)
 {
     int cmp_off =
         (is_load
          ? offsetof(CPUArchState, tlb_table[mem_index][0].addr_read)
          : offsetof(CPUArchState, tlb_table[mem_index][0].addr_write));
     int add_off = offsetof(CPUArchState, tlb_table[mem_index][0].addend);
-    uint16_t retranst;
+    tcg_itype retranst;
     TCGReg base = TCG_AREG0;
 
     /* Extract the page index, shifted into place for tlb index.  */
@@ -648,7 +648,7 @@ static void tcg_out_tlb_check(TCGContext *s, TCGReg r0, TCGReg r1, TCGReg r2,
        This address cannot be used for a tail call, but it's shorter
        than forming an address from scratch.  */
     *label_ptr = s->code_ptr;
-    retranst = ((uint16_t *) s->code_ptr)[1] & ~3;
+    retranst = *s->code_ptr & 0xfffc;
     tcg_out32(s, BC | BI(7, CR_EQ) | retranst | BO_COND_FALSE | LK);
 }
 #endif
@@ -659,7 +659,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
     TCGMemOp opc, bswap;
 #ifdef CONFIG_SOFTMMU
     int mem_index;
-    uint8_t *label_ptr;
+    tcg_itype *label_ptr;
 #endif
 
     datalo = *args++;
@@ -731,7 +731,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
     TCGMemOp opc, bswap, s_bits;
 #ifdef CONFIG_SOFTMMU
     int mem_index;
-    uint8_t *label_ptr;
+    tcg_itype *label_ptr;
 #endif
 
     datalo = *args++;
@@ -914,7 +914,8 @@ static void tcg_target_qemu_prologue (TCGContext *s)
         /* First emit adhoc function descriptor */
         addr = (uint32_t) s->code_ptr + 12;
         tcg_out32 (s, addr);        /* entry point */
-        s->code_ptr += 8;           /* skip TOC and environment pointer */
+        tcg_out32 (s, 0);           /* toc */
+        tcg_out32 (s, 0);           /* environment pointer */
     }
 #endif
     tcg_out32 (s, MFSPR | RT (0) | LR);
@@ -938,7 +939,7 @@ static void tcg_target_qemu_prologue (TCGContext *s)
     tcg_out_mov (s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
     tcg_out32 (s, MTSPR | RS (tcg_target_call_iarg_regs[1]) | CTR);
     tcg_out32 (s, BCCTR | BO_ALWAYS);
-    tb_ret_addr = s->code_ptr;
+    tb_ret_addr = (intptr_t)s->code_ptr;
 
     for (i = 0; i < ARRAY_SIZE (tcg_target_callee_save_regs); ++i)
         tcg_out32 (s, (LWZ
@@ -1069,14 +1070,13 @@ static void tcg_out_bc (TCGContext *s, int bc, int label_index)
 {
     TCGLabel *l = &s->labels[label_index];
 
-    if (l->has_value)
+    if (l->has_value) {
         tcg_out32 (s, bc | reloc_pc14_val (s->code_ptr, l->u.value));
-    else {
-        uint16_t val = *(uint16_t *) &s->code_ptr[2];
-
+    } else {
         /* Thanks to Andrzej Zaborowski */
-        tcg_out32 (s, bc | (val & 0xfffc));
-        tcg_out_reloc (s, s->code_ptr - 4, R_PPC_REL14, label_index, 0);
+        tcg_itype retrans = *s->code_ptr & 0xfffc;
+        tcg_out_reloc(s, s->code_ptr, R_PPC_REL14, label_index, 0);
+        tcg_out32(s, bc | retrans);
     }
 }
 
@@ -1374,13 +1374,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
         if (s->tb_jmp_offset) {
             /* direct jump method */
 
-            s->tb_jmp_offset[args[0]] = s->code_ptr - s->code_buf;
-            s->code_ptr += 16;
-        }
-        else {
+            s->tb_jmp_offset[args[0]]
+                = (uintptr_t)s->code_ptr - (uintptr_t)s->code_buf;
+            s->code_ptr += 4;
+        } else {
             tcg_abort ();
         }
-        s->tb_next_offset[args[0]] = s->code_ptr - s->code_buf;
+        s->tb_next_offset[args[0]]
+            = (uintptr_t)s->code_ptr - (uintptr_t)s->code_buf;
         break;
     case INDEX_op_br:
         {
@@ -1388,13 +1389,11 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
 
             if (l->has_value) {
                 tcg_out_b (s, 0, l->u.value);
-            }
-            else {
-                uint32_t val = *(uint32_t *) s->code_ptr;
-
+            } else {
                 /* Thanks to Andrzej Zaborowski */
-                tcg_out32 (s, B | (val & 0x3fffffc));
-                tcg_out_reloc (s, s->code_ptr - 4, R_PPC_REL24, args[0], 0);
+                tcg_itype retrans = *s->code_ptr & 0x3fffffc;
+                tcg_out_reloc(s, s->code_ptr, R_PPC_REL24, args[0], 0);
+                tcg_out32(s, B | retrans);
             }
         }
         break;
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index e3395e3..52ace65 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -26,6 +26,7 @@
 
 #define TCG_TARGET_WORDS_BIGENDIAN
 #define TCG_TARGET_NB_REGS 32
+#define TCG_TARGET_ITYPE_SIZE 4
 
 typedef enum {
     TCG_REG_R0 = 0,
-- 
1.9.0

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

* [Qemu-devel] [PATCH 7/8] tcg-aarch64: Define TCG_TARGET_ITYPE_SIZE
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
                   ` (5 preceding siblings ...)
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 6/8] tcg-ppc: " Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 8/8] tcg-sparc: " Richard Henderson
  2014-03-29 20:26 ` [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Peter Maydell
  8 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

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

diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
index 661a5af..2614201 100644
--- a/tcg/aarch64/tcg-target.c
+++ b/tcg/aarch64/tcg-target.c
@@ -71,27 +71,23 @@ static const int tcg_target_call_oarg_regs[1] = {
 # endif
 #endif
 
-static inline void reloc_pc26(void *code_ptr, intptr_t target)
+static inline void reloc_pc26(tcg_itype *code_ptr, intptr_t target)
 {
     intptr_t offset = (target - (intptr_t)code_ptr) / 4;
     /* read instruction, mask away previous PC_REL26 parameter contents,
        set the proper offset, then write back the instruction. */
-    uint32_t insn = *(uint32_t *)code_ptr;
-    insn = deposit32(insn, 0, 26, offset);
-    *(uint32_t *)code_ptr = insn;
+    *code_ptr = deposit32(*code_ptr, 0, 26, offset);
 }
 
-static inline void reloc_pc19(void *code_ptr, intptr_t target)
+static inline void reloc_pc19(tcg_itype *code_ptr, intptr_t target)
 {
     intptr_t offset = (target - (intptr_t)code_ptr) / 4;
     /* read instruction, mask away previous PC_REL19 parameter contents,
        set the proper offset, then write back the instruction. */
-    uint32_t insn = *(uint32_t *)code_ptr;
-    insn = deposit32(insn, 5, 19, offset);
-    *(uint32_t *)code_ptr = insn;
+    *code_ptr = deposit32(*code_ptr, 5, 19, offset);
 }
 
-static inline void patch_reloc(uint8_t *code_ptr, int type,
+static inline void patch_reloc(tcg_itype *code_ptr, int type,
                                intptr_t value, intptr_t addend)
 {
     value += addend;
@@ -104,7 +100,6 @@ static inline void patch_reloc(uint8_t *code_ptr, int type,
     case R_AARCH64_CONDBR19:
         reloc_pc19(code_ptr, value);
         break;
-
     default:
         tcg_abort();
     }
@@ -409,8 +404,7 @@ aarch64_ldst_get_type(TCGOpcode tcg_op)
 
 static inline uint32_t tcg_in32(TCGContext *s)
 {
-    uint32_t v = *(uint32_t *)s->code_ptr;
-    return v;
+    return *s->code_ptr;
 }
 
 /* Emit an opcode with "type-checking" of the format.  */
@@ -783,7 +777,7 @@ void aarch64_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr)
         tcg_abort();
     }
 
-    patch_reloc((uint8_t *)jmp_addr, R_AARCH64_JUMP26, target, 0);
+    reloc_pc26((tcg_itype *)jmp_addr, target);
     flush_icache_range(jmp_addr, jmp_addr + 4);
 }
 
@@ -985,7 +979,7 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *lb)
 static void add_qemu_ldst_label(TCGContext *s, int is_ld, int opc,
                                 TCGReg data_reg, TCGReg addr_reg,
                                 int mem_index,
-                                uint8_t *raddr, uint8_t *label_ptr)
+                                tcg_itype *raddr, tcg_itype *label_ptr)
 {
     TCGLabelQemuLdst *label = new_ldst_label(s);
 
@@ -1003,7 +997,7 @@ static void add_qemu_ldst_label(TCGContext *s, int is_ld, int opc,
    the slow path. Generated code returns the host addend in X1,
    clobbers X0,X2,X3,TMP. */
 static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg,
-            int s_bits, uint8_t **label_ptr, int mem_index, int is_read)
+            int s_bits, tcg_itype **label_ptr, int mem_index, int is_read)
 {
     TCGReg base = TCG_AREG0;
     int tlb_offset = is_read ?
@@ -1140,7 +1134,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, int opc)
     TCGReg addr_reg, data_reg;
 #ifdef CONFIG_SOFTMMU
     int mem_index, s_bits;
-    uint8_t *label_ptr;
+    tcg_itype *label_ptr;
 #endif
     data_reg = args[0];
     addr_reg = args[1];
@@ -1163,7 +1157,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, int opc)
     TCGReg addr_reg, data_reg;
 #ifdef CONFIG_SOFTMMU
     int mem_index, s_bits;
-    uint8_t *label_ptr;
+    tcg_itype *label_ptr;
 #endif
     data_reg = args[0];
     addr_reg = args[1];
@@ -1182,7 +1176,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, int opc)
 #endif /* CONFIG_SOFTMMU */
 }
 
-static uint8_t *tb_ret_addr;
+static intptr_t tb_ret_addr;
 
 /* callee stack use example:
    stp     x29, x30, [sp,#-32]!
@@ -1255,7 +1249,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
     switch (opc) {
     case INDEX_op_exit_tb:
         tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_X0, a0);
-        tcg_out_goto(s, (intptr_t)tb_ret_addr);
+        tcg_out_goto(s, tb_ret_addr);
         break;
 
     case INDEX_op_goto_tb:
@@ -1263,11 +1257,11 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
 #error "USE_DIRECT_JUMP required for aarch64"
 #endif
         assert(s->tb_jmp_offset != NULL); /* consistency for USE_DIRECT_JUMP */
-        s->tb_jmp_offset[a0] = s->code_ptr - s->code_buf;
+        s->tb_jmp_offset[a0] = (intptr_t)s->code_ptr - (intptr_t)s->code_buf;
         /* actual branch destination will be patched by
            aarch64_tb_set_jmp_target later, beware retranslation. */
         tcg_out_goto_noaddr(s);
-        s->tb_next_offset[a0] = s->code_ptr - s->code_buf;
+        s->tb_next_offset[a0] = (intptr_t)s->code_ptr - (intptr_t)s->code_buf;
         break;
 
     case INDEX_op_call:
@@ -1820,7 +1814,7 @@ static void tcg_target_qemu_prologue(TCGContext *s)
     tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
     tcg_out_gotor(s, tcg_target_call_iarg_regs[1]);
 
-    tb_ret_addr = s->code_ptr;
+    tb_ret_addr = (intptr_t)s->code_ptr;
 
     /* Remove TCG locals stack space.  */
     tcg_out_insn(s, 3401, ADDI, TCG_TYPE_I64, TCG_REG_SP, TCG_REG_SP,
diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
index 988983e..b6cc72d 100644
--- a/tcg/aarch64/tcg-target.h
+++ b/tcg/aarch64/tcg-target.h
@@ -15,6 +15,7 @@
 
 #undef TCG_TARGET_WORDS_BIGENDIAN
 #undef TCG_TARGET_STACK_GROWSUP
+#define TCG_TARGET_ITYPE_SIZE 4
 
 typedef enum {
     TCG_REG_X0, TCG_REG_X1, TCG_REG_X2, TCG_REG_X3, TCG_REG_X4,
-- 
1.9.0

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

* [Qemu-devel] [PATCH 8/8] tcg-sparc: Define TCG_TARGET_ITYPE_SIZE
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
                   ` (6 preceding siblings ...)
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 7/8] tcg-aarch64: " Richard Henderson
@ 2014-03-29  0:27 ` Richard Henderson
  2014-03-29 20:26 ` [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Peter Maydell
  8 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-29  0:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, aurelien

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

diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 152335c..44d8995 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -253,37 +253,32 @@ static inline int check_fit_i32(uint32_t val, unsigned int bits)
     return ((val << (32 - bits)) >> (32 - bits)) == val;
 }
 
-static void patch_reloc(uint8_t *code_ptr, int type,
+static void patch_reloc(tcg_itype *code_ptr, int type,
                         intptr_t value, intptr_t addend)
 {
     uint32_t insn;
+
     value += addend;
     switch (type) {
-    case R_SPARC_32:
-        if (value != (uint32_t)value) {
-            tcg_abort();
-        }
-        *(uint32_t *)code_ptr = value;
-        break;
     case R_SPARC_WDISP16:
         value -= (intptr_t)code_ptr;
         if (!check_fit_tl(value >> 2, 16)) {
             tcg_abort();
         }
-        insn = *(uint32_t *)code_ptr;
+        insn = *code_ptr;
         insn &= ~INSN_OFF16(-1);
         insn |= INSN_OFF16(value);
-        *(uint32_t *)code_ptr = insn;
+        *code_ptr = insn;
         break;
     case R_SPARC_WDISP19:
         value -= (intptr_t)code_ptr;
         if (!check_fit_tl(value >> 2, 19)) {
             tcg_abort();
         }
-        insn = *(uint32_t *)code_ptr;
+        insn = *code_ptr;
         insn &= ~INSN_OFF19(-1);
         insn |= INSN_OFF19(value);
-        *(uint32_t *)code_ptr = insn;
+        *code_ptr = insn;
         break;
     default:
         tcg_abort();
@@ -544,10 +539,10 @@ static void tcg_out_bpcc(TCGContext *s, int scond, int flags, int label)
     int off19;
 
     if (l->has_value) {
-        off19 = INSN_OFF19(l->u.value - (unsigned long)s->code_ptr);
+        off19 = INSN_OFF19(l->u.value - (uintptr_t)s->code_ptr);
     } else {
         /* Make sure to preserve destinations during retranslation.  */
-        off19 = *(uint32_t *)s->code_ptr & INSN_OFF19(-1);
+        off19 = *s->code_ptr & INSN_OFF19(-1);
         tcg_out_reloc(s, s->code_ptr, R_SPARC_WDISP19, label, 0);
     }
     tcg_out_bpcc0(s, scond, flags, off19);
@@ -592,10 +587,10 @@ static void tcg_out_brcond_i64(TCGContext *s, TCGCond cond, TCGArg arg1,
         int off16;
 
         if (l->has_value) {
-            off16 = INSN_OFF16(l->u.value - (unsigned long)s->code_ptr);
+            off16 = INSN_OFF16(l->u.value - (uintptr_t)s->code_ptr);
         } else {
             /* Make sure to preserve destinations during retranslation.  */
-            off16 = *(uint32_t *)s->code_ptr & INSN_OFF16(-1);
+            off16 = *s->code_ptr & INSN_OFF16(-1);
             tcg_out_reloc(s, s->code_ptr, R_SPARC_WDISP16, label, 0);
         }
         tcg_out32(s, INSN_OP(0) | INSN_OP2(3) | BPR_PT | INSN_RS1(arg1)
@@ -1046,7 +1041,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
     TCGReg addrz, param;
     uintptr_t func;
     int memi;
-    uint32_t *label_ptr[2];
+    tcg_itype *label_ptr[2];
 #endif
 
     datalo = *args++;
@@ -1065,7 +1060,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
         int reg64;
 
         /* bne,pn %[xi]cc, label0 */
-        label_ptr[0] = (uint32_t *)s->code_ptr;
+        label_ptr[0] = s->code_ptr;
         tcg_out_bpcc0(s, COND_NE, BPCC_PN
                       | (TARGET_LONG_BITS == 64 ? BPCC_XCC : BPCC_ICC), 0);
         tcg_out_nop(s);
@@ -1082,7 +1077,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
         }
 
         /* b,a,pt label1 */
-        label_ptr[1] = (uint32_t *)s->code_ptr;
+        label_ptr[1] = s->code_ptr;
         tcg_out_bpcc0(s, COND_A, BPCC_A | BPCC_PT, 0);
     } else {
         /* The fast path is exactly one insn.  Thus we can perform the
@@ -1091,7 +1086,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
 
         /* beq,a,pt %[xi]cc, label0 */
         label_ptr[0] = NULL;
-        label_ptr[1] = (uint32_t *)s->code_ptr;
+        label_ptr[1] = s->code_ptr;
         tcg_out_bpcc0(s, COND_E, BPCC_A | BPCC_PT
                       | (TARGET_LONG_BITS == 64 ? BPCC_XCC : BPCC_ICC), 0);
         /* delay slot */
@@ -1101,8 +1096,8 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
     /* TLB Miss.  */
 
     if (label_ptr[0]) {
-        *label_ptr[0] |= INSN_OFF19((unsigned long)s->code_ptr -
-                                    (unsigned long)label_ptr[0]);
+        *label_ptr[0] |= INSN_OFF19((uintptr_t)s->code_ptr -
+                                    (uintptr_t)label_ptr[0]);
     }
 
     param = TCG_REG_O1;
@@ -1140,8 +1135,8 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args, bool is64)
         break;
     }
 
-    *label_ptr[1] |= INSN_OFF19((unsigned long)s->code_ptr -
-                                (unsigned long)label_ptr[1]);
+    *label_ptr[1] |= INSN_OFF19((uintptr_t)s->code_ptr -
+                                (uintptr_t)label_ptr[1]);
 #else
     if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
         tcg_out_arithi(s, TCG_REG_T1, addrlo, 0, SHIFT_SRL);
@@ -1174,7 +1169,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
     TCGReg addrz, datafull, param;
     uintptr_t func;
     int memi;
-    uint32_t *label_ptr;
+    tcg_itype *label_ptr;
 #endif
 
     datalo = *args++;
@@ -1201,7 +1196,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
     /* The fast path is exactly one insn.  Thus we can perform the entire
        TLB Hit in the (annulled) delay slot of the branch over TLB Miss.  */
     /* beq,a,pt %[xi]cc, label0 */
-    label_ptr = (uint32_t *)s->code_ptr;
+    label_ptr = s->code_ptr;
     tcg_out_bpcc0(s, COND_E, BPCC_A | BPCC_PT
                   | (TARGET_LONG_BITS == 64 ? BPCC_XCC : BPCC_ICC), 0);
     /* delay slot */
@@ -1225,8 +1220,8 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
     /* delay slot */
     tcg_out_movi(s, TCG_TYPE_REG, param, memi);
 
-    *label_ptr |= INSN_OFF19((unsigned long)s->code_ptr -
-                             (unsigned long)label_ptr);
+    *label_ptr |= INSN_OFF19((uintptr_t)s->code_ptr -
+                             (uintptr_t)label_ptr);
 #else
     if (TCG_TARGET_REG_BITS == 64 && TARGET_LONG_BITS == 32) {
         tcg_out_arithi(s, TCG_REG_T1, addrlo, 0, SHIFT_SRL);
@@ -1259,8 +1254,9 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
     case INDEX_op_goto_tb:
         if (s->tb_jmp_offset) {
             /* direct jump method */
-            uint32_t old_insn = *(uint32_t *)s->code_ptr;
-            s->tb_jmp_offset[args[0]] = s->code_ptr - s->code_buf;
+            uint32_t old_insn = *s->code_ptr;
+            s->tb_jmp_offset[args[0]]
+                = (uintptr_t)s->code_ptr - (uintptr_t)s->code_buf;
             /* Make sure to preserve links during retranslation.  */
             tcg_out32(s, CALL | (old_insn & ~INSN_OP(-1)));
         } else {
@@ -1269,7 +1265,8 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
             tcg_out_arithi(s, TCG_REG_G0, TCG_REG_T1, 0, JMPL);
         }
         tcg_out_nop(s);
-        s->tb_next_offset[args[0]] = s->code_ptr - s->code_buf;
+        s->tb_next_offset[args[0]]
+            = (uintptr_t)s->code_ptr - (uintptr_t)s->code_buf;
         break;
     case INDEX_op_call:
         if (const_args[0]) {
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index 3abf1b4..ab00af1 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -33,7 +33,7 @@
 #endif
 
 #define TCG_TARGET_WORDS_BIGENDIAN
-
+#define TCG_TARGET_ITYPE_SIZE 4
 #define TCG_TARGET_NB_REGS 32
 
 typedef enum {
-- 
1.9.0

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

* Re: [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr
  2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
                   ` (7 preceding siblings ...)
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 8/8] tcg-sparc: " Richard Henderson
@ 2014-03-29 20:26 ` Peter Maydell
  2014-03-30 15:31   ` Richard Henderson
  8 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2014-03-29 20:26 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers, Aurelien Jarno

On 29 March 2014 00:27, Richard Henderson <rth@twiddle.net> wrote:
> Here's where I think we should go with the cleanup that Peter started.
>
> I've only converted a couple of the backends as examples.  It's not 100%
> mechanical, since one has to be aware of the change in the semantics of
> pointer arithmetic (e.g. s->code_ptr - s->code_buf).
>
> Taking Sparc as an example, the before code size of tcg.o is 0xad60, and
> the after code size is 0xa200.  A savings of a bit less than 3k.
>
> Thoughts?

Looks good to me -- I like the way a lot of the casts go away.
It seems like it might be worth abstracting out "give me the byte
difference between these two code pointers" rather than having
inline (uintptr_t)codeptr1 - (uintptr_t)codeptr2, but I dunno.

Is there a better name than 'tcg_itype' ? Putting 'type' in the
name of a type is a bit redundant, and suggests it contains
a type rather than an insn.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr
  2014-03-29 20:26 ` [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Peter Maydell
@ 2014-03-30 15:31   ` Richard Henderson
  2014-03-31  3:19     ` Richard Henderson
  2014-04-01 12:05     ` Alex Bennée
  0 siblings, 2 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-30 15:31 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, Aurelien Jarno

On 03/29/2014 01:26 PM, Peter Maydell wrote:
> It seems like it might be worth abstracting out "give me the byte
> difference between these two code pointers" rather than having
> inline (uintptr_t)codeptr1 - (uintptr_t)codeptr2, but I dunno.

Yeah, I dithered about that.  I couldn't come up with a good name that
adequately described what I wanted that was really any shorter than just having
the casts.

> Is there a better name than 'tcg_itype' ? Putting 'type' in the
> name of a type is a bit redundant, and suggests it contains
> a type rather than an insn.

I'm open to suggestions there as well.  On x86 and ia64, it won't hold an
entire insn, so "tcg_insn" seemed inappropriate.


r~

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

* Re: [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr
  2014-03-30 15:31   ` Richard Henderson
@ 2014-03-31  3:19     ` Richard Henderson
  2014-04-01 12:05     ` Alex Bennée
  1 sibling, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2014-03-31  3:19 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, Aurelien Jarno

On 03/30/2014 08:31 AM, Richard Henderson wrote:
> On 03/29/2014 01:26 PM, Peter Maydell wrote:
>> It seems like it might be worth abstracting out "give me the byte
>> difference between these two code pointers" rather than having
>> inline (uintptr_t)codeptr1 - (uintptr_t)codeptr2, but I dunno.
> 
> Yeah, I dithered about that.  I couldn't come up with a good name that
> adequately described what I wanted that was really any shorter than just having
> the casts.

static inline ptrdiff_t tcg_ptr_byte_diff(void *a, void *b)
{
    return a - b;
}

static inline ptrdiff_t tcg_pcrel_diff(TCGContext *s, void *target)
{
    return tcg_ptr_byte_diff(target, s->code_ptr);
}

static inline size_t tcg_current_code_size(TCGContext *s)
{
    return tcg_ptr_byte_diff(s->code_ptr, s->code_buf);
}

>> Is there a better name than 'tcg_itype' ? Putting 'type' in the
>> name of a type is a bit redundant, and suggests it contains
>> a type rather than an insn.
> 
> I'm open to suggestions there as well.  On x86 and ia64, it won't hold an
> entire insn, so "tcg_insn" seemed inappropriate.

tcg_{isa,insn}_{part,elt} ?


r~

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

* Re: [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr
  2014-03-30 15:31   ` Richard Henderson
  2014-03-31  3:19     ` Richard Henderson
@ 2014-04-01 12:05     ` Alex Bennée
  2014-04-01 12:19       ` Alex Bennée
  1 sibling, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2014-04-01 12:05 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Peter Maydell, QEMU Developers, Aurelien Jarno


Richard Henderson <rth@twiddle.net> writes:

> On 03/29/2014 01:26 PM, Peter Maydell wrote:
>> It seems like it might be worth abstracting out "give me the byte
>> difference between these two code pointers" rather than having
>> inline (uintptr_t)codeptr1 - (uintptr_t)codeptr2, but I dunno.
>
> Yeah, I dithered about that.  I couldn't come up with a good name that
> adequately described what I wanted that was really any shorter than just having
> the casts.
>
>> Is there a better name than 'tcg_itype' ? Putting 'type' in the
>> name of a type is a bit redundant, and suggests it contains
>> a type rather than an insn.
>
> I'm open to suggestions there as well.  On x86 and ia64, it won't hold an
> entire insn, so "tcg_insn" seemed inappropriate.
<snip>

tcg_insn_stream?

-- 
Alex Bennée

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

* Re: [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefinedbehaviour patching x86 jumpss
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps Richard Henderson
@ 2014-04-01 12:09   ` Alex Bennée
  2014-04-01 12:18     ` Peter Maydell
  0 siblings, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2014-04-01 12:09 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-devel, aurelien


Richard Henderson <rth@twiddle.net> writes:

> From: Peter Maydell <peter.maydell@linaro.org>
>
> The code which patches x86 jump instructions assumes it can do an
> unaligned write of a uint32_t. This is actually safe on x86, but it's
> still undefined behaviour. We have infrastructure for doing efficient
> unaligned accesses which doesn't engage in undefined behaviour, so
> use it.
>
> This is technically fractionally less efficient, at least with gcc 4.6;
> instead of one instruction:
>  7b2:   89 3e                   mov    %edi,(%rsi)
> we get an extra spurious store to the stack slot:
>  7b2:   89 7c 24 64             mov    %edi,0x64(%rsp)
>  7b6:   89 3e                   mov    %edi,(%rsi)

Ehh? Is that gcc just being silly and putting parameters for an inline
on the stack frame?

>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
<snip>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

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

* Re: [Qemu-devel] [PATCH 2/8] tcg: Avoid stores to unaligned addresses
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 2/8] tcg: Avoid stores to unaligned addresses Richard Henderson
@ 2014-04-01 12:12   ` Alex Bennée
  0 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2014-04-01 12:12 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-devel, aurelien


Richard Henderson <rth@twiddle.net> writes:

> From: Peter Maydell <peter.maydell@linaro.org>
>
> Avoid stores to unaligned addresses in TCG code generation, by using the
> usual memcpy() approach. (Using bswap.h would drag a lot of QEMU baggage
> into TCG, so it's simpler just to do direct memcpy() here.)

I notice bswap.h has an interesting exultation to the efficiency of
inline memcpy's which I hope is justified.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/tcg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index f1e0763..60f06c5 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -125,21 +125,21 @@ static inline void tcg_out8(TCGContext *s, uint8_t v)
>  static inline void tcg_out16(TCGContext *s, uint16_t v)
>  {
>      uint8_t *p = s->code_ptr;
> -    *(uint16_t *)p = v;
> +    memcpy(p, &v, sizeof(v));
>      s->code_ptr = p + 2;
>  }
>  
>  static inline void tcg_out32(TCGContext *s, uint32_t v)
>  {
>      uint8_t *p = s->code_ptr;
> -    *(uint32_t *)p = v;
> +    memcpy(p, &v, sizeof(v));
>      s->code_ptr = p + 4;
>  }
>  
>  static inline void tcg_out64(TCGContext *s, uint64_t v)
>  {
>      uint8_t *p = s->code_ptr;
> -    *(uint64_t *)p = v;
> +    memcpy(p, &v, sizeof(v));
>      s->code_ptr = p + 8;
>  }

-- 
Alex Bennée

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

* Re: [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patchingcode at unaligned addressess
  2014-03-29  0:27 ` [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patching code at " Richard Henderson
@ 2014-04-01 12:13   ` Alex Bennée
  0 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2014-04-01 12:13 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-devel, aurelien


Richard Henderson <rth@twiddle.net> writes:

> From: Peter Maydell <peter.maydell@linaro.org>
>
> To avoid C undefined behaviour when patching generated code,
> provide wrappers tcg_patch8/16/32/64 which use the usual memcpy
> trick, and use them in the i386 backend.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

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

* Re: [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefinedbehaviour patching x86 jumpss
  2014-04-01 12:09   ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefinedbehaviour patching x86 jumpss Alex Bennée
@ 2014-04-01 12:18     ` Peter Maydell
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2014-04-01 12:18 UTC (permalink / raw)
  To: Alex Bennée; +Cc: QEMU Developers, Aurelien Jarno, Richard Henderson

On 1 April 2014 13:09, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Richard Henderson <rth@twiddle.net> writes:
>
>> From: Peter Maydell <peter.maydell@linaro.org>
>>
>> The code which patches x86 jump instructions assumes it can do an
>> unaligned write of a uint32_t. This is actually safe on x86, but it's
>> still undefined behaviour. We have infrastructure for doing efficient
>> unaligned accesses which doesn't engage in undefined behaviour, so
>> use it.
>>
>> This is technically fractionally less efficient, at least with gcc 4.6;
>> instead of one instruction:
>>  7b2:   89 3e                   mov    %edi,(%rsi)
>> we get an extra spurious store to the stack slot:
>>  7b2:   89 7c 24 64             mov    %edi,0x64(%rsp)
>>  7b6:   89 3e                   mov    %edi,(%rsi)
>
> Ehh? Is that gcc just being silly and putting parameters for an inline
> on the stack frame?

It's gcc being dumb and not noticing that it has no requirement
to store the inline parameter to the stack frame because it's
optimised away the reference to the address of the parameter.
Possibly more recent gcc versions do better.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr
  2014-04-01 12:05     ` Alex Bennée
@ 2014-04-01 12:19       ` Alex Bennée
  0 siblings, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2014-04-01 12:19 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Peter Maydell, QEMU Developers, Aurelien Jarno


Alex Bennée <alex.bennee@linaro.org> writes:

> Richard Henderson <rth@twiddle.net> writes:
>
>> On 03/29/2014 01:26 PM, Peter Maydell wrote:
<snip>
>>> Is there a better name than 'tcg_itype' ? Putting 'type' in the
>>> name of a type is a bit redundant, and suggests it contains
>>> a type rather than an insn.
>>
>> I'm open to suggestions there as well.  On x86 and ia64, it won't hold an
>> entire insn, so "tcg_insn" seemed inappropriate.
> <snip>
>
> tcg_insn_stream?

Or even the briefer tcg_istream?

-- 
Alex Bennée

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

* Re: [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr
@ 2014-03-31  8:25 Jay Foad
  0 siblings, 0 replies; 19+ messages in thread
From: Jay Foad @ 2014-03-31  8:25 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

>>> Is there a better name than 'tcg_itype' ? Putting 'type' in the
>>> name of a type is a bit redundant, and suggests it contains
>>> a type rather than an insn.
>>
>> I'm open to suggestions there as well.  On x86 and ia64, it won't hold an
>> entire insn, so "tcg_insn" seemed inappropriate.
>
> tcg_{isa,insn}_{part,elt} ?

tcg_insn_unit ?

Jay.

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

end of thread, other threads:[~2014-04-01 12:59 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-29  0:27 [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Richard Henderson
2014-03-29  0:27 ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps Richard Henderson
2014-04-01 12:09   ` [Qemu-devel] [PATCH 1/8] exec-all.h: Use stl_p to avoid undefinedbehaviour patching x86 jumpss Alex Bennée
2014-04-01 12:18     ` Peter Maydell
2014-03-29  0:27 ` [Qemu-devel] [PATCH 2/8] tcg: Avoid stores to unaligned addresses Richard Henderson
2014-04-01 12:12   ` Alex Bennée
2014-03-29  0:27 ` [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patching code at " Richard Henderson
2014-04-01 12:13   ` [Qemu-devel] [PATCH 3/8] tcg: Avoid undefined behaviour patchingcode at unaligned addressess Alex Bennée
2014-03-29  0:27 ` [Qemu-devel] [PATCH 4/8] tcg: Define tcg_itype for code pointers Richard Henderson
2014-03-29  0:27 ` [Qemu-devel] [PATCH 5/8] tcg-ppc64: Define TCG_TARGET_ITYPE_SIZE Richard Henderson
2014-03-29  0:27 ` [Qemu-devel] [PATCH 6/8] tcg-ppc: " Richard Henderson
2014-03-29  0:27 ` [Qemu-devel] [PATCH 7/8] tcg-aarch64: " Richard Henderson
2014-03-29  0:27 ` [Qemu-devel] [PATCH 8/8] tcg-sparc: " Richard Henderson
2014-03-29 20:26 ` [Qemu-devel] [PATCH 0/8] tcg: tidy the type of code_ptr Peter Maydell
2014-03-30 15:31   ` Richard Henderson
2014-03-31  3:19     ` Richard Henderson
2014-04-01 12:05     ` Alex Bennée
2014-04-01 12:19       ` Alex Bennée
2014-03-31  8:25 Jay Foad

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.