Richard Henderson writes: > On 09/14/2017 08:20 AM, Lluís Vilanova wrote: >> Richard Henderson writes: >> >>> On 09/10/2017 09:27 AM, Lluís Vilanova wrote: >>>> TCG BBLs and instructions have multiple exit points from where to raise >>>> tracing events, but some of the necessary information in the generic >>>> disassembly infrastructure is not available until after generating these >>>> exit points. >>>> >>>> This patch adds support for "inline points" (where the tracing code will >>>> be placed), and "inline regions" (which identify the TCG code that must >>>> be inlined). The TCG compiler will basically copy each inline region to >>>> any inline points that reference it. >> >>> I am not keen on this. >> >>> Is there a reason you can't just emit the tracing code at the appropriate >>> place >>> to begin with? Perhaps I have to wait to see how this is used... >> >> As I tried to briefly explain on next patch, the main problem without >> inlining >> is that we will see guest_tb_after_trans twice on the trace for each TB in >> conditional instructions on the guest, since they have two exit points >> (which we >> capture when emitting goto_tb in TCG).
> Without seeing the code, I suspect this is because you didn't examine the > argument to tcg_gen_exit_tb. You can tell when goto_tb must have been emitted > and avoid logging twice. The generated tracing code for 'guest_*_after' must be right before the "goto_tb" opcode at the end of a TB (AFAIU generated by tcg_gen_lookup_and_goto_ptr()), and we have two of those when decoding a guest conditional jump. If we couple this with the semantics of the trace_*_tcg functions (trace the event at translation time, and generate TCG code to trace the event at execution time), we get the case I described (we don't want to call trace_tb_after_tcg() or trace_insn_after_tcg() twice for the same TB or instruction). That is, unless I've missed something. The only alternative I can think of is changing tracetool to offer an additional API that provides separate functions for translation-time tracing and execution-time generation. So from this: static inline void trace_event_tcg(CPUState *cpu, TCGv_env env, ...) { trace_event_trans(cpu, ...); if (trace_event_get_vcpu_state(cpu, EVENT_EXEC)) { gen_helper_trace_event_exec(env, ...); } } We can extend it into this: static inline void gen_trace_event_exec(TCGv_env env, ...) if (trace_event_get_vcpu_state(cpu, EVENT_EXEC)) { gen_helper_trace_event_exec(env, ...); } } static inline void trace_event_tcg(CPUState *cpu, TCGv_env env, ...) { trace_event_trans(cpu, ...); gen_trace_event_exec(env, ...); } Cheers, Lluis