[PPC] struct pointer representation missing atomic llibrary

nenadv commented 10 years ago

While testing atomics with PPC struct representation, tests failed to link with the following error:

upc_atomic.upc:(.text+0x638c): undefined reference to `__atomic_load'
upc_atomic.upc:(.text+0x63cc): undefined reference to `__atomic_store'
upc_atomic.upc:(.text+0x6408): undefined reference to `__atomic_exchange'
upc_atomic.upc:(.text+0x64dc): undefined reference to `__atomic_compare_exchange'

I think we have to install 'compiler-rt' LLVM project that provides libgcc functionality.

nenadv commented 10 years ago

This might be also be present in x86_64 as we never tested atomics and struct pointer representation.

nenadv commented 10 years ago

I filed the following on llvm as I am having trouble building compiler-rt.

http://llvm.org/bugs/show_bug.cgi?id=19214

PHHargrove commented 10 years ago

Nenad,

It occurs to me that the generated code might be trying to perform 128-bit atomic operations on the PTS type. If that is the case then at least for gcc the expected behavior for operands of unsupported size is to generate external function calls. While it is not clearly stated, I believe the gcc runtime does not provide the implementation in that case, and I therefore wouldn't expect the clang runtime to either. That seems consistent with what you report.

Quoting from the GCC online docs [emphasis mine]:

The four non-arithmetic functions (load, store, exchange, and compare_exchange) all have a generic version as well. This generic version works on any data type. If the data type size maps to one of the integral sizes that may have lock free support, the generic version utilizes the lock free built-in function. Otherwise an external call is left to be resolved at run time. This external call is the same format with the addition of a ‘size_t’ parameter inserted as the first parameter indicating the size of the object being pointed to. All objects must be the same size.

nenadv commented 10 years ago

I think it is trying to do 128-bit atomic operation, an after looking at the source http://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/builtins/atomic.c it should be supported. But, it is not lock free support.

On 3/20/2014 5:35 PM, Paul H. Hargrove wrote:

Nenad,

It occurs to me that the generated code might be trying to perform 128-bit atomic operations on the PTS type. If that is the case then at least for gcc the expected behavior for operands of unsupported size is to generate external function calls. While it is not clearly stated, I believe the gcc runtime does not provide the implementation in that case, and I therefore wouldn't expect the clang runtime to either. That seems consistent with what you report.

Quoting from the GCC online docs http://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html [emphasis mine]:
The four non-arithmetic functions (load, store, exchange, and
compare_exchange) all have a generic version as well. This generic
version works on any data type. If the data type size maps to one
of the integral sizes that may have lock free support, the generic
version utilizes the lock free built-in function. *Otherwise an
external call is left to be resolved at run time.* This external
call is the same format with the addition of a ‘size_t’ parameter
inserted as the first parameter indicating the size of the object
being pointed to. All objects must be the same size.
— Reply to this email directly or view it on GitHub https://github.com/Intrepid/clang-upc/issues/49#issuecomment-38237226.

nenadv commented 10 years ago

I got the compile-rt to build on x86_64 target. There is an issue with Cmake files caused by this update to LLVM - https://github.com/llvm-mirror/llvm/commit/742c278977f1804c507201ba2e871dfe184fbbba and compiler-rt started using the new variables after the 3.4 release. However, I had the similar problem when I used the trunk version and at some point I'll go back and try to figure this out, if applicabale at all.

With this patch to llvm-upc I was able to build compile-rt library:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a68e7e1..8d3540a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -89,6 +89,10 @@ endif()

 string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)

+# They are used as destination of target generators.
+set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
+set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib)
+
 set(LLVM_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_SRC_DIR}/include)
 set(LLVM_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})

All went fine, except that 'atomic.c' file is not being compiled:

