On Fri, Aug 18, 2017 at 8:53 PM, Chih-Wei Huang <cwhu...@android-x86.org> wrote: > 2017-08-19 8:27 GMT+08:00 Emil Velikov <emil.l.veli...@gmail.com>: >> On 18 August 2017 at 20:46, Rob Herring <r...@kernel.org> wrote: >>> Both statically linking libLLVMCore and dynamically linking libLLVM causes >>> duplicated symbols in gallium_dri.so and it fails to dlopen. We don't >>> really need to link libLLVMCore, but just need generated headers to be >>> built first. Dynamically linking to libLLVM instead is enough to do >>> that. Thanks to Qiang Yu for finding the root cause. >>> >> Nice find indeed, thanks. >> >> This reminds me - a small task for a rainy day. >> - Wire the version script files into the Android build - see the >> autoconf snippet below. >> It will hide the hundreds of symbols when static linking LLVM (aka >> sidestep the current issue) and make the binary noticeably smaller. >> >> gallium_dri_la_LDFLAGS += \ >> -Wl,--version-script=$(top_srcdir)/src/gallium/targets/dri/dri.sym >> >>> With this change, we can align all versions and just have libLLVM as a >>> shared lib dependency. >>> >>> This also requires changes in the M and N versions of LLVM to export the >>> include paths for libLLVM. AOSP master is okay. >>> >> Perfect :-) >> >>> Fixes: 26aee6f4d5a ("Android: rework LLVM build support") >>> Reported-by: Mauro Rossi <issor.or...@gmail.com> >>> Cc: Emil Velikov <emil.l.veli...@gmail.com> >>> Cc: 17.2 <mesa-sta...@lists.freedesktop.org> >>> Signed-off-by: Qiang Yu <qiang...@amd.com> >>> Signed-off-by: Rob Herring <r...@kernel.org> >> >> Reviewed-by: Emil Velikov <emil.veli...@collabora.com> >> >>> --- >>> Android.mk | 12 ++++-------- >>> src/amd/Android.common.mk | 4 +--- >>> src/gallium/drivers/radeon/Android.mk | 2 +- >>> src/gallium/drivers/radeonsi/Android.mk | 2 +- >>> 4 files changed, 7 insertions(+), 13 deletions(-) >>> >>> diff --git a/Android.mk b/Android.mk >>> index 6571161c8783..dc4041364551 100644 >>> --- a/Android.mk >>> +++ b/Android.mk >>> @@ -92,16 +92,12 @@ define mesa-build-with-llvm >>> $(if $(filter $(MESA_ANDROID_MAJOR_VERSION), 4 5), \ >>> $(warning Unsupported LLVM version in Android >>> $(MESA_ANDROID_MAJOR_VERSION)),) \ >>> $(if $(filter 6,$(MESA_ANDROID_MAJOR_VERSION)), \ >>> - $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 -DMESA_LLVM_VERSION_PATCH=0) >>> \ >>> - $(eval LOCAL_STATIC_LIBRARIES += libLLVMCore) \ >>> - $(eval LOCAL_C_INCLUDES += external/llvm/include >>> external/llvm/device/include),) \ >>> + $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0307 >>> -DMESA_LLVM_VERSION_PATCH=0),) \ > > Hmm, why do we need an extra comma? > Does it correspond to the else case of $(if ...)? > If so it could be omitted.
Indeed. > >>> $(if $(filter 7,$(MESA_ANDROID_MAJOR_VERSION)), \ >>> - $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0308 -DMESA_LLVM_VERSION_PATCH=0) >>> \ >>> - $(eval LOCAL_STATIC_LIBRARIES += libLLVMCore) \ >>> - $(eval LOCAL_C_INCLUDES += external/llvm/include >>> external/llvm/device/include),) \ >>> + $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0308 >>> -DMESA_LLVM_VERSION_PATCH=0),) \ >>> $(if $(filter O,$(MESA_ANDROID_MAJOR_VERSION)), \ >>> - $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_PATCH=0) >>> \ >>> - $(eval LOCAL_HEADER_LIBRARIES += llvm-headers),) >>> + $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 >>> -DMESA_LLVM_VERSION_PATCH=0),) \ >>> + $(eval LOCAL_SHARED_LIBRARIES += libLLVM) >> Am I the only person getting tad confused by amount of brackets? >> As mentioned by Chih-Wei - a shell switch is not possible, but how >> about a test vague like the following? >> >> test "x$(MESA_ANDROID_MAJOR_VERSION)" = "xO" && >> $(eval LOCAL_CFLAGS += -DHAVE_LLVM=0x0309 -DMESA_LLVM_VERSION_PATCH=0) > > Only possible if you put it into $(shell ...) > That gives me an idea. Maybe we ca do like > > $(shell case "$(MESA_ANDROID_MAJOR_VERSION)" in \ > 6) echo ... ;; \ > 7) echo ... ;; \ > *) echo ... ;; \ > esac) > > I haven't really try it yet. What does either really buy us? It's really just bike shedding and unrelated to fixing the problem at hand. I have another idea which is to use llvm-config and avoid the conditionals altogether. I haven't looked into that closely though. Rob _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev