Patchwork [RFC,qom-cpu,23/41] target-xtensa: Change gen_intermediate_code_internal() arg to XtensaCPU

login
register
mail settings
Submitter Andreas Färber
Date June 29, 2013, 8:01 p.m.
Message ID <1372536117-28167-24-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/255775/
State New
Headers show

Comments

Andreas Färber - June 29, 2013, 8:01 p.m.
Also use bool type while at it.

Prepares for moving singlestep_enabled field to CPUState.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-xtensa/translate.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
Richard Henderson - July 1, 2013, 5:16 p.m.
On 06/29/2013 01:01 PM, Andreas Färber wrote:
> Also use bool type while at it.
> 
> Prepares for moving singlestep_enabled field to CPUState.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  target-xtensa/translate.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

All of 10-23/41,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
Andreas Färber - July 1, 2013, 5:51 p.m.
Am 01.07.2013 19:16, schrieb Richard Henderson:
> On 06/29/2013 01:01 PM, Andreas Färber wrote:
>> Also use bool type while at it.
>>
>> Prepares for moving singlestep_enabled field to CPUState.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  target-xtensa/translate.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> All of 10-23/41,
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

I wonder, all targets seem to implement the same pattern of passing a
hard-coded bool argument to their internal function.

Is there any reason not to have translate-all.c call the function with
that true/false directly?

That would then call for using CPUState *cs argument and leaving FooCPU
*cpu as local variable only where needed. In that case it would be a
central change covering all targets in one patch. :)

Regards,
Andreas
Richard Henderson - July 1, 2013, 6:03 p.m.
On 07/01/2013 10:51 AM, Andreas Färber wrote:
> I wonder, all targets seem to implement the same pattern of passing a
> hard-coded bool argument to their internal function.
> 
> Is there any reason not to have translate-all.c call the function with
> that true/false directly?

The idea is to avoid runtime checks for the rare search_pc case.

Instead we pass constants to inline functions and transform what would be
runtime checks into compile-time optimized code paths.



r~
Andreas Färber - July 1, 2013, 9:46 p.m.
Am 01.07.2013 20:03, schrieb Richard Henderson:
> On 07/01/2013 10:51 AM, Andreas Färber wrote:
>> I wonder, all targets seem to implement the same pattern of passing a
>> hard-coded bool argument to their internal function.
>>
>> Is there any reason not to have translate-all.c call the function with
>> that true/false directly?
> 
> The idea is to avoid runtime checks for the rare search_pc case.
> 
> Instead we pass constants to inline functions and transform what would be
> runtime checks into compile-time optimized code paths.

Thanks for the explanation!

Not every target seems to be aware of that, I'll put together a patch.

Andreas
Andreas Färber - July 2, 2013, 9:13 p.m.
Am 01.07.2013 19:16, schrieb Richard Henderson:
> On 06/29/2013 01:01 PM, Andreas Färber wrote:
>> Also use bool type while at it.
>>
>> Prepares for moving singlestep_enabled field to CPUState.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  target-xtensa/translate.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> All of 10-23/41,
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks, rebased on inline additions and inserted into qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

Patch

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index dcb90a5..d5f2068 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -2875,9 +2875,11 @@  static void gen_ibreak_check(CPUXtensaState *env, DisasContext *dc)
     }
 }
 
-static void gen_intermediate_code_internal(
-        CPUXtensaState *env, TranslationBlock *tb, int search_pc)
+static void gen_intermediate_code_internal(XtensaCPU *cpu,
+                                           TranslationBlock *tb,
+                                           bool search_pc)
 {
+    CPUXtensaState *env = &cpu->env;
     DisasContext dc;
     int insn_count = 0;
     int j, lj = -1;
@@ -3006,12 +3008,12 @@  static void gen_intermediate_code_internal(
 
 void gen_intermediate_code(CPUXtensaState *env, TranslationBlock *tb)
 {
-    gen_intermediate_code_internal(env, tb, 0);
+    gen_intermediate_code_internal(xtensa_env_get_cpu(env), tb, false);
 }
 
 void gen_intermediate_code_pc(CPUXtensaState *env, TranslationBlock *tb)
 {
-    gen_intermediate_code_internal(env, tb, 1);
+    gen_intermediate_code_internal(xtensa_env_get_cpu(env), tb, true);
 }
 
 void xtensa_cpu_dump_state(CPUState *cs, FILE *f,