From ecedcbc0bfb72d8eb2f7a34fa98a9cad052bdad8 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 7 Oct 2026 23:18:02 +0100 Subject: [PATCH] Add `@(futex=.Wait)` and `@(futex=.Wake)` with `intrinsics.Futex_Operation`, so the atomic analysis checks futex words, and use relaxed ordering for `Atomic_Cond`'s counter --- base/intrinsics/intrinsics.odin | 6 +++ core/sync/primitives.odin | 4 ++ core/sync/primitives_atomic.odin | 4 +- src/check_atomics.cpp | 80 +++++++++++++++++++------------- src/check_decl.cpp | 12 +++++ src/check_expr.cpp | 4 ++ src/checker.cpp | 26 +++++++++++ src/checker.hpp | 9 ++-- src/entity.cpp | 8 ++++ src/types.cpp | 13 ++++++ 10 files changed, 130 insertions(+), 36 deletions(-) diff --git a/base/intrinsics/intrinsics.odin b/base/intrinsics/intrinsics.odin index 1baca245c..5941671e2 100644 --- a/base/intrinsics/intrinsics.odin +++ b/base/intrinsics/intrinsics.odin @@ -112,6 +112,12 @@ Atomic_Memory_Order :: enum { Seq_Cst = 5, } +// For `@(futex=...)` on a procedure, which waits on or wakes what its first parameter points to +Futex_Operation :: enum { + Wait = 0, // Atomically compares it with what is expected, with no ordering, and sleeps while they are equal + Wake = 1, // Wakes what waits on it, without accessing it +} + atomic_type_is_lock_free :: proc($T: typeid) -> bool --- atomic_thread_fence :: proc(order: Atomic_Memory_Order) --- diff --git a/core/sync/primitives.odin b/core/sync/primitives.odin index f10653a98..90dec8659 100644 --- a/core/sync/primitives.odin +++ b/core/sync/primitives.odin @@ -554,6 +554,7 @@ If the value of the futex is `expected`, this procedure blocks the execution of the current thread, until the futex is woken up, or until a spurious wakeup occurs. */ +@(futex=.Wait) futex_wait :: proc "contextless" (f: ^Futex, expected: u32) { if u32(atomic_load_explicit(f, .Acquire)) != expected { return @@ -572,6 +573,7 @@ until a spurious wakeup occurs. This procedure returns `false` if the timeout was reached, `true` otherwise. */ +@(futex=.Wait) futex_wait_with_timeout :: proc "contextless" (f: ^Futex, expected: u32, duration: time.Duration) -> bool { if u32(atomic_load_explicit(f, .Acquire)) != expected { return true @@ -586,6 +588,7 @@ futex_wait_with_timeout :: proc "contextless" (f: ^Futex, expected: u32, duratio /* Wake up a single thread waiting on a futex. */ +@(futex=.Wake) futex_signal :: proc "contextless" (f: ^Futex) { _futex_signal(f) } @@ -593,6 +596,7 @@ futex_signal :: proc "contextless" (f: ^Futex) { /* Wake up multiple threads waiting on a futex. */ +@(futex=.Wake) futex_broadcast :: proc "contextless" (f: ^Futex) { _futex_broadcast(f) } diff --git a/core/sync/primitives_atomic.odin b/core/sync/primitives_atomic.odin index 9d5bdf7ee..b19e5e883 100644 --- a/core/sync/primitives_atomic.odin +++ b/core/sync/primitives_atomic.odin @@ -331,12 +331,12 @@ atomic_cond_wait_with_timeout :: proc "contextless" (c: ^Atomic_Cond, m: ^Atomic atomic_cond_signal :: proc "contextless" (c: ^Atomic_Cond) { - atomic_add_explicit(&c.state, 1, .Release) + atomic_add_explicit(&c.state, 1, .Relaxed) futex_signal(&c.state) } atomic_cond_broadcast :: proc "contextless" (c: ^Atomic_Cond) { - atomic_add_explicit(&c.state, 1, .Release) + atomic_add_explicit(&c.state, 1, .Relaxed) futex_broadcast(&c.state) } diff --git a/src/check_atomics.cpp b/src/check_atomics.cpp index 1941fd4d2..eebb5ad39 100644 --- a/src/check_atomics.cpp +++ b/src/check_atomics.cpp @@ -4,6 +4,8 @@ // stored with relaxed ordering, orders nothing, and is warned about, unless the address of the location is taken // elsewhere, as it may then be accessed through it. Only loads and stores are what it would pair with, as a // read-modify-write asking for an ordering which nothing pairs with, e.g. to count, is harmless. +// A call to a procedure with `@(futex=.Wait)` is a relaxed load of what its first argument points to, as the OS only +// compares it, and one with `@(futex=.Wake)` accesses nothing; neither is taking its address elsewhere. // With -vet-atomic-access, a plain read of a location accessed atomically is an error, unless a lock is taken. struct AtomicUses { @@ -478,40 +480,51 @@ gb_internal void check_atomics(Checker *c) { OdinAtomicMemoryOrder failure = OdinAtomicMemoryOrder_relaxed; bool reads = true; bool writes = true; - switch (a.id) { - case BuiltinProc_atomic_store: - explicit_order = false; - reads = false; + switch (a.futex) { + case ProcedureFutex_None: + switch (a.id) { + case BuiltinProc_atomic_store: + explicit_order = false; + reads = false; + break; + case BuiltinProc_atomic_store_explicit: + order = check_atomic_order_of(call->CallExpr.args[2]); + reads = false; + break; + case BuiltinProc_atomic_load: + explicit_order = false; + writes = false; + break; + case BuiltinProc_atomic_load_explicit: + order = check_atomic_order_of(call->CallExpr.args[1]); + writes = false; + break; + case BuiltinProc_atomic_add_explicit: + case BuiltinProc_atomic_sub_explicit: + case BuiltinProc_atomic_and_explicit: + case BuiltinProc_atomic_nand_explicit: + case BuiltinProc_atomic_or_explicit: + case BuiltinProc_atomic_xor_explicit: + case BuiltinProc_atomic_exchange_explicit: + order = check_atomic_order_of(call->CallExpr.args[2]); + break; + case BuiltinProc_atomic_compare_exchange_strong_explicit: + case BuiltinProc_atomic_compare_exchange_weak_explicit: + order = check_atomic_order_of(call->CallExpr.args[3]); + failure = check_atomic_order_of(call->CallExpr.args[4]); + break; + default: + explicit_order = false; + break; + } break; - case BuiltinProc_atomic_store_explicit: - order = check_atomic_order_of(call->CallExpr.args[2]); - reads = false; - break; - case BuiltinProc_atomic_load: + case ProcedureFutex_Wait: explicit_order = false; + order = OdinAtomicMemoryOrder_relaxed; writes = false; break; - case BuiltinProc_atomic_load_explicit: - order = check_atomic_order_of(call->CallExpr.args[1]); - writes = false; - break; - case BuiltinProc_atomic_add_explicit: - case BuiltinProc_atomic_sub_explicit: - case BuiltinProc_atomic_and_explicit: - case BuiltinProc_atomic_nand_explicit: - case BuiltinProc_atomic_or_explicit: - case BuiltinProc_atomic_xor_explicit: - case BuiltinProc_atomic_exchange_explicit: - order = check_atomic_order_of(call->CallExpr.args[2]); - break; - case BuiltinProc_atomic_compare_exchange_strong_explicit: - case BuiltinProc_atomic_compare_exchange_weak_explicit: - order = check_atomic_order_of(call->CallExpr.args[3]); - failure = check_atomic_order_of(call->CallExpr.args[4]); - break; - default: - explicit_order = false; - break; + case ProcedureFutex_Wake: + continue; } AtomicFences f = {}; @@ -593,7 +606,12 @@ gb_internal void check_atomics(Checker *c) { char const *other = token_pos_to_string(ast_token(r.other).pos); if (r.release) { warning(r.site, "'%s' is written with release ordering, but it is only loaded with relaxed ordering, so the release orders nothing", str); - error_line("\tSuggestion: Load it with .Acquire, e.g. at %s, or write it with .Relaxed\n", other); + Entity *e = entity_of_node(r.other->CallExpr.proc); + if (e != nullptr && e->kind == Entity_Procedure && e->Procedure.futex == ProcedureFutex_Wait) { + error_line("\tSuggestion: A futex wait does not acquire, so load it with .Acquire after the wait at %s, or write it with .Relaxed\n", other); + } else { + error_line("\tSuggestion: Load it with .Acquire, e.g. at %s, or write it with .Relaxed\n", other); + } } else { warning(r.site, "'%s' is read with acquire ordering, but it is only stored with relaxed ordering, so the acquire orders nothing", str); error_line("\tSuggestion: Store it with .Release, e.g. at %s, or read it with .Relaxed\n", other); diff --git a/src/check_decl.cpp b/src/check_decl.cpp index 3cb291fb9..8e3b43115 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -1551,6 +1551,18 @@ gb_internal void check_proc_decl(CheckerContext *ctx, Entity *e, DeclInfo *d) { e->Procedure.fast_math_flags = ac.fast_math_flags; + e->Procedure.futex = cast(ProcedureFutex)ac.futex; + if (ac.futex != ProcedureFutex_None) { + // what it waits on or wakes, which may only be known for each instantiation + Type *word = nullptr; + if (pt->param_count > 0) { + word = pt->params->Tuple.variables[0]->type; + } + if (word == nullptr || !is_type_pointer(word) || !(is_type_integer(type_deref(word)) || is_type_polymorphic(type_deref(word)))) { + error(e->token, "A procedure with @(futex) must take a pointer to an integer as its first parameter, which is what it waits on or wakes"); + } + } + e->deprecated_message = ac.deprecated_message; e->warning_message = ac.warning_message; ac.link_name = handle_link_name(ctx, e->token, ac.link_name, ac.link_prefix, ac.link_suffix); diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 91a4bcd0a..47c10c620 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -838,6 +838,7 @@ gb_internal bool find_or_generate_polymorphic_procedure(CheckerContext *old_c, E entity->flags = 0; entity->Procedure.optimization_mode = base_entity->Procedure.optimization_mode; + entity->Procedure.futex = base_entity->Procedure.futex; entity->Procedure.generated_from_polymorphic = true; if (base_entity->flags & EntityFlag_Cold) { @@ -9563,6 +9564,9 @@ gb_internal ExprKind check_call_expr(CheckerContext *c, Operand *operand, Ast *c if (c->curr_proc_decl != nullptr && analysis_in_use(AnalysisFlag_Escape)) { per_thread_array_add(&c->info->checked_calls_queue, CheckedCall{c->curr_proc_decl, callee}); } + if (callee->Procedure.futex != ProcedureFutex_None && call->CallExpr.args.count > 0 && analysis_in_use(AnalysisFlag_Atomic)) { + per_thread_array_add(&c->info->checked_atomics_queue, CheckedAtomic{call, c->curr_proc_decl, BuiltinProc_Invalid, callee->Procedure.futex}); + } } if (result_type == t_invalid) { diff --git a/src/checker.cpp b/src/checker.cpp index b1c82aba8..9bfe088b2 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -1390,6 +1390,17 @@ gb_internal void init_universal(void) { scope_insert(intrinsics_pkg->scope, t_atomic_memory_order->Named.type_name); } + { + GlobalEnumValue values[OdinFutexOperation_COUNT] = { + {OdinFutexOperation_strings[OdinFutexOperation_Wait], OdinFutexOperation_Wait}, + {OdinFutexOperation_strings[OdinFutexOperation_Wake], OdinFutexOperation_Wake}, + }; + + add_global_enum_type(str_lit("Futex_Operation"), values, gb_count_of(values), &t_futex_operation); + GB_ASSERT(t_futex_operation->kind == Type_Named); + scope_insert(intrinsics_pkg->scope, t_futex_operation->Named.type_name); + } + { GlobalEnumValue values[ProcCC_MAX] = { {"Invalid", ProcCC_Invalid}, @@ -4613,6 +4624,21 @@ gb_internal DECL_ATTRIBUTE_PROC(proc_decl_attribute) { } } return true; + } else if (name == "futex") { + ExactValue ev = check_decl_attribute_value(c, value, t_futex_operation); + if (value != nullptr && value->tav.mode == Addressing_Invalid) { + // already reported + return true; + } + if (value == nullptr || ev.kind != ExactValue_Integer || !are_types_identical(value->tav.type, t_futex_operation)) { + error(elem, "Expected a constant of type 'intrinsics.Futex_Operation' for '%.*s', i.e. '.Wait' or '.Wake'", LIT(name)); + return true; + } + switch (exact_value_to_i64(ev)) { + case OdinFutexOperation_Wait: ac->futex = ProcedureFutex_Wait; break; + case OdinFutexOperation_Wake: ac->futex = ProcedureFutex_Wake; break; + } + return true; } return false; } diff --git a/src/checker.hpp b/src/checker.hpp index 51992a4ad..249c4092b 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -167,6 +167,8 @@ struct AttributeContext { u64 fast_math_flags; + u8 futex; // ProcedureFutex + bool raddbg_type_view; String raddbg_type_view_string; }; @@ -283,9 +285,10 @@ struct CheckedCall { }; struct CheckedAtomic { - Ast * call; - DeclInfo *decl; - i32 id; // BuiltinProcId + Ast * call; + DeclInfo * decl; + i32 id; // BuiltinProcId, of an atomic intrinsic + u8 futex; // ProcedureFutex, or of a call to a procedure with `@(futex=...)` }; struct CheckedAddress { diff --git a/src/entity.cpp b/src/entity.cpp index 2ba419b41..a82ceab71 100644 --- a/src/entity.cpp +++ b/src/entity.cpp @@ -140,6 +140,13 @@ enum ProcedureOptimizationMode : u8 { ProcedureOptimizationMode_FavorSize, }; +// see `@(futex=...)` +enum ProcedureFutex : u8 { + ProcedureFutex_None, + ProcedureFutex_Wait, + ProcedureFutex_Wake, +}; + BlockingMutex global_type_name_objc_metadata_mutex; @@ -308,6 +315,7 @@ struct Entity { struct GenProcsData *gen_procs; BlockingMutex gen_procs_mutex; ProcedureOptimizationMode optimization_mode; + ProcedureFutex futex; u64 fast_math_flags; diff --git a/src/types.cpp b/src/types.cpp index 02720149a..9540b1545 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -813,6 +813,19 @@ char const *OdinAtomicMemoryOrder_strings[OdinAtomicMemoryOrder_COUNT] = { gb_global Type *t_atomic_memory_order = nullptr; +enum OdinFutexOperation : i32 { + OdinFutexOperation_Wait = 0, + OdinFutexOperation_Wake = 1, + OdinFutexOperation_COUNT, +}; + +char const *OdinFutexOperation_strings[OdinFutexOperation_COUNT] = { + "Wait", + "Wake", +}; + +gb_global Type *t_futex_operation = nullptr; +