Hi Robert,
There's a few minor comments below. Feel free to address here or as
follow-up, if applicable.
On 24 February 2017 at 13:58, Robert Bragg <rob...@sixbynine.org> wrote:
> Avoiding lots of error prone boilerplate and easing our ability to add +
> maintain support for multiple OA performance counter queries for each
> generation:
>
> This adds a python script to generate code for building up
> performance_queries from the metric sets and counters described in
> brw_oa_hsw.xml as well as functions to normalize each counter based on
> the RPN expressions given.
>
> Although the XML file currently only includes a single metric set, the
> code generated assumes there could be many sets.
>
> The metrics as described in XML get translated into C structures
> which are registered in a brw->perfquery.oa_metrics_table hash table
> keyed by the GUID of the metric set in XML.
>
Mauro, Tapani, we have another piece which will break Androids. Feel
free to send a fixup for Robert to squash.
> Signed-off-by: Robert Bragg <rob...@sixbynine.org>
> ---
> src/mesa/drivers/dri/i965/Makefile.am | 15 +-
> src/mesa/drivers/dri/i965/Makefile.sources | 2 +
> src/mesa/drivers/dri/i965/brw_oa.py | 543
> +++++++++++++++++++++++++++++
> 3 files changed, 559 insertions(+), 1 deletion(-)
> create mode 100644 src/mesa/drivers/dri/i965/brw_oa.py
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.am
> b/src/mesa/drivers/dri/i965/Makefile.am
> index f87fa67ef8..0130afff5f 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.am
> +++ b/src/mesa/drivers/dri/i965/Makefile.am
> @@ -93,7 +93,9 @@ BUILT_SOURCES = $(i965_compiler_GENERATED_FILES)
> CLEANFILES = $(BUILT_SOURCES)
>
> EXTRA_DIST = \
> - brw_nir_trig_workarounds.py
> + brw_nir_trig_workarounds.py \
> + brw_oa_hsw.xml \
> + brw_oa.py
>
> TEST_LIBS = \
> libi965_compiler.la \
> @@ -169,3 +171,14 @@ test_eu_validate_SOURCES = \
> test_eu_validate_LDADD = \
> $(top_builddir)/src/gtest/libgtest.la \
> $(TEST_LIBS)
> +
> +BUILT_SOURCES = \
> + brw_oa_hsw.h \
> + brw_oa_hsw.c
> +
> +brw_oa_hsw.h brw_oa_hsw.c: brw_oa_hsw.xml brw_oa.py Makefile
Eric reminded me that this is a bit buggy/racy. Something like the
following should be better:
brw_oa_hsw.h: brw_oa_hsw.c
brw_oa_hsw.c: brw_oa_hsw.xml brw_oa.py
...
> + $(PYTHON2) $(PYTHON_FLAGS) $(srcdir)/brw_oa.py \
> + --header=$(builddir)/brw_oa_hsw.h \
> + --code=$(builddir)/brw_oa_hsw.c \
> + --chipset="hsw" \
> + $(srcdir)/brw_oa_hsw.xml
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
> b/src/mesa/drivers/dri/i965/Makefile.sources
> index 5278e86339..60acd15d41 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -135,6 +135,8 @@ i965_FILES = \
> brw_nir_uniforms.cpp \
> brw_object_purgeable.c \
> brw_pipe_control.c \
> + brw_oa_hsw.h \
> + brw_oa_hsw.c \
H is after C ;-)
> brw_performance_query.h \
> brw_performance_query.c \
> brw_program.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_oa.py
> b/src/mesa/drivers/dri/i965/brw_oa.py
> new file mode 100644
> index 0000000000..2c622531af
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_oa.py
> @@ -0,0 +1,543 @@
> +#!/usr/bin/env python2
Thanks for omitting the execute bit !
Maybe drop this, since one should/can not execute the file directly ?
> +
> +if args.header:
> + header_file = open(args.header, 'w')
> +
> +
> +h(copyright)
> +h("""#pragma once
Use proper ifndef guards please.
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
