[Mesa-dev] [RFC] opencl: mega-cl

[Francisco Jerez]

Seems like a serious hack to me to work around broken linking...  IMO we
should just fix the linking issue.  The symbols for the various GLSL
types need to be linked with the proper binding and visibility -- I
assume that the cause of your problem is that people are making
assumptions about the equality of GLSL types based on their memory
addresses *and* marking the symbols as hidden *and* passing pointers to
GLSL types across shared objects?  That sounds like a recipe for
disaster.

The current code is likely being correctly linked based on the
assumption that the third condition from my question above doesn't hold
(i.e. that the hidden symbols are really being kept local to the GLSL
compiler), but your branch probably breaks this assumption?  It's a bug
to pass around pointers to hidden symbols to other shared objects except
where you know with confidence that the identity of the objects you're
passing around (as in their memory address) is unimportant.

Rob Clark <robdclark at gmail.com> writes:

> This is a sort of workaround for an issue encountered with the ongoing
> work for clover clc->spirv->nir work for supporting OpenCL with drivers
> that support consuming nir instead of llvm-ir.  The problem is that
> libnir (and therefore glsl_types) ends up statically linked into both
> clover and the pipe driver it dynamically loads.  Which causes *much*
> hillarity.
>
> An easy solution is to statically link the drivers into clover.  In fact
> I'm not even really sure what other solution there is, but maybe there
> is some linker magic I am not aware of.
>
> (Note, if mesa state tracker ever used libpipe_loader_dynamic, I think
> it would hit the same problem.  If we still support that config, we
> should drop it ASAP.)
>
> The down side is we end up statically linking in drivers and other
> things that don't make sense (ie. only support TGSI or don't support
> compute shaders, etc).  I'm thinking a sane option here is to have for
> each driver, "driver_${name}" plus "driver_${name}_stub", the stub
> containing an entry point that just returns an error.  (Maybe also do
> similar for winsys and some other things?)  This way, a target like
> opencl can pick either the real driver or the stub as needed.
>
> Btw, I'm kinda thinking we should drop libpipe_loader_dynamic
> completely, only only use libpipe_loader_static, and use the stub
> drivers to avoid pulling in stuff that doesn't make sense.  (I noticed,
> for example, the omx target statically links a bunch of drivers that
> don't support the video related gallium APIs.. this stub driver approach
> would help there too.)
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
>  src/gallium/targets/opencl/meson.build | 18 +++++++++++++++---
>  src/gallium/targets/opencl/target.c    |  2 ++
>  2 files changed, 17 insertions(+), 3 deletions(-)
>  create mode 100644 src/gallium/targets/opencl/target.c
>
> diff --git a/src/gallium/targets/opencl/meson.build b/src/gallium/targets/opencl/meson.build
> index bebe0547d45..8d5fd81b55e 100644
> --- a/src/gallium/targets/opencl/meson.build
> +++ b/src/gallium/targets/opencl/meson.build
> @@ -35,13 +35,19 @@ opencl_libname = with_opencl_icd ? 'MesaOpenCL' : 'OpenCL'
>  
>  libopencl = shared_library(
>    opencl_libname,
> -  [],
> +  ['target.c'],
> +  include_directories : [
> +    inc_common, inc_util, inc_gallium_winsys, inc_gallium_drivers,
> +  ],
>    link_args : [ld_args_gc_sections, opencl_link_args],
>    link_depends : opencl_link_deps,
>    link_whole : libclover,
> -  link_with : [libpipe_loader_dynamic, libgallium, libmesa_util],
> +  link_with : [
> +    libpipe_loader_static, libgallium, libmesa_util,
> +    libgalliumvl, libws_null, libwsw, libswdri, libswkmsdri,
> +  ],
>    dependencies : [
> -    dep_thread, dep_clock, dep_dl, dep_unwind, dep_elf, dep_expat,
> +    dep_libdrm, dep_thread, dep_clock, dep_dl, dep_unwind, dep_elf, dep_expat,
>      cpp.find_library('clangCodeGen', dirs : llvm_libdir),
>      cpp.find_library('clangFrontendTool', dirs : llvm_libdir),
>      cpp.find_library('clangFrontend', dirs : llvm_libdir),
> @@ -54,6 +60,12 @@ libopencl = shared_library(
>      cpp.find_library('clangEdit', dirs : llvm_libdir),
>      cpp.find_library('clangLex', dirs : llvm_libdir),
>      cpp.find_library('clangBasic', dirs : llvm_libdir),
> +    # TODO only a small subset of these makes sense, but libpipe_loader_static
> +    # depends on all.. maybe there is a way to have stub versions for the
> +    # drivers which don't support OpenCL?
> +    driver_swrast, driver_r300, driver_r600, driver_radeonsi, driver_nouveau,
> +    driver_pl111, driver_vc4, driver_vc5, driver_freedreno, driver_etnaviv,
> +    driver_imx, driver_i915, driver_svga, driver_virgl, driver_swr,
>    ],
>    version : opencl_version,
>    install : true,
> diff --git a/src/gallium/targets/opencl/target.c b/src/gallium/targets/opencl/target.c
> new file mode 100644
> index 00000000000..308e23bb4a0
> --- /dev/null
> +++ b/src/gallium/targets/opencl/target.c
> @@ -0,0 +1,2 @@
> +#include "target-helpers/drm_helper.h"
> +#include "target-helpers/sw_helper.h"
> -- 
> 2.14.3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180225/da840722/attachment.sig>