Re: [PATCH 2/3] tracetool: introduce generate_unconditional

["Alex Bennée" <alex.bennee@linaro.org>]

Tanish Desai <tanishdesai37@gmail.com> writes:

> This patch separates the generation logic of trace_foo() for the UST and DTrace backends from other backends.
> The motivation is to remove the unnecessary if (true) in the _no_check function, as UST and DTrace do not require a trace_event_get_state check without introducing a seperate function it is very difficult to generate code which keeps them out of unified if condition.
> With this separation, we can safely move the trace_event_get_state check into trace_foo for the other backends only, keeping UST/DTrace generation paths clean.
> A new generate_h_unconditional function has been introduced for UST
> and DTrace. It behaves similarly to generate_h, but is defined only in
> UST and DTrace backends. This ensures that generate_h is used by the
> other backends, while UST/DTrace selectively use
> generate_h_unconditional.

Please fix the word wrap on the commit at 78 chars.

>
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
>  scripts/tracetool/backend/__init__.py |  3 +++
>  scripts/tracetool/backend/dtrace.py   |  3 ++-
>  scripts/tracetool/backend/ust.py      |  2 +-
>  scripts/tracetool/format/h.py         | 10 +++++++---
>  4 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
> index 7bfcc86cc5..c4456a5efd 100644
> --- a/scripts/tracetool/backend/__init__.py
> +++ b/scripts/tracetool/backend/__init__.py
> @@ -118,6 +118,9 @@ def generate_begin(self, events, group):
>      def generate(self, event, group):
>          self._run_function("generate_%s", event, group)
>  
> +    def generate_unconditional(self, event, group):
> +        self._run_function("generate_%s_unconditional", event, group)
> +    
>      def generate_backend_dstate(self, event, group):
>          self._run_function("generate_%s_backend_dstate", event, group)
>  
> diff --git a/scripts/tracetool/backend/dtrace.py b/scripts/tracetool/backend/dtrace.py
> index e17edc9b9d..171b7e09ed 100644
> --- a/scripts/tracetool/backend/dtrace.py
> +++ b/scripts/tracetool/backend/dtrace.py
> @@ -61,7 +61,8 @@ def generate_h_begin(events, group):
>              '#endif',
>              uppername=e.name.upper())
>  
> -def generate_h(event, group):
> +

stray newline

> +def generate_h_unconditional(event, group):
>      out('    QEMU_%(uppername)s(%(argnames)s);',
>          uppername=event.name.upper(),
>          argnames=", ".join(event.args.names()))
> diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
> index c857516f21..1564b490ec 100644
> --- a/scripts/tracetool/backend/ust.py
> +++ b/scripts/tracetool/backend/ust.py
> @@ -30,7 +30,7 @@ def generate_h_begin(events, group):
>          '')
>  
>  
> -def generate_h(event, group):
> +def generate_h_unconditional(event, group):
>      argnames = ", ".join(event.args.names())
>      if len(event.args) > 0:
>          argnames = ", " + argnames
> 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)

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro