Re: [Qemu-devel] [PATCH] tcg: Initialize cpu_env generically

2017-10-12 Thread Richard Henderson 
On 10/12/2017 01:32 PM, Lluís Vilanova wrote:
> Richard Henderson writes:
> 
>> On 10/11/2017 11:43 PM, Lluís Vilanova wrote:
 /* Track which vCPU triggers events */
 CPUState *cpu;  /* *_trans */
 -TCGv_env tcg_env;   /* *_exec  */
>>>
>>> I would rather keep it here instead of making a new global variable, since 
>>> that
>>> should make it easier in the future to have multiple translation contexts.
> 
>> Why do you believe this prevents it?  The variable is literally identical for
>> *all* targets.
> 
> If someone decides to make tcg_ctx thread-local or have one per target
> architecture (supporting multiple target archs concurrently), having all that
> info on the tcg_ctx object is easier to track and modify, compared to having
> multiple global variables.

We have patches on-list that do exactly this, from Emilio Cota.  I'm in the
process of reviewing them now.

We've made sure that patch set works with the cpu_* global variables created
once within target/*/translate.c.  This one is no different.


r~

Re: [Qemu-devel] [PATCH] tcg: Initialize cpu_env generically

2017-10-12 Thread Lluís Vilanova 
Richard Henderson writes:

> On 10/11/2017 11:43 PM, Lluís Vilanova wrote:
>>> /* Track which vCPU triggers events */
>>> CPUState *cpu;  /* *_trans */
>>> -TCGv_env tcg_env;   /* *_exec  */
>> 
>> I would rather keep it here instead of making a new global variable, since 
>> that
>> should make it easier in the future to have multiple translation contexts.

> Why do you believe this prevents it?  The variable is literally identical for
> *all* targets.

If someone decides to make tcg_ctx thread-local or have one per target
architecture (supporting multiple target archs concurrently), having all that
info on the tcg_ctx object is easier to track and modify, compared to having
multiple global variables.


> r~

> PS: Everyone, please trim context that you don't care about.  If you just add
> two lines in the middle of 1000, I'll not always find it.

So true; I just went with the flow of the list, sorry.


Cheers,
  Lluis

Re: [Qemu-devel] [PATCH] tcg: Initialize cpu_env generically

2017-10-12 Thread Richard Henderson 
On 10/11/2017 11:43 PM, Lluís Vilanova wrote:
>>  /* Track which vCPU triggers events */
>>  CPUState *cpu;  /* *_trans */
>> -TCGv_env tcg_env;   /* *_exec  */
> 
> I would rather keep it here instead of making a new global variable, since 
> that
> should make it easier in the future to have multiple translation contexts.

Why do you believe this prevents it?  The variable is literally identical for
*all* targets.


r~

PS: Everyone, please trim context that you don't care about.  If you just add
two lines in the middle of 1000, I'll not always find it.

Re: [Qemu-devel] [PATCH] tcg: Initialize cpu_env generically

2017-10-12 Thread Lluís Vilanova 
Richard Henderson writes:

> This is identical for each target.  So, move the initialization to
> common code.  Move the variable itself out of tcg_ctx and name it
> cpu_env to minimize changes within targets.

> This also means we can remove tcg_global_reg_new_{ptr,i32,i64},
> since there are no longer global-register temps created by targets.

> Signed-off-by: Richard Henderson 
> ---
>  include/exec/gen-icount.h |  8 
>  target/arm/translate.h|  1 -
>  tcg/tcg.h |  9 +
>  target/alpha/translate.c  |  4 
>  target/arm/translate.c|  4 
>  target/cris/translate.c   |  3 ---
>  target/cris/translate_v10.c   |  2 --
>  target/hppa/translate.c   |  4 
>  target/i386/translate.c   |  3 ---
>  target/lm32/translate.c   |  4 
>  target/m68k/translate.c   |  5 -
>  target/microblaze/translate.c |  4 
>  target/mips/translate.c   |  4 
>  target/moxie/translate.c  |  7 ++-
>  target/nios2/translate.c  |  3 ---
>  target/openrisc/translate.c   |  3 ---
>  target/ppc/translate.c| 10 +++---
>  target/s390x/translate.c  |  6 --
>  target/sh4/translate.c|  7 +--
>  target/sparc/translate.c  |  4 
>  target/tilegx/translate.c |  3 ---
>  target/tricore/translate.c|  6 ++
>  target/unicore32/translate.c  |  4 
>  target/xtensa/translate.c |  3 ---
>  tcg/tcg-op.c  | 30 +++---
>  tcg/tcg.c | 31 +++
>  26 files changed, 35 insertions(+), 137 deletions(-)

> diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
> index 9b3cb14dfa..de52a67ee8 100644
> --- a/include/exec/gen-icount.h
> +++ b/include/exec/gen-icount.h
> @@ -19,7 +19,7 @@ static inline void gen_tb_start(TranslationBlock *tb)
>  count = tcg_temp_new_i32();
>  }
 
> -tcg_gen_ld_i32(count, tcg_ctx.tcg_env,
> +tcg_gen_ld_i32(count, cpu_env,
> -ENV_OFFSET + offsetof(CPUState, icount_decr.u32));
 
>  if (tb->cflags & CF_USE_ICOUNT) {
> @@ -37,7 +37,7 @@ static inline void gen_tb_start(TranslationBlock *tb)
>  tcg_gen_brcondi_i32(TCG_COND_LT, count, 0, exitreq_label);
 
>  if (tb->cflags & CF_USE_ICOUNT) {
> -tcg_gen_st16_i32(count, tcg_ctx.tcg_env,
> +tcg_gen_st16_i32(count, cpu_env,
>   -ENV_OFFSET + offsetof(CPUState, 
> icount_decr.u16.low));
>  }
 
> @@ -62,7 +62,7 @@ static inline void gen_tb_end(TranslationBlock *tb, int 
> num_insns)
>  static inline void gen_io_start(void)
>  {
>  TCGv_i32 tmp = tcg_const_i32(1);
> -tcg_gen_st_i32(tmp, tcg_ctx.tcg_env,
> +tcg_gen_st_i32(tmp, cpu_env,
> -ENV_OFFSET + offsetof(CPUState, can_do_io));
>  tcg_temp_free_i32(tmp);
>  }
> @@ -70,7 +70,7 @@ static inline void gen_io_start(void)
>  static inline void gen_io_end(void)
>  {
>  TCGv_i32 tmp = tcg_const_i32(0);
> -tcg_gen_st_i32(tmp, tcg_ctx.tcg_env,
> +tcg_gen_st_i32(tmp, cpu_env,
> -ENV_OFFSET + offsetof(CPUState, can_do_io));
>  tcg_temp_free_i32(tmp);
>  }
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 3c96aec956..410ba79c0d 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -80,7 +80,6 @@ typedef struct DisasCompare {
>  } DisasCompare;
 
>  /* Share the TCG temporaries common between 32 and 64 bit modes.  */
> -extern TCGv_env cpu_env;
>  extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
>  extern TCGv_i64 cpu_exclusive_addr;
>  extern TCGv_i64 cpu_exclusive_val;
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index b2d42e3136..da1fefd6f1 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -702,7 +702,6 @@ struct TCGContext {
 
>  /* Track which vCPU triggers events */
>  CPUState *cpu;  /* *_trans */
> -TCGv_env tcg_env;   /* *_exec  */

I would rather keep it here instead of making a new global variable, since that
should make it easier in the future to have multiple translation contexts.


Thanks,
  Lluis

 
>  /* These structures are private to tcg-target.inc.c.  */
>  #ifdef TCG_TARGET_NEED_LDST_LABELS
> @@ -727,6 +726,7 @@ struct TCGContext {
>  };
 
>  extern TCGContext tcg_ctx;
> +extern TCGv_env cpu_env;
>  extern bool parallel_cpus;
 
>  static inline void tcg_set_insn_param(int op_idx, int arg, TCGArg v)
> @@ -783,9 +783,6 @@ void tcg_set_frame(TCGContext *s, TCGReg reg, intptr_t 
> start, intptr_t size);
 
>  int tcg_global_mem_new_internal(TCGType, TCGv_ptr, intptr_t, const char *);
 
> -TCGv_i32 tcg_global_reg_new_i32(TCGReg reg, const char *name);
> -TCGv_i64 tcg_global_reg_new_i64(TCGReg reg, const char *name);
> -
>  TCGv_i32 tcg_temp_new_internal_i32(int temp_local);
>  TCGv_i64 tcg_temp_new_internal_i64(int temp_local);
 
> @@ -904,8 +901,6 @@ do {\
>  #define TCGV_PTR_TO_NAT(n)

Re: [Qemu-devel] [PATCH] tcg: Initialize cpu_env generically

2017-10-10 Thread Emilio G. Cota 
On Tue, Oct 10, 2017 at 14:45:40 -0700, Richard Henderson wrote:
> This is identical for each target.  So, move the initialization to
> common code.  Move the variable itself out of tcg_ctx and name it
> cpu_env to minimize changes within targets.
> 
> This also means we can remove tcg_global_reg_new_{ptr,i32,i64},
> since there are no longer global-register temps created by targets.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Emilio G. Cota 

E.

Re: [Qemu-devel] [PATCH] tcg: Initialize cpu_env generically

2017-10-10 Thread Philippe Mathieu-Daudé 
On 10/10/2017 06:45 PM, Richard Henderson wrote:
> This is identical for each target.  So, move the initialization to
> common code.  Move the variable itself out of tcg_ctx and name it
> cpu_env to minimize changes within targets.
> 
> This also means we can remove tcg_global_reg_new_{ptr,i32,i64},
> since there are no longer global-register temps created by targets.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  include/exec/gen-icount.h |  8 
>  target/arm/translate.h|  1 -
>  tcg/tcg.h |  9 +
>  target/alpha/translate.c  |  4 
>  target/arm/translate.c|  4 
>  target/cris/translate.c   |  3 ---
>  target/cris/translate_v10.c   |  2 --
>  target/hppa/translate.c   |  4 
>  target/i386/translate.c   |  3 ---
>  target/lm32/translate.c   |  4 
>  target/m68k/translate.c   |  5 -
>  target/microblaze/translate.c |  4 
>  target/mips/translate.c   |  4 
>  target/moxie/translate.c  |  7 ++-
>  target/nios2/translate.c  |  3 ---
>  target/openrisc/translate.c   |  3 ---
>  target/ppc/translate.c| 10 +++---
>  target/s390x/translate.c  |  6 --
>  target/sh4/translate.c|  7 +--
>  target/sparc/translate.c  |  4 
>  target/tilegx/translate.c |  3 ---
>  target/tricore/translate.c|  6 ++
>  target/unicore32/translate.c  |  4 
>  target/xtensa/translate.c |  3 ---
>  tcg/tcg-op.c  | 30 +++---
>  tcg/tcg.c | 31 +++
>  26 files changed, 35 insertions(+), 137 deletions(-)
> 
> diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
> index 9b3cb14dfa..de52a67ee8 100644
> --- a/include/exec/gen-icount.h
> +++ b/include/exec/gen-icount.h
> @@ -19,7 +19,7 @@ static inline void gen_tb_start(TranslationBlock *tb)
>  count = tcg_temp_new_i32();
>  }
>  
> -tcg_gen_ld_i32(count, tcg_ctx.tcg_env,
> +tcg_gen_ld_i32(count, cpu_env,
> -ENV_OFFSET + offsetof(CPUState, icount_decr.u32));
>  
>  if (tb->cflags & CF_USE_ICOUNT) {
> @@ -37,7 +37,7 @@ static inline void gen_tb_start(TranslationBlock *tb)
>  tcg_gen_brcondi_i32(TCG_COND_LT, count, 0, exitreq_label);
>  
>  if (tb->cflags & CF_USE_ICOUNT) {
> -tcg_gen_st16_i32(count, tcg_ctx.tcg_env,
> +tcg_gen_st16_i32(count, cpu_env,
>   -ENV_OFFSET + offsetof(CPUState, 
> icount_decr.u16.low));
>  }
>  
> @@ -62,7 +62,7 @@ static inline void gen_tb_end(TranslationBlock *tb, int 
> num_insns)
>  static inline void gen_io_start(void)
>  {
>  TCGv_i32 tmp = tcg_const_i32(1);
> -tcg_gen_st_i32(tmp, tcg_ctx.tcg_env,
> +tcg_gen_st_i32(tmp, cpu_env,
> -ENV_OFFSET + offsetof(CPUState, can_do_io));
>  tcg_temp_free_i32(tmp);
>  }
> @@ -70,7 +70,7 @@ static inline void gen_io_start(void)
>  static inline void gen_io_end(void)
>  {
>  TCGv_i32 tmp = tcg_const_i32(0);
> -tcg_gen_st_i32(tmp, tcg_ctx.tcg_env,
> +tcg_gen_st_i32(tmp, cpu_env,
> -ENV_OFFSET + offsetof(CPUState, can_do_io));
>  tcg_temp_free_i32(tmp);
>  }
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 3c96aec956..410ba79c0d 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -80,7 +80,6 @@ typedef struct DisasCompare {
>  } DisasCompare;
>  
>  /* Share the TCG temporaries common between 32 and 64 bit modes.  */
> -extern TCGv_env cpu_env;
>  extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
>  extern TCGv_i64 cpu_exclusive_addr;
>  extern TCGv_i64 cpu_exclusive_val;
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index b2d42e3136..da1fefd6f1 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -702,7 +702,6 @@ struct TCGContext {
>  
>  /* Track which vCPU triggers events */
>  CPUState *cpu;  /* *_trans */
> -TCGv_env tcg_env;   /* *_exec  */
>  
>  /* These structures are private to tcg-target.inc.c.  */
>  #ifdef TCG_TARGET_NEED_LDST_LABELS
> @@ -727,6 +726,7 @@ struct TCGContext {
>  };
>  
>  extern TCGContext tcg_ctx;
> +extern TCGv_env cpu_env;
>  extern bool parallel_cpus;
>  
>  static inline void tcg_set_insn_param(int op_idx, int arg, TCGArg v)
> @@ -783,9 +783,6 @@ void tcg_set_frame(TCGContext *s, TCGReg reg, intptr_t 
> start, intptr_t size);
>  
>  int tcg_global_mem_new_internal(TCGType, TCGv_ptr, intptr_t, const char *);
>  
> -TCGv_i32 tcg_global_reg_new_i32(TCGReg reg, const char *name);
> -TCGv_i64 tcg_global_reg_new_i64(TCGReg reg, const char *name);
> -
>  TCGv_i32 tcg_temp_new_internal_i32(int temp_local);
>  TCGv_i64 tcg_temp_new_internal_i64(int temp_local);
>  
> @@ -904,8 +901,6 @@ do {\
>  #define TCGV_PTR_TO_NAT(n) MAKE_TCGV_I32(GET_TCGV_PTR(n))
>  
>  #define tcg_const_ptr(V)

[Qemu-devel] [PATCH] tcg: Initialize cpu_env generically

2017-10-10 Thread Richard Henderson 
This is identical for each target.  So, move the initialization to
common code.  Move the variable itself out of tcg_ctx and name it
cpu_env to minimize changes within targets.

This also means we can remove tcg_global_reg_new_{ptr,i32,i64},
since there are no longer global-register temps created by targets.

Signed-off-by: Richard Henderson 
---
 include/exec/gen-icount.h |  8 
 target/arm/translate.h|  1 -
 tcg/tcg.h |  9 +
 target/alpha/translate.c  |  4 
 target/arm/translate.c|  4 
 target/cris/translate.c   |  3 ---
 target/cris/translate_v10.c   |  2 --
 target/hppa/translate.c   |  4 
 target/i386/translate.c   |  3 ---
 target/lm32/translate.c   |  4 
 target/m68k/translate.c   |  5 -
 target/microblaze/translate.c |  4 
 target/mips/translate.c   |  4 
 target/moxie/translate.c  |  7 ++-
 target/nios2/translate.c  |  3 ---
 target/openrisc/translate.c   |  3 ---
 target/ppc/translate.c| 10 +++---
 target/s390x/translate.c  |  6 --
 target/sh4/translate.c|  7 +--
 target/sparc/translate.c  |  4 
 target/tilegx/translate.c |  3 ---
 target/tricore/translate.c|  6 ++
 target/unicore32/translate.c  |  4 
 target/xtensa/translate.c |  3 ---
 tcg/tcg-op.c  | 30 +++---
 tcg/tcg.c | 31 +++
 26 files changed, 35 insertions(+), 137 deletions(-)

diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index 9b3cb14dfa..de52a67ee8 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -19,7 +19,7 @@ static inline void gen_tb_start(TranslationBlock *tb)
 count = tcg_temp_new_i32();
 }
 
-tcg_gen_ld_i32(count, tcg_ctx.tcg_env,
+tcg_gen_ld_i32(count, cpu_env,
-ENV_OFFSET + offsetof(CPUState, icount_decr.u32));
 
 if (tb->cflags & CF_USE_ICOUNT) {
@@ -37,7 +37,7 @@ static inline void gen_tb_start(TranslationBlock *tb)
 tcg_gen_brcondi_i32(TCG_COND_LT, count, 0, exitreq_label);
 
 if (tb->cflags & CF_USE_ICOUNT) {
-tcg_gen_st16_i32(count, tcg_ctx.tcg_env,
+tcg_gen_st16_i32(count, cpu_env,
  -ENV_OFFSET + offsetof(CPUState, 
icount_decr.u16.low));
 }
 
@@ -62,7 +62,7 @@ static inline void gen_tb_end(TranslationBlock *tb, int 
num_insns)
 static inline void gen_io_start(void)
 {
 TCGv_i32 tmp = tcg_const_i32(1);
-tcg_gen_st_i32(tmp, tcg_ctx.tcg_env,
+tcg_gen_st_i32(tmp, cpu_env,
-ENV_OFFSET + offsetof(CPUState, can_do_io));
 tcg_temp_free_i32(tmp);
 }
@@ -70,7 +70,7 @@ static inline void gen_io_start(void)
 static inline void gen_io_end(void)
 {
 TCGv_i32 tmp = tcg_const_i32(0);
-tcg_gen_st_i32(tmp, tcg_ctx.tcg_env,
+tcg_gen_st_i32(tmp, cpu_env,
-ENV_OFFSET + offsetof(CPUState, can_do_io));
 tcg_temp_free_i32(tmp);
 }
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 3c96aec956..410ba79c0d 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -80,7 +80,6 @@ typedef struct DisasCompare {
 } DisasCompare;
 
 /* Share the TCG temporaries common between 32 and 64 bit modes.  */
-extern TCGv_env cpu_env;
 extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
 extern TCGv_i64 cpu_exclusive_addr;
 extern TCGv_i64 cpu_exclusive_val;
diff --git a/tcg/tcg.h b/tcg/tcg.h
index b2d42e3136..da1fefd6f1 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -702,7 +702,6 @@ struct TCGContext {
 
 /* Track which vCPU triggers events */
 CPUState *cpu;  /* *_trans */
-TCGv_env tcg_env;   /* *_exec  */
 
 /* These structures are private to tcg-target.inc.c.  */
 #ifdef TCG_TARGET_NEED_LDST_LABELS
@@ -727,6 +726,7 @@ struct TCGContext {
 };
 
 extern TCGContext tcg_ctx;
+extern TCGv_env cpu_env;
 extern bool parallel_cpus;
 
 static inline void tcg_set_insn_param(int op_idx, int arg, TCGArg v)
@@ -783,9 +783,6 @@ void tcg_set_frame(TCGContext *s, TCGReg reg, intptr_t 
start, intptr_t size);
 
 int tcg_global_mem_new_internal(TCGType, TCGv_ptr, intptr_t, const char *);
 
-TCGv_i32 tcg_global_reg_new_i32(TCGReg reg, const char *name);
-TCGv_i64 tcg_global_reg_new_i64(TCGReg reg, const char *name);
-
 TCGv_i32 tcg_temp_new_internal_i32(int temp_local);
 TCGv_i64 tcg_temp_new_internal_i64(int temp_local);
 
@@ -904,8 +901,6 @@ do {\
 #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((intptr_t)(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())
@@ -915,8 +910,6 @@ do {\
 #define TCGV_PTR_TO_NAT(n)