Re: [Qemu-devel] [PATCH v11 01/29] Pass generic CPUState to gen_intermediate_code()

2017-07-01 Thread Richard Henderson 

On 06/28/2017 05:20 AM, Lluís Vilanova wrote:

-void gen_intermediate_code(CPUArchState *env, struct TranslationBlock *tb);
+void gen_intermediate_code(CPUState *env, struct TranslationBlock *tb);


Ought to change the argument name too.


r~

Re: [Qemu-devel] [PATCH v11 01/29] Pass generic CPUState to gen_intermediate_code()

2017-06-30 Thread Richard Henderson 

On 06/29/2017 03:52 PM, Emilio G. Cota wrote:

On Wed, Jun 28, 2017 at 15:20:42 +0300, Lluís Vilanova wrote:

Needed to implement a target-agnostic gen_intermediate_code() in the
future.

Signed-off-by: Lluís Vilanova 
Reviewed-by: David Gibson 
Reviewed-by: Richard Henderson 
---

(snip)

-void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb)
+void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb)
  {
-AlphaCPU *cpu = alpha_env_get_cpu(env);
-CPUState *cs = CPU(cpu);
+CPUAlphaState *env = cpu->env_ptr;


I'd keep the original variable names, i.e. cs for CPUState in this case,
just like you did for a64:


-void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb)
+void gen_intermediate_code_a64(CPUState *cs, TranslationBlock *tb)
  {
-CPUState *cs = CPU(cpu);
-CPUARMState *env = &cpu->env;
+CPUARMState *env = cs->env_ptr;
+ARMCPU *cpu = arm_env_get_cpu(env);


This will keep the diff size to a minimum.


I asked for the same thing many revisions ago.


r~

Re: [Qemu-devel] [PATCH v11 01/29] Pass generic CPUState to gen_intermediate_code()

2017-06-29 Thread Emilio G. Cota 
On Wed, Jun 28, 2017 at 15:20:42 +0300, Lluís Vilanova wrote:
> Needed to implement a target-agnostic gen_intermediate_code() in the
> future.
> 
> Signed-off-by: Lluís Vilanova 
> Reviewed-by: David Gibson 
> Reviewed-by: Richard Henderson 
> ---
(snip)
> -void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb)
> +void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb)
>  {
> -AlphaCPU *cpu = alpha_env_get_cpu(env);
> -CPUState *cs = CPU(cpu);
> +CPUAlphaState *env = cpu->env_ptr;

I'd keep the original variable names, i.e. cs for CPUState in this case,
just like you did for a64:

> -void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb)
> +void gen_intermediate_code_a64(CPUState *cs, TranslationBlock *tb)
>  {
> -CPUState *cs = CPU(cpu);
> -CPUARMState *env = &cpu->env;
> +CPUARMState *env = cs->env_ptr;
> +ARMCPU *cpu = arm_env_get_cpu(env);

This will keep the diff size to a minimum.

E.

[Qemu-devel] [PATCH v11 01/29] Pass generic CPUState to gen_intermediate_code()

2017-06-28 Thread Lluís Vilanova 
Needed to implement a target-agnostic gen_intermediate_code() in the
future.

Signed-off-by: Lluís Vilanova 
Reviewed-by: David Gibson 
Reviewed-by: Richard Henderson 
---
 accel/tcg/translate-all.c |2 +-
 include/exec/exec-all.h   |2 +-
 target/alpha/translate.c  |   11 +--
 target/arm/translate-a64.c|6 +++---
 target/arm/translate.c|   18 +-
 target/arm/translate.h|4 ++--
 target/cris/translate.c   |   17 -
 target/hppa/translate.c   |5 ++---
 target/i386/translate.c   |   13 ++---
 target/lm32/translate.c   |   22 +++---
 target/m68k/translate.c   |   15 +++
 target/microblaze/translate.c |   22 +++---
 target/mips/translate.c   |   15 +++
 target/moxie/translate.c  |   14 +++---
 target/nios2/translate.c  |5 ++---
 target/openrisc/translate.c   |   20 ++--
 target/ppc/translate.c|   15 +++
 target/s390x/translate.c  |   13 ++---
 target/sh4/translate.c|   15 +++
 target/sparc/translate.c  |   11 +--
 target/tilegx/translate.c |7 +++
 target/tricore/translate.c|9 -
 target/unicore32/translate.c  |   17 -
 target/xtensa/translate.c |   13 ++---
 24 files changed, 138 insertions(+), 153 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index f6ad46b613..2dc93f420a 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1299,7 +1299,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 tcg_func_start(&tcg_ctx);
 
 tcg_ctx.cpu = ENV_GET_CPU(env);
-gen_intermediate_code(env, tb);
+gen_intermediate_code(cpu, tb);
 tcg_ctx.cpu = NULL;
 
 trace_translate_block(tb, tb->pc, tb->tc_ptr);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 724ec73dce..a7cd106587 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -43,7 +43,7 @@ typedef ram_addr_t tb_page_addr_t;
 
 #include "qemu/log.h"
 
-void gen_intermediate_code(CPUArchState *env, struct TranslationBlock *tb);
+void gen_intermediate_code(CPUState *env, struct TranslationBlock *tb);
 void restore_state_to_opc(CPUArchState *env, struct TranslationBlock *tb,
   target_ulong *data);
 
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 232af9e177..c1227c51b0 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -2911,10 +2911,9 @@ static ExitStatus translate_one(DisasContext *ctx, 
uint32_t insn)
 return ret;
 }
 
-void gen_intermediate_code(CPUAlphaState *env, struct TranslationBlock *tb)
+void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb)
 {
-AlphaCPU *cpu = alpha_env_get_cpu(env);
-CPUState *cs = CPU(cpu);
+CPUAlphaState *env = cpu->env_ptr;
 DisasContext ctx, *ctxp = &ctx;
 target_ulong pc_start;
 target_ulong pc_mask;
@@ -2929,7 +2928,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct 
TranslationBlock *tb)
 ctx.pc = pc_start;
 ctx.mem_idx = cpu_mmu_index(env, false);
 ctx.implver = env->implver;
-ctx.singlestep_enabled = cs->singlestep_enabled;
+ctx.singlestep_enabled = cpu->singlestep_enabled;
 
 #ifdef CONFIG_USER_ONLY
 ctx.ir = cpu_std_ir;
@@ -2972,7 +2971,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct 
TranslationBlock *tb)
 tcg_gen_insn_start(ctx.pc);
 num_insns++;
 
-if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) {
+if (unlikely(cpu_breakpoint_test(cpu, ctx.pc, BP_ANY))) {
 ret = gen_excp(&ctx, EXCP_DEBUG, 0);
 /* The address covered by the breakpoint must be included in
[tb->pc, tb->pc + tb->size) in order to for it to be
@@ -3047,7 +3046,7 @@ void gen_intermediate_code(CPUAlphaState *env, struct 
TranslationBlock *tb)
 && qemu_log_in_addr_range(pc_start)) {
 qemu_log_lock();
 qemu_log("IN: %s\n", lookup_symbol(pc_start));
-log_target_disas(cs, pc_start, ctx.pc - pc_start, 1);
+log_target_disas(cpu, pc_start, ctx.pc - pc_start, 1);
 qemu_log("\n");
 qemu_log_unlock();
 }
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index e55547d95d..f9bd1a9679 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -11190,10 +11190,10 @@ static void disas_a64_insn(CPUARMState *env, 
DisasContext *s)
 free_tmp_a64(s);
 }
 
-void gen_intermediate_code_a64(ARMCPU *cpu, TranslationBlock *tb)
+void gen_intermediate_code_a64(CPUState *cs, TranslationBlock *tb)
 {
-CPUState *cs = CPU(cpu);
-CPUARMState *env = &cpu->env;
+CPUARMState *env = cs->env_ptr;
+ARMCPU *cpu = arm_env_get_cpu(env);
 DisasContext dc1, *dc = &dc1;
 target_ulong pc_start;
 target_ulong next_page_start;
diff --