[Mesa-stable] [PATCH] clover: Properly initialize LLVM targets when linking with component libs

[Emil Velikov]

On 8 August 2015 at 12:11, Francisco Jerez <currojerez at riseup.net> wrote:
> Tom Stellard <thomas.stellard at amd.com> writes:
>
>> Calls to LLVMIntialize* fail when we are linking against individual
>> component libraries rather than one large shared object, because
>> we only include component libraries that are required by the drivers.
>>
>> We need to make sure to only initialize the targets that we need.
>>
>> CC: 10.6 <mesa-stable at lists.freedesktop.org>
>> ---
>>  configure.ac                                          |  4 ++++
>>  src/gallium/state_trackers/clover/Makefile.am         |  3 ++-
>>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 17 +++++++++++++----
>>  3 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 36197d3..e1a7d7a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -2040,8 +2040,10 @@ require_egl_drm() {
>>  radeon_llvm_check() {
>>      if test ${LLVM_VERSION_INT} -lt 307; then
>>          amdgpu_llvm_target_name='r600'
>> +     CLOVER_CPP_FLAGS="${CLOVER_CPP_FLAGS} -DCLOVER_INIT_R600_TARGET"
>>      else
>>          amdgpu_llvm_target_name='amdgpu'
>> +     CLOVER_CPP_FLAGS="${CLOVER_CPP_FLAGS} -DCLOVER_INIT_AMDGPU_TARGET"
>>      fi
>>      if test "x$enable_gallium_llvm" != "xyes"; then
>>          AC_MSG_ERROR([--enable-gallium-llvm is required when building $1])
>> @@ -2285,6 +2287,8 @@ AC_SUBST([XA_MINOR], $XA_MINOR)
>>  AC_SUBST([XA_TINY], $XA_TINY)
>>  AC_SUBST([XA_VERSION], "$XA_MAJOR.$XA_MINOR.$XA_TINY")
>>
>> +AC_SUBST([CLOVER_CPP_FLAGS], $CLOVER_CPP_FLAGS)
>> +
>>  dnl Restore LDFLAGS and CPPFLAGS
>>  LDFLAGS="$_SAVE_LDFLAGS"
>>  CPPFLAGS="$_SAVE_CPPFLAGS"
>> diff --git a/src/gallium/state_trackers/clover/Makefile.am b/src/gallium/state_trackers/clover/Makefile.am
>> index fd0ccf8..975b36f 100644
>> --- a/src/gallium/state_trackers/clover/Makefile.am
>> +++ b/src/gallium/state_trackers/clover/Makefile.am
>> @@ -45,7 +45,8 @@ libclllvm_la_CXXFLAGS = \
>>       $(DEFINES) \
>>       -DLIBCLC_INCLUDEDIR=\"$(LIBCLC_INCLUDEDIR)/\" \
>>       -DLIBCLC_LIBEXECDIR=\"$(LIBCLC_LIBEXECDIR)/\" \
>> -     -DCLANG_RESOURCE_DIR=\"$(CLANG_RESOURCE_DIR)\"
>> +     -DCLANG_RESOURCE_DIR=\"$(CLANG_RESOURCE_DIR)\" \
>> +     $(CLOVER_CPP_FLAGS)
>>
>>  libclllvm_la_SOURCES = $(LLVM_SOURCES)
>>
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index 86859af..361a149 100644
>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> @@ -786,10 +786,19 @@ namespace {
>>     init_targets() {
>>        static bool targets_initialized = false;
>>        if (!targets_initialized) {
>> -         LLVMInitializeAllTargets();
>> -         LLVMInitializeAllTargetInfos();
>> -         LLVMInitializeAllTargetMCs();
>> -         LLVMInitializeAllAsmPrinters();
>> +#ifdef CLOVER_INIT_AMDGPU_TARGET
>> +         LLVMInitializeAMDGPUTarget();
>> +         LLVMInitializeAMDGPUTargetInfo();
>> +         LLVMInitializeAMDGPUTargetMC();
>> +         LLVMInitializeAMDGPUAsmPrinter();
>> +#endif
>> +
>> +#ifdef CLOVER_INIT_R600_TARGET
>> +         LLVMInitializeR600Target();
>> +         LLVMInitializeR600TargetInfo();
>> +         LLVMInitializeR600TargetMC();
>> +         LLVMInitializeR600AsmPrinter();
>> +#endif
>
> Doesn't this feel like a layering violation?  Why should clover
> initialize specific LLVM back-ends?  And isn't it a build-system bug if,
> say, LLVMInitializeAllTargets() is being pulled in but some symbol it
> depends on isn't?
>
Hi Tom,

Based of Francisco's comment it seems that this patch goes in the bit
bucket. Can you please confirm if that's not the case ?

Thanks
Emil