Skip to content

AtomicU128 uses __atomic_load function on OSX #39590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alexcrichton opened this issue Feb 6, 2017 · 5 comments
Closed

AtomicU128 uses __atomic_load function on OSX #39590

alexcrichton opened this issue Feb 6, 2017 · 5 comments
Labels
C-bug Category: This is a bug. O-macos Operating system: macOS

Comments

@alexcrichton
Copy link
Member

Last night's nightly build failed with these logs:

error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-m64" "-L" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1/lib/rustlib/x86_64-apple-darwin/lib" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/release/deps/std-a260b5db713b337f.0.o" "-o" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/release/deps/libstd-a260b5db713b337f.dylib" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/release/deps/std-a260b5db713b337f.metadata.o" "-Wl,-dead_strip" "-nodefaultlibs" "-L" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/release/deps" "-L" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1-std/release/deps" "-L" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/native/jemalloc/lib" "-L" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/release/build/compiler_builtins-069104501ba665d8/out" "-L" "/Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/obj/build/x86_64-apple-darwin/stage1/lib/rustlib/x86_64-apple-darwin/lib" "-l" "System" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/libpanic_unwind-18bf5d50673f1daa.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/libunwind-6b24ec54aa474d14.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/libcollections-4e19d5a43d7fdd2c.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/liballoc-99e0cdfb2e11773a.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/liballoc_jemalloc-7b14c3e63843fe84.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/liblibc-109501e572ed7296.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/libstd_unicode-49cd3c7af2b2f27f.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/librand-664091cbac310259.rlib" "-Wl,-force_load,/tmp/rustc.K4tN3uRshYIw/libcore-f1125930d2c15bcd.rlib" "/tmp/rustc.K4tN3uRshYIw/libcompiler_builtins-594db3b2ae45efeb.rlib" "-l" "pthread" "-l" "c" "-l" "m" "-dynamiclib" "-Wl,-dylib" "-Wl,-install_name,@rpath/libstd-a260b5db713b337f.dylib" "-Wl,-rpath,@loader_path/../lib"
  = note: Undefined symbols for architecture x86_64:
            "___atomic_load", referenced from:
                _$LT$core..sync..atomic..AtomicI128$u20$as$u20$core..fmt..Debug$GT$::fmt::hc3b52e65322d95c5 in libcore-f1125930d2c15bcd.rlib(core-f1125930d2c15bcd.0.o)
                _$LT$core..sync..atomic..AtomicU128$u20$as$u20$core..fmt..Debug$GT$::fmt::h78335460c4fed629 in libcore-f1125930d2c15bcd.rlib(core-f1125930d2c15bcd.0.o)
          ld: symbol(s) not found for architecture x86_64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)
          

error: aborting due to previous error

error: Could not compile `std`.

Notably it looks like this code:

#![feature(i128_type, i128)]

use std::sync::atomic::{AtomicU128, Ordering};

#[no_mangle]
pub extern fn foo(a: &AtomicU128) -> u128 {
    a.load(Ordering::SeqCst)
}

generates this assembly:

	.section	__TEXT,__text,regular,pure_instructions
	.globl	_foo
	.p2align	4, 0x90
_foo:
	.cfi_startproc
	pushq	%rbp
Ltmp0:
	.cfi_def_cfa_offset 16
Ltmp1:
	.cfi_offset %rbp, -16
	movq	%rsp, %rbp
Ltmp2:
	.cfi_def_cfa_register %rbp
	subq	$16, %rsp
	movq	%rdi, %rax
	leaq	-16(%rbp), %rdx
	movl	$16, %edi
	movl	$5, %ecx
	movq	%rax, %rsi
	callq	___atomic_load
	movq	-16(%rbp), %rax
	movq	-8(%rbp), %rdx
	addq	$16, %rsp
	popq	%rbp
	retq
	.cfi_endproc


.subsections_via_symbols

cc @Amanieu and #38959

I'm going to temporarily revert #38959 to fix the nightlies while we figure out a solution

bors added a commit that referenced this issue Feb 6, 2017
Revert "Add 128-bit atomics"

This reverts commit 9903975.

Unfortunately 128-bit atomics have broken our nightly builds (#39590) and while we investigate I'm posting a temporary revert of the PR that added them. If we can figure out a solution though before this lands I'd be happy to close!
@parched
Copy link
Contributor

parched commented Feb 6, 2017

Does atomic.c just need to be added to compiler_builtins? It looks like that's where it is defined.

@alexcrichton
Copy link
Member Author

Probably, yeah, but the libs team has previously decided that we won't be providing atomics that don't have a true underlying hardware implementation, so in that sense that wouldn't be our desired solution for such a problem.

bors added a commit that referenced this issue Feb 7, 2017
Revert "Add 128-bit atomics"

This reverts commit 9903975.

Unfortunately 128-bit atomics have broken our nightly builds (#39590) and while we investigate I'm posting a temporary revert of the PR that added them. If we can figure out a solution though before this lands I'd be happy to close!
@ranma42
Copy link
Contributor

ranma42 commented Feb 7, 2017

The cmpxchg16b instruction is not guaranteed to be available on all processors that support x86_64, but in the case of OSX the base cpu is set to core2, which supposedly supports that instruction:

base.cpu = "core2".to_string();
base.max_atomic_width = Some(128); // core2 support cmpxchg16b

@Amanieu
Copy link
Member

Amanieu commented Feb 7, 2017

I think that you need to add +cx16 to the set of LLVM features to enable the use of cmpxchg16b.

@steveklabnik steveklabnik added the O-macos Operating system: macOS label Feb 7, 2017
@kennytm
Copy link
Member

kennytm commented May 4, 2017

cc #35118 (i128 tracking issue)

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 27, 2017
johshoff added a commit to johshoff/concurrent_queue that referenced this issue Jul 7, 2018
The `asm!` directive didn't compile on nightly anymore as it was. The
new code still has issues, most obviously that Bit Test and Set
instruction is not available.

Another issue is the fact that most of these are available through
stable atomics interfaces, so those should be used instead. However,
they are not available for u128, even though they were for a short time:

rust-lang/rust#39590

Lastly, memory-ordering is not specified. However, this is not much
different from what it used to be like.
bors added a commit that referenced this issue Nov 5, 2018
Correct alignment of atomic types and (re)add Atomic{I,U}128

This is a updated #53514 to also make atomic types `repr(C)` as per comment in #53514 (comment).

Fixes #39590
Closes #53514

r? @pnkfelix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-macos Operating system: macOS
Projects
None yet
Development

No branches or pull requests

7 participants