* [Qemu-devel] [RCF PATCH 0/2] Improving TCG debug output @ 2014-03-12 14:12 alex.bennee 2014-03-12 14:12 ` [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info alex.bennee 2014-03-12 14:12 ` [Qemu-devel] [RCF PATCH 2/2] tcg: add debug helpers tcg_debug_dump_i(32|64) alex.bennee 0 siblings, 2 replies; 6+ messages in thread From: alex.bennee @ 2014-03-12 14:12 UTC (permalink / raw) To: qemu-devel; +Cc: Alex Bennée From: Alex Bennée <alex.bennee@linaro.org> Hi, These two patches have been sitting in my personal tree for a while and I thought it was worth soliciting feedback as to their wider usefulness. The first is simply an attempt to make tcg abort failures a little less terse. The second I found useful when I was debugging a complex set of TCG ops for a round, shift and narrow implementation. The alternative was to set up GDB and step through the generated target code (or just infer from the copious dumps). The macro magic might be a bit much though. Alex Bennée (2): tcg: add tcg_abort_dbg() for additional debug info tcg: add debug helpers tcg_debug_dump_i(32|64) Makefile.target | 2 +- target-arm/helper.h | 2 ++ tcg/i386/tcg-target.c | 4 ++-- tcg/optimize.c | 2 +- tcg/tcg-helpers.c | 32 +++++++++++++++++++++++++++++ tcg/tcg-helpers.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++ tcg/tcg.h | 7 +++++-- 7 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 tcg/tcg-helpers.c create mode 100644 tcg/tcg-helpers.h -- 1.9.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info 2014-03-12 14:12 [Qemu-devel] [RCF PATCH 0/2] Improving TCG debug output alex.bennee @ 2014-03-12 14:12 ` alex.bennee 2014-03-12 15:02 ` Peter Maydell 2014-03-12 14:12 ` [Qemu-devel] [RCF PATCH 2/2] tcg: add debug helpers tcg_debug_dump_i(32|64) alex.bennee 1 sibling, 1 reply; 6+ messages in thread From: alex.bennee @ 2014-03-12 14:12 UTC (permalink / raw) To: qemu-devel; +Cc: Alex Bennée, Richard Henderson From: Alex Bennée <alex.bennee@linaro.org> There are times the tcg aborts with a fatal but terse error which isn't overly helpful. This adds an alternative macro that can be used to show a little more helper information when an abort occurs. diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c index f832282..1a6c565 100644 --- a/tcg/i386/tcg-target.c +++ b/tcg/i386/tcg-target.c @@ -1342,7 +1342,7 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l) } break; default: - tcg_abort(); + tcg_abort_dbg("bad opc:%x", opc); } /* Jump to the code corresponding to next IR of qemu_st */ @@ -1519,7 +1519,7 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi, } break; default: - tcg_abort(); + tcg_abort_dbg("bad memop=%x", memop); } } diff --git a/tcg/optimize.c b/tcg/optimize.c index 7777743..ae1a3f8 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -407,7 +407,7 @@ static bool do_constant_folding_cond_eq(TCGCond c) case TCG_COND_EQ: return 1; default: - tcg_abort(); + tcg_abort_dbg("bad condition:%d", c); } } diff --git a/tcg/tcg.h b/tcg/tcg.h index f7efcb4..bfde09f 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -657,12 +657,15 @@ typedef struct TCGTargetOpDef { const char *args_ct_str[TCG_MAX_OP_ARGS]; } TCGTargetOpDef; -#define tcg_abort() \ +#define tcg_abort_dbg(fmt, ...)\ do {\ - fprintf(stderr, "%s:%d: tcg fatal error\n", __FILE__, __LINE__);\ + fprintf(stderr, "%s:%d: tcg fatal error " fmt "\n",\ + __FILE__, __LINE__, ## __VA_ARGS__);\ abort();\ } while (0) +#define tcg_abort() tcg_abort_dbg("") + #ifdef CONFIG_DEBUG_TCG # define tcg_debug_assert(X) do { assert(X); } while (0) #elif QEMU_GNUC_PREREQ(4, 5) -- 1.9.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info 2014-03-12 14:12 ` [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info alex.bennee @ 2014-03-12 15:02 ` Peter Maydell 2014-03-12 15:05 ` Richard Henderson 0 siblings, 1 reply; 6+ messages in thread From: Peter Maydell @ 2014-03-12 15:02 UTC (permalink / raw) To: Alex Bennée; +Cc: QEMU Developers, Richard Henderson On 12 March 2014 14:12, <alex.bennee@linaro.org> wrote: > From: Alex Bennée <alex.bennee@linaro.org> > > There are times the tcg aborts with a fatal but terse error which isn't > overly helpful. This adds an alternative macro that can be used to show > a little more helper information when an abort occurs. > > diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c > index f832282..1a6c565 100644 > --- a/tcg/i386/tcg-target.c > +++ b/tcg/i386/tcg-target.c > @@ -1342,7 +1342,7 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l) > } > break; > default: > - tcg_abort(); > + tcg_abort_dbg("bad opc:%x", opc); > } > > /* Jump to the code corresponding to next IR of qemu_st */ > @@ -1519,7 +1519,7 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi, > } > break; > default: > - tcg_abort(); > + tcg_abort_dbg("bad memop=%x", memop); > } > } > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index 7777743..ae1a3f8 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -407,7 +407,7 @@ static bool do_constant_folding_cond_eq(TCGCond c) > case TCG_COND_EQ: > return 1; > default: > - tcg_abort(); > + tcg_abort_dbg("bad condition:%d", c); This is the wrong place to be diagnosing this. We should be (when TCG debug is enabled) checking the condition when the tcg_gen_setcond or tcg_set_brcond function is called, so that you get a useful backtrace that points directly at the buggy frontend code. I suspect the other two cases are similar. thanks -- PMM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info 2014-03-12 15:02 ` Peter Maydell @ 2014-03-12 15:05 ` Richard Henderson 0 siblings, 0 replies; 6+ messages in thread From: Richard Henderson @ 2014-03-12 15:05 UTC (permalink / raw) To: Peter Maydell, Alex Bennée; +Cc: QEMU Developers On 03/12/2014 08:02 AM, Peter Maydell wrote: > This is the wrong place to be diagnosing this. We should be (when > TCG debug is enabled) checking the condition when the tcg_gen_setcond > or tcg_set_brcond function is called, so that you get a useful backtrace > that points directly at the buggy frontend code. I suspect the other > two cases are similar. Indeed. See the asserts in tcg_gen_deposit_i32 for example. r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [RCF PATCH 2/2] tcg: add debug helpers tcg_debug_dump_i(32|64) 2014-03-12 14:12 [Qemu-devel] [RCF PATCH 0/2] Improving TCG debug output alex.bennee 2014-03-12 14:12 ` [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info alex.bennee @ 2014-03-12 14:12 ` alex.bennee 1 sibling, 0 replies; 6+ messages in thread From: alex.bennee @ 2014-03-12 14:12 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Ákos Kovács, Fam Zheng, Paolo Bonzini, Alex Bennée, Andreas Färber, Richard Henderson [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=true, Size: 4956 bytes --] From: Alex Bennée <alex.bennee@linaro.org> The helpers are useful for debugging if you want to inspect interim values of tcg temp variables while executing TCG generated code. This is an alternative to tracing with logs or inspecting with GDB. The functions do take format strings but to prevent massive memory loss it will default to a constant string after a short number of uses. create mode 100644 tcg/tcg-helpers.c create mode 100644 tcg/tcg-helpers.h diff --git a/Makefile.target b/Makefile.target index ba12340..a87b7d7 100644 --- a/Makefile.target +++ b/Makefile.target @@ -73,7 +73,7 @@ all: $(PROGS) stap ######################################################### # cpu emulator library obj-y = exec.o translate-all.o cpu-exec.o -obj-y += tcg/tcg.o tcg/optimize.o +obj-y += tcg/tcg.o tcg/optimize.o tcg/tcg-helpers.o obj-$(CONFIG_TCG_INTERPRETER) += tci.o obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o obj-y += fpu/softfloat.o diff --git a/target-arm/helper.h b/target-arm/helper.h index 8923f8a..7600c21 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -507,4 +507,6 @@ DEF_HELPER_FLAGS_3(crc32c, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) #include "helper-a64.h" #endif +#include "tcg/tcg-helpers.h" + #include "exec/def-helper.h" diff --git a/tcg/tcg-helpers.c b/tcg/tcg-helpers.c new file mode 100644 index 0000000..83bfee6 --- /dev/null +++ b/tcg/tcg-helpers.c @@ -0,0 +1,32 @@ +/* + * TCG Common Helper Functions + * + * Copyright (c) 2014 + * Written by Alex Bennée <alex.bennee@linaro.org> + * + * This code is licensed under the GNU GPL v2. + * + * This file contains common non-architecture specific helper + * functions that can be used from TCG generated code. Currently these + * are mainly helpful for debugging. + */ + +#include <stdlib.h> +#include <stdio.h> + +#include "cpu.h" +#include "exec/exec-all.h" +#include "helper.h" + +/* TCG Value Dumpers */ +uint32_t HELPER(dump_u32)(uint32_t val, void *string) +{ + fprintf(stderr,"%s %x\n", (char *) string, val); + return val; +} + +uint64_t HELPER(dump_u64)(uint64_t val, void *string) +{ + fprintf(stderr,"%s %lx\n", (char *) string, val); + return val; +} diff --git a/tcg/tcg-helpers.h b/tcg/tcg-helpers.h new file mode 100644 index 0000000..510d8b5 --- /dev/null +++ b/tcg/tcg-helpers.h @@ -0,0 +1,57 @@ +/* + * TCG Common Helpers + * + * Copyright (c) 2014 + * Written by Alex Bennée <alex.bennee@linaro.org> + * + * This code is licensed under the GNU GPL v2. + * + * WARNING: intended to be included in $ARCH/helper.h + */ + +DEF_HELPER_2(dump_u32, i32, i32, ptr) +DEF_HELPER_2(dump_u64, i64, i64, ptr) + + +/* + * Formatting macros + * + * To be useful we would like to format a string with a message to be + * associated with this dump. As these strings have to live the + * lifetime of the generated code we are essentially leaking memory so + * the more times a given dump call is generated the more memory is + * consumed (not so for each time the TCG code itself is called) + */ + +/* This limits the number of malloc'ed strings *per call site* */ +#define TCG_DEBUG_DUMP_MAX_STRINGS 10 + +/* String macro magic */ +#define STRINGIFY_DETAIL(x) #x +#define STRINGIFY(x) STRINGIFY_DETAIL(x) + +#define tcg_debug_dump_i32(tcg32, fmt, ...) \ +do { \ + static int tcg_debug_alloc_strings = 0; \ + gchar *debug_string = (gchar *) __FILE__ ":" STRINGIFY(__LINE__) ":"; \ + TCGv_ptr tcg_string; \ + if (tcg_debug_alloc_strings++ < TCG_DEBUG_DUMP_MAX_STRINGS) { \ + debug_string = g_strdup_printf(fmt ":", ## __VA_ARGS__); \ + } \ + tcg_string = tcg_const_ptr(debug_string); \ + gen_helper_dump_u32(tcg32, tcg32, tcg_string); \ + tcg_temp_free_ptr(tcg_string); \ +} while (0); + +#define tcg_debug_dump_i64(tcg64, fmt, ...) \ +do { \ + static int tcg_debug_alloc_strings = 0; \ + gchar *debug_string = (gchar *) __FILE__ ":" STRINGIFY(__LINE__) ":"; \ + TCGv_ptr tcg_string; \ + if (tcg_debug_alloc_strings++ < TCG_DEBUG_DUMP_MAX_STRINGS) { \ + debug_string = g_strdup_printf(fmt ":", ## __VA_ARGS__); \ + } \ + tcg_string = tcg_const_ptr(debug_string); \ + gen_helper_dump_u64(tcg64, tcg64, tcg_string); \ + tcg_temp_free_ptr(tcg_string); \ +} while (0); -- 1.9.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [RCF PATCH 0/2] Improving TCG debug output @ 2014-03-12 13:49 alex.bennee 2014-03-12 13:49 ` [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info alex.bennee 0 siblings, 1 reply; 6+ messages in thread From: alex.bennee @ 2014-03-12 13:49 UTC (permalink / raw) To: qemu-devel From: Alex Bennée <alex.bennee@linaro.org> Hi, These two patches have been sitting in my personal tree for a while and I thought it was worth soliciting feedback as to their wider usefulness. The first is simply an attempt to make tcg abort failures a little less terse. The second I found useful when I was debugging a complex set of TCG ops for a round, shift and narrow implementation. The alternative was to set up GDB and step through the generated target code (or just infer from the copious dumps). The macro magic might be a bit much though. Alex Bennée (2): tcg: add tcg_abort_dbg() for additional debug info tcg: add debug helpers tcg_debug_dump_i(32|64) Makefile.target | 2 +- target-arm/helper.h | 2 ++ tcg/i386/tcg-target.c | 4 ++-- tcg/optimize.c | 2 +- tcg/tcg-helpers.c | 32 +++++++++++++++++++++++++++++ tcg/tcg-helpers.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++ tcg/tcg.h | 7 +++++-- 7 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 tcg/tcg-helpers.c create mode 100644 tcg/tcg-helpers.h -- 1.9.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info 2014-03-12 13:49 [Qemu-devel] [RCF PATCH 0/2] Improving TCG debug output alex.bennee @ 2014-03-12 13:49 ` alex.bennee 0 siblings, 0 replies; 6+ messages in thread From: alex.bennee @ 2014-03-12 13:49 UTC (permalink / raw) To: qemu-devel From: Alex Bennée <alex.bennee@linaro.org> There are times the tcg aborts with a fatal but terse error which isn't overly helpful. This adds an alternative macro that can be used to show a little more helper information when an abort occurs. diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c index f832282..1a6c565 100644 --- a/tcg/i386/tcg-target.c +++ b/tcg/i386/tcg-target.c @@ -1342,7 +1342,7 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l) } break; default: - tcg_abort(); + tcg_abort_dbg("bad opc:%x", opc); } /* Jump to the code corresponding to next IR of qemu_st */ @@ -1519,7 +1519,7 @@ static void tcg_out_qemu_ld_direct(TCGContext *s, TCGReg datalo, TCGReg datahi, } break; default: - tcg_abort(); + tcg_abort_dbg("bad memop=%x", memop); } } diff --git a/tcg/optimize.c b/tcg/optimize.c index 7777743..ae1a3f8 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -407,7 +407,7 @@ static bool do_constant_folding_cond_eq(TCGCond c) case TCG_COND_EQ: return 1; default: - tcg_abort(); + tcg_abort_dbg("bad condition:%d", c); } } diff --git a/tcg/tcg.h b/tcg/tcg.h index f7efcb4..bfde09f 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -657,12 +657,15 @@ typedef struct TCGTargetOpDef { const char *args_ct_str[TCG_MAX_OP_ARGS]; } TCGTargetOpDef; -#define tcg_abort() \ +#define tcg_abort_dbg(fmt, ...)\ do {\ - fprintf(stderr, "%s:%d: tcg fatal error\n", __FILE__, __LINE__);\ + fprintf(stderr, "%s:%d: tcg fatal error " fmt "\n",\ + __FILE__, __LINE__, ## __VA_ARGS__);\ abort();\ } while (0) +#define tcg_abort() tcg_abort_dbg("") + #ifdef CONFIG_DEBUG_TCG # define tcg_debug_assert(X) do { assert(X); } while (0) #elif QEMU_GNUC_PREREQ(4, 5) -- 1.9.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-12 15:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-12 14:12 [Qemu-devel] [RCF PATCH 0/2] Improving TCG debug output alex.bennee 2014-03-12 14:12 ` [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info alex.bennee 2014-03-12 15:02 ` Peter Maydell 2014-03-12 15:05 ` Richard Henderson 2014-03-12 14:12 ` [Qemu-devel] [RCF PATCH 2/2] tcg: add debug helpers tcg_debug_dump_i(32|64) alex.bennee -- strict thread matches above, loose matches on Subject: below -- 2014-03-12 13:49 [Qemu-devel] [RCF PATCH 0/2] Improving TCG debug output alex.bennee 2014-03-12 13:49 ` [Qemu-devel] [RCF PATCH 1/2] tcg: add tcg_abort_dbg() for additional debug info alex.bennee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).