[Qemu-devel] [PATCH 0/2] tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

[Qemu-devel] [PATCH 0/2] tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

[Peter Maydell]

This patch series enhances the type checking of TCG values done when
compiling with debugging enabled, so that it can detect confusion of
TCGv_ptr values with whichever of TCGv_i32 and TCGv_i64 corresponds to
the pointer-width type on the compile host. This means that such
errors will always be compile failures, rather than only failing on
one of the two kinds of compile host.

In particular, this would have caught the recent compile error
I introduced...

I've tested this on both 32 and 64 bit hosts, with a full compile
for all targets. Note that to get a clean debug compile for both
hosts (whether with or without this patchset) you'll need to have
applied the following recent patches which all fix compile problems:

 * http://patchwork.ozlabs.org/patch/97559/
   (target-arm: Fix compilation failure for 64 bit hosts)
 * http://patchwork.ozlabs.org/patch/97418/
   (target-s390x: Fix wrong argument in call of tcg_gen_shl_i64())
 * http://patchwork.ozlabs.org/patch/96665/
   (ppc: Fix compilation for ppc64-softmmu)

Peter Maydell (2):
  tcg/tcg-op.h: Fix prototypes for ld/st functions on 64 bit hosts
  tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

 tcg/tcg-op.h |   48 +++++++++++++++++++++++++++---------------------
 tcg/tcg.h    |   52 ++++++++++++++++++++++++++++++++++------------------
 2 files changed, 61 insertions(+), 39 deletions(-)

[Qemu-devel] [PATCH 1/2] tcg/tcg-op.h: Fix prototypes for ld/st functions on 64 bit hosts

[Peter Maydell]

The prototypes for the ld/st functions on a 64 bit host declared
the address parameter as a TCGv_i64 rather than a TCGv_ptr. This
worked OK (since the two are aliases), but needs to be fixed to
allow extension of TCG type debugging to i64/i32/ptr mismatches.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tcg/tcg-op.h |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 207a89f..6529655 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -1063,66 +1063,66 @@ static inline void tcg_gen_movi_i64(TCGv_i64 ret, int64_t arg)
     tcg_gen_op2i_i64(INDEX_op_movi_i64, ret, arg);
 }
 
-static inline void tcg_gen_ld8u_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld8u_i64(TCGv_i64 ret, TCGv_ptr arg2,
                                     tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_ld8u_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld8s_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld8s_i64(TCGv_i64 ret, TCGv_ptr arg2,
                                     tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_ld8s_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld16u_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld16u_i64(TCGv_i64 ret, TCGv_ptr arg2,
                                      tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_ld16u_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld16s_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld16s_i64(TCGv_i64 ret, TCGv_ptr arg2,
                                      tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_ld16s_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld32u_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld32u_i64(TCGv_i64 ret, TCGv_ptr arg2,
                                      tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_ld32u_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld32s_i64(TCGv_i64 ret, TCGv_i64 arg2,
+static inline void tcg_gen_ld32s_i64(TCGv_i64 ret, TCGv_ptr arg2,
                                      tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_ld32s_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_ld_i64(TCGv_i64 ret, TCGv_i64 arg2, tcg_target_long offset)
+static inline void tcg_gen_ld_i64(TCGv_i64 ret, TCGv_ptr arg2, tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_ld_i64, ret, arg2, offset);
 }
 
-static inline void tcg_gen_st8_i64(TCGv_i64 arg1, TCGv_i64 arg2,
+static inline void tcg_gen_st8_i64(TCGv_i64 arg1, TCGv_ptr arg2,
                                    tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_st8_i64, arg1, arg2, offset);
 }
 
-static inline void tcg_gen_st16_i64(TCGv_i64 arg1, TCGv_i64 arg2,
+static inline void tcg_gen_st16_i64(TCGv_i64 arg1, TCGv_ptr arg2,
                                     tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_st16_i64, arg1, arg2, offset);
 }
 
-static inline void tcg_gen_st32_i64(TCGv_i64 arg1, TCGv_i64 arg2,
+static inline void tcg_gen_st32_i64(TCGv_i64 arg1, TCGv_ptr arg2,
                                     tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_st32_i64, arg1, arg2, offset);
 }
 
-static inline void tcg_gen_st_i64(TCGv_i64 arg1, TCGv_i64 arg2, tcg_target_long offset)
+static inline void tcg_gen_st_i64(TCGv_i64 arg1, TCGv_ptr arg2, tcg_target_long offset)
 {
     tcg_gen_ldst_op_i64(INDEX_op_st_i64, arg1, arg2, offset);
 }
-- 
1.7.1

[Qemu-devel] [PATCH 2/2] tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

[Peter Maydell]

When compiling with DEBUG_TCGV enabled, make the TCGv_ptr type distinct
from TCGv_i32/TCGv_i64. This means that using an i32 or i64 TCG op to
manipulate a TCGv_ptr will always be detected at compile time, rather
than only if compiling on a host system with the other word size.

NB: the tcg_add_ptr and tcg_sub_ptr macros have been removed as they
were not used anywhere.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tcg/tcg-op.h |   26 ++++++++++++++++----------
 tcg/tcg.h    |   52 ++++++++++++++++++++++++++++++++++------------------
 2 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 6529655..ebf5e13 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -2304,8 +2304,8 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv addr, int mem_index)
 #endif
 }
 
-#define tcg_gen_ld_ptr tcg_gen_ld_i32
-#define tcg_gen_discard_ptr tcg_gen_discard_i32
+#define tcg_gen_ld_ptr(R, A, O) tcg_gen_ld_i32(TCGV_PTR_TO_NAT(R), (A), (O))
+#define tcg_gen_discard_ptr(A) tcg_gen_discard_i32(TCGV_PTR_TO_NAT(A))
 
 #else /* TCG_TARGET_REG_BITS == 32 */
 
@@ -2372,8 +2372,8 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv addr, int mem_index)
     tcg_gen_qemu_ldst_op_i64(INDEX_op_qemu_st64, arg, addr, mem_index);
 }
 
-#define tcg_gen_ld_ptr tcg_gen_ld_i64
-#define tcg_gen_discard_ptr tcg_gen_discard_i64
+#define tcg_gen_ld_ptr(R, A, O) tcg_gen_ld_i64(TCGV_PTR_TO_NAT(R), (A), (O))
+#define tcg_gen_discard_ptr(A) tcg_gen_discard_i64(TCGV_PTR_TO_NAT(A))
 
 #endif /* TCG_TARGET_REG_BITS != 32 */
 
@@ -2523,11 +2523,17 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv addr, int mem_index)
 #endif
 
 #if TCG_TARGET_REG_BITS == 32
-#define tcg_gen_add_ptr tcg_gen_add_i32
-#define tcg_gen_addi_ptr tcg_gen_addi_i32
-#define tcg_gen_ext_i32_ptr tcg_gen_mov_i32
+#define tcg_gen_add_ptr(R, A, B) tcg_gen_add_i32(TCGV_PTR_TO_NAT(R), \
+                                               TCGV_PTR_TO_NAT(A), \
+                                               TCGV_PTR_TO_NAT(B))
+#define tcg_gen_addi_ptr(R, A, B) tcg_gen_addi_i32(TCGV_PTR_TO_NAT(R), \
+                                                 TCGV_PTR_TO_NAT(A), (B))
+#define tcg_gen_ext_i32_ptr(R, A) tcg_gen_mov_i32(TCGV_PTR_TO_NAT(R), (A))
 #else /* TCG_TARGET_REG_BITS == 32 */
-#define tcg_gen_add_ptr tcg_gen_add_i64
-#define tcg_gen_addi_ptr tcg_gen_addi_i64
-#define tcg_gen_ext_i32_ptr tcg_gen_ext_i32_i64
+#define tcg_gen_add_ptr(R, A, B) tcg_gen_add_i64(TCGV_PTR_TO_NAT(R), \
+                                               TCGV_PTR_TO_NAT(A), \
+                                               TCGV_PTR_TO_NAT(B))
+#define tcg_gen_addi_ptr(R, A, B) tcg_gen_addi_i64(TCGV_PTR_TO_NAT(R),   \
+                                                 TCGV_PTR_TO_NAT(A), (B))
+#define tcg_gen_ext_i32_ptr(R, A) tcg_gen_ext_i32_i64(TCGV_PTR_TO_NAT(R), (A))
 #endif /* TCG_TARGET_REG_BITS != 32 */
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 2b985ac..1ec7a51 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -150,12 +150,19 @@ typedef struct
     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)
@@ -165,10 +172,17 @@ typedef struct
 
 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)
 
 #if TCG_TARGET_REG_BITS == 32
 #define TCGV_LOW(t) (t)
@@ -459,25 +473,27 @@ do {\
 void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdefs);
 
 #if TCG_TARGET_REG_BITS == 32
-#define tcg_const_ptr tcg_const_i32
-#define tcg_add_ptr tcg_add_i32
-#define tcg_sub_ptr tcg_sub_i32
-#define TCGv_ptr TCGv_i32
-#define GET_TCGV_PTR GET_TCGV_I32
-#define tcg_global_reg_new_ptr tcg_global_reg_new_i32
-#define tcg_global_mem_new_ptr tcg_global_mem_new_i32
-#define tcg_temp_new_ptr tcg_temp_new_i32
-#define tcg_temp_free_ptr tcg_temp_free_i32
+#define TCGV_NAT_TO_PTR(n) MAKE_TCGV_PTR(GET_TCGV_I32(n))
+#define TCGV_PTR_TO_NAT(n) MAKE_TCGV_I32(GET_TCGV_PTR(n))
+
+#define tcg_const_ptr(V) TCGV_NAT_TO_PTR(tcg_const_i32(V))
+#define tcg_global_reg_new_ptr(R, N) \
+    TCGV_NAT_TO_PTR(tcg_global_reg_new_i32((R), (N)))
+#define tcg_global_mem_new_ptr(R, O, N) \
+    TCGV_NAT_TO_PTR(tcg_global_mem_new_i32((R), (O), (N)))
+#define tcg_temp_new_ptr() TCGV_NAT_TO_PTR(tcg_temp_new_i32())
+#define tcg_temp_free_ptr(T) tcg_temp_free_i32(TCGV_PTR_TO_NAT(T))
 #else
-#define tcg_const_ptr tcg_const_i64
-#define tcg_add_ptr tcg_add_i64
-#define tcg_sub_ptr tcg_sub_i64
-#define TCGv_ptr TCGv_i64
-#define GET_TCGV_PTR GET_TCGV_I64
-#define tcg_global_reg_new_ptr tcg_global_reg_new_i64
-#define tcg_global_mem_new_ptr tcg_global_mem_new_i64
-#define tcg_temp_new_ptr tcg_temp_new_i64
-#define tcg_temp_free_ptr tcg_temp_free_i64
+#define TCGV_NAT_TO_PTR(n) MAKE_TCGV_PTR(GET_TCGV_I64(n))
+#define TCGV_PTR_TO_NAT(n) MAKE_TCGV_I64(GET_TCGV_PTR(n))
+
+#define tcg_const_ptr(V) TCGV_NAT_TO_PTR(tcg_const_i64(V))
+#define tcg_global_reg_new_ptr(R, N) \
+    TCGV_NAT_TO_PTR(tcg_global_reg_new_i64((R), (N)))
+#define tcg_global_mem_new_ptr(R, O, N) \
+    TCGV_NAT_TO_PTR(tcg_global_mem_new_i64((R), (O), (N)))
+#define tcg_temp_new_ptr() TCGV_NAT_TO_PTR(tcg_temp_new_i64())
+#define tcg_temp_free_ptr(T) tcg_temp_free_i64(TCGV_PTR_TO_NAT(T))
 #endif
 
 void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
-- 
1.7.1

Re: [Qemu-devel] [PATCH 0/2] tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64

[Aurelien Jarno]

On Fri, May 27, 2011 at 01:12:11PM +0100, Peter Maydell wrote:
> This patch series enhances the type checking of TCG values done when
> compiling with debugging enabled, so that it can detect confusion of
> TCGv_ptr values with whichever of TCGv_i32 and TCGv_i64 corresponds to
> the pointer-width type on the compile host. This means that such
> errors will always be compile failures, rather than only failing on
> one of the two kinds of compile host.
> 
> In particular, this would have caught the recent compile error
> I introduced...
> 
> I've tested this on both 32 and 64 bit hosts, with a full compile
> for all targets. Note that to get a clean debug compile for both
> hosts (whether with or without this patchset) you'll need to have
> applied the following recent patches which all fix compile problems:
> 
>  * http://patchwork.ozlabs.org/patch/97559/
>    (target-arm: Fix compilation failure for 64 bit hosts)
>  * http://patchwork.ozlabs.org/patch/97418/
>    (target-s390x: Fix wrong argument in call of tcg_gen_shl_i64())
>  * http://patchwork.ozlabs.org/patch/96665/
>    (ppc: Fix compilation for ppc64-softmmu)
> 
> Peter Maydell (2):
>   tcg/tcg-op.h: Fix prototypes for ld/st functions on 64 bit hosts
>   tcg: If DEBUG_TCGV, distinguish TCGv_ptr from TCGv_i32/TCGv_i64
> 
>  tcg/tcg-op.h |   48 +++++++++++++++++++++++++++---------------------
>  tcg/tcg.h    |   52 ++++++++++++++++++++++++++++++++++------------------
>  2 files changed, 61 insertions(+), 39 deletions(-)
> 

Thanks, both applied.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net