Skip to content
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

128bit atomic support on non-Intel machines #18706

Open
vchuravy opened this issue Sep 27, 2016 · 3 comments
Open

128bit atomic support on non-Intel machines #18706

vchuravy opened this issue Sep 27, 2016 · 3 comments
Assignees
Labels
system:powerpc PowerPC upstream The issue is with an upstream dependency, e.g. LLVM

Comments

@vchuravy
Copy link
Member

On non-intel machines (or machines that don't have the cx16 feature) llvm issues call to the __sync* libcalls. See the recent discussion in #14818, #18701 and #16066 (comment)

Possible solutions
1; Use compiler-rt and revert llvm-mirror/compiler-rt@fa3eee4
2; Resolve __sync to jl__sync in jitlayers and provide implementations in libjulia. I started doing this in https://github.com/JuliaLang/julia/tree/vc/int128_atomics, but it is a horrible hack
3; Fix llvm upstream to call into libatomic. https://llvm.org/bugs/show_bug.cgi?id=30471

@vchuravy vchuravy added upstream The issue is with an upstream dependency, e.g. LLVM system:powerpc PowerPC labels Sep 27, 2016
@vchuravy vchuravy self-assigned this Sep 27, 2016
@yuyichao
Copy link
Contributor

For power8, the right solution should be using the instructions directly since that seems to be what GCC does. For ARM, the solution is to not use atomic intrinsics and use the libatomic generic call directly. The __sync shouldn't be needed anywhere we support.

@yuyichao
Copy link
Contributor

yuyichao commented Sep 27, 2016

So to summarize, if LLVM and GCC emit different code for the atomic, then LLVM needs to be fixed (unless it's a GCC bug). After that, we need to follow what clang does.

  1. x86 without cx16

    We need a __atomic call in the assembly. LLVM 3.9+ does this automatically so there's nothing we need to fix in codegen. We do need to remove the hard coded requirement of the cx16 feature though.

  2. ARM

    There's no builtin support for some of the i128 atomics and there's no C interger type of size 16 so neither GCC nor Clang support it. The generic __atomic functions does support any size and we need to emit that directly.

  3. AArch64

    Same as x86 without cx16 (The instruction exists but cannot be used for atomic load since it requires writing to the memory). On LLVM 3.9 the __atomic call is generated automatically by LLVM and there's nothing we need to do.

  4. Power8

    IIUC the instruction is required by the ISA and GCC emit that directly. We need to make sure Clang/LLVM emit the instruction too and there's no codegen change needed.

The only changes that we need to do are

  1. Upgrade to LLVM 3.9
  2. Remove the hard coded cx16 requirement
  3. Reenable 128bit atomic on ARM with a different implementation.

@vchuravy
Copy link
Member Author

For PPC https://reviews.llvm.org/rGb9c3941cd61d landed in LLVM 13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:powerpc PowerPC upstream The issue is with an upstream dependency, e.g. LLVM
Projects
None yet
Development

No branches or pull requests

2 participants