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? >>> >> 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
