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

[Andreas Färber]

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?

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