Re: [Qemu-devel] [Qemu-Devel][PATCH 1/3] Adding an optional tb execution counter.

[Richard Henderson]

On 6/14/19 6:53 AM, vandersonmr wrote:
> +void HELPER(inc_exec_freq)(void *ptr)
> +{
> +    TranslationBlock* tb = (TranslationBlock*) ptr;
> +    atomic_inc(&tb->exec_freq);
> +}
...
> +DEF_HELPER_FLAGS_1(inc_exec_freq, TCG_CALL_NO_RWG, void, ptr)
...
>      uint32_t flags; /* flags defining in which context the code was 
> generated */
>      uint16_t size;      /* size of target code for this block (1 <=
>                             size <= TARGET_PAGE_SIZE) */
> +    uint64_t exec_freq;

It's not a frequency, but a count.

>      uint16_t icount;
>      uint32_t cflags;    /* compile flags */

Consider where you've placed the data with respect to the packing of other
members of the structure.

>  static inline void gen_tb_start(TranslationBlock *tb)
>  {
>      TCGv_i32 count, imm;
>  
> +    if (enable_freq_count) {
> +        TCGv_ptr tb_ptr = tcg_temp_new_ptr();
> +        tcg_gen_trunc_i64_ptr(tb_ptr, tcg_const_i64((int64_t) tb));
> +        gen_helper_inc_exec_freq(tb_ptr);
> +    }
> +
>      tcg_ctx->exitreq_label = gen_new_label();
>      if (tb_cflags(tb) & CF_USE_ICOUNT) {
>          count = tcg_temp_local_new_i32();

By placing the increment before the exit for interrupt check instead of after,
you're kinda counting the wrong thing, because the TB hasn't executed.


> diff --git a/linux-user/main.c b/linux-user/main.c
> index a59ae9439d..1bf7155670 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -58,6 +58,7 @@ static const char *seed_optarg;
>  unsigned long mmap_min_addr;
>  unsigned long guest_base;
>  int have_guest_base;
> +bool enable_freq_count = false;

This is being declared in multiple places and initialized in multiple places.
This needs to go elsewhere.


r~