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.
>>> + */
>>> +typedef struct TCGProfile {
>>> +
>>> +    struct {
>>> +        int nb_guest_insns;
>>> +        int nb_spills;
>>> +        int nb_ops_pre_opt;
>>> +
>>> +        int del_op_count;
>>> +        int temp_count;
>>> +    } translation;
>>> +
>>> +    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
* 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.

Thanks,
Fei.

> Thanks,
> Fei.
> 
>>
>> r~
> 


Reply via email to