On Fri, Aug 18, 2017 at 11:03 AM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> Hi Qiang Yu,
>
> On 18 August 2017 at 01:30, Qiang Yu <qiang...@amd.com> wrote:
>> The problem is in gallium/winsys/amdgpu/drm/Android.mk
>> which will have duplacated symbols when linking
>> gallium_dri.so for libLLVMCore and libLLVM.
>>
> Skimming through the commit message and the change inspires some confusion.
>
> How about something like the following:
> "Android: rework LLVM linking to avoid multiple symbol definitions
>
> Currently we static against libLLVMCore alongside the shared link
> against libLLVM.
> This leads to duplicate symbols and run/build (please correct me) time.
>
> Fix that by consistently dropping the libLLVMCore + llvm-headers bits
> and consistently using a shared libLLVM link.
> "
>
>> Signed-off-by: Qiang Yu <qiang...@amd.com>
>> Signed-off-by: Mauro Rossi <issor.or...@gmail.com>
>> Signed-off-by: Rob Herring <robherri...@gmail.com>
> I don't recall seeing patches from Mauro/RobH about this. I think you
> meant - Reviewed-by/Acked-by/other?
>
> Fixes: 26aee6f4d5a ("Android: rework LLVM build support")
> Fixes: d31a2b4d492 ("Android: Add LLVM support for Android O")
Only the first one is correct. The latter one was just to get a clean revert.
>> ---
>> Android.mk | 7 ++++---
>> 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(+), 8 deletions(-)
>>
>> diff --git a/Android.mk b/Android.mk
>> index 6571161..5154a56 100644
>> --- a/Android.mk
>> +++ b/Android.mk
>> @@ -93,15 +93,16 @@ define mesa-build-with-llvm
>> $(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_SHARED_LIBRARIES += libLLVM) \
>> $(eval LOCAL_C_INCLUDES += external/llvm/include
>> external/llvm/device/include),) \
>> $(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_SHARED_LIBRARIES += libLLVM) \
>> $(eval LOCAL_C_INCLUDES += external/llvm/include
>> external/llvm/device/include),) \
>> $(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_SHARED_LIBRARIES += libLLVM) \
>> + $(eval LOCAL_C_INCLUDES += external/llvm/include
>> external/llvm/device/include),)
The Android O lines are wrong and should be dropped. They were just
for my testing purposes.
> With these two lines being identical across the board, we can have
> them only once.
> At a later stage one can export the includes from the LLVM module.
That didn't work for some reason for Mauro.
I already have the correct patch in my tree. I'll polish it up and
send it out, then commit it.
Rob
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
