Summary: | mesa fails to check for gcc atomic primitives before using them | ||
---|---|---|---|
Product: | Mesa | Reporter: | Jonathan Gray <jsg> |
Component: | Mesa core | Assignee: | mesa-dev |
Status: | RESOLVED MOVED | QA Contact: | mesa-dev |
Severity: | normal | ||
Priority: | medium | CC: | bugzilla, mattst88 |
Version: | git | ||
Hardware: | All | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: | i486 __sync_val_compare_and_swap_8 disable debug message |
Description
Jonathan Gray
2015-11-24 01:38:53 UTC
Matt: I know you're an Alpha fan. Suggestions? It seems mips64 got the sync builtins later than other platforms, they are available in later versions of gcc but not gcc 4.2. Alpha also hit an ICE so for now we stopped building Mesa on alpha/mips64 in OpenBSD. lib/mesa/src/mesa/main/format_pack.c -fPIC -DPIC -o main/.libs/format_pack.o /usr/xenocara/lib/mesa/src/mesa/main/format_pack.c: In function 'pack_float_bgr_srgb8': /usr/xenocara/lib/mesa/src/mesa/main/format_pack.c:6890: error: unrecognizable insn: (insn 259 11 13 2 (set (reg:SF 1 $1) (const_double:SF 1.220703125e-4 [0x0.8p-12])) -1 (nil) (nil)) /usr/xenocara/lib/mesa/src/mesa/main/format_pack.c:6890: internal compiler error: in extract_insn, at recog.c:2077 For other architectures we currently build with: #elif defined(__ARM_ARCH_4__) || defined(__ARM_ARCH_5__) || \ defined(__hppa__) || defined(__sparc__) || defined(__sh__) #define PIPE_ATOMIC_UNLOCKED ... #if defined(PIPE_ATOMIC_UNLOCKED) #define PIPE_ATOMIC "Unlocked" #define p_atomic_set(_v, _i) (*(_v) = (_i)) #define p_atomic_read(_v) (*(_v)) #define p_atomic_dec_zero(_v) ((*(_v) -= 1) == 0) #define p_atomic_inc(_v) (*(_v) += 1) #define p_atomic_dec(_v) (*(_v) -= 1) #define p_atomic_add(_v, _i) (*(_v) += (_i)) #define p_atomic_inc_return(_v) (*(_v) += 1) #define p_atomic_dec_return(_v) (*(_v) -= 1) #define p_atomic_cmpxchg(_v, old, _new) ({ \ __typeof(*_v) _r = *(_v); \ if (*(_v) == old) \ *(_v) = (_new); \ _r; \ }) #endif Does OpenBSD supply stdatomic.h? I wonder if we should start using it on all platforms that supply it. No. And it seems gcc only started supporting C11 atomics in 4.9. libdrm currently checks for gcc __sync builtins, libatomic-ops and Solaris atomic.h I think if OpenBSD insists on compiling current software with an 8 year old compiler, they should expect to implement some widely available features in their fork... I added support for the gnu binary integer constant extension to gcc specifically for the intel driver in Mesa. We've also changed the behaviour of the sync builtins with PIC on i386 to not clobber ebx as well and people are working on implementing the sync builtins for mips64. Mesa won't build with newer versions of out of tree compilers anyway due to the problem misdetecting SSE4.1 there is another PR for (https://bugs.freedesktop.org/show_bug.cgi?id=91806). Even with the latest GCC these atomic builtins won't be available on all platforms anyway. Mesa is rapidly becoming i386/amd64/powerpc/sparc64 only. What Matt is saying is that surely Mesa is not the only project out there which relies on atomics, thus adding (backporting) those in GCC wouldn't be that bad of an idea. That aside I'm thinking that porting the libdrm solution wouldn't be that bad of an idea. There is no way to backport builtins for architectures that lack atomics at the ISA level. On arm/hppa/sh linux has a syscall which the gcc documentation claims can only do 64 bit values on arm: https://gcc.gnu.org/wiki/Atomic Even on Linux with the latest toolchains this should be checked as there are cases where some or none of the atomics may be present. We're hitting this on TinyCore, and I see Alpine hit it a while back too [1]. We build with -march=i486, and some of the atomics require a 586. Things build, but some drivers fail to load: libGL: dlopen /usr/local/lib/dri/i915_dri.so failed (/usr/local/lib/dri/i915_dri.so: undefined symbol: __sync_val_compare_and_swap_8) libGL error: unable to load driver: i915_dri.so libGL error: driver pointer missing They should fail to build at the very least. Changing the hw and os to all. [1] https://bugs.alpinelinux.org/issues/4254 Created attachment 126862 [details] [review] i486 __sync_val_compare_and_swap_8 disable debug message I came across the same problem when I compiled Mesa for the new Plop Linux i486 release. As the affected code is only in a debug message routine in "intel_debug.c", I simply disabled it. Now it compiles fine with "march=i486". I attached the patch. Previously 64 bit atomics were contained to i965. The on disk shader cache introduced in Mesa 13.0 in 87ab26b2ab35a29d446ae66f1795d40c184c0739 now requires 64 bit atomics as well which breaks Mesa on 32 bit powerpc which doesn't have 64 bit atomic instructions: glxinfo:/usr/X11R6/lib/modules/dri/r300_dri.so: undefined symbol '__sync_add_and_fetch_8' libGL error: unable to load driver: r300_dri.so libGL error: driver pointer missing libGL error: failed to load driver: r300 glxinfo:/usr/X11R6/lib/modules/dri/swrast_dri.so: undefined symbol '__sync_add_and_fetch_8' libGL error: unable to load driver: swrast_dri.so libGL error: failed to load driver: swrast X Error of failed request: GLXBadContext Major opcode of failed request: 155 (GLX) Minor opcode of failed request: 6 (X_GLXIsDirect) Serial number of failed request: 47 Current serial number in output stream: 0 name of display: :0 This was reported by Stefan Sperling on OpenBSD/macppc along with the workaround of using --disable-shader-cache. Fixes (partial at least) pushed as a6a38a038bd62e6d9558905f00bef81b5e7e6fcc and b384c23b9e804916f125354e06c735bd3bb22811. Please reopen if there is still a problem and specify the environment (architecture, compiler version, any extra flags). This is still not handled for 32 bit atomics. Yes but any affected platforms we actually care about? Newer gcc should cover some of the cases mentioned in this bug. I think pre-v6 ARM now has 32bit atomics on Linux thanks to a kernel helper. Others you mentioned are hppa and superh, is anyone actually building mesa for those? Otherwise it will just be wasted effort for some dead code that nobody ever uses. Shocking as it may seem Mesa runs on more than just Linux. (In reply to Jonathan Gray from comment #15) > Shocking as it may seem Mesa runs on more than just Linux. Don't act like that. Grazvydas wrote code to fix a bug you reported more than two years ago. You should be thankful, not sarcastic. Presumably if you don't piss him off he'd be amenable to fixing other things in this area. (In reply to Grazvydas Ignotas from comment #14) > Yes but any affected platforms we actually care about? > > Newer gcc should cover some of the cases mentioned in this bug. I think > pre-v6 ARM now has 32bit atomics on Linux thanks to a kernel helper. > Others you mentioned are hppa and superh, is anyone actually building mesa > for those? > > Otherwise it will just be wasted effort for some dead code that nobody ever > uses. Gentoo supports both hppa and sh. I find it doubtful that anyone would use Mesa on sh, but hppa is well maintained. I should have a system running in a few days on which I can test. You figured out the solution -- I'm happy to copy-and-paste your small amount of code to support hppa. (In reply to Matt Turner from comment #16) > (In reply to Jonathan Gray from comment #15) > > Shocking as it may seem Mesa runs on more than just Linux. > > Don't act like that. Grazvydas wrote code to fix a bug you reported more > than two years ago. You should be thankful, not sarcastic. > > Presumably if you don't piss him off he'd be amenable to fixing other things > in this area. The problem was clearly stated in the original report. The changes that went in have not addressed this. Only a variant that concerns a subset. Closing the bug because it fixed the case he cared about and not the actual reported problem isn't helping anyone. I've had to workaround the breakage for years to be able to ship builds of Mesa on multiple architectures. (In reply to Jonathan Gray from comment #18) > (In reply to Matt Turner from comment #16) > > (In reply to Jonathan Gray from comment #15) > > > Shocking as it may seem Mesa runs on more than just Linux. > > > > Don't act like that. Grazvydas wrote code to fix a bug you reported more > > than two years ago. You should be thankful, not sarcastic. > > > > Presumably if you don't piss him off he'd be amenable to fixing other things > > in this area. > > The problem was clearly stated in the original report. The changes that > went in have not addressed this. Only a variant that concerns a subset. > Closing the bug because it fixed the case he cared about and not the actual > reported problem isn't helping anyone. I don't think he cared about it, he was trying to fix the bug for you. > > I've had to workaround the breakage for years to be able to ship builds of > Mesa on multiple architectures. Shocking as it may seem Mesa development in mainly done on Linux for Linux on non-niche modern hardware. Occasionally things are going to break on systems untested during development, complaining about an inevitability is not very helpful. This is a community project with limited resources, we will always need contributions from people interested in keeping such platforms building/running optimally. (In reply to Jonathan Gray from comment #18) > (In reply to Matt Turner from comment #16) > > (In reply to Jonathan Gray from comment #15) > > > Shocking as it may seem Mesa runs on more than just Linux. > > > > Don't act like that. Grazvydas wrote code to fix a bug you reported more > > than two years ago. You should be thankful, not sarcastic. > > > > Presumably if you don't piss him off he'd be amenable to fixing other things > > in this area. > > The problem was clearly stated in the original report. The changes that > went in have not addressed this. Only a variant that concerns a subset. > Closing the bug because it fixed the case he cared about and not the actual > reported problem isn't helping anyone. > > I've had to workaround the breakage for years to be able to ship builds of > Mesa on multiple architectures. I think you're inappropriately attributing malice to comments 12 and 14. This also popped up on archlinux32 when trying to package mesa for i486: /usr/bin/ld: src/intel/vulkan/libanv_common.a(anv_allocator.c.o): in function `anv_block_pool_alloc_new': anv_allocator.c:(.text+0x1aa): undefined reference to `__sync_fetch_and_add_8' /usr/bin/ld: anv_allocator.c:(.text+0x307): undefined reference to `__sync_lock_test_and_set_8' /usr/bin/ld: src/intel/vulkan/libanv_common.a(anv_allocator.c.o): in function `anv_free_list_pop': anv_allocator.c:(.text+0x440): undefined reference to `__sync_val_compare_and_swap_8' /usr/bin/ld: src/intel/vulkan/libanv_common.a(anv_allocator.c.o): in function `anv_free_list_push': anv_allocator.c:(.text+0x526): undefined reference to `__sync_val_compare_and_swap_8' /usr/bin/ld: src/intel/vulkan/libanv_common.a(anv_allocator.c.o): in function `anv_state_pool_alloc_no_vg': anv_allocator.c:(.text+0x755): undefined reference to `__sync_fetch_and_add_8' /usr/bin/ld: anv_allocator.c:(.text+0x7a3): undefined reference to `__sync_lock_test_and_set_8' I seem unable to find a source file (only the mentioned *.a and *.o) which actually tries to use these atomics - any idea, where I should look / try to patch? > gcc --version gcc (GCC) 8.2.1 20180831 > uname -a Linux arch32-i486-bs0 4.17.6-1-ARCH #1 SMP PREEMPT Wed Jul 18 08:34:04 UTC 2018 i486 GNU/Linux Best Regards, Erich (In reply to freedesktop from comment #21) > This also popped up on archlinux32 when trying to package mesa for i486: > > /usr/bin/ld: src/intel/vulkan/libanv_common.a(anv_allocator.c.o): in > function `anv_block_pool_alloc_new': ... There is no GPU supported by the Intel Vulkan driver that can possibly exist with a 486 CPU. Don't build that driver with i486 restrictions. Same goes for i915_dri.so and i965_dri.so. None of those can exist with ARM, Alpha, or MIPS either. Use the proper options to configure.ac or Meson to exclude those drivers on those platforms. -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/997. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.