Re: [Mesa-dev] [PATCH mesa] meson: add dep_thread to every lib that includes threads.h
Quoting Emil Velikov (2017-12-12 07:04:09) > On 11 December 2017 at 22:22, Dylan Bakerwrote: > > Quoting Emil Velikov (2017-12-11 12:06:35) > >> On 7 December 2017 at 17:25, Dylan Baker wrote: > >> > Quoting Emil Velikov (2017-12-07 08:40:27) > >> >> On 7 December 2017 at 14:51, Eric Engestrom > >> >> wrote: > >> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104141 > >> >> > Signed-off-by: Eric Engestrom > >> >> > --- > >> >> > src/broadcom/meson.build| 2 +- > >> >> > src/gallium/auxiliary/meson.build | 2 +- > >> >> > src/gallium/state_trackers/nine/meson.build | 1 + > >> >> > src/gallium/targets/xa/meson.build | 2 +- > >> >> > src/gallium/targets/xvmc/meson.build| 2 +- > >> >> > src/gbm/meson.build | 2 +- > >> >> > src/intel/common/meson.build| 2 +- > >> >> > src/loader/meson.build | 2 +- > >> >> > src/util/meson.build| 2 +- > >> >> > 9 files changed, 9 insertions(+), 8 deletions(-) > >> >> > > >> >> I doubt we can continue and pretend to be libpthread.so free. > >> >> To make it even funnier, depending on moon cycle or other fun factors, > >> >> we could get the pthread dependency implicitly satisfied as one of the > >> >> other shared libraries already pulls the library. > >> >> > >> >> So how about we simply append -pthread to CC/CXX with at global scope > >> >> and drop the all the individual dependencies? > >> >> It will safe us a few characters to type, plus will ensure that newly > >> >> added binaries don't fall victim of the same issue. > >> > > >> > Absolutely not. The meson build has dep_thread for a reason, because > >> > meson > >> > guarantees that calling `dependency('threads')` will always return the > >> > right > >> > value for your platform, even if that platform is windows and doesn't > >> > have > >> > pthreads at all (but does the right thing for cygwin). > >> > > >> I would recommend looking through clang/gcc. AFAICS any* platform/arch > >> combo supported by Mesa handles -pthread and that toggle does the > >> "right thing". > >> Obviously that can seem a bit hacky, so a better way to avoid all the > >> copy/paste is for meson to grow an option that allows folding the > >> required cflags/libs with the compiler directive. > > > > That's all fine, but the meson build is planning on supporting haiku and > > plain > > windows (with msvc), neither of which have pthreads (haiku does, but it's > > not a > > standalone library and you don't pass -pthreads to the compiler or linker > > and > > it's an error to do so). macOS clang also warns when passing -pthreads to > > the > > linker (but only the one shipped with xcode), not if you build clang > > yourself. > > > > If you feel strongly about it, open a bug upstream and discuss it with > > upstream. > > If they agree and add a mechanism to do so I'd be fine using it. > > > >> > The reason that we're running into this problem is as you guessed that > >> > some > >> > dependencies pull in pthreads implicitly, for example LLVM, which is why > >> > we're > >> > seeing this so often in gallium. > >> > > >> Precisely. Due to the combinatoric explosions things are bound to > >> break again, hence my earlier suggestion. > >> I doubt you or anyone on the team will be excited to see things break. > > > > That's possible, obviously. I also think these sort of issues will work > > themselves out fairly quickly, while I'm very concerned adding -pthread > > into the > > list of arguments we pass unconditionally is going to break whole platforms > > in > > subtle and hard to fix ways, and really goes against the philosophy of > > meson, > > which is to solve these sort of problems in meson itself, rather than each > > build > > system solving them again and again, usually incorrectly. > > > > If we want to trot out the big hammer, I'd be happier just to add > > dep_thread to > > every shared library and binary than trying to add the right combination of > > -pthreads and -lpthreads for each platform ourselves to the C and C++ flags. > > > > There's about 350 uses of pthread symbols in mesa itself, of that there are > > 56 > > unique files containing pthread symbols (some of which are generators), and > > of > > that there are only 23 unique folders containing pthread symbols. I think > > that > > getting this right is very doable. > > > > I'll start auditing the meson build to see if there's any place that we're > > missing passing pthreads directly. > > > Guess I should have made it more obvious: > > I'm trying to save you (amongst others) the annoyance as things break > - since they will break :-( > It's entirely up-to you to decide on the best approach to mitigate or > even avoid that. > > HTH > Emil I do appreciate it, I know you've dealt with a number of these problems
Re: [Mesa-dev] [PATCH mesa] meson: add dep_thread to every lib that includes threads.h
On 11 December 2017 at 22:22, Dylan Bakerwrote: > Quoting Emil Velikov (2017-12-11 12:06:35) >> On 7 December 2017 at 17:25, Dylan Baker wrote: >> > Quoting Emil Velikov (2017-12-07 08:40:27) >> >> On 7 December 2017 at 14:51, Eric Engestrom >> >> wrote: >> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104141 >> >> > Signed-off-by: Eric Engestrom >> >> > --- >> >> > src/broadcom/meson.build| 2 +- >> >> > src/gallium/auxiliary/meson.build | 2 +- >> >> > src/gallium/state_trackers/nine/meson.build | 1 + >> >> > src/gallium/targets/xa/meson.build | 2 +- >> >> > src/gallium/targets/xvmc/meson.build| 2 +- >> >> > src/gbm/meson.build | 2 +- >> >> > src/intel/common/meson.build| 2 +- >> >> > src/loader/meson.build | 2 +- >> >> > src/util/meson.build| 2 +- >> >> > 9 files changed, 9 insertions(+), 8 deletions(-) >> >> > >> >> I doubt we can continue and pretend to be libpthread.so free. >> >> To make it even funnier, depending on moon cycle or other fun factors, >> >> we could get the pthread dependency implicitly satisfied as one of the >> >> other shared libraries already pulls the library. >> >> >> >> So how about we simply append -pthread to CC/CXX with at global scope >> >> and drop the all the individual dependencies? >> >> It will safe us a few characters to type, plus will ensure that newly >> >> added binaries don't fall victim of the same issue. >> > >> > Absolutely not. The meson build has dep_thread for a reason, because meson >> > guarantees that calling `dependency('threads')` will always return the >> > right >> > value for your platform, even if that platform is windows and doesn't have >> > pthreads at all (but does the right thing for cygwin). >> > >> I would recommend looking through clang/gcc. AFAICS any* platform/arch >> combo supported by Mesa handles -pthread and that toggle does the >> "right thing". >> Obviously that can seem a bit hacky, so a better way to avoid all the >> copy/paste is for meson to grow an option that allows folding the >> required cflags/libs with the compiler directive. > > That's all fine, but the meson build is planning on supporting haiku and plain > windows (with msvc), neither of which have pthreads (haiku does, but it's not > a > standalone library and you don't pass -pthreads to the compiler or linker and > it's an error to do so). macOS clang also warns when passing -pthreads to the > linker (but only the one shipped with xcode), not if you build clang yourself. > > If you feel strongly about it, open a bug upstream and discuss it with > upstream. > If they agree and add a mechanism to do so I'd be fine using it. > >> > The reason that we're running into this problem is as you guessed that some >> > dependencies pull in pthreads implicitly, for example LLVM, which is why >> > we're >> > seeing this so often in gallium. >> > >> Precisely. Due to the combinatoric explosions things are bound to >> break again, hence my earlier suggestion. >> I doubt you or anyone on the team will be excited to see things break. > > That's possible, obviously. I also think these sort of issues will work > themselves out fairly quickly, while I'm very concerned adding -pthread into > the > list of arguments we pass unconditionally is going to break whole platforms in > subtle and hard to fix ways, and really goes against the philosophy of meson, > which is to solve these sort of problems in meson itself, rather than each > build > system solving them again and again, usually incorrectly. > > If we want to trot out the big hammer, I'd be happier just to add dep_thread > to > every shared library and binary than trying to add the right combination of > -pthreads and -lpthreads for each platform ourselves to the C and C++ flags. > > There's about 350 uses of pthread symbols in mesa itself, of that there are 56 > unique files containing pthread symbols (some of which are generators), and of > that there are only 23 unique folders containing pthread symbols. I think that > getting this right is very doable. > > I'll start auditing the meson build to see if there's any place that we're > missing passing pthreads directly. > Guess I should have made it more obvious: I'm trying to save you (amongst others) the annoyance as things break - since they will break :-( It's entirely up-to you to decide on the best approach to mitigate or even avoid that. HTH Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] meson: add dep_thread to every lib that includes threads.h
Quoting Emil Velikov (2017-12-11 12:06:35) > On 7 December 2017 at 17:25, Dylan Bakerwrote: > > Quoting Emil Velikov (2017-12-07 08:40:27) > >> On 7 December 2017 at 14:51, Eric Engestrom > >> wrote: > >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104141 > >> > Signed-off-by: Eric Engestrom > >> > --- > >> > src/broadcom/meson.build| 2 +- > >> > src/gallium/auxiliary/meson.build | 2 +- > >> > src/gallium/state_trackers/nine/meson.build | 1 + > >> > src/gallium/targets/xa/meson.build | 2 +- > >> > src/gallium/targets/xvmc/meson.build| 2 +- > >> > src/gbm/meson.build | 2 +- > >> > src/intel/common/meson.build| 2 +- > >> > src/loader/meson.build | 2 +- > >> > src/util/meson.build| 2 +- > >> > 9 files changed, 9 insertions(+), 8 deletions(-) > >> > > >> I doubt we can continue and pretend to be libpthread.so free. > >> To make it even funnier, depending on moon cycle or other fun factors, > >> we could get the pthread dependency implicitly satisfied as one of the > >> other shared libraries already pulls the library. > >> > >> So how about we simply append -pthread to CC/CXX with at global scope > >> and drop the all the individual dependencies? > >> It will safe us a few characters to type, plus will ensure that newly > >> added binaries don't fall victim of the same issue. > > > > Absolutely not. The meson build has dep_thread for a reason, because meson > > guarantees that calling `dependency('threads')` will always return the right > > value for your platform, even if that platform is windows and doesn't have > > pthreads at all (but does the right thing for cygwin). > > > I would recommend looking through clang/gcc. AFAICS any* platform/arch > combo supported by Mesa handles -pthread and that toggle does the > "right thing". > Obviously that can seem a bit hacky, so a better way to avoid all the > copy/paste is for meson to grow an option that allows folding the > required cflags/libs with the compiler directive. That's all fine, but the meson build is planning on supporting haiku and plain windows (with msvc), neither of which have pthreads (haiku does, but it's not a standalone library and you don't pass -pthreads to the compiler or linker and it's an error to do so). macOS clang also warns when passing -pthreads to the linker (but only the one shipped with xcode), not if you build clang yourself. If you feel strongly about it, open a bug upstream and discuss it with upstream. If they agree and add a mechanism to do so I'd be fine using it. > > The reason that we're running into this problem is as you guessed that some > > dependencies pull in pthreads implicitly, for example LLVM, which is why > > we're > > seeing this so often in gallium. > > > Precisely. Due to the combinatoric explosions things are bound to > break again, hence my earlier suggestion. > I doubt you or anyone on the team will be excited to see things break. That's possible, obviously. I also think these sort of issues will work themselves out fairly quickly, while I'm very concerned adding -pthread into the list of arguments we pass unconditionally is going to break whole platforms in subtle and hard to fix ways, and really goes against the philosophy of meson, which is to solve these sort of problems in meson itself, rather than each build system solving them again and again, usually incorrectly. If we want to trot out the big hammer, I'd be happier just to add dep_thread to every shared library and binary than trying to add the right combination of -pthreads and -lpthreads for each platform ourselves to the C and C++ flags. There's about 350 uses of pthread symbols in mesa itself, of that there are 56 unique files containing pthread symbols (some of which are generators), and of that there are only 23 unique folders containing pthread symbols. I think that getting this right is very doable. I'll start auditing the meson build to see if there's any place that we're missing passing pthreads directly. Dylan > > -Emil > > * I've checked Cygwin, Solaris-like, BSD-like, Linuxen, Hurd and > Android, on both 32 and 64bit x86, ARM and PPC. > I believe msys/mingw should be fine as well, but I'm short on details. signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] meson: add dep_thread to every lib that includes threads.h
On 7 December 2017 at 17:25, Dylan Bakerwrote: > Quoting Emil Velikov (2017-12-07 08:40:27) >> On 7 December 2017 at 14:51, Eric Engestrom >> wrote: >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104141 >> > Signed-off-by: Eric Engestrom >> > --- >> > src/broadcom/meson.build| 2 +- >> > src/gallium/auxiliary/meson.build | 2 +- >> > src/gallium/state_trackers/nine/meson.build | 1 + >> > src/gallium/targets/xa/meson.build | 2 +- >> > src/gallium/targets/xvmc/meson.build| 2 +- >> > src/gbm/meson.build | 2 +- >> > src/intel/common/meson.build| 2 +- >> > src/loader/meson.build | 2 +- >> > src/util/meson.build| 2 +- >> > 9 files changed, 9 insertions(+), 8 deletions(-) >> > >> I doubt we can continue and pretend to be libpthread.so free. >> To make it even funnier, depending on moon cycle or other fun factors, >> we could get the pthread dependency implicitly satisfied as one of the >> other shared libraries already pulls the library. >> >> So how about we simply append -pthread to CC/CXX with at global scope >> and drop the all the individual dependencies? >> It will safe us a few characters to type, plus will ensure that newly >> added binaries don't fall victim of the same issue. > > Absolutely not. The meson build has dep_thread for a reason, because meson > guarantees that calling `dependency('threads')` will always return the right > value for your platform, even if that platform is windows and doesn't have > pthreads at all (but does the right thing for cygwin). > I would recommend looking through clang/gcc. AFAICS any* platform/arch combo supported by Mesa handles -pthread and that toggle does the "right thing". Obviously that can seem a bit hacky, so a better way to avoid all the copy/paste is for meson to grow an option that allows folding the required cflags/libs with the compiler directive. > The reason that we're running into this problem is as you guessed that some > dependencies pull in pthreads implicitly, for example LLVM, which is why we're > seeing this so often in gallium. > Precisely. Due to the combinatoric explosions things are bound to break again, hence my earlier suggestion. I doubt you or anyone on the team will be excited to see things break. -Emil * I've checked Cygwin, Solaris-like, BSD-like, Linuxen, Hurd and Android, on both 32 and 64bit x86, ARM and PPC. I believe msys/mingw should be fine as well, but I'm short on details. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] meson: add dep_thread to every lib that includes threads.h
Reviewed-by: Dylan BakerQuoting Eric Engestrom (2017-12-07 06:51:41) > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104141 > Signed-off-by: Eric Engestrom > --- > src/broadcom/meson.build| 2 +- > src/gallium/auxiliary/meson.build | 2 +- > src/gallium/state_trackers/nine/meson.build | 1 + > src/gallium/targets/xa/meson.build | 2 +- > src/gallium/targets/xvmc/meson.build| 2 +- > src/gbm/meson.build | 2 +- > src/intel/common/meson.build| 2 +- > src/loader/meson.build | 2 +- > src/util/meson.build| 2 +- > 9 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/src/broadcom/meson.build b/src/broadcom/meson.build > index dbeee091139dcd05ec5a..6072fd1b997bb29b307a 100644 > --- a/src/broadcom/meson.build > +++ b/src/broadcom/meson.build > @@ -39,6 +39,6 @@ if with_gallium_vc5 > c_args : [c_vis_args, no_override_init_args], > link_whole : [libbroadcom_compiler, libbroadcom_qpu], > build_by_default : false, > -dependencies: dep_valgrind, > +dependencies: [dep_valgrind, dep_thread], >) > endif > diff --git a/src/gallium/auxiliary/meson.build > b/src/gallium/auxiliary/meson.build > index 3e623fd099fe4b60a5de..4d59f07fd31f3a154934 100644 > --- a/src/gallium/auxiliary/meson.build > +++ b/src/gallium/auxiliary/meson.build > @@ -495,7 +495,7 @@ libgallium = static_library( >], >c_args : [c_vis_args, c_msvc_compat_args], >cpp_args : [cpp_vis_args, cpp_msvc_compat_args], > - dependencies : [dep_libdrm, dep_llvm, dep_unwind, dep_dl, dep_m], > + dependencies : [dep_libdrm, dep_llvm, dep_unwind, dep_dl, dep_m, > dep_thread], >build_by_default : false, > ) > > diff --git a/src/gallium/state_trackers/nine/meson.build > b/src/gallium/state_trackers/nine/meson.build > index 797ef389a69f164d7440..491b7937ab19d754e498 100644 > --- a/src/gallium/state_trackers/nine/meson.build > +++ b/src/gallium/state_trackers/nine/meson.build > @@ -66,4 +66,5 @@ libnine_st = static_library( >include_directories : [ > inc_d3d9, inc_gallium, inc_include, inc_src, inc_gallium_aux, >], > + dependencies : dep_thread, > ) > diff --git a/src/gallium/targets/xa/meson.build > b/src/gallium/targets/xa/meson.build > index f16921dfc9c443a166b4..f25999d1603c2873897a 100644 > --- a/src/gallium/targets/xa/meson.build > +++ b/src/gallium/targets/xa/meson.build > @@ -45,7 +45,7 @@ libxatracker = shared_library( >], >link_depends : xa_link_depends, >dependencies : [ > -dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_xcb_dri3, dep_libdrm, > +dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_xcb_dri3, dep_libdrm, dep_thread, > driver_nouveau, driver_i915, driver_svga, driver_freedreno, >], >install : true, > diff --git a/src/gallium/targets/xvmc/meson.build > b/src/gallium/targets/xvmc/meson.build > index 76de816efed7559602a3..48759de910f88e66b82b 100644 > --- a/src/gallium/targets/xvmc/meson.build > +++ b/src/gallium/targets/xvmc/meson.build > @@ -45,7 +45,7 @@ libxvmc_gallium = shared_library( > libpipe_loader_static, libws_null, libwsw, >], >dependencies : [ > -dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_xcb_dri3, dep_libdrm, > +dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_xcb_dri3, dep_libdrm, dep_thread, > driver_r600, driver_nouveau, >], >link_depends : xvmc_link_depends, > diff --git a/src/gbm/meson.build b/src/gbm/meson.build > index 4b550e06f2790ae9a13d..6e512996add1cfadd189 100644 > --- a/src/gbm/meson.build > +++ b/src/gbm/meson.build > @@ -54,7 +54,7 @@ libgbm = shared_library( >c_args : [c_vis_args, args_gbm], >link_args : [ld_args_gc_sections], >link_with : [libloader, libmesa_util, libxmlconfig], > - dependencies : [deps_gbm, dep_dl], > + dependencies : [deps_gbm, dep_dl, dep_thread], >version : '1.0.0', >install : true, > ) > diff --git a/src/intel/common/meson.build b/src/intel/common/meson.build > index cbcf6647531193142cd5..19472e306f4577ebc231 100644 > --- a/src/intel/common/meson.build > +++ b/src/intel/common/meson.build > @@ -40,5 +40,5 @@ libintel_common = static_library( >files_libintel_common, >include_directories : [inc_common, inc_intel], >c_args : [c_vis_args, no_override_init_args], > - dependencies : [dep_expat, dep_libdrm], > + dependencies : [dep_expat, dep_libdrm, dep_thread], > ) > diff --git a/src/loader/meson.build b/src/loader/meson.build > index 05268f58f3f138fe0eb0..e4455e9a72b24891f58b 100644 > --- a/src/loader/meson.build > +++ b/src/loader/meson.build > @@ -41,6 +41,6 @@ libloader = static_library( > xmlpool_options_h], >c_args : [c_vis_args, '-DUSE_DRICONF'], >include_directories : [inc_include, inc_src, inc_util], > - dependencies : dep_libdrm, > + dependencies : [dep_libdrm, dep_thread], >build_by_default : false, > ) > diff --git
Re: [Mesa-dev] [PATCH mesa] meson: add dep_thread to every lib that includes threads.h
Quoting Emil Velikov (2017-12-07 08:40:27) > On 7 December 2017 at 14:51, Eric Engestromwrote: > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104141 > > Signed-off-by: Eric Engestrom > > --- > > src/broadcom/meson.build| 2 +- > > src/gallium/auxiliary/meson.build | 2 +- > > src/gallium/state_trackers/nine/meson.build | 1 + > > src/gallium/targets/xa/meson.build | 2 +- > > src/gallium/targets/xvmc/meson.build| 2 +- > > src/gbm/meson.build | 2 +- > > src/intel/common/meson.build| 2 +- > > src/loader/meson.build | 2 +- > > src/util/meson.build| 2 +- > > 9 files changed, 9 insertions(+), 8 deletions(-) > > > I doubt we can continue and pretend to be libpthread.so free. > To make it even funnier, depending on moon cycle or other fun factors, > we could get the pthread dependency implicitly satisfied as one of the > other shared libraries already pulls the library. > > So how about we simply append -pthread to CC/CXX with at global scope > and drop the all the individual dependencies? > It will safe us a few characters to type, plus will ensure that newly > added binaries don't fall victim of the same issue. Absolutely not. The meson build has dep_thread for a reason, because meson guarantees that calling `dependency('threads')` will always return the right value for your platform, even if that platform is windows and doesn't have pthreads at all (but does the right thing for cygwin). The reason that we're running into this problem is as you guessed that some dependencies pull in pthreads implicitly, for example LLVM, which is why we're seeing this so often in gallium. > -Emil > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] meson: add dep_thread to every lib that includes threads.h
On 7 December 2017 at 14:51, Eric Engestromwrote: > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104141 > Signed-off-by: Eric Engestrom > --- > src/broadcom/meson.build| 2 +- > src/gallium/auxiliary/meson.build | 2 +- > src/gallium/state_trackers/nine/meson.build | 1 + > src/gallium/targets/xa/meson.build | 2 +- > src/gallium/targets/xvmc/meson.build| 2 +- > src/gbm/meson.build | 2 +- > src/intel/common/meson.build| 2 +- > src/loader/meson.build | 2 +- > src/util/meson.build| 2 +- > 9 files changed, 9 insertions(+), 8 deletions(-) > I doubt we can continue and pretend to be libpthread.so free. To make it even funnier, depending on moon cycle or other fun factors, we could get the pthread dependency implicitly satisfied as one of the other shared libraries already pulls the library. So how about we simply append -pthread to CC/CXX with at global scope and drop the all the individual dependencies? It will safe us a few characters to type, plus will ensure that newly added binaries don't fall victim of the same issue. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev