Richard Henderson writes: > On 07/15/2016 09:42 PM, Lluís Vilanova wrote: >> Signed-off-by: Lluís Vilanova <vilan...@ac.upc.edu> >> --- >> include/exec/translate-all_template.h | 58 ++++++++++++ >> include/qom/cpu.h | 21 ++++ >> translate-all_template.h | 160 >> +++++++++++++++++++++++++++++++++ >> 3 files changed, 239 insertions(+) >> create mode 100644 include/exec/translate-all_template.h >> create mode 100644 translate-all_template.h >> >> diff --git a/include/exec/translate-all_template.h >> b/include/exec/translate-all_template.h >> new file mode 100644 >> index 0000000..9e0c361 >> --- /dev/null >> +++ b/include/exec/translate-all_template.h >> @@ -0,0 +1,58 @@ >> +/* >> + * Generic intermediate code generation. >> + * >> + * Copyright (C) 2016 Lluís Vilanova <vilan...@ac.upc.edu> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> + >> +#ifndef EXEC__TRANSLATE_ALL_TEMPLATE_H >> +#define EXEC__TRANSLATE_ALL_TEMPLATE_H >> + >> +/* >> + * Include this header from a target-specific file, and add a >> + * >> + * DisasContextBase base; >> + * >> + * member in your target-specific DisasContext. >> + */ >> + >> + >> +#include "exec/exec-all.h" >> + >> + >> +/** >> + * DisasJumpType: >> + * @DJ_NEXT: Next instruction in program order >> + * @DJ_TOO_MANY: Too many instructions executed >> + * @DJ_TARGET: Start of target-specific conditions >> + * >> + * What instruction to disassemble next. >> + */ >> +typedef enum DisasJumpType >> +{ >> + DJ_NEXT, >> + DJ_TOO_MANY, >> + DJ_TARGET, >> +} DisasJumpType;
> I think you might as well add the common cases: exit tb via exception, exit > via > goto_tb, exit via indirect jump (pc updated), exit for state change (pc not > updated). > See the set used for alpha. Do you mean the "DISAS_*" values in "exec-all.h"? I looked into it, but could not fully understand when they are supposed to be used (seem inconsistent across targets), so I decided to defer this to the target. >> +void gen_intermediate_code(CPUState *cpu, struct TranslationBlock *tb) >> +{ >> + CPUArchState *env = cpu->env_ptr; >> + DisasContext dc1, *dc = &dc1; >> + int num_insns; >> + int max_insns; >> + >> + /* Initialize DisasContext */ >> + dc->base.tb = tb; >> + dc->base.singlestep_enabled = cpu->singlestep_enabled; >> + dc->base.pc_first = tb->pc; >> + dc->base.pc_next = dc->base.pc_first; >> + dc->base.jmp_type = DJ_NEXT; >> + gen_intermediate_code_target_init_disas_context(dc, env); >> + >> + /* Target-specific globals */ >> + gen_intermediate_code_target_init_globals(dc, env); >> + >> + /* Instruction counting */ >> + num_insns = 0; >> + max_insns = dc->base.tb->cflags & CF_COUNT_MASK; >> + if (max_insns == 0) { >> + max_insns = CF_COUNT_MASK; >> + } >> + if (max_insns > TCG_MAX_INSNS) { >> + max_insns = TCG_MAX_INSNS; >> + } > I've started adding the singlestep check here, outside the loop, setting > max_insns to 1. That makes sense, and could eliminate one field in DisasContextBase. >> + >> + /* Start translating */ >> + gen_tb_start(dc->base.tb); >> + >> + while (true) { >> + CPUBreakpoint *bp; >> + >> + tcg_gen_insn_start(dc->base.pc_next, dc->cc_op); > You've probably discovered that this will have to be its own hook. Yes, sorry about not fixing it before sending. >> + num_insns++; >> + >> + /* Pass breakpoint hits to target for further processing */ >> + bp = NULL; >> + do { >> + bp = cpu_breakpoint_get(cpu, dc->base.pc_next, bp); >> + if (unlikely(bp)) { >> + if (gen_intermediate_code_target_breakpoint_hit(dc, env, >> bp)) { >> + goto done_generating; >> + } >> + } >> + } while (bp != NULL); > Why would you need to loop here? cpu_breakpoint_get() just returns breakpoints matching a PC, and delays other checks (like flags) to target code. That's why cpu_breakpoint_get() is now used as a looping construct (thus the change in qlist to continue a previous foreach). Thanks, Lluis