On Wed, Mar 1, 2017 at 8:49 AM, Robert Bragg <rob...@sixbynine.org> wrote: > On Fri, Feb 24, 2017 at 2:50 PM, Emil Velikov <emil.l.veli...@gmail.com> > wrote: >> 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. > > Ah, can you maybe give me a bit more of a hint about what I'm breaking > for Android? Just extra work running python scripts as part of an > Android build I guess and requiring some change to > ./src/mesa/drivers/dri/i965/Android.gen.mk? > >> >>> 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 > > as one issue, I can see that brw_oa_hsw.h isn't declared as a > prerequisite for compiling brw_oa_hsw.c, am I missing more issues? > Based on that I'd imagine adding: > > brw_oa_hsw.c: brw_oa_hsw.h > > as a way to ensure we can't attempt to compile brw_oa_hsw.c before > brw_oa_hsw.h is generated. > Is there a reason for removing the Makefile prerequisite? If I were to > mess with the arguments passed to the script in the codegen rules I'd > like a Makefile change to result in re-running those rules.
We depend on the Makefile in src/intel/Makefile.genxml.am, and one unintended side-effect of that is that after every ./configure the files that depend on Makefile have to be recompiled. In that case, contents of the Makefile end up in the resulting files, so I think the dependency is okay if a bit annoying. This patch, however, I'd remove the Makefile dependency, since the Makefile just contains the execution of the script. I'm not sure why the script arguments would change... _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
