From 3df8459362f73ba713bf7c63c6f6ecf6e36ef117 Mon Sep 17 00:00:00 2001
From: Nathan Daly <nhdaly@gmail.com>
Date: Fri, 23 Mar 2018 11:42:29 -0500
Subject: [PATCH] Add support for Atomic{Bool} (Fix #26542).

Adds `Bool` to list of types supported by `Atomic{T}`.

Defines all `atomic_*!` for `Bool`, except `atomic_add!` and `atomic_sub!`
since `add(::Bool, ::Bool)` returns an `Int`.

Also adds tests for those methods to `test/threads.jl`.
---
 base/atomics.jl | 24 +++++++++++++++---------
 test/threads.jl | 19 ++++++++++++++++++-
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/base/atomics.jl b/base/atomics.jl
index b9a3289b68944..f10792240d717 100644
--- a/base/atomics.jl
+++ b/base/atomics.jl
@@ -27,9 +27,11 @@ else
 end
 const floattypes = (Float16, Float32, Float64)
 # TODO: Support Bool, Ptr
-const atomictypes = (inttypes..., floattypes...)
+const arithmetictypes = (inttypes..., floattypes...)
+const atomictypes = (arithmetictypes..., Bool)
 const IntTypes = Union{inttypes...}
 const FloatTypes = Union{floattypes...}
+const ArithmeticTypes = Union{arithmetictypes...}
 const AtomicTypes = Union{atomictypes...}
 
 """
@@ -39,8 +41,8 @@ Holds a reference to an object of type `T`, ensuring that it is only
 accessed atomically, i.e. in a thread-safe manner.
 
 Only certain "simple" types can be used atomically, namely the
-primitive integer and float-point types. These are `Int8`...`Int128`,
-`UInt8`...`UInt128`, and `Float16`...`Float64`.
+primitive boolean, integer, and float-point types. These are `Bool`,
+`Int8`...`Int128`, `UInt8`...`UInt128`, and `Float16`...`Float64`.
 
 New atomic objects can be created from a non-atomic values; if none is
 specified, the atomic object is initialized with zero.
@@ -130,11 +132,12 @@ julia> x[]
 function atomic_xchg! end
 
 """
-    Threads.atomic_add!(x::Atomic{T}, val::T) where T
+    Threads.atomic_add!(x::Atomic{T}, val::T) where T <: ArithmeticTypes
 
 Atomically add `val` to `x`
 
-Performs `x[] += val` atomically. Returns the **old** value.
+Performs `x[] += val` atomically. Returns the **old** value. Not defined for
+`Atomic{Bool}`.
 
 For further details, see LLVM's `atomicrmw add` instruction.
 
@@ -153,11 +156,12 @@ julia> x[]
 function atomic_add! end
 
 """
-    Threads.atomic_sub!(x::Atomic{T}, val::T) where T
+    Threads.atomic_sub!(x::Atomic{T}, val::T) where T <: ArithmeticTypes
 
 Atomically subtract `val` from `x`
 
-Performs `x[] -= val` atomically. Returns the **old** value.
+Performs `x[] -= val` atomically. Returns the **old** value. Not defined for
+`Atomic{Bool}`.
 
 For further details, see LLVM's `atomicrmw sub` instruction.
 
@@ -317,7 +321,7 @@ unsafe_convert(::Type{Ptr{T}}, x::Atomic{T}) where {T} = convert(Ptr{T}, pointer
 setindex!(x::Atomic{T}, v) where {T} = setindex!(x, convert(T, v))
 
 const llvmtypes = IdDict{Any,String}(
-    Bool => "i1",
+    Bool => "i8",  # julia represents bools with 8-bits for now. # TODO: is this okay?
     Int8 => "i8", UInt8 => "i8",
     Int16 => "i16", UInt16 => "i16",
     Int32 => "i32", UInt32 => "i32",
@@ -380,13 +384,15 @@ for typ in atomictypes
                      unsafe_convert(Ptr{$typ}, x), cmp, new)
     end
 
-    for rmwop in [:xchg, :add, :sub, :and, :nand, :or, :xor, :max, :min]
+    arithmetic_ops = [:add, :sub]
+    for rmwop in [arithmetic_ops..., :xchg, :and, :nand, :or, :xor, :max, :min]
         rmw = string(rmwop)
         fn = Symbol("atomic_", rmw, "!")
         if (rmw == "max" || rmw == "min") && typ <: Unsigned
             # LLVM distinguishes signedness in the operation, not the integer type.
             rmw = "u" * rmw
         end
+        if rmwop in arithmetic_ops && !(typ <: ArithmeticTypes) continue end
         if typ <: Integer
             @eval $fn(x::Atomic{$typ}, v::$typ) =
                 llvmcall($"""
diff --git a/test/threads.jl b/test/threads.jl
index 7c943265ff2e5..642c97b0c4483 100644
--- a/test/threads.jl
+++ b/test/threads.jl
@@ -171,10 +171,27 @@ end
 end
 
 # Ensure only LLVM-supported types can be atomic
-@test_throws TypeError Atomic{Bool}
 @test_throws TypeError Atomic{BigInt}
 @test_throws TypeError Atomic{ComplexF64}
 
+function test_atomic_bools()
+    x = Atomic{Bool}(false)
+    # Arithmetic functions are not defined.
+    @test_throws MethodError atomic_add!(x, true)
+    @test_throws MethodError atomic_sub!(x, true)
+    # All the rest are:
+    for v in [true, false]
+        @test x[] == atomic_xchg!(x, v)
+        @test v == atomic_cas!(x, v, !v)
+    end
+    x = Atomic{Bool}(false)
+    @test false == atomic_max!(x, true); @test x[] == true
+    x = Atomic{Bool}(true)
+    @test true == atomic_and!(x, false); @test x[] == false
+end
+
+test_atomic_bools()
+
 # Test atomic memory ordering with load/store
 mutable struct CommBuf
     var1::Atomic{Int}