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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Atomics and Locks (ARM, AArch64, X86) #14293

Merged
merged 11 commits into from
Mar 5, 2024
Merged

Fix: Atomics and Locks (ARM, AArch64, X86) #14293

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f6b724d
Add explicit memory orderings to Atomic(T)
ysbaddaden Feb 12, 2024
c74327a
Fix: atomic locks for X86, ARM and AArch64 targets
ysbaddaden Feb 12, 2024
5a170f5
src/atomic.cr
ysbaddaden Feb 12, 2024
cd15f7b
fixup! Fix: invalid fallback to llvm 12 and below
ysbaddaden Feb 12, 2024
cbd5540
More compact LLVM <= 12 fallback
ysbaddaden Feb 15, 2024
be9e9ad
fixup! More compact LLVM <= 12 fallback
ysbaddaden Feb 15, 2024
5ef1f21
fixup! More compact LLVM <= 12 fallback
ysbaddaden Feb 15, 2024
fd8d209
Fix: raise ArgumentError
ysbaddaden Feb 15, 2024
6617523
Fix: don't depend on LLVM version to validate cmpxchg ordering
ysbaddaden Feb 15, 2024
a53ab72
CAS: add comment about no stronger rule
ysbaddaden Feb 26, 2024
7fafb28
Docs for #compare_and_set with explicit orderings
ysbaddaden Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
99 changes: 28 additions & 71 deletions src/atomic.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,76 +70,33 @@ struct Atomic(T)
end

def compare_and_set(cmp : T, new : T, success_ordering : Ordering, failure_ordering : Ordering) : {T, Bool}
ysbaddaden marked this conversation as resolved.
Show resolved Hide resolved
{% if compare_versions(Crystal::LLVM_VERSION, "13.0.0") >= 0 %}
# LLVM since 13.0.0 accepts any combination of success & failure ordering
case success_ordering
{% for s_ordering in Ordering.constants %}
in Ordering::{{s_ordering}}
case failure_ordering
{% for f_ordering in Ordering.constants %}
in Ordering::{{f_ordering}}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T),
{{ s_ordering.stringify == "Relaxed" ? :monotonic : s_ordering.underscore.symbolize }},
{{ f_ordering.stringify == "Relaxed" ? :monotonic : f_ordering.underscore.symbolize }})
{% end %}
end
{% end %}
case {success_ordering, failure_ordering}
when {.relaxed?, .relaxed?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :monotonic, :monotonic)
when {.acquire?, .relaxed?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire, :monotonic)
when {.acquire?, .acquire?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire, :acquire)
when {.release?, .relaxed?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :release, :monotonic)
when {.release?, .acquire?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :release, :acquire)
when {.acquire_release?, .relaxed?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire_release, :monotonic)
when {.acquire_release?, .acquire?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire_release, :acquire)
when {.sequentially_consistent?, .relaxed?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :sequentially_consistent, :monotonic)
when {.sequentially_consistent?, .acquire?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :sequentially_consistent, :acquire)
when {.sequentially_consistent?, .sequentially_consistent?}
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :sequentially_consistent, :sequentially_consistent)
else
if failure_ordering.release? || failure_ordering.acquire_release?
raise ArgumentError.new("Failure ordering cannot include release semantics")
end
{% else %}
# LLVM until 12.0.0 only accepts some combinations of success & failure ordering
case success_ordering
in .relaxed?
if failure_ordering.relaxed?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :monotonic, :monotonic)
else
raise "BUG: failure ordering shall be no stronger than success ordering"
end
in .acquire?
case failure_ordering
in .relaxed?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire, :monotonic)
in .acquire?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire, :acquire)
in .release?
raise "BUG: failure ordering cannot include release semantics"
in .acquire_release?, .sequentially_consistent?
raise "BUG: failure ordering shall be no stronger than success ordering"
end
in .release?
case failure_ordering
in .relaxed?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :release, :monotonic)
in .acquire?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :release, :acquire)
in .release?
raise "BUG: failure ordering cannot include release semantics"
in .acquire_release?, .sequentially_consistent?
raise "BUG: failure ordering shall be no stronger than success ordering"
end
in .acquire_release?
case failure_ordering
in .relaxed?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire_release, :monotonic)
in .acquire?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :acquire_release, :acquire)
in .release?, .acquire_release?
raise "BUG: failure ordering cannot include release semantics"
in .sequentially_consistent?
raise "BUG: failure ordering shall be no stronger than success ordering"
end
in .sequentially_consistent?
case failure_ordering
in .relaxed?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :sequentially_consistent, :monotonic)
in .acquire?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :sequentially_consistent, :acquire)
in .release?, .acquire_release?
raise "BUG: failure ordering cannot include release semantics"
in .sequentially_consistent?
Ops.cmpxchg(pointerof(@value), cmp.as(T), new.as(T), :sequentially_consistent, :sequentially_consistent)
end
end
{% end %}
raise ArgumentError.new("Failure ordering shall be no stronger than success ordering")
end
end

