Bug 230888 - Missing 64 bit atomic functions for i386 (libatomic)
Summary: Missing 64 bit atomic functions for i386 (libatomic)
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: i386 Any
: --- Affects Some People
Assignee: freebsd-toolchain mailing list
URL:
Keywords: needs-patch, needs-qa
: 220822 233725 234976 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-08-25 12:53 UTC by Dimitry Andric
Modified: 2019-06-11 04:05 UTC (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitry Andric freebsd_committer 2018-08-25 12:53:48 UTC
We have always been missing 64 bit atomic functions for i386, and this issue has now unfortunately come up again when testing llvm 7.0.0 release candidates[1].

Our old gcc 4.2 never supported the necessary C11 constructs, so it was never a problem there.  But since clang has been introduced, it has silently emitted "lock cmpxchg8b" instructions, even though these are only supported by i586 and higher CPUs.

Upstream llvm attempted to fix this a number of times, most recently in <https://reviews.llvm.org/rL323281>.  However, for the default target CPU on i386-freebsd, which is i486, it will be forced to emit function calls to atomic primitives, as __atomic_load_8(), __atomic_fetch_add_8() and such.

Demonstration with ports gcc:

$ cat test-c11-atomic-2.c
_Atomic(long long) ll;

int main(void)
{
        ++ll;
        return 0;
}

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc8/gcc/i386-portbld-freebsd12.0/8.2.0/lto-wrapper
Target: i386-portbld-freebsd12.0
Configured with: /wrkdirs/share/dim/ports/lang/gcc8/work/gcc-8.2.0/configure --disable-multilib --disable-bootstrap --disable-nls --enable-gnu-indirect-function --libdir=/usr/local/lib/gcc8 --libexecdir=/usr/local/libexec/gcc8 --program-suffix=8 --with-as=/usr/local/bin/as --with-gmp=/usr/local --with-gxx-include-dir=/usr/local/lib/gcc8/include/c++/ --with-ld=/usr/local/bin/ld --with-pkgversion='FreeBSD Ports Collection' --with-system-zlib --with-isl=/usr/local --enable-languages=c,c++,objc,fortran --prefix=/usr/local --localstatedir=/var --mandir=/usr/local/man --infodir=/usr/local/info/gcc8 --build=i386-portbld-freebsd12.0
Thread model: posix
gcc version 8.2.0 (FreeBSD Ports Collection)

$ gcc -O2 -S test-c11-atomic-2.c -o -
        .file   "test-c11-atomic-2.c"
        .text
        .section        .text.startup,"ax",@progbits
        .p2align 4,,15
        .globl  main
        .type   main, @function
main:
.LFB0:
        .cfi_startproc
        leal    4(%esp), %ecx
        .cfi_def_cfa 1, 0
        andl    $-16, %esp
        pushl   -4(%ecx)
        pushl   %ebp
        .cfi_escape 0x10,0x5,0x2,0x75,0
        movl    %esp, %ebp
        pushl   %ecx
        .cfi_escape 0xf,0x3,0x75,0x7c,0x6
        subl    $4, %esp
        pushl   $5
        pushl   $0
        pushl   $1
        pushl   $ll
        call    __atomic_fetch_add_8
        movl    -4(%ebp), %ecx
        .cfi_def_cfa 1, 0
        addl    $16, %esp
        xorl    %eax, %eax
        leave
        .cfi_restore 5
        leal    -4(%ecx), %esp
        .cfi_def_cfa 4, 4
        ret
        .cfi_endproc
.LFE0:
        .size   main, .-main
        .comm   ll,8,8
        .ident  "GCC: (FreeBSD Ports Collection) 8.2.0"
        .section        .note.GNU-stack,"",@progbits

$ gcc -O2 test-c11-atomic-2.c
/tmp//cc4hCSj6.o: In function `main':
test-c11-atomic-2.c:(.text.startup+0x1d): undefined reference to `__atomic_fetch_add_8'
collect2: error: ld returned 1 exit status

We should really make a choice and either:
1) Implement __atomic_load_8, __atomic_fetch_8 and others for i386, with or without kernel support, and add them to libc or libgcc
2) Change the default target CPU to i586 (which is actually the de facto default for a few years now), or even i686.

I assume 1) is quite a lot of work, so 2) is likely the way of least effort, but might meet with some resistance to change and POLA complaints.

[1] http://lists.llvm.org/pipermail/release-testers/2018-August/000753.html
[2] https://reviews.llvm.org/rL323281
Comment 1 Dimitry Andric freebsd_committer 2018-08-25 13:01:19 UTC
See also bug 220822, bug 229605, and probably lots more.

Assigning to toolchain@ since that is a better place than bugs@, which I think nobody reads. :)
Comment 2 Conrad Meyer freebsd_committer 2018-08-25 21:59:18 UTC
Isn't (1) just: kernel: mask all interrupts, do operation, unmask; userspace: take a global mutex over the op?

Do we care if 64-bit atomics don't work from NMI contexts on i486-class hardware?  I doubt it.

I'm also pro gone_in(12)'ing i486 and lower.
Comment 3 David Chisnall freebsd_committer 2018-08-26 10:44:30 UTC
Is there a reason, given that LLVM's atomic.c is in contrib, that we don't just connect it to the libgcc_s build?

Note that the problem is not just 64-bit.  An increasing amount of x86-64 code depends on 128-bit atomics (which are single instructions if compiling with -mcx16, libcalls otherwise).  C11 allows arbitrary sized atomics.  It's allowed to write _Atomic(struct X) for any arbitrary X.  The code in atomic.c handles this, though not for atomic types in shared memory (which can't be supported without changing the ABI - something that WG21 thought through by making std::atomic a library feature and allowing larger atomic types to be implemented with an inline lock, and which WG14 completely messed up).
Comment 4 Conrad Meyer freebsd_committer 2018-08-26 17:22:24 UTC
(In reply to David Chisnall from comment #3)
Seems like a C lock implementation for large types is allowed to place the lock in the shared memory with the object.  (Not that using shared memory or _Atomic structs is necessarily a good idea.)

"The size, representation, and alignment of an atomic type need not be the same as those of the corresponding unqualified type."
Comment 5 Harlan Stenn 2018-08-27 10:57:53 UTC
This is also an issue for MariaDB and unpatched haproxy-1.8 .

As far as i486 goes, I'd rather see this fixed for the easy cases now, and perhaps the more difficult cases can be addressed later, if possible.

H
Comment 6 Dimitry Andric freebsd_committer 2019-03-09 15:14:43 UTC
*** Bug 233725 has been marked as a duplicate of this bug. ***
Comment 7 Dimitry Andric freebsd_committer 2019-03-09 15:14:52 UTC
*** Bug 220822 has been marked as a duplicate of this bug. ***
Comment 8 Dimitry Andric freebsd_committer 2019-03-09 15:15:17 UTC
*** Bug 234976 has been marked as a duplicate of this bug. ***
Comment 9 Jan Beich freebsd_committer 2019-04-27 18:17:43 UTC
Also affects LLVM openmp e.g.,
http://beefy11.nyi.freebsd.org/data/head-i386-default/p499924_s346654/logs/errors/colmap-3.5_7.log

$ cat a.c
#include <stdint.h>

int main(void)
{
	int64_t j, i = 1;
#pragma omp atomic read
	j = i;
	return 0;
}

$ cc -fopenmp a.c
ld: error: undefined symbol: __atomic_load
>>> referenced by foo.c
>>>               /tmp/foo-fdf421.o:(main)
cc: error: linker command failed with exit code 1 (use -v to see invocation)
Comment 10 Jan Beich freebsd_committer 2019-04-27 18:19:22 UTC
Err, I meant Clang -fopenmp, not LLVM openmp (library).