From 2a5dd2a2f5c5f34e89229cd4b4ef9b0700072676 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 2 Dec 2016 10:53:31 +0900 Subject: [PATCH] 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 f8e3e125951f179a4b1f037076804747d925a7d0) ref #19482 --- base/atomics.jl | 15 +++++++++------ src/jitlayers.cpp | 4 ++-- src/julia_internal.h | 1 + src/threading.c | 7 +++++++ test/threads.jl | 3 ++- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/base/atomics.jl b/base/atomics.jl index f602eb8812666..f2cd218d9a607 100644 --- 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,31 +216,31 @@ 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)) @@ -245,7 +248,7 @@ for typ in atomictypes 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 index b8f02a88d50d9..22099ae65e853 100644 --- 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 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 index 0b7ae35b870f2..8658e93eeb4c0 100644 --- 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 index 828cdecc504bd..fbbc57b3b8309 100644 --- 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 index 07ad70fd684a8..eb5f6707fbeb0 100644 --- 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)