[Mesa-dev] [PATCH] android: vulkan: add support for libmesa_vulkan_{util, wsi}

Tue Feb 28 11:58:36 UTC 2017

On Tuesday, 2017-02-28 01:31:01 +0100, Mauro Rossi wrote:
> Thanks a lot for your feedbacks.
> Here follows final version.
> Mauro
> 
> From d0db1d0fd07fd0b5fb7580bb8dddf109389f28ba Mon Sep 17 00:00:00 2001
> From: Mauro Rossi <issor.oruam at gmail.com>
> Date: Tue, 28 Feb 2017 01:24:41 +0100
> Subject: [PATCH] android: vulkan: add support for libmesa_vulkan_util
> 
> The following changes are implemented:
> 
> Add src/vulkan/Android.mk to build libmesa_vulkan_util
> Android.mk: add src/vulkan to SUBDIR to build new module
> intel/vulkan: fix libmesa_vulkan_util,vk_enum_to_str.h dependencies
> Add -o OUTPUT_PATH option in src/vulkan/util/gen_enum_to_str.py script
> Use -o OUTPUT_PATH option in automake generation rules for vk_enum_to_str.{c,h}
> 
> Fixes: e9dcb17 "vulkan/util: Add generator for enum_to_str functions"
> Fixes: 8e03250 "vulkan: Combine wsi and util makefiles"
> ---
>  Android.mk                         |  3 ++-
>  src/intel/vulkan/Android.mk        |  8 ++++--
>  src/vulkan/Android.mk              | 55 ++++++++++++++++++++++++++++++++++++++
>  src/vulkan/Makefile.am             |  2 +-
>  src/vulkan/util/gen_enum_to_str.py |  9 +++++--
>  5 files changed, 71 insertions(+), 6 deletions(-)
>  create mode 100644 src/vulkan/Android.mk
> 
> diff --git a/Android.mk b/Android.mk
> index 4168b4d..bb70321 100644
> --- a/Android.mk
> +++ b/Android.mk
> @@ -92,7 +92,8 @@ SUBDIRS := \
>   src/egl \
>   src/amd \
>   src/intel \
> - src/mesa/drivers/dri
> + src/mesa/drivers/dri \
> + src/vulkan
> 
>  INC_DIRS := $(call all-named-subdir-makefiles,$(SUBDIRS))
> 
> diff --git a/src/intel/vulkan/Android.mk b/src/intel/vulkan/Android.mk
> index 1e53970..a6a7d26 100644
> --- a/src/intel/vulkan/Android.mk
> +++ b/src/intel/vulkan/Android.mk
> @@ -74,7 +74,8 @@ include $(BUILD_STATIC_LIBRARY)
>  ANV_INCLUDES := \
>   $(VULKAN_COMMON_INCLUDES) \
>   $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_anv_entrypoints,,) \
> - $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_nir,,)/nir
> + $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_nir,,)/nir \
> + $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_vulkan_util,,)/util
> 
>  #
>  # libanv for gen7
> @@ -172,7 +173,10 @@ LOCAL_C_INCLUDES := \
>   $(ANV_INCLUDES) \
>   $(MESA_TOP)/src/compiler
> 
> -LOCAL_WHOLE_STATIC_LIBRARIES := libmesa_anv_entrypoints libmesa_genxml
> +LOCAL_WHOLE_STATIC_LIBRARIES := \
> + libmesa_anv_entrypoints \
> + libmesa_genxml \
> + libmesa_vulkan_util
> 
>  LOCAL_GENERATED_SOURCES += $(intermediates)/anv_entrypoints.c
> 
> diff --git a/src/vulkan/Android.mk b/src/vulkan/Android.mk
> new file mode 100644
> index 0000000..871d7f3
> --- /dev/null
> +++ b/src/vulkan/Android.mk
> @@ -0,0 +1,55 @@
> +# Copyright © 2017 Mauro Rossi <issor.oruam at gmail.com>
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> +# IN THE SOFTWARE.
> +
> +LOCAL_PATH := $(call my-dir)
> +
> +# Import variables
> +include $(LOCAL_PATH)/Makefile.sources
> +
> +#
> +# libmesa_vulkan_util
> +#
> +
> +include $(CLEAR_VARS)
> +LOCAL_MODULE := libmesa_vulkan_util
> +LOCAL_MODULE_CLASS := STATIC_LIBRARIES
> +
> +intermediates := $(call local-generated-sources-dir)
> +
> +LOCAL_C_INCLUDES := \
> + $(MESA_TOP)/include/vulkan
> +
> +LOCAL_GENERATED_SOURCES := \
> + $(intermediates)/util/vk_enum_to_str.c \
> + $(intermediates)/util/vk_enum_to_str.h
> +
> +vulkan_api_xml = $(MESA_TOP)/src/vulkan/registry/vk.xml
> +
> +$(LOCAL_GENERATED_SOURCES):
> $(MESA_TOP)/src/vulkan/util/gen_enum_to_str.py $(vulkan_api_xml)
> + @echo "target Generated: $(PRIVATE_MODULE) <= $(notdir $(@))"
> + @mkdir -p $(dir $@)
> + $(hide) $(MESA_PYTHON2)
> $(MESA_TOP)/src/vulkan/util/gen_enum_to_str.py -o
> $(intermediates)/util
> +
> +LOCAL_EXPORT_C_INCLUDE_DIRS := \
> +        $(intermediates)
> +
> +include $(MESA_COMMON_MK)
> +include $(BUILD_STATIC_LIBRARY)
> diff --git a/src/vulkan/Makefile.am b/src/vulkan/Makefile.am
> index 5cdffbf..019da13 100644
> --- a/src/vulkan/Makefile.am
> +++ b/src/vulkan/Makefile.am
> @@ -17,7 +17,7 @@ BUILT_SOURCES = \
> 
>  util/vk_enum_to_str.c util/vk_enum_to_str.h: util/gen_enum_to_str.py
> $(vulkan_api_xml)
>   $(MKDIR_GEN)
> - $(PYTHON_GEN) $(srcdir)/util/gen_enum_to_str.py
> + $(PYTHON_GEN) $(srcdir)/util/gen_enum_to_str.py -o
> $(top_builddir)/src/vulkan/util

