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

[Francisco Jerez]

Tom Stellard <thomas.stell...@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-sta...@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?

>           targets_initialized = true;
>        }
>     }
> -- 
> 2.0.4

signature.asc
Description: PGP signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev