On 10/7/19 11:28 AM, Alex Bennée wrote: > @@ -1795,6 +1799,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu, > if (flag & TB_EXEC_STATS) { > tb->tb_stats->stats_enabled |= TB_EXEC_STATS; > } > + > + if (flag & TB_JIT_STATS) { > + tb->tb_stats->stats_enabled |= TB_JIT_STATS; > + }
So, assuming that you really meant this, and not replacing as I was guessing vs patch 2, then this is tb->tb_stats->stats_enabled |= flag & (TB_EXEC_STATS | TB_JIT_STATS); But I still think it's weird to be wanting to modify the shared structure. What does that mean for concurrent code generation? > + /* > + * Collect JIT stats when enabled. We batch them all up here to > + * avoid spamming the cache with atomic accesses > + */ > + if (tb_stats_enabled(tb, TB_JIT_STATS)) { > + TBStatistics *ts = tb->tb_stats; > + qemu_mutex_lock(&ts->jit_stats_lock); > + > + ts->code.num_guest_inst += prof->translation.nb_guest_insns; > + ts->code.num_tcg_ops += prof->translation.nb_ops_pre_opt; > + ts->code.num_tcg_ops_opt += tcg_ctx->nb_ops; > + ts->code.spills += prof->translation.nb_spills; > + ts->code.out_len += tb->tc.size; > + > + ts->translations.total++; > + if (phys_page2 != -1) { > + ts->translations.spanning++; > + } > + > + g_ptr_array_add(ts->tbs, tb); > + > + qemu_mutex_unlock(&ts->jit_stats_lock); > + } Hmm. So we're to interpret all of code.field as sums, the average of which can be obtained by dividing by translations.total. Ok, but that should probably be mentioned in a comment in/near the structure definition. What are we planning to do with the set of all tb's collected here? > @@ -3125,6 +3126,7 @@ static void temp_sync > case TEMP_VAL_REG: > tcg_out_st(s, ts->type, ts->reg, > ts->mem_base->reg, ts->mem_offset); > + s->prof.translation.nb_spills++; > break; > > case TEMP_VAL_MEM: This is not a spill in the common compiler definition. This is "write the temp to its backing storage". While this does happen in the course of spilling, the vast majority of these are because we've finished modifying a global temp and must now update memory. Which is not nearly the same thing as "spill". A spill in the compiler definition happens in tcg_reg_alloc, right after the comment "We must spill something". ;-) r~