qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC 16/19] target-ppc: Refactor debug output macros


From: Alexander Graf
Subject: Re: [Qemu-devel] [RFC 16/19] target-ppc: Refactor debug output macros
Date: Sun, 27 Jan 2013 17:10:57 +0100


Am 27.01.2013 um 15:54 schrieb Andreas Färber <address@hidden>:

> Am 27.01.2013 15:46, schrieb Alexander Graf:
>> 
>> On 27.01.2013, at 15:35, Andreas Färber wrote:
>> 
>>> Am 27.01.2013 15:14, schrieb Anthony Liguori:
>>>> Andreas Färber <address@hidden> 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 fully agree. Just recently Scott revamped the openpic debug print code:
>> 
>> 
>> //#define DEBUG_OPENPIC
>> 
>> #ifdef DEBUG_OPENPIC
>> static const int debug_openpic = 1;
>> #else
>> static const int debug_openpic = 0;
>> #endif
>> 
>> #define DPRINTF(fmt, ...) do { \
>>        if (debug_openpic) { \
>>            printf(fmt , ## __VA_ARGS__); \
>>        } \
>>    } while (0)
> 
> Like :)
> 
> I'll wait for more feedback from affected maintainers though before I
> redo or widen this series.
> Or were you proposing to do a ppc-only refactoring à la Scott for 1.4?

No, I think it makes a lot more sense to do this tree-wide after 1.4 :).

Alex

> 
> Andreas
> 
>> I like that approach. It keeps all users identical. The #define stays 
>> identical. The callers stay identical. But we do get proper compile time 
>> checks. Of course Anthony's approach works too, but the thing I'd definitely 
>> like to see is that the #defines don't become numerical, but rather stay of 
>> an #ifdef basis.
>> 
>> 
>> Alex
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



reply via email to

[Prev in Thread] Current Thread [Next in Thread]