From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Subject: Re: [RFC 16/19] target-ppc: Refactor debug output macros Date: Sun, 27 Jan 2013 15:35:08 +0100 Message-ID: <51053B1C.8080205@suse.de> References: <1359293537-8251-1-git-send-email-afaerber@suse.de> <1359293537-8251-17-git-send-email-afaerber@suse.de> <87txq2sofb.fsf@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: qemu-devel@nongnu.org, Alexander Graf , Gleb Natapov , Marcelo Tosatti , PowerPC , Overall To: Anthony Liguori Return-path: Received: from cantor2.suse.de ([195.135.220.15]:50138 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754965Ab3A0OfU (ORCPT ); Sun, 27 Jan 2013 09:35:20 -0500 In-Reply-To: <87txq2sofb.fsf@codemonkey.ws> Sender: kvm-owner@vger.kernel.org List-ID: Am 27.01.2013 15:14, schrieb Anthony Liguori: > Andreas F=C3=A4rber writes: >> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c >> index 0a1ac86..54722c4 100644 >> --- a/target-ppc/excp_helper.c >> +++ b/target-ppc/excp_helper.c >> @@ -21,14 +21,14 @@ >> =20 >> #include "helper_regs.h" >> =20 >> -//#define DEBUG_OP >> -//#define DEBUG_EXCEPTIONS >> +#define DEBUG_OP 0 >> +#define DEBUG_EXCEPTIONS 0 >> =20 >> -#ifdef DEBUG_EXCEPTIONS >> -# define LOG_EXCP(...) qemu_log(__VA_ARGS__) >> -#else >> -# define LOG_EXCP(...) do { } while (0) >> -#endif >> +#define LOG_EXCP(...) G_STMT_START \ >> + if (DEBUG_EXCEPTIONS) { \ >> + qemu_log(__VA_ARGS__); \ >> + } \ >> + G_STMT_END >=20 > Just thinking out loud a bit.. This form becomes pretty common and i= t's > ashame to use a macro here if we don't have to. >=20 > I think: >=20 > static inline void LOG_EXCP(const char *fmt, ...) > { > if (debug_exceptions) { > va_list ap; > va_start(ap, fmt); > qemu_logv(fmt, ap); > va_end(ap); > } > } >=20 > Probably would have equivalent performance. debug_exception would be > read-mostly and ought to be very predictable as a result. I strongly > expect that the compiler would actually inline LOG_EXCP too. Thanks for your early feedback. I merely tried to stay close to the original code. I wouldn't mind inline functions either. Or even more harmonization for that matter. > I see LOG_EXCP and LOG_DIS in this series. Perhaps we could just > introduce these functions and then make these flags run-time > controllable? I was feeling conservative during that series in light of compile-time decided conditional; if we want to go down that route we should probabl= y sprinkle quite some unlikely()s for optimization. I think the if (0) { ... } approach would already catch a few things. A= s a next step, some mechanism as proposed by Peter C. (?) to enable thing= s at configure-time could be built on top. Run-time would need some stabilization phase to avoid command line compatibility issues. > BTW, one advantage of this over your original proposal back to your > point is that you still won't catch linker errors with your proposal. > Dead code eliminate will kill off those branches before the linker ev= er > sees them. Linker errors would be limited to renamed/dropped/#ifdef'ed functions, wouldn't they? In the past I caught that using existing --enable-debug. My recurring issue is overlooking env->something after removing fields from CPU_COMMON/CPUArchState. I was hoping that to be caught inside if (0) { ... } during my 3x KVM + BSD + MinGW builds rather than patching individual files. Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3= =BCrnberg