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

[Andreas Färber]

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.

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 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
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.
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.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg