| Message ID | 20210630183226.3290849-19-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | accel/tcg: Introduce translator_use_goto_tb | expand |
On Wed, Jun 30, 2021 at 11:32:16AM -0700, Richard Henderson wrote: > Reorder the cases in openrisc_tr_tb_stop to make this easier to read. Hi Richard, For me the description is a bit misleading. I don't see it as a simple reorder for making things easier to read, because we need to understand what is inside of translator_use_goto_tb now. From the patch basically translator_use_goto_tb indicates that a jump is in the same page and we are not single stepping. The old code was: If single step DEBUG else if not same page tcg_gen_lookup_and_goto_ptr() else *same page* jump same page Now: If translator_use_goto_tb() (same page & not single step) jump same page If single step DEBUG else *not same page* tcg_gen_lookup_and_goto_ptr() It would be a bit easier to understand if the description was something like, Reorder the control statements to allow using the page boundary check from translator_use_goto_tb(). That said it looks correct so: Reviewed-by: Stafford Horne <shorne@gmail.com> > Cc: Stafford Horne <shorne@gmail.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/openrisc/translate.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c > index a9c81f8bd5..2d142d8577 100644 > --- a/target/openrisc/translate.c > +++ b/target/openrisc/translate.c > @@ -1720,16 +1720,17 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs) > /* fallthru */ > > case DISAS_TOO_MANY: > - if (unlikely(dc->base.singlestep_enabled)) { > - tcg_gen_movi_tl(cpu_pc, jmp_dest); > - gen_exception(dc, EXCP_DEBUG); > - } else if ((dc->base.pc_first ^ jmp_dest) & TARGET_PAGE_MASK) { > - tcg_gen_movi_tl(cpu_pc, jmp_dest); > - tcg_gen_lookup_and_goto_ptr(); > - } else { > + if (translator_use_goto_tb(&dc->base, jmp_dest)) { > tcg_gen_goto_tb(0); > tcg_gen_movi_tl(cpu_pc, jmp_dest); > tcg_gen_exit_tb(dc->base.tb, 0); > + break; > + } > + tcg_gen_movi_tl(cpu_pc, jmp_dest); > + if (unlikely(dc->base.singlestep_enabled)) { > + gen_exception(dc, EXCP_DEBUG); > + } else { > + tcg_gen_lookup_and_goto_ptr(); > } > break; > > -- > 2.25.1 >
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index a9c81f8bd5..2d142d8577 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -1720,16 +1720,17 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs) /* fallthru */ case DISAS_TOO_MANY: - if (unlikely(dc->base.singlestep_enabled)) { - tcg_gen_movi_tl(cpu_pc, jmp_dest); - gen_exception(dc, EXCP_DEBUG); - } else if ((dc->base.pc_first ^ jmp_dest) & TARGET_PAGE_MASK) { - tcg_gen_movi_tl(cpu_pc, jmp_dest); - tcg_gen_lookup_and_goto_ptr(); - } else { + if (translator_use_goto_tb(&dc->base, jmp_dest)) { tcg_gen_goto_tb(0); tcg_gen_movi_tl(cpu_pc, jmp_dest); tcg_gen_exit_tb(dc->base.tb, 0); + break; + } + tcg_gen_movi_tl(cpu_pc, jmp_dest); + if (unlikely(dc->base.singlestep_enabled)) { + gen_exception(dc, EXCP_DEBUG); + } else { + tcg_gen_lookup_and_goto_ptr(); } break;
Reorder the cases in openrisc_tr_tb_stop to make this easier to read. Cc: Stafford Horne <shorne@gmail.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/openrisc/translate.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) -- 2.25.1