From c494c7a503712c6c247ccd87b1dc0b08374fd0ba Mon Sep 17 00:00:00 2001 From: gingerBill Date: Thu, 1 Oct 2026 23:17:34 +0100 Subject: [PATCH] Check global entity groups in parallel * CAS-claimed entities, cycle-safe waits for entities and records, lazy entities without lazy_mutex * Build union type names only on error, take the error block only to report proc group overload errors --- src/check_decl.cpp | 55 +++--- src/check_expr.cpp | 25 ++- src/check_stmt.cpp | 4 +- src/check_type.cpp | 12 +- src/checker.cpp | 4 + src/checker.hpp | 4 +- src/checker_global.cpp | 176 +++++++++++------- src/entity.cpp | 1 + src/thread_pool.cpp | 41 ++++ src/threading.cpp | 4 + src/types.cpp | 44 ++--- tests/issues/run.bat | 2 + tests/issues/run.sh | 2 + tests/issues/test_issue_7566.odin | 36 ++++ .../test_issue_foreign_import_attributes.odin | 13 ++ ...test_issue_global_when_cycle_accepted.odin | 28 +++ ...est_issue_global_when_cycle_ambiguous.odin | 5 + 17 files changed, 316 insertions(+), 140 deletions(-) create mode 100644 tests/issues/test_issue_7566.odin create mode 100644 tests/issues/test_issue_foreign_import_attributes.odin create mode 100644 tests/issues/test_issue_global_when_cycle_accepted.odin create mode 100644 tests/issues/test_issue_global_when_cycle_ambiguous.odin diff --git a/src/check_decl.cpp b/src/check_decl.cpp index 4a40ba7a4..008959936 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -1953,6 +1953,7 @@ gb_internal void check_proc_group_decl(CheckerContext *ctx, Entity *pg_entity, D GB_ASSERT(p != q); bool is_invalid = false; + bool different_results = false; TokenPos pos = q->token.pos; @@ -1960,9 +1961,6 @@ gb_internal void check_proc_group_decl(CheckerContext *ctx, Entity *pg_entity, D continue; } - - ERROR_BLOCK(); - if (q->flags & EntityFlag_Disabled) { continue; } @@ -1984,21 +1982,18 @@ gb_internal void check_proc_group_decl(CheckerContext *ctx, Entity *pg_entity, D if (!both_have_where_clauses) switch (kind) { case ProcOverload_Identical: - error(p->token, "Overloaded procedure '%.*s' has the same type as another procedure in the procedure group '%.*s'", LIT(name), LIT(proc_group_name)); is_invalid = true; break; // case ProcOverload_CallingConvention: - // error(p->token, "Overloaded procedure '%.*s' has the same type as another procedure in the procedure group '%.*s'", LIT(name), LIT(proc_group_name)); // is_invalid = true; // break; case ProcOverload_ParamVariadic: - error(p->token, "Overloaded procedure '%.*s' has the same type as another procedure in the procedure group '%.*s'", LIT(name), LIT(proc_group_name)); is_invalid = true; break; case ProcOverload_ResultCount: case ProcOverload_ResultTypes: - error(p->token, "Overloaded procedure '%.*s' has the same parameters but different results in the procedure group '%.*s'", LIT(name), LIT(proc_group_name)); is_invalid = true; + different_results = true; break; case ProcOverload_Polymorphic: break; @@ -2011,6 +2006,13 @@ gb_internal void check_proc_group_decl(CheckerContext *ctx, Entity *pg_entity, D } if (is_invalid) { + // NOTE(bill): only now, as the error block is shared by every thread + ERROR_BLOCK(); + if (different_results) { + error(p->token, "Overloaded procedure '%.*s' has the same parameters but different results in the procedure group '%.*s'", LIT(name), LIT(proc_group_name)); + } else { + error(p->token, "Overloaded procedure '%.*s' has the same type as another procedure in the procedure group '%.*s'", LIT(name), LIT(proc_group_name)); + } error_line("\tprevious procedure at %s\n", token_pos_to_string(pos)); invalid[k] = true; } @@ -2177,16 +2179,25 @@ gb_internal void check_entity_decl(CheckerContext *ctx, Entity *e, DeclInfo *d, return; } defer (global_when_trial_end_entity(&trial_scope)); - bool is_lazy = (e->flags & EntityFlag_Lazy) != 0; - if (is_lazy) { - mutex_lock(&ctx->info->lazy_mutex); - if (e->state == EntityState_Resolved) { - // NOTE: another thread checked it whilst this one waited - mutex_unlock(&ctx->info->lazy_mutex); + + // NOTE(bill): checked by whichever thread claims it first; any other that needs it meanwhile waits for it + i32 owner = 0; + if (!e->checking_thread.compare_exchange_strong(owner, cast(i32)current_thread_index() + 1)) { + if (thread_wait_for_owner(&e->checking_thread, owner, owner)) { return; } - lazy_mutex_depth += 1; + // NOTE: this thread is checking it already, or the thread checking it waits for this one + error(e->token, "Illegal declaration cycle of `%.*s`", LIT(e->token.string)); + return; } + if (e->state == EntityState_Resolved) { + // NOTE: another thread finished it before this one claimed it + e->checking_thread.store(0); + futex_broadcast(&e->checking_thread); + return; + } + + bool is_lazy = (e->flags & EntityFlag_Lazy) != 0; GlobalEntityTimingFrame timing_frame = global_entity_timing_begin(e); String name = e->token.string; @@ -2290,20 +2301,20 @@ end:; global_entity_timing_end(timing_frame, e); // NOTE(bill): Add it to the list of checked entities if (is_lazy) { + mutex_lock(&ctx->info->lazy_mutex); array_add(&ctx->info->entities, e); - lazy_mutex_depth -= 1; mutex_unlock(&ctx->info->lazy_mutex); } + e->checking_thread.store(0); + futex_broadcast(&e->checking_thread); } -// A lazy entity is only ever in progress on the thread holding `lazy_mutex`, so taking it (it is -// recursive) waits out another thread and is a no-op on the thread that is checking it -gb_internal void wait_for_lazy_entity(CheckerContext *ctx, Entity *e) { - if ((e->flags & EntityFlag_Lazy) == 0 || e->state == EntityState_Resolved) { - return; +// An entity in progress on another thread is waited for, unless that thread waits for this one +gb_internal void wait_for_entity(Entity *e) { + i32 owner = e->checking_thread.load(); + if (owner != 0) { + thread_wait_for_owner(&e->checking_thread, owner, owner); } - mutex_lock(&ctx->info->lazy_mutex); - mutex_unlock(&ctx->info->lazy_mutex); } diff --git a/src/check_expr.cpp b/src/check_expr.cpp index f655b206e..50b59f0c1 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -955,7 +955,7 @@ gb_internal i64 check_distance_between_types(CheckerContext *c, Operand *operand } if (is_type_union(dst) && allow_unions) { - wait_signal_until_available(&dst->Union.variants_wait_signal); + wait_for_record_signal(&dst->Union.variants_wait_signal, &dst->Union.checking_thread); for (Type *vt : dst->Union.variants) { if (are_types_identical(vt, s)) { return 1; @@ -1713,7 +1713,7 @@ gb_internal Entity *check_ident(CheckerContext *c, Operand *o, Ast *n, Type *nam if (e->state == EntityState_Unresolved) { check_entity_decl(c, e, nullptr, named_type); } else { - wait_for_lazy_entity(c, e); + wait_for_entity(e); } switch (e->kind) { case Entity_Constant: @@ -5172,7 +5172,7 @@ gb_internal void convert_to_typed(CheckerContext *c, Operand *operand, Type *tar case Type_Union: if (!is_operand_nil(*operand) && !is_operand_uninit(*operand)) { TEMPORARY_ALLOCATOR_GUARD(); - wait_signal_until_available(&t->Union.variants_wait_signal); + wait_for_record_signal(&t->Union.variants_wait_signal, &t->Union.checking_thread); isize count = t->Union.variants.count; ValidIndexAndScore *valids = temporary_alloc_array(count); @@ -5205,9 +5205,6 @@ gb_internal void convert_to_typed(CheckerContext *c, Operand *operand, Type *tar first_success_index = valids[0].index; } - gbString type_str = type_to_string(target_type); - defer (gb_string_free(type_str)); - if (valid_count == 1) { Type *new_type = t->Union.variants[first_success_index]; if (operand->mode != Addressing_Constant || @@ -5219,6 +5216,8 @@ gb_internal void convert_to_typed(CheckerContext *c, Operand *operand, Type *tar break; } else if (valid_count > 1) { ERROR_BLOCK(); + gbString type_str = type_to_string(target_type); + defer (gb_string_free(type_str)); GB_ASSERT(first_success_index >= 0); convert_untyped_error(c, operand, target_type, true); @@ -5244,6 +5243,8 @@ gb_internal void convert_to_typed(CheckerContext *c, Operand *operand, Type *tar target_type = t_untyped_uninit; } else if (!is_type_untyped_nil(operand->type) || !type_has_nil(target_type)) { ERROR_BLOCK(); + gbString type_str = type_to_string(target_type); + defer (gb_string_free(type_str)); convert_untyped_error(c, operand, target_type, true); if (count > 0) { @@ -5746,7 +5747,7 @@ gb_internal Entity *check_entity_from_ident_or_selector(CheckerContext *c, Ast * } if (e != nullptr) { // its kind and type are read by the caller - wait_for_lazy_entity(c, e); + wait_for_entity(e); } return e; } else if (!ident_only) if (node->kind == Ast_SelectorExpr) { @@ -8834,9 +8835,7 @@ gb_internal CallArgumentError check_polymorphic_record_type(CheckerContext *c, O set_base_type(named_type, struct_type); check_open_scope(&ctx, node); - begin_filling_record(struct_type); check_struct_type(&ctx, struct_type, node, &ordered_operands, named_type, original_type, found_gen_types); - end_filling_record(struct_type); // check_struct_type released found_gen_types->mutex after publishing the instantiation. gen_types_locked = false; check_close_scope(&ctx); @@ -9745,7 +9744,7 @@ gb_internal bool attempt_implicit_selector_expr(CheckerContext *c, Operand *o, A TEMPORARY_ALLOCATOR_GUARD(); Type *union_type = base_type(th); - wait_signal_until_available(&union_type->Union.variants_wait_signal); + wait_for_record_signal(&union_type->Union.variants_wait_signal, &union_type->Union.checking_thread); auto operands = array_make(temporary_allocator(), 0, union_type->Union.variants.count); for (Type *vt : union_type->Union.variants) { @@ -11088,7 +11087,7 @@ gb_internal ExprKind check_compound_literal(CheckerContext *c, Operand *o, Ast * // If exactly one matches, retarget to that variant and let the normal path build it (the surrounding assignment then wraps it into the union). // Otherwise report a clear error. if (t->kind == Type_Union && cl->type == nullptr && cl->elems.count > 0) { - wait_signal_until_available(&t->Union.variants_wait_signal); + wait_for_record_signal(&t->Union.variants_wait_signal, &t->Union.checking_thread); auto matches = array_make(temporary_allocator(), 0, t->Union.variants.count); for (Type *variant : t->Union.variants) { Operand trial = {}; @@ -11156,7 +11155,7 @@ gb_internal ExprKind check_compound_literal(CheckerContext *c, Operand *o, Ast * break; } - wait_signal_until_available(&t->Struct.fields_wait_signal); + wait_for_record_signal(&t->Struct.fields_wait_signal, &t->Struct.checking_thread); isize field_count = t->Struct.fields.count; isize min_field_count = t->Struct.fields.count; for (isize i = min_field_count-1; i >= 0; i--) { @@ -12105,7 +12104,7 @@ gb_internal ExprKind check_type_assertion(CheckerContext *c, Operand *o, Ast *no Type *src = type_deref(o->type); Type *bsrc = base_type(src); if (bsrc->kind == Type_Union) { - wait_signal_until_available(&bsrc->Union.variants_wait_signal); + wait_for_record_signal(&bsrc->Union.variants_wait_signal, &bsrc->Union.checking_thread); } diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index dfb046da3..1a8d90a1e 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -849,7 +849,7 @@ gb_internal bool check_using_stmt_entity(CheckerContext *ctx, AstUsingStmt *us, bool is_ptr = is_type_pointer(e->type); Type *t = base_type(type_deref(e->type)); if (t->kind == Type_Struct) { - wait_signal_until_available(&t->Struct.fields_wait_signal); + wait_for_record_signal(&t->Struct.fields_wait_signal, &t->Struct.checking_thread); Scope *found = t->Struct.scope; GB_ASSERT(found != nullptr); @@ -1540,7 +1540,7 @@ gb_internal void check_type_switch_stmt(CheckerContext *ctx, Ast *node, u32 mod_ // TODO(bill): Make robust Type *bt = base_type(type_deref(x.type)); if (bt->kind == Type_Union) { - wait_signal_until_available(&bt->Union.variants_wait_signal); + wait_for_record_signal(&bt->Union.variants_wait_signal, &bt->Union.checking_thread); } Type *case_type = nullptr; diff --git a/src/check_type.cpp b/src/check_type.cpp index 247f52ea3..b7b525702 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -724,6 +724,9 @@ gb_internal void check_struct_type(CheckerContext *ctx, Type *struct_type, Ast * GB_ASSERT(is_type_struct(struct_type)); ast_node(st, StructType, node); + struct_type->Struct.checking_thread.store(cast(i32)current_thread_index() + 1); + defer (struct_type->Struct.checking_thread.store(0)); + String context = str_lit("struct"); isize min_field_count = 0; @@ -846,6 +849,9 @@ gb_internal void check_union_type(CheckerContext *ctx, Type *union_type, Ast *no GB_ASSERT(is_type_union(union_type)); ast_node(ut, UnionType, node); + union_type->Union.checking_thread.store(cast(i32)current_thread_index() + 1); + defer (union_type->Union.checking_thread.store(0)); + union_type->Union.node = node; union_type->Union.scope = ctx->scope; @@ -2254,8 +2260,8 @@ gb_internal SubstResult subst_unify(CheckerContext *c, Type *pattern, Type *sour pattern->Struct.is_packed != source->Struct.is_packed) { return Subst_Unhandled; } - wait_signal_until_available(&pattern->Struct.fields_wait_signal); - wait_signal_until_available(&source->Struct.fields_wait_signal); + wait_for_record_signal(&pattern->Struct.fields_wait_signal, &pattern->Struct.checking_thread); + wait_for_record_signal(&source->Struct.fields_wait_signal, &source->Struct.checking_thread); if (pattern->Struct.fields.count != source->Struct.fields.count) { return Subst_NoMatch; } @@ -4391,7 +4397,7 @@ gb_internal bool complete_soa_type(Checker *checker, Type *t, bool wait_to_finis GB_ASSERT(old_struct->kind == Type_Struct); if (wait_to_finish) { - wait_signal_until_available(&old_struct->Struct.fields_wait_signal); + wait_for_record_signal(&old_struct->Struct.fields_wait_signal, &old_struct->Struct.checking_thread); } else { GB_ASSERT(old_struct->Struct.fields_wait_signal.futex.load() != 0); } diff --git a/src/checker.cpp b/src/checker.cpp index 95961c8a1..ff59665ec 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -7537,8 +7537,12 @@ gb_internal void check_parsed_files(Checker *c) { array_sort(c->info.entities, init_procedures_cmp); TIME_SECTION("check all global entities"); + isize entity_count = c->info.entities.count; check_all_global_entities(c); + // NOTE(bill): lazy entities are added once checked, which with several threads is in no fixed order + gb_sort_array(c->info.entities.data + entity_count, c->info.entities.count - entity_count, init_procedures_cmp); + if (build_context.internal_global_entity_graph) { TIME_SECTION("print global entity graph"); print_global_groups(&global_groups); diff --git a/src/checker.hpp b/src/checker.hpp index 78f579c2d..52d43ad81 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -780,7 +780,7 @@ struct CheckerInfo { BlockingMutex type_and_value_mutex; - RecursiveMutex lazy_mutex; // Mutex required for lazy type checking of specific files + RecursiveMutex lazy_mutex; // for adding checked lazy entities to `entities` // BlockingMutex type_info_mutex; // NOT recursive @@ -970,7 +970,7 @@ struct GlobalEntityTimingFrame { }; gb_internal GlobalEntityTimingFrame global_entity_timing_begin(Entity *e); gb_internal void global_entity_timing_end(GlobalEntityTimingFrame const &f, Entity *e); -gb_internal void wait_for_lazy_entity(CheckerContext *c, Entity *e); +gb_internal void wait_for_entity(Entity *e); gb_internal Ast *remove_type_alias_clutter(Ast *node); gb_internal void check_const_decl(CheckerContext *c, Entity *e, Ast *type_expr, Ast *init_expr, Type *named_type); gb_internal void check_type_decl(CheckerContext *c, Entity *e, Ast *type_expr, Type *def); diff --git a/src/checker_global.cpp b/src/checker_global.cpp index fd8b51130..2ca4e0e27 100644 --- a/src/checker_global.cpp +++ b/src/checker_global.cpp @@ -749,27 +749,32 @@ gb_internal void global_graph_print_entity(Entity *e) { struct GlobalGroup { - i32 start; // into `GlobalGroupGraph::members` - i32 count; - bool done; + i32 start; // into `GlobalGroupGraph::members` + i32 count; + std::atomic done; }; struct GlobalGroupGraph { Array nodes; PtrMap node_of; - Array offsets; // node -> the nodes it names, as `targets[offsets[v]..offsets[v+1]]` + Array offsets; // node -> the nodes it names, as `targets[offsets[v].. targets; Array group_of; - Array groups; // every dependency of a group has a lower index - Array members; // nodes, by group, in source order + Array groups; // every dependency of a group has a lower index + Array members; // nodes by group in source order - bool active; - i32 current_group; - Entity *current_entity; - isize missing_edges; + Array dependent_offsets; // group -> the groups that depend on it as `dependents[dependent_offsets[gi].. dependents; + std::atomic * pending; // per group its dependencies not yet done + + Checker * checker; + bool active; + std::atomic missing_edges; }; gb_global GlobalGroupGraph global_groups; +gb_global gb_thread_local i32 global_group_current = -1; +gb_global gb_thread_local Entity * global_group_current_entity; struct GlobalPlaceholderHit { Scope * scope; @@ -1268,7 +1273,9 @@ gb_internal void build_global_groups(Checker *c, GlobalGroupGraph *g) { array_init(&g->groups, heap_allocator(), group_count); array_init(&g->members, heap_allocator(), node_count); for (i32 gi = 0; gi < group_count; gi++) { - g->groups[gi] = {}; + g->groups[gi].start = 0; + g->groups[gi].count = 0; + g->groups[gi].done.store(false); } for (i32 v = 0; v < node_count; v++) { g->groups[g->group_of[v]].count += 1; @@ -1299,7 +1306,7 @@ gb_internal void global_group_check_edge(CheckerContext *ctx, Entity *e) { } } else { i32 gi = g->group_of[*v]; - if (gi == g->current_group || g->groups[gi].done) { + if (gi == global_group_current || g->groups[gi].done.load()) { return; } } @@ -1312,9 +1319,9 @@ gb_internal void global_group_check_edge(CheckerContext *ctx, Entity *e) { gb_printf_err(" needs "); global_graph_print_entity(e); gb_printf_err(v == nullptr ? ", which is not in the graph" : ""); - if (g->current_entity != by) { + if (global_group_current_entity != by) { gb_printf_err(", while checking "); - global_graph_print_entity(g->current_entity); + global_graph_print_entity(global_group_current_entity); } gb_printf_err("\n"); } @@ -1331,14 +1338,14 @@ gb_internal void check_global_group(Checker *c, GlobalGroupGraph *g, i32 gi) { auto soa_types = array_make(heap_allocator()); global_group_soa_types = &soa_types; - g->current_group = gi; + global_group_current = gi; for (i32 k = 0; k < group->count; k++) { Entity *e = g->nodes[members[k]]; if (e->flags & EntityFlag_Lazy) { // NOTE: only checked when something uses it; the group orders it after what it names continue; } - g->current_entity = e; + global_group_current_entity = e; GlobalEntityTimingFrame timing_frame = global_entity_timing_begin(e); check_single_global_entity(c, e, e->decl_info, &untyped); if (e->type != nullptr && is_type_typed(e->type)) { @@ -1361,41 +1368,24 @@ gb_internal void check_global_group(Checker *c, GlobalGroupGraph *g, i32 gi) { add_untyped_expressions(&c->info, &untyped); map_destroy(&untyped); - group->done = true; - g->current_group = -1; - g->current_entity = nullptr; + group->done.store(true); + global_group_current = -1; + global_group_current_entity = nullptr; } -// Groups in dependency order; with `-internal-shuffle-global-entities`, a random one of the groups whose -// dependencies are done, as a parallel checker might -gb_internal void check_global_groups(Checker *c, GlobalGroupGraph *g) { +gb_internal void build_global_group_dependents(GlobalGroupGraph *g) { i32 group_count = cast(i32)g->groups.count; - u64 seed = build_context.internal_shuffle_global_entities; - if (seed == 0) { - for (i32 gi = 0; gi < group_count; gi++) { - check_global_group(c, g, gi); - } - return; - } - - auto dependents = array_make >(heap_allocator(), group_count); - auto dep_count = array_make (heap_allocator(), group_count); - auto seen = array_make (heap_allocator(), group_count); - auto ready = array_make (heap_allocator(), 0, group_count); - defer ({ - for (auto &d : dependents) { - array_free(&d); - } - array_free(&dependents); - }); - defer (array_free(&dep_count)); + auto edge_from = array_make(heap_allocator(), 0, group_count); + auto edge_to = array_make(heap_allocator(), 0, group_count); + auto seen = array_make(heap_allocator(), group_count); + defer (array_free(&edge_from)); + defer (array_free(&edge_to)); defer (array_free(&seen)); - defer (array_free(&ready)); + g->pending = gb_alloc_array(heap_allocator(), std::atomic, group_count); for (i32 gi = 0; gi < group_count; gi++) { - dep_count[gi] = 0; - dependents[gi] = {}; seen[gi] = -1; + g->pending[gi].store(0); } for (i32 gi = 0; gi < group_count; gi++) { GlobalGroup const &group = g->groups[gi]; @@ -1405,36 +1395,79 @@ gb_internal void check_global_groups(Checker *c, GlobalGroupGraph *g) { i32 dep = g->group_of[g->targets[i]]; if (dep != gi && seen[dep] != gi) { seen[dep] = gi; - dep_count[gi] += 1; - if (dependents[dep].allocator.proc == nullptr) { - array_init(&dependents[dep], heap_allocator()); - } - array_add(&dependents[dep], gi); + array_add(&edge_from, dep); + array_add(&edge_to, gi); + g->pending[gi].fetch_add(1); } } } - if (dep_count[gi] == 0) { + } + global_graph_csr(group_count, edge_from, edge_to, &g->dependent_offsets, &g->dependents); +} + +gb_internal void check_global_group_and_release(GlobalGroupGraph *g, i32 gi, Array *ready); + +gb_internal WORKER_TASK_PROC(check_global_group_worker) { + check_global_group_and_release(&global_groups, cast(i32)cast(intptr)data, nullptr); + return 0; +} + +gb_internal void check_global_group_and_release(GlobalGroupGraph *g, i32 gi, Array *ready) { + check_global_group(g->checker, g, gi); + for (i32 i = g->dependent_offsets[gi]; i < g->dependent_offsets[gi+1]; i++) { + i32 next = g->dependents[i]; + if (g->pending[next].fetch_sub(1) == 1) { + if (ready != nullptr) { + array_add(ready, next); + } else { + thread_pool_add_task(check_global_group_worker, cast(void *)cast(intptr)next); + } + } + } +} + +gb_internal void check_global_groups(Checker *c, GlobalGroupGraph *g) { + i32 group_count = cast(i32)g->groups.count; + u64 seed = build_context.internal_shuffle_global_entities; + g->checker = c; + + if (seed == 0 && (build_context.thread_count <= 1 || build_context.no_threaded_checker)) { + for (i32 gi = 0; gi < group_count; gi++) { + check_global_group(c, g, gi); + } + return; + } + + build_global_group_dependents(g); + + // NOTE: all found before any is checked, as checking one releases others + auto ready = array_make(heap_allocator(), 0, group_count); + defer (array_free(&ready)); + for (i32 gi = 0; gi < group_count; gi++) { + if (g->pending[gi].load() == 0) { array_add(&ready, gi); } } - u64 state = seed; - isize checked = 0; - while (ready.count > 0) { - isize i = cast(isize)(global_group_random(&state) % cast(u64)ready.count); - i32 gi = ready[i]; - ready[i] = ready[ready.count-1]; - array_pop(&ready); - - check_global_group(c, g, gi); - checked += 1; - for (i32 next : dependents[gi]) { - if (--dep_count[next] == 0) { - array_add(&ready, next); - } + if (seed == 0) { + for (i32 gi : ready) { + thread_pool_add_task(check_global_group_worker, cast(void *)cast(intptr)gi); + } + thread_pool_wait(); + } else { + u64 state = seed; + while (ready.count > 0) { + isize i = cast(isize)(global_group_random(&state) % cast(u64)ready.count); + i32 gi = ready[i]; + ready[i] = ready[ready.count-1]; + array_pop(&ready); + check_global_group_and_release(g, gi, &ready); } } - GB_ASSERT(checked == group_count); + + for (i32 gi = 0; gi < group_count; gi++) { + GB_ASSERT(g->groups[gi].done.load()); + } } gb_internal void destroy_global_groups(GlobalGroupGraph *g) { @@ -1445,6 +1478,12 @@ gb_internal void destroy_global_groups(GlobalGroupGraph *g) { array_free(&g->group_of); array_free(&g->groups); array_free(&g->members); + array_free(&g->dependent_offsets); + array_free(&g->dependents); + if (g->pending != nullptr) { + gb_free(heap_allocator(), g->pending); + g->pending = nullptr; + } } gb_internal void check_all_global_entities(Checker *c) { @@ -1470,12 +1509,11 @@ gb_internal void check_all_global_entities(Checker *c) { TIME_SECTION("check all global entities - check groups"); g->active = true; - g->current_group = -1; check_global_groups(c, g); g->active = false; - if (build_context.internal_check_global_edges && g->missing_edges > 0) { - gb_printf_err("%td missing global dependencies\n", g->missing_edges); + if (build_context.internal_check_global_edges && g->missing_edges.load() > 0) { + gb_printf_err("%td missing global dependencies\n", g->missing_edges.load()); gb_exit(1); } @@ -2217,7 +2255,7 @@ gb_internal void print_global_groups(GlobalGroupGraph *g) { f64 critical_ms = critical >= 0 ? global_graph_ms(path[critical], freq) : 0; gb_printf_err("Global entity groups\n"); - gb_printf_err(" entities: %td (%td not timed), dependency edges: %td, missing edges: %td\n", g->nodes.count, untimed, g->targets.count, g->missing_edges); + gb_printf_err(" entities: %td (%td not timed), dependency edges: %td, missing edges: %td\n", g->nodes.count, untimed, g->targets.count, g->missing_edges.load()); gb_printf_err(" self time: %.3f ms in the groups, %.3f ms during 'when' resolution\n", total_ms, global_graph_ms(when_ticks, freq)); gb_printf_err(" groups: %d (%td with a cycle), largest has %d entities\n", group_count, cyclic, largest >= 0 ? g->groups[largest].count : 0); gb_printf_err(" critical path: %.3f ms over %d groups -> at most %.2fx speedup\n", diff --git a/src/entity.cpp b/src/entity.cpp index 845e2b910..4a7800450 100644 --- a/src/entity.cpp +++ b/src/entity.cpp @@ -210,6 +210,7 @@ struct Entity { u64 id; std::atomic flags; std::atomic state; + Futex checking_thread; // 1 + the index of the thread in `check_entity_decl` for it, else 0 std::atomic min_dep_count; Token token; Scope * scope; diff --git a/src/thread_pool.cpp b/src/thread_pool.cpp index 0ffce8046..b62811a5a 100644 --- a/src/thread_pool.cpp +++ b/src/thread_pool.cpp @@ -174,6 +174,47 @@ gb_internal bool thread_pool_add_task(ThreadPool *pool, WorkerTaskProc *proc, vo return true; } +gb_internal bool thread_wait_for_owner(Futex *futex, Footex value, i32 owner) { + if (futex->load() != value) { + return true; + } + Thread *self = current_thread; + if (self != nullptr && owner > 0) { + i32 me = cast(i32)self->idx + 1; + if (owner == me) { + return false; + } + self->waiting_futex.store(futex); + self->waiting_value.store(value); + self->waiting_for.store(owner); + + // NOTE(bill): a thread counts as waiting only while what it waits on is unchanged + // when it clears its own `waiting_for` only once it has woken + Slice threads = self->pool->threads; + i32 t = owner; + for (isize i = 0; i <= threads.count; i++) { + if (t == me) { + self->waiting_for.store(0); + return false; + } + Thread *other = &threads[t-1]; + i32 next = other->waiting_for.load(); + Futex *f = other->waiting_futex.load(); + if (next == 0 || f == nullptr || f->load() != other->waiting_value.load()) { + break; + } + t = next; + } + } + while (futex->load() == value) { + futex_wait(futex, value); + } + if (self != nullptr) { + self->waiting_for.store(0); + } + return true; +} + gb_internal void thread_pool_wait(ThreadPool *pool) { WorkerTask task; diff --git a/src/threading.cpp b/src/threading.cpp index 524c70e29..28882ea24 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -73,6 +73,10 @@ struct Thread { struct Arena *permanent_arena; struct Arena *temporary_arena; + + std::atomic *> waiting_futex; + std::atomic waiting_value; + std::atomic waiting_for; }; typedef std::atomic Futex; diff --git a/src/types.cpp b/src/types.cpp index aad687ac7..129a5afd1 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -155,6 +155,7 @@ struct TypeStruct { i32 soa_count; StructSoaKind soa_kind; Wait_Signal fields_wait_signal; + Futex checking_thread; // 1 + the index of the thread in `check_struct_type` for it, else 0 BlockingMutex soa_mutex; BlockingMutex offset_mutex; // for settings offsets @@ -181,6 +182,7 @@ struct TypeUnion { Type * polymorphic_parent; Wait_Signal polymorphic_wait_signal; Wait_Signal variants_wait_signal; // signalled once `variants` is populated (mirrors TypeStruct.fields_wait_signal) + Futex checking_thread; // 1 + the index of the thread in `check_union_type` for it, else 0 std::atomic tag_size; bool is_polymorphic; @@ -413,6 +415,7 @@ gb_internal bool is_type_simple_compare(Type *t); gb_internal Type *type_deref(Type *t, bool allow_multi_pointer=false); gb_internal Type *base_type(Type *t); gb_internal Type *alloc_type_multi_pointer(Type *elem); +gb_internal void wait_for_record_signal(Wait_Signal *signal, Futex *checking_thread); gb_internal u32 type_info_flags_of_type(Type *type) { if (type == nullptr) { @@ -2477,13 +2480,13 @@ gb_internal TypeTuple *get_record_polymorphic_params(Type *t) { t = base_type(t); switch (t->kind) { case Type_Struct: - wait_signal_until_available(&t->Struct.polymorphic_wait_signal); + wait_for_record_signal(&t->Struct.polymorphic_wait_signal, &t->Struct.checking_thread); if (t->Struct.polymorphic_params) { return &t->Struct.polymorphic_params->Tuple; } break; case Type_Union: - wait_signal_until_available(&t->Union.polymorphic_wait_signal); + wait_for_record_signal(&t->Union.polymorphic_wait_signal, &t->Union.checking_thread); if (t->Union.polymorphic_params) { return &t->Union.polymorphic_params->Tuple; } @@ -3560,7 +3563,7 @@ gb_internal bool union_variant_index_types_equal(Type *v, Type *vt) { gb_internal i64 union_variant_index_checked(Type *u, Type *v) { u = base_type(u); GB_ASSERT(u->kind == Type_Union); - wait_signal_until_available(&u->Union.variants_wait_signal); + wait_for_record_signal(&u->Union.variants_wait_signal, &u->Union.checking_thread); for_array(i, u->Union.variants) { Type *vt = u->Union.variants[i]; @@ -3579,7 +3582,7 @@ gb_internal i64 union_variant_index_checked(Type *u, Type *v) { gb_internal bool union_is_variant_of(Type *u, Type *v) { u = base_type(u); GB_ASSERT(u->kind == Type_Union); - wait_signal_until_available(&u->Union.variants_wait_signal); + wait_for_record_signal(&u->Union.variants_wait_signal, &u->Union.checking_thread); for_array(i, u->Union.variants) { Type *vt = u->Union.variants[i]; @@ -3770,7 +3773,7 @@ gb_internal Selection lookup_field_from_index(Type *type, i64 index) { isize max_count = 0; switch (type->kind) { case Type_Struct: - wait_signal_until_available(&type->Struct.fields_wait_signal); + wait_for_record_signal(&type->Struct.fields_wait_signal, &type->Struct.checking_thread); max_count = type->Struct.fields.count; break; case Type_Tuple: max_count = type->Tuple.variables.count; break; @@ -3782,7 +3785,7 @@ gb_internal Selection lookup_field_from_index(Type *type, i64 index) { switch (type->kind) { case Type_Struct: { - wait_signal_until_available(&type->Struct.fields_wait_signal); + wait_for_record_signal(&type->Struct.fields_wait_signal, &type->Struct.checking_thread); for (isize i = 0; i < max_count; i++) { Entity *f = type->Struct.fields[i]; if (f->kind == Entity_Variable) { @@ -3951,7 +3954,7 @@ gb_internal Selection lookup_field_with_selection(Type *type_, InternedString fi // NOTE(bill): A polymorphic struct has no fields, this only hits in the case of an error return sel; } - wait_signal_until_available(&type->Struct.fields_wait_signal); + wait_for_record_signal(&type->Struct.fields_wait_signal, &type->Struct.checking_thread); isize field_count = type->Struct.fields.count; if (field_count != 0) for_array(i, type->Struct.fields) { Entity *f = type->Struct.fields[i]; @@ -4421,33 +4424,16 @@ gb_internal i64 type_target_max_align(void) { return max_align; } -// Polymorphic record instances being filled by this thread, which another may already have found in the cache -gb_internal gb_thread_local Array records_being_filled; - -gb_internal void begin_filling_record(Type *t) { - if (records_being_filled.allocator.proc == nullptr) { - array_init(&records_being_filled, heap_allocator()); +gb_internal void wait_for_record_signal(Wait_Signal *signal, Futex *checking_thread) { + if (signal->futex.load() == 0) { + thread_wait_for_owner(&signal->futex, 0, checking_thread->load()); } - array_add(&records_being_filled, t); } -gb_internal void end_filling_record(Type *t) { - GB_ASSERT(records_being_filled.count > 0 && records_being_filled[records_being_filled.count-1] == t); - array_pop(&records_being_filled); -} - -gb_internal gb_thread_local i32 lazy_mutex_depth; - gb_internal void wait_for_struct_fields(Type *t) { - if (t->Struct.polymorphic_parent == nullptr || t->Struct.fields_wait_signal.futex.load() != 0 || lazy_mutex_depth > 0) { - return; + if (t->Struct.polymorphic_parent != nullptr) { + wait_for_record_signal(&t->Struct.fields_wait_signal, &t->Struct.checking_thread); } - for (Type *r : records_being_filled) { - if (r == t) { - return; - } - } - wait_signal_until_available(&t->Struct.fields_wait_signal); } gb_internal i64 type_align_of_internal(Type *t, TypePath *path) { diff --git a/tests/issues/run.bat b/tests/issues/run.bat index b5b90339c..28fee09f4 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -61,6 +61,7 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin check ..\test_issue_7336.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin check ..\test_issue_ellipsis_type_call.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "10" || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration.odin -no-entry-point %COMMON% || exit /b +..\..\..\odin check ..\test_issue_foreign_import_attributes.odin -no-entry-point %COMMON% || exit /b ..\..\..\odin check ..\test_issue_foreign_redeclaration_mismatch.odin -no-entry-point %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b ..\..\..\odin doc ..\test_issue_asm_doc_category.odin -file 2>&1 | find /c "asm templates" | findstr /x "1" || exit /b ..\..\..\odin build ..\test_issue_7037.odin %COMMON% -o:none || exit /b @@ -77,6 +78,7 @@ clang -c ..\test_issue_sysv_abi.c -o test_issue_sysv_abi_c.o || exit /b ..\..\..\odin run ..\test_issue_7482.odin %COMMON% || exit /b ..\..\..\odin run ..\test_issue_7562.odin %COMMON% -no-crt -no-thread-local || exit /b ..\..\..\odin run ..\test_issue_7562.odin %COMMON% -no-crt -no-thread-local -o:speed || exit /b +..\..\..\odin test ..\test_issue_7566.odin %COMMON% || exit /b ..\..\..\odin test ..\test_issue_7587.odin %COMMON% || exit /b ..\..\..\odin run ..\test_issue_7596.odin %COMMON% || exit /b diff --git a/tests/issues/run.sh b/tests/issues/run.sh index 8bc063be0..209d2923d 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -101,6 +101,7 @@ $ODIN check ../test_issue_7012.odin -no-entry-point $COMMON_CHECK $ODIN build ../test_issue_7037.odin $COMMON -o:none $ODIN run ../test_issue_7482.odin $COMMON $ODIN run ../test_issue_7564.odin $COMMON +$ODIN test ../test_issue_7566.odin $COMMON $ODIN test ../test_issue_7587.odin $COMMON $ODIN run ../test_issue_7596.odin $COMMON $ODIN test ../test_issue_7421.odin $COMMON @@ -151,6 +152,7 @@ else fi $ODIN check ../test_issue_foreign_redeclaration.odin -no-entry-point $COMMON_CHECK +$ODIN check ../test_issue_foreign_import_attributes.odin -no-entry-point $COMMON_CHECK if [[ $($ODIN check ../test_issue_foreign_redeclaration_mismatch.odin -no-entry-point $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 1 ]]; then echo "SUCCESSFUL 1/1" else diff --git a/tests/issues/test_issue_7566.odin b/tests/issues/test_issue_7566.odin new file mode 100644 index 000000000..3bbef41e7 --- /dev/null +++ b/tests/issues/test_issue_7566.odin @@ -0,0 +1,36 @@ +// Tests issue #7566 https://github.com/odin-lang/Odin/issues/7566 +// Polymorphic instances whose constant parameters are spelt the same but have different values +package test_issues + +import "core:testing" + +Dim :: enum{ C, F } +Dims :: [Dim]int +Arr2 :: [2][2]int + +get_dims :: proc($dims: Dims) -> Dims { return dims } +get_arr2 :: proc($a: Arr2) -> Arr2 { return a } + +make_and_check :: proc($A, $I: int) -> Dims { + dims :: Dims{.C = I, .F = A} + return get_dims(dims) +} + +make_inline :: proc($A, $I: int) -> Dims { + return get_dims(Dims{.C = I, .F = A}) +} + +make_nested :: proc($A: int) -> Arr2 { + a :: Arr2{{A, 1}, {2, A}} + return get_arr2(a) +} + +@(test) +test_issue_7566 :: proc(t: ^testing.T) { + testing.expect_value(t, make_and_check(2, 2), Dims{.C = 2, .F = 2}) + testing.expect_value(t, make_and_check(3, 3), Dims{.C = 3, .F = 3}) + testing.expect_value(t, make_inline(4, 5), Dims{.C = 5, .F = 4}) + testing.expect_value(t, make_inline(6, 7), Dims{.C = 7, .F = 6}) + testing.expect_value(t, make_nested(8), Arr2{{8, 1}, {2, 8}}) + testing.expect_value(t, make_nested(9), Arr2{{9, 1}, {2, 9}}) +} diff --git a/tests/issues/test_issue_foreign_import_attributes.odin b/tests/issues/test_issue_foreign_import_attributes.odin new file mode 100644 index 000000000..d7dd1cb80 --- /dev/null +++ b/tests/issues/test_issue_foreign_import_attributes.odin @@ -0,0 +1,13 @@ +// 'foreign import' attributes may name constants declared after them +package test_issues + +@(priority_index=PRIORITY, extra_linker_flags=FLAGS) +foreign import lib "system:foo" + +foreign lib { + foo :: proc "c" () --- +} + +PRIORITY :: LATER + 1 +FLAGS :: "-L." +LATER :: 1 diff --git a/tests/issues/test_issue_global_when_cycle_accepted.odin b/tests/issues/test_issue_global_when_cycle_accepted.odin new file mode 100644 index 000000000..5d4b1e2a1 --- /dev/null +++ b/tests/issues/test_issue_global_when_cycle_accepted.odin @@ -0,0 +1,28 @@ +// Cycles of global 'when's with exactly one consistent choice of branches +package test_issues + +import "core:testing" + +// only the first branch of the second 'when' is consistent, so `int` stays the builtin +when size_of(int) == 8 { Y :: 1 } +when Y == 1 { Z :: 1 } else { int :: i32 } + +// through an unconditional declaration, whose value is checked for each choice +SIZE :: size_of(uint) +when SIZE == 8 { V :: 1 } +when V == 1 { W :: 2 } else { uint :: u32 } + +// nested and 'else when' branches +when size_of(rawptr) == 8 { + when true { N :: 1 } +} +when N != 1 { rawptr :: u32 } else when N == 1 { M :: 3 } else { rawptr :: u16 } + +@(test) +test_global_when_cycle_accepted :: proc(t: ^testing.T) { + testing.expect_value(t, Z, 1) + testing.expect_value(t, size_of(int), 8) + testing.expect_value(t, SIZE, 8) + testing.expect_value(t, W, 2) + testing.expect_value(t, M, 3) +} diff --git a/tests/issues/test_issue_global_when_cycle_ambiguous.odin b/tests/issues/test_issue_global_when_cycle_ambiguous.odin new file mode 100644 index 000000000..90ebb69db --- /dev/null +++ b/tests/issues/test_issue_global_when_cycle_ambiguous.odin @@ -0,0 +1,5 @@ +// A cycle of global 'when's with two consistent choices of branches, which is an error +package test_issues + +when size_of(int) == 8 { uint :: u32 } +when size_of(uint) == 8 { int :: i32 }