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

128 bit atomic load uses a lock #41280

Closed
tkf opened this issue Jun 18, 2021 · 1 comment · Fixed by #42268
Closed

128 bit atomic load uses a lock #41280

tkf opened this issue Jun 18, 2021 · 1 comment · Fixed by #42268
Labels
multithreading Base.Threads and related functionality performance Must go faster

Comments

@tkf
Copy link
Member

tkf commented Jun 18, 2021

I'm not sure if it's because this is still WIP, but I'm filing this just in case.

julia> mutable struct AtomicRef{T}
           @atomic x::T
       end

julia> f(r) = @atomic :acquire r.x;

julia> @code_llvm f(AtomicRef(UInt128(0)))
;  @ REPL[20]:1 within `f`
define void @julia_f_793(i128* noalias nocapture sret(i128) %0, {}* nonnull align 8 dereferenceable(32) %1) #0 {
top:
; ┌ @ Base.jl:54 within `getproperty`
   %2 = bitcast {}* %1 to i8*
   %3 = getelementptr inbounds i8, i8* %2, i64 16
   call void @jl_lock_value({}* %1)
   %4 = bitcast i8* %3 to i128*
   %5 = load i128, i128* %4, align 8
   call void @jl_unlock_value({}* %1)
; └
  store i128 %5, i128* %0, align 8
  ret void
}

julia> @code_llvm getindex(Threads.Atomic{UInt128}(UInt128(0)))
;  @ atomics.jl:358 within `getindex`
define void @julia_getindex_795(i128* noalias nocapture sret(i128) %0, {}* nonnull align 8 dereferenceable(16) %1) #0 {
top:
  %2 = bitcast {}* %1 to i128*
  %rv.i = load atomic i128, i128* %2 acquire, align 16
  store i128 %rv.i, i128* %0, align 8
  ret void
}

julia> getindex(Threads.Atomic{UInt128}(UInt128(0)))
0x00000000000000000000000000000000
@tkf tkf added the multithreading Base.Threads and related functionality label Jun 18, 2021
@vtjnash
Copy link
Member

vtjnash commented Jun 19, 2021

Yeah, it turned out that I couldn't rely on most C compilers (even just the tiny set on CI) to be able to emit correct code for that case so I had disabled it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multithreading Base.Threads and related functionality performance Must go faster
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants