> 1. nocheck isn't necessary anymore. The body of nocheck could be inlined
> here instead to simplify the generated code.
Yes I agree.I will remove nocheck and inline the body of nocheck in
trace-foo
> 2. "if (%(cond)s) {" is only useful for backends that implement
> .generate(). For example, if only the dtrace backend is enabled then
> "if (trace_event_get_state(...)) {}" will be emitted unnecessarily.
Yes, we should remove unnecessary if (trace_event_get_state(...)){}
blocks.
But It is difficult because backend.generate() calls
_run_function("generate_%s", event, group) which in turn loops on all
backends.Format can't call generate for individual backends.I will need
to make this map in scripts/tracetool/backend/__init__.py:_run_function().(I
think this
will not be a good thing to do).
possible fix would be to create in scripts/tracetool/backend/__init__.py
def is_conditional(self, cond_check):
self._run_function("generate_%s_conditional", cond_check)
now cond_check will be passed to all backends and backend's will have
def is_h_conditional(cond_check):
cond_check = cond_check or True
Finally if cond_check==True in h.py I will generate "if
(trace_event_get_state(...)) {"
else not. As the same condition is re-used this solution would work well.
Since h.py only handles reused/shared logic, it's safe to assume consistent
conditions.
If a new backend requires a different condition, it's better handled
directly in backend/*.py.
On Tue, Jun 24, 2025 at 8:07 PM Stefan Hajnoczi <stefa...@redhat.com> wrote:
> On Fri, Jun 20, 2025 at 02:37:19PM +0000, Tanish Desai wrote:
> > diff --git a/scripts/tracetool/format/h.py
> b/scripts/tracetool/format/h.py
> > index ea126b07ea..89d54b9aff 100644
> > --- a/scripts/tracetool/format/h.py
> > +++ b/scripts/tracetool/format/h.py
> > @@ -76,13 +76,17 @@ def generate(events, backend, group):
> > out('',
> > 'static inline void %(api)s(%(args)s)',
> > '{',
> > - ' if (%(cond)s) {',
> > + api=e.api(),
> > + args=e.args)
> > +
> > + if "disable" not in e.properties:
> > + backend.generate_unconditional(e, group)
> > +
> > + out(' if (%(cond)s) {',
> > ' %(api_nocheck)s(%(names)s);',
> > ' }',
> > '}',
> > - api=e.api(),
> > api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> > - args=e.args,
> > names=", ".join(e.args.names()),
> > cond=cond)
>
> Two thoughts:
>
> 1. nocheck isn't necessary anymore. The body of nocheck could be inlined
> here instead to simplify the generated code.
>
> 2. "if (%(cond)s) {" is only useful for backends that implement
> .generate(). For example, if only the dtrace backend is enabled then
> "if (trace_event_get_state(...)) {}" will be emitted unnecessarily.
>
> Maybe backends should have a .condition() interface so that
> scripts/tracetool/format/h.py:generate() can first collect a dict[cond]
> -> backend. Then it iterates over the map, calling backend.generate()
> within "if (%(cond)s) { ... }". That way only the conditions that are
> actually needed are generated and multiple backends that have the same
> condition will share the same if statement.
>
> Stefan
>
