[Mesa-dev] [PATCH] Android: Fix LLVM duplicated symbols linking for N and M

[Mauro Rossi]

2017-08-21 23:57 GMT+02:00 Rob Herring <robh at kernel.org>:
> On Mon, Aug 21, 2017 at 4:44 PM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>> 2017-08-18 21:46 GMT+02:00 Rob Herring <robh at kernel.org>:
>>> 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.
>>>
>>> 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.
>>>
>>> Fixes: 26aee6f4d5a ("Android: rework LLVM build support")
>>> Reported-by: Mauro Rossi <issor.oruam at gmail.com>
>>> Cc: Emil Velikov <emil.l.velikov at gmail.com>
>>> Cc: 17.2 <mesa-stable at lists.freedesktop.org>
>>> Signed-off-by: Qiang Yu <Qiang.Yu at amd.com>
>>> Signed-off-by: Rob Herring <robh at kernel.org>
>>> ---
>>>  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),) \
>>>    $(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)
>>>  endef
>>
>> Hi Rob,
>>
>> I've just seen now that llvm include paths were removed compared to
>> the original version of this patch.
>
> Which is why I posted the the changed version.
>
>> I had reported build errors without those on android-x86 development
>> mail thread.
>
> I replied in that thread that there was a typo in the LLVM change. I
> had device/include instead of include/device for the include path.
> Were you able to test that?

I had even read the message, but I was not completely "connected"
The current version is more than ok.

Sorry

Mauro

>
>> I'm submitting a correction to mesa-dev ML, please review and apply it
>>
>> It will be necessary also in 17.2 branch when this patch will be
>> backported to 17.2.0.
>> Thanks
>>
>> Mauro
>>
>>>
>>>  # add subdirectories
>>> diff --git a/src/amd/Android.common.mk b/src/amd/Android.common.mk
>>> index 7d08bfd31d79..4e2d0f9c2ffa 100644
>>> --- a/src/amd/Android.common.mk
>>> +++ b/src/amd/Android.common.mk
>>> @@ -55,9 +55,7 @@ LOCAL_C_INCLUDES := \
>>>         $(call generated-sources-dir-for,STATIC_LIBRARIES,libmesa_nir,,)/nir \
>>>         $(MESA_TOP)/src/gallium/include \
>>>         $(MESA_TOP)/src/gallium/auxiliary \
>>> -       $(intermediates)/common \
>>> -       external/llvm/include \
>>> -       external/llvm/device/include
>>> +       $(intermediates)/common
>>>
>>>  LOCAL_EXPORT_C_INCLUDE_DIRS := \
>>>         $(LOCAL_PATH)/common
>>> diff --git a/src/gallium/drivers/radeon/Android.mk b/src/gallium/drivers/radeon/Android.mk
>>> index eb1a32182bb0..c2d3a1cbce60 100644
>>> --- a/src/gallium/drivers/radeon/Android.mk
>>> +++ b/src/gallium/drivers/radeon/Android.mk
>>> @@ -30,7 +30,7 @@ include $(CLEAR_VARS)
>>>
>>>  LOCAL_SRC_FILES := $(C_SOURCES)
>>>
>>> -LOCAL_SHARED_LIBRARIES := libdrm_radeon libLLVM
>>> +LOCAL_SHARED_LIBRARIES := libdrm_radeon
>>>  LOCAL_MODULE := libmesa_pipe_radeon
>>>
>>>  ifeq ($(MESA_ENABLE_LLVM),true)
>>> diff --git a/src/gallium/drivers/radeonsi/Android.mk b/src/gallium/drivers/radeonsi/Android.mk
>>> index 65661a5ea7a5..e72b80c4e807 100644
>>> --- a/src/gallium/drivers/radeonsi/Android.mk
>>> +++ b/src/gallium/drivers/radeonsi/Android.mk
>>> @@ -41,7 +41,7 @@ LOCAL_C_INCLUDES := \
>>>
>>>  LOCAL_STATIC_LIBRARIES := libmesa_amd_common
>>>
>>> -LOCAL_SHARED_LIBRARIES := libdrm_radeon libLLVM
>>> +LOCAL_SHARED_LIBRARIES := libdrm_radeon
>>>  LOCAL_MODULE := libmesa_pipe_radeonsi
>>>
>>>  intermediates := $(call local-generated-sources-dir)
>>> --
>>> 2.11.0
>>>