Re: [PATCH v9 00/13] TCG code quality tracking and perf integration
Patchew URL:
https://patchew.org/QEMU/20191007152839.30804-1-alex.ben...@linaro.org/
Hi,
This series failed the docker-mingw@fedora build test. Please find the testing
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
CC qga/vss-win32.o
CC qga/qapi-generated/qga-qapi-types.o
CC qga/qapi-generated/qga-qapi-visit.o
./qemu-monitor.texi:585: warning: @findex missing argument
CC qga/qapi-generated/qga-qapi-commands.o
./qemu-monitor.texi:585: warning: @findex missing argument
AR libqemuutil.a
LINKelf2dmp.exe
CC qemu-img.o
---
LINKqemu-img.exe
CC aarch64-softmmu/arch_init.o
CC aarch64-softmmu/cpus.o
/tmp/qemu-test/src/disas.c:425:31: error: 'struct _IO_FILE' declared inside
parameter list will not be visible outside of this definition or declaration
[-Werror]
static int fprintf_log(struct _IO_FILE *a, const char *b, ...)
^~~~
/tmp/qemu-test/src/disas.c: In function 'fprintf_log':
/tmp/qemu-test/src/disas.c:431:18: error: passing argument 1 of 'vfprintf' from
incompatible pointer type [-Werror=incompatible-pointer-types]
vfprintf(a, b, ap);
^
In file included from /tmp/qemu-test/src/include/qemu/osdep.h:99,
---
~~^~~~
In file included from /tmp/qemu-test/src/disas.c:3:
/tmp/qemu-test/src/disas.c: In function 'target_disas':
/tmp/qemu-test/src/include/disas/dis-asm.h:478:23: error: assignment to
'fprintf_function' {aka 'int (*)(struct _iobuf *, const char *, ...)'} from
incompatible pointer type 'int (*)(struct _IO_FILE *, const char *, ...)'
[-Werror=incompatible-pointer-types]
(INFO).fprintf_func = (FPRINTF_FUNC), \
^
/tmp/qemu-test/src/include/disas/dis-asm.h:470:3: note: in expansion of macro
'INIT_DISASSEMBLE_INFO_NO_ARCH'
---
/tmp/qemu-test/src/disas.c:450:5: note: in expansion of macro
'INIT_DISASSEMBLE_INFO'
INIT_DISASSEMBLE_INFO(s.info, out, fprintf_log);
^
/tmp/qemu-test/src/disas.c:481:21: error: passing argument 1 of 'fprintf_log'
from incompatible pointer type [-Werror=incompatible-pointer-types]
fprintf_log(out, "0x" TARGET_FMT_lx ": ", pc);
^~~
/tmp/qemu-test/src/disas.c:425:41: note: expected 'struct _IO_FILE *' but
argument is of type 'FILE *' {aka 'struct _iobuf *'}
static int fprintf_log(struct _IO_FILE *a, const char *b, ...)
~^
/tmp/qemu-test/src/disas.c:483:21: error: passing argument 1 of 'fprintf_log'
from incompatible pointer type [-Werror=incompatible-pointer-types]
fprintf_log(out, "\n");
^~~
/tmp/qemu-test/src/disas.c:425:41: note: expected 'struct _IO_FILE *' but
argument is of type 'FILE *' {aka 'struct _iobuf *'}
static int fprintf_log(struct _IO_FILE *a, const char *b, ...)
~^
cc1: all warnings being treated as errors
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: disas.o] Error 1
make[1]: *** Waiting for unfinished jobs
CC x86_64-softmmu/tcg/tcg-op-gvec.o
CC x86_64-softmmu/tcg/tcg-common.o
---
CC x86_64-softmmu/cpus.o
CC x86_64-softmmu/gdbstub.o
CC x86_64-softmmu/balloon.o
/tmp/qemu-test/src/disas.c:425:31: error: 'struct _IO_FILE' declared inside
parameter list will not be visible outside of this definition or declaration
[-Werror]
static int fprintf_log(struct _IO_FILE *a, const char *b, ...)
^~~~
/tmp/qemu-test/src/disas.c: In function 'fprintf_log':
/tmp/qemu-test/src/disas.c:431:18: error: passing argument 1 of 'vfprintf' from
incompatible pointer type [-Werror=incompatible-pointer-types]
vfprintf(a, b, ap);
^
In file included from /tmp/qemu-test/src/include/qemu/osdep.h:99,
---
~~^~~~
In file included from /tmp/qemu-test/src/disas.c:3:
/tmp/qemu-test/src/disas.c: In function 'target_disas':
/tmp/qemu-test/src/include/disas/dis-asm.h:478:23: error: assignment to
'fprintf_function' {aka 'int (*)(struct _iobuf *, const char *, ...)'} from
incompatible pointer type 'int (*)(struct _IO_FILE *, const char *, ...)'
[-Werror=incompatible-pointer-types]
(INFO).fprintf_func = (FPRINTF_FUNC), \
^
/tmp/qemu-test/src/include/disas/dis-asm.h:470:3: note: in expansion of macro
'INIT_DISASSEMBLE_INFO_NO_ARCH'
---
/tmp/qemu-test/src/disas.c:450:5: note: in expansion of macro
'INIT_DISASSEMBLE_INFO'
INIT_DISASSEMBLE_INFO(s.info, out, fprintf_log);
^
/tmp/qemu-test/src/disas.c:481:21: error: passing argument 1 of 'fprintf_log'
from incompatible pointer type [-Werror=incompatible-pointer-types]
fprintf_log(out, "0x"
Re: [PATCH v9 00/13] TCG code quality tracking and perf integration
Patchew URL:
https://patchew.org/QEMU/20191007152839.30804-1-alex.ben...@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH v9 00/13] TCG code quality tracking and perf integration
Message-id: 20191007152839.30804-1-alex.ben...@linaro.org
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
* [new tag] patchew/20191007152839.30804-1-alex.ben...@linaro.org ->
patchew/20191007152839.30804-1-alex.ben...@linaro.org
Switched to a new branch 'test'
2344fa6 configure: remove the final bits of --profiler support
f20376c tb-stats: adding TBStatistics info into perf dump
3ce3a87 accel/tcg: adding integration with linux perf
f430d85 tb-stats: dump hot TBs at the end of the execution
acc3b84 Adding info [tb-list|tb] commands to HMP (WIP)
cdf6f72 tb-stats: reset the tracked TBs on a tb_flush
9fe7b93 monitor: adding tb_stats hmp command
1358a45 debug: add -d tb_stats to control TBStatistics collection:
c1bf3a8 accel: adding TB_JIT_TIME and full replacing CONFIG_PROFILER
947c81f accel: replacing part of CONFIG_PROFILER with TBStats
b3bf3ff accel: collecting JIT statistics
c7359e9 accel: collecting TB execution count
ac71d5f accel/tcg: introduce TBStatistics structure
=== OUTPUT BEGIN ===
1/13 Checking commit ac71d5f77f09 (accel/tcg: introduce TBStatistics structure)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#37:
new file mode 100644
WARNING: Block comments use a leading /* on a separate line
#231: FILE: include/exec/tb-context.h:26:
+/* Page tracking code uses ram addresses in system mode, and virtual
WARNING: Block comments use * on subsequent lines
#232: FILE: include/exec/tb-context.h:27:
+/* Page tracking code uses ram addresses in system mode, and virtual
+ addresses in userspace mode. Define tb_page_addr_t to be an appropriate
WARNING: Block comments use a trailing */ on a separate line
#233: FILE: include/exec/tb-context.h:28:
+ type. */
total: 0 errors, 4 warnings, 271 lines checked
Patch 1/13 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/13 Checking commit c7359e966d69 (accel: collecting TB execution count)
3/13 Checking commit b3bf3ff3a8e4 (accel: collecting JIT statistics)
4/13 Checking commit 947c81f1a9c9 (accel: replacing part of CONFIG_PROFILER
with TBStats)
WARNING: line over 80 characters
#105: FILE: accel/tcg/tb-stats.c:85:
+qemu_printf("JIT cycles %" PRId64 " (%0.3f s at 2.4
GHz)\n",
WARNING: line over 80 characters
#126: FILE: accel/tcg/tb-stats.c:106:
+(double)s->la_time / (s->code_time ? s->code_time : 1) *
100.0);
WARNING: line over 80 characters
#130: FILE: accel/tcg/tb-stats.c:110:
+s->restore_count ? (double)s->restore_time /
s->restore_count : 0);
total: 0 errors, 3 warnings, 345 lines checked
Patch 4/13 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/13 Checking commit c1bf3a89e351 (accel: adding TB_JIT_TIME and full replacing
CONFIG_PROFILER)
WARNING: line over 80 characters
#112: FILE: accel/tcg/tb-stats.c:142:
+(double)jpi->opt_time / (jpi->code_time ? jpi->code_time : 1)
* 100.0);
WARNING: line over 80 characters
#114: FILE: accel/tcg/tb-stats.c:144:
+(double)jpi->la_time / (jpi->code_time ? jpi->code_time : 1) *
100.0);
WARNING: line over 80 characters
#118: FILE: accel/tcg/tb-stats.c:148:
+jpi->restore_count ? (double)jpi->restore_time /
jpi->restore_count : 0);
WARNING: line over 80 characters
#149: FILE: accel/tcg/tb-stats.c:152:
+s->cpu_exec_time, s->cpu_exec_time / (double)
NANOSECONDS_PER_SECOND);
WARNING: line over 80 characters
#589: FILE: tcg/tcg.c:4133:
+qemu_printf("%s %" PRId64 "\n", tcg_op_defs[i].name,
prof.table_op_count[i]);
total: 0 errors, 5 warnings, 557 lines checked
Patch 5/13 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/13 Checking commit 1358a45e85db (debug: add -d tb_stats to control
TBStatistics collection:)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#53:
new file mode 100644
WARNING: line over 80 characters
#206: FILE: util/log.c:279:
+{ CPU_LOG_TB_STATS,
"tb_stats[[,level=(+all+jit+exec+time)][,dump_limit=]]",
WARNING: line over 80 characters
#218: FILE: util/log.c:305:
+
[PATCH v9 00/13] TCG code quality tracking and perf integration
Hi, These are the bits of Vanderson's GSoC project which I think are ready for merge(^) and would like get up-streamed before the code freeze kicks in. It effectively removes the CONFIG_PROFILER support in favour of a more dynamic and configurable TB Statistics subsystem. While not in use it has no impact on the system apart a small amount of wasted space in the TCGContext for each thread which is used to collect stats during translation. When in use a TBStats structure is created for each set of TBs that have the same address/flags combination. The most basic tracking involves counting executions of each TB. You can also enable JIT stats and sort by features such as spill count and host/guest expansion ration. For full performance analysis you can use -perf to output a JIT dump that is compatible with the linux perf tool. This allows you to see exactly which TBs are responsible for the majority of real execution time. With the additional TB stats involved it will also include basic information about the quality of that code. The main changes from Vanderson's last post apart from general clean-ups and g_autoptr'ing is I've dropped the "cfg" and "coverset" commands. I dropped "cfg" because I wasn't happy about it navigated the next TB in the chain. "coverset" tends to dump quite a lot of TBs on a full system emulation so I wanted to think about a slightly smoother UI experience. ^ and finally I think I'll also drop the "tb" command for the next iteration because although useful it fails when the guest has un-mapped pages and results in a exception getting delivered to the guest which is less than ideal. See the comments for get_code_string. However I've left it in this iteration so you can see a taste of the future. There are more notes on the project at the wiki page: https://wiki.qemu.org/Features/TCGCodeQuality Please review. Alex Bennée (2): tb-stats: reset the tracked TBs on a tb_flush configure: remove the final bits of --profiler support Vanderson M. do Rosario (11): accel/tcg: introduce TBStatistics structure accel: collecting TB execution count accel: collecting JIT statistics accel: replacing part of CONFIG_PROFILER with TBStats accel: adding TB_JIT_TIME and full replacing CONFIG_PROFILER debug: add -d tb_stats to control TBStatistics collection: monitor: adding tb_stats hmp command Adding info [tb-list|tb] commands to HMP (WIP) tb-stats: dump hot TBs at the end of the execution accel/tcg: adding integration with linux perf tb-stats: adding TBStatistics info into perf dump accel/tcg/Makefile.objs | 4 +- accel/tcg/cpu-exec.c | 4 + accel/tcg/perf/Makefile.objs | 1 + accel/tcg/perf/jitdump.c | 206 +++ accel/tcg/perf/jitdump.h | 36 ++ accel/tcg/tb-stats.c | 662 ++ accel/tcg/tcg-runtime.c | 7 + accel/tcg/tcg-runtime.h | 2 + accel/tcg/translate-all.c | 173 +++-- accel/tcg/translator.c| 4 + configure | 8 - cpus.c| 14 +- disas.c | 31 +- docs/devel/tcg.rst| 15 + hmp-commands-info.hx | 16 + hmp-commands.hx | 17 + include/exec/exec-all.h | 15 +- include/exec/gen-icount.h | 10 + include/exec/tb-context.h | 12 + include/exec/tb-hash.h| 7 + include/exec/tb-stats-dump.h | 21 ++ include/exec/tb-stats-flags.h | 31 ++ include/exec/tb-stats.h | 165 + include/qemu-common.h | 3 + include/qemu/log-for-trace.h | 4 + include/qemu/log.h| 3 + include/qemu/timer.h | 5 +- linux-user/exit.c | 2 + linux-user/main.c | 7 + monitor/misc.c| 127 +-- os-posix.c| 5 + qemu-options.hx | 11 + stubs/Makefile.objs | 1 + stubs/tb-stats.c | 32 ++ tcg/tcg.c | 221 +++- tcg/tcg.h | 34 +- util/log.c| 89 - vl.c | 10 +- 38 files changed, 1727 insertions(+), 288 deletions(-) create mode 100644 accel/tcg/perf/Makefile.objs create mode 100644 accel/tcg/perf/jitdump.c create mode 100644 accel/tcg/perf/jitdump.h create mode 100644 accel/tcg/tb-stats.c create mode 100644 include/exec/tb-stats-dump.h create mode 100644 include/exec/tb-stats-flags.h create mode 100644 include/exec/tb-stats.h create mode 100644 stubs/tb-stats.c -- 2.20.1
