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 <robdcl...@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 <robdcl...@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
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev