| 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
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~
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
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~
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
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,
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(-)