--- a/lib/builtins/CMakeLists.txt
+++ b/lib/builtins/CMakeLists.txt
@@ -17,7 +17,7 @@ set(GENERIC_SOURCES
   ashrdi3.c
   ashrti3.c
   # FIXME: atomic.c may only be compiled if host compiler understands _Atomic
-  # atomic.c
+  atomic.c
   clear_cache.c

I enabled compilation, but gcc 4.7.2 that we have does not support _Atimic (c11 spec). It was added to gcc 6 months after the 4.7.2 release date. Which begs the question of compiling compiler-rt library with gcc instead of the freshly built clang (?)

However, even without atomics, I was not able to link with compiler-rt libraries. They are all installed in 'lib/clang/3.4/lib/linux':

libclang_rt.asan-i386.a          libclang_rt.lsan-x86_64.a       libclang_rt.tsan-x86_64.a.syms
libclang_rt.asan-x86_64.a        libclang_rt.msan-x86_64.a       libclang_rt.ubsan_cxx-i386.a
libclang_rt.asan-x86_64.a.syms   libclang_rt.msan-x86_64.a.syms  libclang_rt.ubsan_cxx-x86_64.a
libclang_rt.dd-x86_64.a          libclang_rt.profile-i386.a      libclang_rt.ubsan_cxx-x86_64.a.syms
libclang_rt.dfsan-libc-x86_64.a  libclang_rt.profile-x86_64.a    libclang_rt.ubsan-i386.a
libclang_rt.dfsan-x86_64.a       libclang_rt.san-i386.a          libclang_rt.ubsan-x86_64.a
libclang_rt.dfsan-x86_64.a.syms  libclang_rt.san-x86_64.a        libclang_rt.ubsan-x86_64.a.syms
libclang_rt.i386.a               libclang_rt.tsan-x86_64.a       libclang_rt.x86_64.a

No library path, no library specified on the clang driver link line. More to research on this.

At this point I am more inclined in providing our own low level atomic library.

nenadv commented 10 years ago

After more checking on compiler-rt library I determined that it might not be what we need to support UPC SMP atomics. We probably need to build compiler-rt (as instructed by the Clang install instructions) just to make sure we don;t have problem with it. But the main problem is that they use the host CC to compile the library, which in our case does not work as it does not support _Atomic type and we cannot compiler lib/binutils/atomic.c. Additional problem is that this files currently commented out from the comiler-rt build and there is no mechanism to configure it in or out.

This is what we need to resolve this issue:

If Clang does not have _atomic builtin supported in hardware, external calls to load,store,exchange, cswap are created with the first argument being the size of the request.
Template for those functions can be found in the atomic.c from the compiler-rt library.
A hack like this is used to define a builtin function:

#pragma redefine_extname __atomic_load_c __atomic_load
void                                                                            
__atomic_load_c (int size, void * src, void * dest, int memmodel)               
{                                                                               
  LOCK_ACCESS;                                                                  
  memcpy (dest, src, size);                                                     
  UNLOCK_ACCESS;                                                                
}

I am able to compile/pass struct implementation on x86_64 with this approach. More testing needed.

nenadv commented 10 years ago

Fixed by this commit https://github.com/Intrepid/clang-upc/commit/07e142808fd1bf9694f2512e6aeb494b63c3fd4b After extensive testing of the Clang builtin atomics library:

128 bits builtins are not supported. Calls to generic libatomic functions are being generated. Those functions are defined in compiler-rt library but we cannot build it on some systems that do not have C11 support for _Atomic types.
64 bits atomics on 32 bit target are broken. External ".._8" calls are being generated and some of the calls are incorrectly implemented.

See http://llvm.org/bugs/show_bug.cgi?id=19149 for description.

With above mentioned fix we are implementing a generic atomic library that uses a spin lock to protect critical regions of code. We will most likely continue to use this until atomics on 32 bit are fixed. For 128 bits we'll need to provide additional code.

Note that libupc has an issue the way configure/make works in Clang. cmake/configure runs at the beginning and compiler used for testing the host features (e.g. does it have sync functions) is usually the host compiler which is not the same as the compiler we will be using and compiling the UPC library with.

clangupc / clang-upc

[PPC] struct pointer representation missing atomic llibrary #49