Just like the other Makefile patch a few days ago, this rule will run
the python script twice, script which writes both files.
Not performant, race condition, etc.

The clean fix is to give `-o $@` to the script (ie. the target of the rule)
and add an `if` in the script to only write the file given.

The quick and dirty fix is to add a rule forcing the serialisation, like
this:
  util/vk_enum_to_str.c: util/vk_enum_to_str.h
This fixes the race condition, but still writes both files twice for no
reason.

With either of these fixes applied:
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

> 
>  libvulkan_util_la_SOURCES = $(VULKAN_UTIL_FILES)
> 
> diff --git a/src/vulkan/util/gen_enum_to_str.py
> b/src/vulkan/util/gen_enum_to_str.py
> index 4b6fdf3..d45bcec 100644
> --- a/src/vulkan/util/gen_enum_to_str.py
> +++ b/src/vulkan/util/gen_enum_to_str.py
> @@ -22,6 +22,7 @@
>  """Create enum to string functions for vulking using vk.xml."""
> 
>  from __future__ import print_function
> +import argparse
>  import os
>  import textwrap
>  import xml.etree.cElementTree as et
> @@ -30,6 +31,10 @@ from mako.template import Template
> 
>  VK_XML = os.path.join(os.path.dirname(__file__), '..', 'registry', 'vk.xml')
> 
> +parser = argparse.ArgumentParser()
> +parser.add_argument('-o', '--output-path', required=True)
> +args = parser.parse_args()
> +
>  COPYRIGHT = textwrap.dedent(u"""\
>      * Copyright © 2017 Intel Corporation
>      *
> @@ -159,8 +164,8 @@ def xml_parser(filename):
> 
>  def main():
>      enums = xml_parser(VK_XML)
> -    for template, file_ in [(C_TEMPLATE, 'util/vk_enum_to_str.c'),
> -                            (H_TEMPLATE, 'util/vk_enum_to_str.h')]:
> +    for template, file_ in [(C_TEMPLATE,
> os.path.join(args.output_path, 'vk_enum_to_str.c')),
> +                            (H_TEMPLATE,
> os.path.join(args.output_path, 'vk_enum_to_str.h'))]:
>          with open(file_, 'wb') as f:
>              f.write(template.render(
>                  file=os.path.basename(__file__),
> -- 
> 2.9.3