Re: [Qemu-devel] [RFC PATCH] tcg: add ability to dump /tmp/perf-<pid>.map files

[Richard Henderson <rth@twiddle.net>]

On 03/27/2014 09:37 AM, alex.bennee@linaro.org wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> This allows the perf tool to map samples to each individual translation
> block. This could be expanded for user space but currently it gives
> enough information to find any hotblocks by other means.

Plausible, I suppose.

>  TCGOpDef tcg_op_defs[] = {
>  #define DEF(s, oargs, iargs, cargs, flags) { #s, oargs, iargs, cargs, iargs + oargs + cargs, flags },
> @@ -2575,6 +2579,8 @@ static inline int tcg_gen_code_common(TCGContext *s, uint64_t target_pc,
>   the_end:
>      /* Generate TB finalization at the end of block */
>      tcg_out_tb_finalize(s);
> +
> +    tcg_write_perfmap(gen_code_buf, s->code_ptr - gen_code_buf, target_pc);
>      return -1;
>  }
>  
> @@ -2666,6 +2672,21 @@ void tcg_dump_info(FILE *f, fprintf_function cpu_fprintf)
>  }
>  #endif
>  
> +static FILE *tcg_perfmap = NULL;
> +void qemu_tcg_enable_perfmap(void) {
> +    gchar * map_file = g_strdup_printf("/tmp/perf-%d.map", getpid());
> +    tcg_perfmap = g_fopen(map_file, "w");
> +    g_free(map_file);
> +}
> +
> +static void tcg_write_perfmap(uint8_t *start, uint64_t size, uint64_t target_pc)
> +{
> +    if (tcg_perfmap) {
> +        g_fprintf(tcg_perfmap, "%lx %lx subject-0x%lx\n",
> +                  (uint64_t) start, size, target_pc);
> +    }
> +}

Why oh why do you feel the need to use g_fprintf?  That's just gratuitous glib
obfuscation, that.

For this small of a single-use function, I think it would be clearer to just do
the printf directly in tcg_gen_code_common.  Certainly no need to use uint64_t
all over the place; ptrdiff_t and target_ulong are exactly the right thing.


r~