On 6/1/2023 10:40 AM, Richard Henderson wrote:
> On 5/30/23 01:35, Fei Wu wrote:
>> +static void do_dump_tbs_info(int total, int sort_by)
>> +{
>> + id = 1;
>> + GList *i;
>> + int count = total;
>> +
>> + g_list_free(last_search);
>> + last_search = NULL;
>> +
>> + qht_iter(&tb_ctx.tb_stats, collect_tb_stats, NULL);
>> +
>> + last_search = g_list_sort_with_data(last_search, inverse_sort_tbs,
>> + &sort_by);
>> +
>
> Why are you sorting on a list and not an array?
> Intuitively, sorting a list of 1 million elements seems like the wrong
> choice.
>
> Why did you put all the comparisons in one inverse_sort_tbs function,
> and require examining sort_by? Better would be N sorting functions
> where sort_by is evaluated once before starting the sort.
>
>
>> +++ b/disas/disas.c
>> @@ -8,6 +8,8 @@
>> #include "hw/core/cpu.h"
>> #include "exec/memory.h"
>>
>> +#include "qemu/log-for-trace.h"
>> +
>> /* Filled in by elfload.c. Simplistic, but will do for now. */
>> struct syminfo *syminfos = NULL;
>>
>> @@ -199,6 +201,24 @@ static void initialize_debug_host(CPUDebug *s)
>> #endif
>> }
>>
>> +static int
>> +__attribute__((format(printf, 2, 3)))
>> +fprintf_log(FILE *a, const char *b, ...)
>> +{
>> + va_list ap;
>> + va_start(ap, b);
>> +
>> + if (!to_string) {
>> + vfprintf(a, b, ap);
>> + } else {
>> + qemu_vlog(b, ap);
>> + }
>> +
>> + va_end(ap);
>> +
>> + return 1;
>> +}
>> +
>
> Not need on this either. Global variable being checked on each
> callback, instead of selecting the proper callback earlier -- preferably
> without the global variable.
>
> Did you really need something different than monitor_disas? You almost
> certainly want to read physical memory and not virtual anyway.
>
This makes me think the necessity of 'info tb', the primary extra info
it adds is guest instruction, which can be gotten from 'x/i' w/o calling
tb_gen_code.
(qemu) info tb 1
------------------------------
TB id:1 | phys:0x79bc86 virt:0xffffffff8059bc86 flags:0x01024001 0 inv/1
| exec:56962444/0 guest inst cov:0.61%
| trans:1 ints: g:8 op:27 op_opt:22 spills:0
| h/g (host bytes / guest insts): 26.000000
| time to gen at 2.4GHz => code:747.08(ns) IR:477.92(ns)
----------------
IN:
Priv: 1; Virt: 0
0xffffffff8059bc86: 00000013 nop
0xffffffff8059bc8a: 00000013 nop
0xffffffff8059bc8e: 00000013 nop
0xffffffff8059bc92: 00000013 nop
0xffffffff8059bc96: 1141 addi sp,sp,-16
0xffffffff8059bc98: e422 sd s0,8(sp)
0xffffffff8059bc9a: 0800 addi s0,sp,16
0xffffffff8059bc9c: c0102573 rdtime a0
------------------------------
(qemu) x/8i 0xffffffff8059bc86
x/8i 0xffffffff8059bc86
0xffffffff8059bc86: 00000013 nop
0xffffffff8059bc8a: 00000013 nop
0xffffffff8059bc8e: 00000013 nop
0xffffffff8059bc92: 00000013 nop
0xffffffff8059bc96: 1141 addi sp,sp,-16
0xffffffff8059bc98: e422 sd s0,8(sp)
0xffffffff8059bc9a: 0800 addi s0,sp,16
0xffffffff8059bc9c: c0102573 rdtime a0
Thanks,
Fei.
>> +void qemu_log_to_monitor(bool enable)
>> +{
>> + to_monitor = enable;
>> +}
>> +
>> +void qemu_log_to_string(bool enable, GString *s)
>> +{
>> + to_string = enable;
>> + string = s;
>> +}
>
> What are these for, and why do you need them?
> Why would to_string ever be anything other than (string != NULL)?
> Why are you using such very generic names for global variables?
>
>
> r~
