Re: [Qemu-devel] [PATCH v2 00/15] Debug output revamp

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Am 21.02.2013 17:10, schrieb Richard Henderson:
>> On 2013-02-20 20:24, Andreas Färber wrote:
>>> v2 replaces macros with static functions, adopting Scott's scheme of
>>> const
>>> variables set through #ifdefs (keeping their defined() semantics) and
>>> adopting
>>> Anthony's proposal of using va_list for argument passing.
>>> v1 had changed some #ifdefs into ifs due to the #ifdef -> #if change;
>>> while not
>>> strictly necessary anymore, I have not reverted this (mostly sparc).
>> 
>> Sorry I missed the discussion that must have went into v1, but I'm not
>> thrilled about this.
>> 
>> My gcc intuition says that varargs means that inlining won't happen, and
>> that static const means that the code won't be deleted as dead at
>> compile time.  Which means that we're going to have more overhead both
>> when the debugging is disabled and when it is enabled.
>>
>> Is there an especially good reason why we're not still using a macro,
>> but one controlled by an if (0) when debugging is disabled?
>
> v1 had that.
>
> static const bool: Coming from openpic, suggested by Alex to keep #ifdef
> rather than #if semantics. Would a #define be any better?
>
> if (0) vs. #ifdef: See my tmp105 debug output patch that went in between
> v1 and v2, it used #ifdef inside the static function.
> It was suggested that if the functions have no side-effects, GCC would
> optimize them out in the if (0) case.
> I checked in one case that a helper function cc_name() passed as vararg
> to D_LOG() did not show up in objdump in the if (0) case. I admit I
> probably didn't check for the D_LOG() function itself.

We'd have to check every compiler we care about, and even then we
wouldn't really be certain the optimizer gets rid of the debug code
reliably.

Relying on dead code elimination is fine in simple cases, but relying on
automatic inlining + optimizing load of const variable into value of its
initializer + dead code elminination is asking for quite a bit more.

> Macros: Need for \ linebreaks and heated discussions about
> G_STMT_START/END vs. do {...} while (0) vs. functions. Really don't want
> to repeat that.

If I remember correctly, that was you vs. everybody else, so an easy way
not to repeat it is to do it the way everybody else does it ;-P

> If you want to make a proposal please do, it was a lot of dumb typing
> work... For example you could use tracepoints to avoid the issue in

Thanks for that!  I hope we can salvage it.

> alpha and s390x, but others such as Alex and Peter don't like the
> tracing infrastructure and it'd require non-mechanical changes so we
> seem to be stuck with the current macros.
>
> Disabled ppc #ifdefs caused issues recently in a target-ppc series
> removing ppc_def_t.

Annoying, but not the end of the world.

> And my QOM CPUState part 9 series needs to do ugly variable definitions
> in target-cris/helper.c due to the disabled D(x) and D_LOG(x) macros.

Even more annoying.

I got one more: debug print macros are defined all over the place, with
minor variations.