# Performs `atomic_value &+= value`. Returns the old value.
Expand Down Expand Up @@ -314,7 +271,7 @@ struct Atomic(T)
in .sequentially_consistent?
Ops.store(pointerof(@value), value.as(T), :sequentially_consistent, true)
in .acquire?, .acquire_release?
raise "BUG: Atomic store cannot have acquire semantic"
raise ArgumentError.new("Atomic store cannot have acquire semantic")
end
value
end
Expand All @@ -341,7 +298,7 @@ struct Atomic(T)
in .sequentially_consistent?
Ops.load(pointerof(@value), :sequentially_consistent, true)
in .release?, .acquire_release?
raise "BUG: Atomic load cannot have release semantic"
raise ArgumentError.new("Atomic load cannot have release semantic")
end
end

Expand Down
48 changes: 11 additions & 37 deletions src/compiler/crystal/codegen/primitives.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,10 +1169,7 @@ class Crystal::CodeGenVisitor
failure_node = call.args[-1]
success_ordering = atomic_ordering_get_const(success_node, success_ordering)
failure_ordering = atomic_ordering_get_const(failure_node, failure_ordering)

{% if LibLLVM::IS_LT_130 %}
validate_atomic_cmpxchg_ordering(success_node, success_ordering, failure_node, failure_ordering)
{% end %}
validate_atomic_cmpxchg_ordering(success_node, success_ordering, failure_node, failure_ordering)

value = builder.cmpxchg(ptr, cmp, new, success_ordering, failure_ordering)
value_type = node.type.as(TupleInstanceType)
Expand Down Expand Up @@ -1299,41 +1296,18 @@ class Crystal::CodeGenVisitor
end
end

{% if LibLLVM::IS_LT_130 %}
def validate_atomic_cmpxchg_ordering(success_node, success_ordering, failure_node, failure_ordering)
success_node.raise "must be atomic" if success_ordering.not_atomic?
failure_node.raise "must be atomic" if failure_ordering.not_atomic?
def validate_atomic_cmpxchg_ordering(success_node, success_ordering, failure_node, failure_ordering)
success_node.raise "must be atomic" if success_ordering.not_atomic?
success_node.raise "cannot be unordered" if success_ordering.unordered?

success_node.raise "cannot be unordered" if success_ordering.unordered?
failure_node.raise "cannot be unordered" if failure_ordering.unordered?
failure_node.raise "must be atomic" if failure_ordering.not_atomic?
failure_node.raise "cannot be unordered" if failure_ordering.unordered?
failure_node.raise "cannot include release semantics" if failure_ordering.release? || failure_ordering.acquire_release?

case success_ordering
when .monotonic?
unless failure_ordering.monotonic?
failure_node.raise "shall be no stronger than success ordering"
end
when .acquire?, .release?
case failure_ordering
when .release?
failure_node.raise "cannot include release semantics"
when .acquire_release?, .sequentially_consistent?
failure_node.raise "shall be no stronger than success ordering"
end
when .acquire_release?
case failure_ordering
when .release?, .acquire_release?
failure_node.raise "cannot include release semantics"
when .sequentially_consistent?
failure_node.raise "shall be no stronger than success ordering"
end
when .sequentially_consistent?
case failure_ordering
when .release?, .acquire_release?
failure_node.raise "cannot include release semantics"
end
end
end
{% end %}
{% if LibLLVM::IS_LT_130 %}
failure_node.raise "shall be no stronger than success ordering" if failure_ordering > success_ordering
{% end %}
ysbaddaden marked this conversation as resolved.
Show resolved Hide resolved
end

def bool_from_bool_literal(node)
unless node.is_a?(BoolLiteral)
Expand Down