On 18 September 2015 at 05:55, Richard Henderson <r...@twiddle.net> wrote: > Reduce the boilerplate required for each target. At the same time, > move the test for breakpoint after calling tcg_gen_insn_start. > > Signed-off-by: Richard Henderson <r...@twiddle.net> > --- > include/qom/cpu.h | 16 +++++++++++++++ > target-alpha/translate.c | 13 ++++-------- > target-arm/translate-a64.c | 20 +++++++------------ > target-arm/translate.c | 46 > ++++++++++++++++++++----------------------- > target-cris/translate.c | 27 ++++++++----------------- > target-i386/translate.c | 17 +++++++--------- > target-lm32/translate.c | 25 +++++++---------------- > target-m68k/translate.c | 18 ++++++----------- > target-microblaze/translate.c | 36 ++++++++++++--------------------- > target-mips/translate.c | 25 ++++++++++------------- > target-moxie/translate.c | 19 +++++++----------- > target-openrisc/translate.c | 24 +++++++--------------- > target-ppc/translate.c | 14 +++++-------- > target-s390x/translate.c | 16 ++++++--------- > target-sh4/translate.c | 20 ++++++++----------- > target-sparc/translate.c | 23 ++++++++++------------ > target-unicore32/translate.c | 24 ++++++++++------------ > target-xtensa/translate.c | 25 +++++++---------------- > 18 files changed, 159 insertions(+), 249 deletions(-) > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 302673d..e11dca3 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -679,6 +679,7 @@ void cpu_single_step(CPUState *cpu, int enabled); > /* 0x08 currently unused */ > #define BP_GDB 0x10 > #define BP_CPU 0x20 > +#define BP_ANY (BP_GDB | BP_CPU) > #define BP_WATCHPOINT_HIT_READ 0x40 > #define BP_WATCHPOINT_HIT_WRITE 0x80 > #define BP_WATCHPOINT_HIT (BP_WATCHPOINT_HIT_READ | BP_WATCHPOINT_HIT_WRITE) > @@ -689,6 +690,21 @@ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int > flags); > void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *breakpoint); > void cpu_breakpoint_remove_all(CPUState *cpu, int mask); > > +/* Return true if PC matches an installed breakpoint. */ > +static inline bool cpu_breakpoint_test(CPUState *cpu, vaddr pc, int mask) > +{ > + CPUBreakpoint *bp; > + > + if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) { > + QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) { > + if (bp->pc == pc && (bp->flags & mask)) { > + return true; > + } > + } > + } > + return false; > +}
This won't work with the fix for ARM breakpoints Sergey currently has on list: http://patchwork.ozlabs.org/patch/517359/ where we need to behave differently for "there's a GDB breakpoint here" and "there's a CPU breakpoint here" (because the complex conditions on the latter require us to call a helper function to see if we need to actually generate an EXCP_DEBUG exception). > @@ -2913,14 +2912,6 @@ static inline void > gen_intermediate_code_internal(AlphaCPU *cpu, > > gen_tb_start(tb); > do { > - if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) { > - QTAILQ_FOREACH(bp, &cs->breakpoints, entry) { > - if (bp->pc == ctx.pc) { > - gen_excp(&ctx, EXCP_DEBUG, 0); > - break; > - } > - } > - } > if (search_pc) { > j = tcg_op_buf_count(); > if (lj < j) { > @@ -2936,6 +2927,10 @@ static inline void > gen_intermediate_code_internal(AlphaCPU *cpu, > tcg_gen_insn_start(ctx.pc); > num_insns++; > > + if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) { > + gen_excp(&ctx, EXCP_DEBUG, 0); > + break; > + } > if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) { > gen_io_start(); > } > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 4670941..c1efd30 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -11007,7 +11007,6 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu, > CPUState *cs = CPU(cpu); > CPUARMState *env = &cpu->env; > DisasContext dc1, *dc = &dc1; > - CPUBreakpoint *bp; > int j, lj; > target_ulong pc_start; > target_ulong next_page_start; > @@ -11079,18 +11078,6 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu, > tcg_clear_temp_count(); > > do { > - if (unlikely(!QTAILQ_EMPTY(&cs->breakpoints))) { > - QTAILQ_FOREACH(bp, &cs->breakpoints, entry) { > - if (bp->pc == dc->pc) { > - gen_exception_internal_insn(dc, 0, EXCP_DEBUG); > - /* Advance PC so that clearing the breakpoint will > - invalidate this TB. */ > - dc->pc += 2; > - goto done_generating; > - } > - } > - } > - > if (search_pc) { > j = tcg_op_buf_count(); > if (lj < j) { > @@ -11106,6 +11093,13 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu, > tcg_gen_insn_start(dc->pc); > num_insns++; > > + if (unlikely(cpu_breakpoint_test(cs, dc->pc, BP_ANY))) { > + gen_exception_internal_insn(dc, 0, EXCP_DEBUG); > + /* Advance PC so that clearing the breakpoint will > + invalidate this TB. */ > + dc->pc += 2; > + goto done_generating; > + } > if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) { > gen_io_start(); > } Do you know why some but not all targets do this "advance PC" thing if there's a breakpoint? thanks -- PMM