Am 27.01.2013 15:14, schrieb Anthony Liguori:
> Andreas Färber <afaerber@xxxxxxx> 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 @@
>>
>> #include "helper_regs.h"
>>
>> -//#define DEBUG_OP
>> -//#define DEBUG_EXCEPTIONS
>> +#define DEBUG_OP 0
>> +#define DEBUG_EXCEPTIONS 0
>>
>> -#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
>
> Just thinking out loud a bit.. This form becomes pretty common and it's
> ashame to use a macro here if we don't have to.
>
> I think:
>
> 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);
> }
> }
>
> 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 probably
sprinkle quite some unlikely()s for optimization.
I think the if (0) { ... } approach would already catch a few things. As
a next step, some mechanism as proposed by Peter C. (?) to enable things
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 ever
> 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
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
