On 10/7/19 11:28 AM, Alex Bennée wrote: > +struct jit_profile_info { > + uint64_t translations; > + uint64_t aborted; > + uint64_t ops; > + unsigned ops_max; > + uint64_t del_ops; > + uint64_t temps; > + unsigned temps_max; > + uint64_t host; > + uint64_t guest; > + uint64_t search_data; > +};
Hmm. Maybe better to define a single structure that you can share between the collection here and the storage in TCGProfile? Also, "host" and "guest" are not helpful names... > + jpi->host += tbs->code.out_len; > + jpi->guest += tbs->code.in_len; ... code_int_len and code_out_len are helpful names. > @@ -1807,6 +1805,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, > tb->tb_stats = NULL; > } > > + > tcg_func_start(tcg_ctx); > > tcg_ctx->cpu = env_cpu(env); Watch the random whitespace. > +#define stat_per_translation(stat, name) \ > + (stat->translations.total ? stat->name / stat->translations.total : 0) Does this really need to go in the header? > +static void collect_jit_profile_info(void *p, uint32_t hash, void *userp) > +{ > + struct jit_profile_info *jpi = userp; > + TBStatistics *tbs = p; > + > + jpi->translations += tbs->translations.total; > + jpi->ops += tbs->code.num_tcg_ops; > + if (stat_per_translation(tbs, code.num_tcg_ops) > jpi->ops_max) { > + jpi->ops_max = stat_per_translation(tbs, code.num_tcg_ops); > + } > + jpi->del_ops += tbs->code.deleted_ops; > + jpi->temps += tbs->code.temps; > + if (stat_per_translation(tbs, code.temps) > jpi->temps_max) { > + jpi->temps_max = stat_per_translation(tbs, code.temps); > + } > + jpi->host += tbs->code.out_len; > + jpi->guest += tbs->code.in_len; > + jpi->search_data += tbs->code.search_out_len; > +} E.g. unsigned long total = tbs->translations.total; if (total) { unsigned tmp; tmp = tbs->code.num_tcg_ops / total; jpi->ops_max = MAX(jpi->ops_max, tmp); tmp = tbs->code.temps / total; jpi->temps_max = MAX(jpi->temps_max, tmp); } It does occur to me to wonder why code.num_guest_inst etc are "unsigned" while translations.total are "unsigned long", given that they are both accumulations over many TBs. r~