diff --git a/core/sync/primitives_atomic.odin b/core/sync/primitives_atomic.odin index 8cec36602..dd432745f 100644 --- a/core/sync/primitives_atomic.odin +++ b/core/sync/primitives_atomic.odin @@ -77,7 +77,7 @@ atomic_mutex_unlock :: proc(m: ^Atomic_Mutex) { // atomic_mutex_try_lock tries to lock m, will return true on success, and false on failure atomic_mutex_try_lock :: proc(m: ^Atomic_Mutex) -> bool { - _, ok := atomic_compare_exchange_strong_explicit(&m.state, .Unlocked, .Locked, .acquire, .acquire) + _, ok := atomic_compare_exchange_strong_explicit(&m.state, .Unlocked, .Locked, .acquire, .consume) return ok } diff --git a/src/check_builtin.cpp b/src/check_builtin.cpp index 3fe0f1048..a82577b31 100644 --- a/src/check_builtin.cpp +++ b/src/check_builtin.cpp @@ -379,7 +379,7 @@ bool check_builtin_objc_procedure(CheckerContext *c, Operand *operand, Ast *call } } -bool check_atomic_memory_order_argument(CheckerContext *c, Ast *expr, String const &builtin_name, char const *extra_message = nullptr) { +bool check_atomic_memory_order_argument(CheckerContext *c, Ast *expr, String const &builtin_name, OdinAtomicMemoryOrder *memory_order_, char const *extra_message = nullptr) { Operand x = {}; check_expr_with_type_hint(c, &x, expr, t_atomic_memory_order); if (x.mode == Addressing_Invalid) { @@ -400,6 +400,9 @@ bool check_atomic_memory_order_argument(CheckerContext *c, Ast *expr, String con error(x.expr, "Illegal Atomic_Memory_Order value, got %lld", cast(long long)value); return false; } + if (memory_order_) { + *memory_order_ = cast(OdinAtomicMemoryOrder)value; + } return true; @@ -3232,7 +3235,7 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32 case BuiltinProc_atomic_thread_fence: case BuiltinProc_atomic_signal_fence: - if (!check_atomic_memory_order_argument(c, ce->args[0], builtin_name)) { + if (!check_atomic_memory_order_argument(c, ce->args[0], builtin_name, nullptr)) { return false; } operand->mode = Addressing_NoValue; @@ -3269,9 +3272,17 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32 check_expr_with_type_hint(c, &x, ce->args[1], elem); check_assignment(c, &x, elem, builtin_name); - if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name)) { + OdinAtomicMemoryOrder memory_order = {}; + if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name, &memory_order)) { return false; } + switch (memory_order) { + case OdinAtomicMemoryOrder_consume: + case OdinAtomicMemoryOrder_acquire: + case OdinAtomicMemoryOrder_acq_rel: + error(ce->args[2], "Illegal memory order .%s for %.*s", OdinAtomicMemoryOrder_strings[memory_order], LIT(builtin_name)); + break; + } operand->type = nullptr; operand->mode = Addressing_NoValue; @@ -3303,10 +3314,18 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32 return false; } - if (!check_atomic_memory_order_argument(c, ce->args[1], builtin_name)) { + OdinAtomicMemoryOrder memory_order = {}; + if (!check_atomic_memory_order_argument(c, ce->args[1], builtin_name, &memory_order)) { return false; } + switch (memory_order) { + case OdinAtomicMemoryOrder_release: + case OdinAtomicMemoryOrder_acq_rel: + error(ce->args[1], "Illegal memory order .%s for %.*s", OdinAtomicMemoryOrder_strings[memory_order], LIT(builtin_name)); + break; + } + operand->type = elem; operand->mode = Addressing_Value; break; @@ -3352,7 +3371,7 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32 check_assignment(c, &x, elem, builtin_name); - if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name)) { + if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name, nullptr)) { return false; } @@ -3396,13 +3415,72 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32 check_assignment(c, &x, elem, builtin_name); check_assignment(c, &y, elem, builtin_name); - if (!check_atomic_memory_order_argument(c, ce->args[3], builtin_name, "success ordering")) { + OdinAtomicMemoryOrder success_memory_order = {}; + OdinAtomicMemoryOrder failure_memory_order = {}; + if (!check_atomic_memory_order_argument(c, ce->args[3], builtin_name, &success_memory_order, "success ordering")) { return false; } - if (!check_atomic_memory_order_argument(c, ce->args[4], builtin_name, "failure ordering")) { + if (!check_atomic_memory_order_argument(c, ce->args[4], builtin_name, &failure_memory_order, "failure ordering")) { return false; } + bool invalid_combination = false; + + switch (success_memory_order) { + case OdinAtomicMemoryOrder_relaxed: + case OdinAtomicMemoryOrder_release: + if (failure_memory_order != OdinAtomicMemoryOrder_relaxed) { + invalid_combination = true; + } + break; + case OdinAtomicMemoryOrder_consume: + switch (failure_memory_order) { + case OdinAtomicMemoryOrder_relaxed: + case OdinAtomicMemoryOrder_consume: + break; + default: + invalid_combination = true; + break; + } + break; + case OdinAtomicMemoryOrder_acquire: + case OdinAtomicMemoryOrder_acq_rel: + switch (failure_memory_order) { + case OdinAtomicMemoryOrder_relaxed: + case OdinAtomicMemoryOrder_consume: + case OdinAtomicMemoryOrder_acquire: + break; + default: + invalid_combination = true; + break; + } + break; + case OdinAtomicMemoryOrder_seq_cst: + switch (failure_memory_order) { + case OdinAtomicMemoryOrder_relaxed: + case OdinAtomicMemoryOrder_consume: + case OdinAtomicMemoryOrder_acquire: + case OdinAtomicMemoryOrder_seq_cst: + break; + default: + invalid_combination = true; + break; + } + break; + default: + invalid_combination = true; + break; + } + + + if (invalid_combination) { + error(ce->args[3], "Illegal memory order pairing for %.*s, success = .%s, failure = .%s", + LIT(builtin_name), + OdinAtomicMemoryOrder_strings[success_memory_order], + OdinAtomicMemoryOrder_strings[failure_memory_order] + ); + } + operand->mode = Addressing_OptionalOk; operand->type = elem; break; diff --git a/src/types.cpp b/src/types.cpp index 25e29820c..16d07d392 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -693,8 +693,8 @@ gb_global Type *t_objc_SEL = nullptr; gb_global Type *t_objc_Class = nullptr; enum OdinAtomicMemoryOrder : i32 { - OdinAtomicMemoryOrder_relaxed = 0, - OdinAtomicMemoryOrder_consume = 1, + OdinAtomicMemoryOrder_relaxed = 0, // unordered + OdinAtomicMemoryOrder_consume = 1, // monotonic OdinAtomicMemoryOrder_acquire = 2, OdinAtomicMemoryOrder_release = 3, OdinAtomicMemoryOrder_acq_rel = 4, @@ -702,6 +702,15 @@ enum OdinAtomicMemoryOrder : i32 { OdinAtomicMemoryOrder_COUNT, }; +char const *OdinAtomicMemoryOrder_strings[OdinAtomicMemoryOrder_COUNT] = { + "relaxed", + "consume", + "acquire", + "release", + "acq_rel", + "seq_cst", +}; + gb_global Type *t_atomic_memory_order = nullptr;