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

["Andreas Färber" <afaerber@suse.de>]

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 <afaerber@suse.de> 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