Correct alignment of atomic load and stores

This makes sure that atomic load and stores have the correct alignment as guaranteed by the gc. (cherry picked from commit f8e3e12) ref #19482
JuliaLang · May 2, 2017 · 2a5dd2a · 2a5dd2a
1 parent a676636
commit 2a5dd2a
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 9 deletions.
diff --git a/base/atomics.jl b/base/atomics.jl
@@ -202,6 +202,9 @@ inttype(::Type{Float16}) = Int16
 inttype(::Type{Float32}) = Int32
 inttype(::Type{Float64}) = Int64
 
+
+alignment{T}(::Type{T}) = ccall(:jl_alignment, Cint, (Csize_t,), sizeof(T))
+
 # All atomic operations have acquire and/or release semantics, depending on
 # whether the load or store values. Most of the time, this is what one wants
 # anyway, and it's only moderately expensive on most hardware.
@@ -213,39 +216,39 @@ for typ in atomictypes
     if VersionNumber(Base.libllvm_version) >= v"3.8"
         @eval getindex(x::Atomic{$typ}) =
             llvmcall($"""
-                     %rv = load atomic $rt %0 acquire, align $(WORD_SIZE ÷ 8)
+                     %rv = load atomic $rt %0 acquire, align $(alignment(typ))
                      ret $lt %rv
                      """, $typ, Tuple{Ptr{$typ}}, unsafe_convert(Ptr{$typ}, x))
         @eval setindex!(x::Atomic{$typ}, v::$typ) =
             llvmcall($"""
-                     store atomic $lt %1, $lt* %0 release, align $(WORD_SIZE ÷ 8)
+                     store atomic $lt %1, $lt* %0 release, align $(alignment(typ))
                      ret void
                      """, Void, Tuple{Ptr{$typ},$typ}, unsafe_convert(Ptr{$typ}, x), v)
     else
         if typ <: Integer
             @eval getindex(x::Atomic{$typ}) =
                 llvmcall($"""
-                         %rv = load atomic $rt %0 acquire, align $(WORD_SIZE ÷ 8)
+                         %rv = load atomic $rt %0 acquire, align $(alignment(typ))
                          ret $lt %rv
                          """, $typ, Tuple{Ptr{$typ}}, unsafe_convert(Ptr{$typ}, x))
             @eval setindex!(x::Atomic{$typ}, v::$typ) =
                 llvmcall($"""
-                         store atomic $lt %1, $lt* %0 release, align $(WORD_SIZE ÷ 8)
+                         store atomic $lt %1, $lt* %0 release, align $(alignment(typ))
                          ret void
                          """, Void, Tuple{Ptr{$typ},$typ}, unsafe_convert(Ptr{$typ}, x), v)
         else
             @eval getindex(x::Atomic{$typ}) =
                 llvmcall($"""
                          %iptr = bitcast $lt* %0 to $ilt*
-                         %irv = load atomic $irt %iptr acquire, align $(WORD_SIZE ÷ 8)
+                         %irv = load atomic $irt %iptr acquire, align $(alignment(typ))
                          %rv = bitcast $ilt %irv to $lt
                          ret $lt %rv
                          """, $typ, Tuple{Ptr{$typ}}, unsafe_convert(Ptr{$typ}, x))
             @eval setindex!(x::Atomic{$typ}, v::$typ) =
                 llvmcall($"""
                          %iptr = bitcast $lt* %0 to $ilt*
                          %ival = bitcast $lt %1 to $ilt
-                         store atomic $ilt %ival, $ilt* %iptr release, align $(WORD_SIZE ÷ 8)
+                         store atomic $ilt %ival, $ilt* %iptr release, align $(alignment(typ))
                          ret void
                          """, Void, Tuple{Ptr{$typ},$typ}, unsafe_convert(Ptr{$typ}, x), v)
         end

diff --git a/src/jitlayers.cpp b/src/jitlayers.cpp
@@ -289,7 +289,7 @@ void NotifyDebugger(jit_code_entry *JITCodeEntry)
 }
 // ------------------------ END OF TEMPORARY COPY FROM LLVM -----------------
 
-#ifdef _OS_LINUX_
+#if defined(_OS_LINUX_)
 // Resolve non-lock free atomic functions in the libatomic library.
 // This is the library that provides support for c11/c++11 atomic operations.
 static uint64_t resolve_atomic(const char *name)
@@ -506,7 +506,7 @@ void JuliaOJIT::addModule(std::unique_ptr<Module> M)
                         // Step 2: Search the program symbols
                         if (uint64_t addr = SectionMemoryManager::getSymbolAddressInProcess(Name))
                             return RuntimeDyld::SymbolInfo(addr, JITSymbolFlags::Exported);
-#ifdef _OS_LINUX_
+#if defined(_OS_LINUX_)
                         if (uint64_t addr = resolve_atomic(Name.c_str()))
                             return RuntimeDyld::SymbolInfo(addr, JITSymbolFlags::Exported);
 #endif

diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -116,6 +116,7 @@ STATIC_INLINE int jl_gc_alignment(size_t sz)
     return 16;
 #endif
 }
+JL_DLLEXPORT int jl_alignment(size_t sz);
 
 STATIC_INLINE int JL_CONST_FUNC jl_gc_szclass(size_t sz)
 {

diff --git a/src/threading.c b/src/threading.c
@@ -780,6 +780,13 @@ void jl_start_threads(void) { }
 
 #endif // !JULIA_ENABLE_THREADING
 
+// Make gc alignment available for threading
+// see threads.jl alignment
+JL_DLLEXPORT int jl_alignment(size_t sz)
+{
+    return jl_gc_alignment(sz);
+}
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/test/threads.jl b/test/threads.jl
@@ -273,7 +273,8 @@ let atomic_types = [Int8, Int16, Int32, Int64, Int128,
                     Float16, Float32, Float64]
     # Temporarily omit 128-bit types on 32bit x86
     # 128-bit atomics do not exist on AArch32.
-    # And we don't support them yet on power.
+    # And we don't support them yet on power, because they are lowered
+    # to `__sync_lock_test_and_set_16`.
     if Sys.ARCH === :i686 || startswith(string(Sys.ARCH), "arm") ||
        Sys.ARCH === :powerpc64le || Sys.ARCH === :ppc64le
         filter!(T -> sizeof(T)<=8, atomic_types)