2017-08-03 20:50 GMT+02:00 Rob Herring <r...@kernel.org>: > On Thu, Aug 3, 2017 at 11:12 AM, Mauro Rossi <issor.or...@gmail.com> wrote: >> 2017-08-03 16:52 GMT+02:00 Emil Velikov <emil.l.veli...@gmail.com>: >>> On 3 August 2017 at 13:24, Mauro Rossi <issor.or...@gmail.com> wrote: >>>> 2017-08-03 11:57 GMT+02:00 Emil Velikov <emil.l.veli...@gmail.com>: >>>>> Hi Mauro, >>>>> >>>>> Thanks for the series. I'll pull 2&3 in a second - there's a minor >>>>> suggestion in this patch. >>>>> >>>>> On 3 August 2017 at 01:55, Mauro Rossi <issor.or...@gmail.com> wrote: >>>>>> Android build changes to avoid the following building error: >>>>>> >>>>>> target C: libmesa_pipe_radeonsi <= >>>>>> external/mesa/src/gallium/drivers/radeonsi/si_pipe.c >>>>>> ... >>>>>> In file included from >>>>>> external/mesa/src/gallium/drivers/radeonsi/si_pipe.c:38: >>>>>> external/mesa/src/compiler/nir/nir.h:48:10: fatal error: 'nir_opcodes.h' >>>>>> file not found >>>>>> #include "nir_opcodes.h" >>>>>> ^ >>>>>> 1 error generated. >>>>>> >>>>>> Fixes: da62a31c5b "radeonsi: add nir include paths" >>>>>> --- >>>>>> src/gallium/drivers/radeonsi/Android.mk | 3 ++- >>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/src/gallium/drivers/radeonsi/Android.mk >>>>>> b/src/gallium/drivers/radeonsi/Android.mk >>>>>> index 6fff91f6f7..452bba3af8 100644 >>>>>> --- a/src/gallium/drivers/radeonsi/Android.mk >>>>>> +++ b/src/gallium/drivers/radeonsi/Android.mk >>>>>> @@ -36,7 +36,8 @@ LOCAL_MODULE_CLASS := STATIC_LIBRARIES >>>>>> >>>>>> LOCAL_C_INCLUDES := \ >>>>>> $(MESA_TOP)/src/amd/common \ >>>>>> - $(call >>>>>> generated-sources-dir-for,STATIC_LIBRARIES,libmesa_amd_common,,)/common >>>>>> + $(call >>>>>> generated-sources-dir-for,STATIC_LIBRARIES,libmesa_amd_common,,)/common \ >>>>>> + $(call >>>>>> generated-sources-dir-for,STATIC_LIBRARIES,libmesa_nir,,)/nir >>>>> The more robust solution is to add LOCAL_EXPORT_C_INCLUDE_DIRS for >>>>> libmesa_nir. >>>>> >>>>> With that in place we can drop the existing four instances of the >>>>> $(call generated-sources-dir-for... libmesa_nir... from the codebase. >>>>> >>>>> -Emil >>>> >>>> The current patch proposed is in principle already robust and mimiking >>>> automake, but if you prefer other way, I would propose doing this: >>>> >>>> LOCAL_C_INCLUDES := \ >>>> - $(call >>>> generated-sources-dir-for,STATIC_LIBRARIES,libmesa_amd_common,,)/common >>>> + $(call >>>> generated-sources-dir-for,STATIC_LIBRARIES,libmesa_amd_common,,)/common >>>> \ >>>> + $(dir $(MESA_GEN_NIR_H)) >>>> >>>> because LOCAL_EXPORT_C_INCLUDE_DIRS in Android.nir.mk would then >>>> require static linking of libmesa_nir and we would get double symbols >>>> at whole static linking in gallium_dri target due the current >>>> dri/target building rules. > > Can't you link against the libmesa_nir (i.e. add to > LOCAL_STATIC_LIBRARIES) and not add it to GALLIUM_LIBS?
As we speak GALLIUM_LIBS adds LOCAL_STATIC_LIBRARIES, so a different approach would be required. > >>>> >>> Was under the impression that there'll be symbol problems if both >>> places use WHOLE_STATIC. >> >> The issue would happen if libmesa_nir added to GALLIUM_LIBS in >> gallium/drivers/radeonsi/Android.mk, after Rob Herring simplifications >> to driver building rules and then libmesa_nir being listed twice in >> WHOLE_STATIC in gallium/targets/dri/Android.mk > > The fix is to either simply sort the whole > LOCAL_WHOLE_STATIC_LIBRARIES (not just GALLIUM_LIBS) or push the > libmesa_nir inclusion into the drivers that need it. The latter is the > better fix IMO. Probably the same thing should be done for > libmesa_glsl and libmesa_compiler. Additionally, the need for the rest > of the libs (not in GALLIUM_LIBS) could be moved to > LOCAL_STATIC_LIBRARIES. > > Rob We can do it with further patch, I would suggest to continue as Emil said to un-break the build, with macro that are anyway canonic for Android. Then we can have (afterwards) a look on the options: - LOCAL_EXPORT_C_INCLUDE_DIRS in libmesa_nir - or add $(dir $(MESA_GEN_NIR_H)) in LOCAL_C_INCLUDES as done for MESA_GEN_GLSL_H. - or add LOCAL_GENERATED_SOURCES += MESA_GEN_NIR_H as done in other places Mauro _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
