"Wu, Fei" <fei2...@intel.com> writes:
> On 5/30/2023 1:01 PM, Wu, Fei wrote: >> On 5/30/2023 12:07 PM, Richard Henderson wrote: >>> On 5/29/23 04:49, Fei Wu wrote: >>>> +/* >>>> + * The TCGProfile structure holds data for analysing the quality of >>>> + * the code generation. The data is split between stuff that is valid >>>> + * for the lifetime of a single translation and things that are valid >>>> + * for the lifetime of the translator. As the former is reset for each >>>> + * new translation so it should be copied elsewhere if you want to >>>> + * keep it. >>>> + * >>>> + * The structure is safe to access within the context of translation >>>> + * but accessing the data from elsewhere should be done with safe >>>> + * work. >>>> + */ <snip> >>>> + int64_t cpu_exec_time; >>>> + int64_t op_count; /* total insn count */ >>>> + int64_t code_in_len; >>>> + int64_t code_out_len; >>>> + int64_t search_out_len; >>>> + >>>> + /* Timestamps during translation */ >>>> + uint64_t gen_start_time; >>>> + uint64_t gen_ir_done_time; >>>> + uint64_t gen_opt_done_time; >>>> + uint64_t gen_la_done_time; >>>> + uint64_t gen_code_done_time; >>>> + >>>> + /* Lifetime count of TCGOps per TCGContext */ >>>> + uint64_t table_op_count[NB_OPS]; >>>> +} TCGProfile; >>>> + >>> >>> Why have you added this back? >>> >>> The whole point of removing CONFIG_PROFILE to begin was to have all new >>> code. Not to remove it then reintroduce it unchanged. >>> >>> In tcg_gen_code, you have access to the TranslationBlock as s->gen_tb. >>> There is zero point to accumulating into TCGProfile, when you could be >>> accumulating into TCGStatistics directly. >>> >> TCGProfile contains global wide (per TCGContext) stats, but TBStatistics >> is TB specific, some info in TCGProfile such as table_op_count is not >> able to be summed up from TBStatistics. The per-translation stats in >> TCGProfile may be removed indeed. >> > After some cleanup locally, these are the remains in TCGProfile: > * cpu_exec_time - which is not guarded by tb_stats_enabled, it could be > moved out as an individual variable? > * gen_xxx_time - which in kinda global variables across functions to > calc ts->gen_times Given the work on JIT profiling I think there is an argument to drop the time profile bits and pieces. I think you can get the same information from a perf run although it does amortise the cost of generation over all translations. Do we see any particular TBs that are particularly expensive to translate by more than a standard deviation? > * table_op_count - it's indeed guarded by tb_stats_enabled, but it's a > large array, it might be too large to put into TBStaticstics. Probably. This is probably more interesting information as an aggregate than per TB. > > Thanks, > Fei. > >> Thanks, >> Fei. >> >>> >>> r~ >> -- Alex Bennée Virtualisation Tech Lead @ Linaro
