[Mesa-dev] [PATCH] Android: glsl: add rules to generate ir_expression*.h header files

[Emil Velikov]

On 2 September 2016 at 05:35, Tapani Pälli <tapani.palli at intel.com> wrote:
> Hi;
>
> Thanks, I was trying to work on the same issue but did not quite get it
> working .. will test this one with N build.
>
> FYI this patch contains both of these (which I believe were issues already
> before mentioned changes, just somehow revealed only with N build?):
>
The issue is makefile order/parallel jobs related.

IIRC latest builds use intermediate tools to convert the makefiles,
thus they could easily reshuffle the former or make the latter easier
to hit.

> https://patchwork.freedesktop.org/patch/107759/
> https://patchwork.freedesktop.org/patch/106788/
>
Was going to ask the question that Rob covered in his summary - why
don't we opt for the 'Android way' of pulling dependencies (generated
files and/or includes) by referencing the respective static/shared
lib.

>
>
> On 09/01/2016 10:06 PM, Rob Herring wrote:
>>
>> Recent changes to generate ir_expression*.h header files broke Android
>> builds. This adds the generation rules. This change is complicated due to
>> creating a circular dependency between libmesa_glsl, libmesa_nir, and
>> libmesa_compiler. Normally, we add static libraries so that include paths
>> are added even if there's no linking dependency. That is the case here.
>> Instead, we explicitly add the include path using $(MESA_GEN_GLSL_H) to
>> libmesa_compiler. This in turn requires shuffling the order of make
>> includes. It also uncovered missing dependency tracking of glsl_parser.h.
>>
Looking at this and the inherent over-linking that one gets when
referencing shared libraries makes me wonder if those were things
considered by the Android build system engineers.
/me bites his tongue not to prevent ranting on the topic.

Have you seen/heard anything on the topic of deprecating the old
Android.mk files and/or a tool to assist/automate the conversion ?

>> Signed-off-by: Rob Herring <robh at kernel.org>
>> ---
>>  src/compiler/Android.glsl.gen.mk    | 25 +++++++++++++++++++++++++
>>  src/compiler/Android.glsl.mk        |  1 -
>>  src/compiler/Android.mk             | 12 ++++++++----
>>  src/mesa/Android.libmesa_dricore.mk |  6 +++++-
>>  src/mesa/Android.libmesa_st_mesa.mk |  4 +++-
>>  src/mesa/program/Android.mk         |  6 ++++--
>>  6 files changed, 45 insertions(+), 9 deletions(-)
>>
Looks perfectly sane and is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

I'll quickly double-check and push as the coffee kicks in.

>> +$(intermediates)/glsl/ir_expression_operation_strings.h:
>> $(LOCAL_PATH)/glsl/ir_expression_operation.py
>> +       @mkdir -p $(dir $@)
>> +       $(hide) $(MESA_PYTHON2) $< strings > $@
Friendly suggestion about moving these/such build rules into the
Makefile.glsl.gen (or alike) and reusing them.
It will spare you/others some time and frustration:

Something like the below should work, but is untested.

$ cat Android...

MESA_BUILD_DIR := $(intermediates)
AM_V_at := @
MKDIR_P := mkdir -p
AM_V_GEN := $(hide)
srcdir := $(LOCAL_PATH)
// most/all of the above should be safe to move to Android.common.mk,
although one might need to change some := to =

include Makefile.glsl.gen
...

$ cat Makefile.glsl.am

MESA_BUILD_DIR = $(builddir)
MESA_PYTHON2 = $(PYTHON2)

include Makefile.glsl.gen
...

$ cat  Makefile.glsl.gen

MKDIR_GEN = $(AM_V_at)$(MKDIR_P) $(@D)
PYTHON_GEN = $(AM_V_GEN)$(MESA_PYTHON2) $(PYTHON_FLAGS)
...

$(MESA_BUILD_DIR)/glsl/ir_expression_operation_strings.h:
glsl/ir_expression_operation.py
    $(MKDIR_GEN)
    $(PYTHON_GEN) $(srcdir)/glsl/ir_expression_operation.py constant >
$@ || ($(RM) $@; false)

...


Regards,
Emil