From 9f7154a03941f1ffad629e8e558040911e9886ba Mon Sep 17 00:00:00 2001 From: gingerBill Date: Sat, 10 Jul 2021 15:14:25 +0100 Subject: [PATCH] Prepare for multithreading the semantic checker by giving mutexes to variables of contention NOTE(bill): I know this is dodgy, but I want to make sure it is correct logic before improve those data structures --- src/check_decl.cpp | 3 + src/check_expr.cpp | 39 ++++++++----- src/check_type.cpp | 13 +++-- src/checker.cpp | 131 ++++++++++++++++++++++++++++++++----------- src/checker.hpp | 12 +++- src/llvm_backend.cpp | 2 +- 6 files changed, 144 insertions(+), 56 deletions(-) diff --git a/src/check_decl.cpp b/src/check_decl.cpp index a2c40c9a2..84817e9d8 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -786,6 +786,7 @@ void check_proc_decl(CheckerContext *ctx, Entity *e, DeclInfo *d) { GB_ASSERT(pl->body->kind == Ast_BlockStmt); if (!pt->is_polymorphic) { + // check_procedure_now(ctx->checker, ctx->file, e->token, d, proc_type, pl->body, pl->tags); check_procedure_later(ctx->checker, ctx->file, e->token, d, proc_type, pl->body, pl->tags); } } else if (!is_foreign) { @@ -808,7 +809,9 @@ void check_proc_decl(CheckerContext *ctx, Entity *e, DeclInfo *d) { if (ac.deferred_procedure.entity != nullptr) { e->Procedure.deferred_procedure = ac.deferred_procedure; + gb_mutex_lock(&ctx->checker->procs_with_deferred_to_check_mutex); array_add(&ctx->checker->procs_with_deferred_to_check, e); + gb_mutex_unlock(&ctx->checker->procs_with_deferred_to_check_mutex); } if (is_foreign) { diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 4d2cd56d4..9399f94dc 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -48,8 +48,8 @@ struct CallArgumentData { }; struct PolyProcData { - Entity * gen_entity; - ProcInfo proc_info; + Entity * gen_entity; + ProcInfo *proc_info; }; struct ValidIndexAndScore { @@ -279,7 +279,7 @@ bool find_or_generate_polymorphic_procedure(CheckerContext *c, Entity *base_enti }); - + CheckerInfo *info = c->info; CheckerContext nctx = *c; Scope *scope = create_scope(base_entity->scope); @@ -294,7 +294,6 @@ bool find_or_generate_polymorphic_procedure(CheckerContext *c, Entity *base_enti } - auto *pt = &src->Proc; // NOTE(bill): This is slightly memory leaking if the type already exists @@ -306,8 +305,13 @@ bool find_or_generate_polymorphic_procedure(CheckerContext *c, Entity *base_enti return false; } - auto *found_gen_procs = map_get(&nctx.info->gen_procs, hash_pointer(base_entity->identifier)); + gb_mutex_lock(&info->gen_procs_mutex); + auto *found_gen_procs = map_get(&info->gen_procs, hash_pointer(base_entity->identifier)); + gb_mutex_unlock(&info->gen_procs_mutex); if (found_gen_procs) { + gb_mutex_lock(&info->gen_procs_mutex); + defer (gb_mutex_unlock(&info->gen_procs_mutex)); + auto procs = *found_gen_procs; for_array(i, procs) { Entity *other = procs[i]; @@ -344,6 +348,9 @@ bool find_or_generate_polymorphic_procedure(CheckerContext *c, Entity *base_enti } if (found_gen_procs) { + gb_mutex_lock(&info->gen_procs_mutex); + defer (gb_mutex_unlock(&info->gen_procs_mutex)); + auto procs = *found_gen_procs; for_array(i, procs) { Entity *other = procs[i]; @@ -403,23 +410,25 @@ bool find_or_generate_polymorphic_procedure(CheckerContext *c, Entity *base_enti } } - ProcInfo proc_info = {}; - proc_info.file = file; - proc_info.token = token; - proc_info.decl = d; - proc_info.type = final_proc_type; - proc_info.body = pl->body; - proc_info.tags = tags; - proc_info.generated_from_polymorphic = true; - proc_info.poly_def_node = poly_def_node; + ProcInfo *proc_info = gb_alloc_item(permanent_allocator(), ProcInfo); + proc_info->file = file; + proc_info->token = token; + proc_info->decl = d; + proc_info->type = final_proc_type; + proc_info->body = pl->body; + proc_info->tags = tags; + proc_info->generated_from_polymorphic = true; + proc_info->poly_def_node = poly_def_node; + gb_mutex_lock(&info->gen_procs_mutex); if (found_gen_procs) { array_add(found_gen_procs, entity); } else { auto array = array_make(heap_allocator()); array_add(&array, entity); - map_set(&nctx.checker->info.gen_procs, hash_pointer(base_entity->identifier), array); + map_set(&info->gen_procs, hash_pointer(base_entity->identifier), array); } + gb_mutex_unlock(&info->gen_procs_mutex); GB_ASSERT(entity != nullptr); diff --git a/src/check_type.cpp b/src/check_type.cpp index 578538106..e7832272a 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -236,7 +236,10 @@ bool check_custom_align(CheckerContext *ctx, Ast *node, i64 *align_) { Entity *find_polymorphic_record_entity(CheckerContext *ctx, Type *original_type, isize param_count, Array const &ordered_operands, bool *failure) { - auto *found_gen_types = map_get(&ctx->checker->info.gen_types, hash_pointer(original_type)); + gb_mutex_lock(&ctx->info->gen_types_mutex); + defer (gb_mutex_unlock(&ctx->info->gen_types_mutex)); + + auto *found_gen_types = map_get(&ctx->info->gen_types, hash_pointer(original_type)); if (found_gen_types != nullptr) { for_array(i, *found_gen_types) { Entity *e = (*found_gen_types)[i]; @@ -315,14 +318,16 @@ void add_polymorphic_record_entity(CheckerContext *ctx, Ast *node, Type *named_t named_type->Named.type_name = e; - auto *found_gen_types = map_get(&ctx->checker->info.gen_types, hash_pointer(original_type)); + gb_mutex_lock(&ctx->info->gen_types_mutex); + auto *found_gen_types = map_get(&ctx->info->gen_types, hash_pointer(original_type)); if (found_gen_types) { array_add(found_gen_types, e); } else { auto array = array_make(heap_allocator()); array_add(&array, e); - map_set(&ctx->checker->info.gen_types, hash_pointer(original_type), array); + map_set(&ctx->info->gen_types, hash_pointer(original_type), array); } + gb_mutex_unlock(&ctx->info->gen_types_mutex); } Type *check_record_polymorphic_params(CheckerContext *ctx, Ast *polymorphic_params, @@ -2796,7 +2801,7 @@ Type *check_type_expr(CheckerContext *ctx, Ast *e, Type *named_type) { #endif if (is_type_typed(type)) { - add_type_and_value(&ctx->checker->info, e, Addressing_Type, type, empty_exact_value); + add_type_and_value(ctx->info, e, Addressing_Type, type, empty_exact_value); } else { gbString name = type_to_string(type); error(e, "Invalid type definition of %s", name); diff --git a/src/checker.cpp b/src/checker.cpp index 7ca56cccc..7a00eb495 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -4,6 +4,8 @@ void check_expr(CheckerContext *c, Operand *operand, Ast *expression); void check_expr_or_type(CheckerContext *c, Operand *operand, Ast *expression, Type *type_hint=nullptr); void add_comparison_procedures_for_fields(CheckerContext *c, Type *t); +void check_proc_info(Checker *c, ProcInfo *pi); + bool is_operand_value(Operand o) { switch (o.mode) { @@ -850,6 +852,11 @@ void init_checker_info(CheckerInfo *i) { array_init(&i->identifier_uses, a); } + gb_mutex_init(&i->untyped_mutex); + gb_mutex_init(&i->gen_procs_mutex); + gb_mutex_init(&i->gen_types_mutex); + gb_mutex_init(&i->type_info_mutex); + } void destroy_checker_info(CheckerInfo *i) { @@ -867,6 +874,11 @@ void destroy_checker_info(CheckerInfo *i) { array_free(&i->identifier_uses); array_free(&i->required_foreign_imports_through_force); array_free(&i->required_global_variables); + + gb_mutex_destroy(&i->untyped_mutex); + gb_mutex_destroy(&i->gen_procs_mutex); + gb_mutex_destroy(&i->gen_types_mutex); + gb_mutex_destroy(&i->type_info_mutex); } CheckerContext make_checker_context(Checker *c) { @@ -942,6 +954,9 @@ bool init_checker(Checker *c, Parser *parser) { isize arena_size = 2 * item_size * total_token_count; c->builtin_ctx = make_checker_context(c); + + gb_mutex_init(&c->procs_to_check_mutex); + gb_mutex_init(&c->procs_with_deferred_to_check_mutex); return true; } @@ -952,6 +967,9 @@ void destroy_checker(Checker *c) { array_free(&c->procs_with_deferred_to_check); destroy_checker_context(&c->builtin_ctx); + + gb_mutex_destroy(&c->procs_to_check_mutex); + gb_mutex_destroy(&c->procs_with_deferred_to_check_mutex); } @@ -1028,14 +1046,19 @@ Scope *scope_of_node(Ast *node) { return node->scope; } ExprInfo *check_get_expr_info(CheckerInfo *i, Ast *expr) { + gb_mutex_lock(&i->untyped_mutex); + ExprInfo *res = nullptr; ExprInfo **found = map_get(&i->untyped, hash_node(expr)); if (found) { - return *found; + res = *found; } - return nullptr; + gb_mutex_unlock(&i->untyped_mutex); + return res; } void check_remove_expr_info(CheckerInfo *i, Ast *expr) { + gb_mutex_lock(&i->untyped_mutex); map_remove(&i->untyped, hash_node(expr)); + gb_mutex_unlock(&i->untyped_mutex); } @@ -1046,6 +1069,8 @@ isize type_info_index(CheckerInfo *info, Type *type, bool error_on_failure) { type = t_bool; } + gb_mutex_lock(&info->type_info_mutex); + isize entry_index = -1; HashKey key = hash_type(type); isize *found_entry_index = map_get(&info->type_info_map, key); @@ -1067,6 +1092,8 @@ isize type_info_index(CheckerInfo *info, Type *type, bool error_on_failure) { } } + gb_mutex_unlock(&info->type_info_mutex); + if (error_on_failure && entry_index < 0) { compiler_error("Type_Info for '%s' could not be found", type_to_string(type)); } @@ -1084,7 +1111,9 @@ void add_untyped(CheckerInfo *i, Ast *expression, bool lhs, AddressingMode mode, if (mode == Addressing_Constant && type == t_invalid) { compiler_error("add_untyped - invalid type: %s", type_to_string(type)); } + gb_mutex_lock(&i->untyped_mutex); map_set(&i->untyped, hash_node(expression), make_expr_info(mode, type, value, lhs)); + gb_mutex_unlock(&i->untyped_mutex); } void add_type_and_value(CheckerInfo *i, Ast *expr, AddressingMode mode, Type *type, ExactValue value) { @@ -1285,6 +1314,9 @@ void add_type_info_type(CheckerContext *c, Type *t) { add_type_info_dependency(c->decl, t); + gb_mutex_lock(&c->info->type_info_mutex); + defer (gb_mutex_unlock(&c->info->type_info_mutex)); + auto found = map_get(&c->info->type_info_map, hash_type(t)); if (found != nullptr) { // Types have already been added @@ -1478,19 +1510,22 @@ void add_type_info_type(CheckerContext *c, Type *t) { } } -void check_procedure_later(Checker *c, ProcInfo info) { - GB_ASSERT(info.decl != nullptr); +void check_procedure_later(Checker *c, ProcInfo *info) { + GB_ASSERT(info != nullptr); + GB_ASSERT(info->decl != nullptr); + gb_mutex_lock(&c->procs_to_check_mutex); array_add(&c->procs_to_check, info); + gb_mutex_unlock(&c->procs_to_check_mutex); } void check_procedure_later(Checker *c, AstFile *file, Token token, DeclInfo *decl, Type *type, Ast *body, u64 tags) { - ProcInfo info = {}; - info.file = file; - info.token = token; - info.decl = decl; - info.type = type; - info.body = body; - info.tags = tags; + ProcInfo *info = gb_alloc_item(permanent_allocator(), ProcInfo); + info->file = file; + info->token = token; + info->decl = decl; + info->type = type; + info->body = body; + info->tags = tags; check_procedure_later(c, info); } @@ -4226,37 +4261,40 @@ void calculate_global_init_order(Checker *c) { } -void check_proc_info(Checker *c, ProcInfo pi) { - if (pi.type == nullptr) { +void check_proc_info(Checker *c, ProcInfo *pi) { + if (pi == nullptr) { + return; + } + if (pi->type == nullptr) { return; } CheckerContext ctx = make_checker_context(c); defer (destroy_checker_context(&ctx)); - reset_checker_context(&ctx, pi.file); - ctx.decl = pi.decl; + reset_checker_context(&ctx, pi->file); + ctx.decl = pi->decl; - TypeProc *pt = &pi.type->Proc; - String name = pi.token.string; + TypeProc *pt = &pi->type->Proc; + String name = pi->token.string; if (pt->is_polymorphic && !pt->is_poly_specialized) { - Token token = pi.token; - if (pi.poly_def_node != nullptr) { - token = ast_token(pi.poly_def_node); + Token token = pi->token; + if (pi->poly_def_node != nullptr) { + token = ast_token(pi->poly_def_node); } error(token, "Unspecialized polymorphic procedure '%.*s'", LIT(name)); return; } if (pt->is_polymorphic && pt->is_poly_specialized) { - Entity *e = pi.decl->entity; + Entity *e = pi->decl->entity; if ((e->flags & EntityFlag_Used) == 0) { // NOTE(bill, 2019-08-31): It was never used, don't check return; } } - bool bounds_check = (pi.tags & ProcTag_bounds_check) != 0; - bool no_bounds_check = (pi.tags & ProcTag_no_bounds_check) != 0; + bool bounds_check = (pi->tags & ProcTag_bounds_check) != 0; + bool no_bounds_check = (pi->tags & ProcTag_no_bounds_check) != 0; if (bounds_check) { ctx.state_flags |= StateFlag_bounds_check; @@ -4266,17 +4304,19 @@ void check_proc_info(Checker *c, ProcInfo pi) { ctx.state_flags &= ~StateFlag_bounds_check; } - check_proc_body(&ctx, pi.token, pi.decl, pi.type, pi.body); - if (pi.body != nullptr && pi.decl->entity != nullptr) { - pi.decl->entity->flags |= EntityFlag_ProcBodyChecked; + check_proc_body(&ctx, pi->token, pi->decl, pi->type, pi->body); + if (pi->body != nullptr && pi->decl->entity != nullptr) { + pi->decl->entity->flags |= EntityFlag_ProcBodyChecked; } } +GB_STATIC_ASSERT(sizeof(isize) == sizeof(void *)); + GB_THREAD_PROC(check_proc_info_worker_proc) { if (thread == nullptr) return 0; auto *c = cast(Checker *)thread->user_data; - isize index = thread->user_index; - check_proc_info(c, c->procs_to_check[index]); + ProcInfo *pi = cast(ProcInfo *)cast(uintptr)thread->user_index; + check_proc_info(c, pi); return 0; } @@ -4310,7 +4350,7 @@ void check_unchecked_bodies(Checker *c) { continue; } - check_proc_info(c, pi); + check_proc_info(c, &pi); } } } @@ -4347,6 +4387,33 @@ void check_test_names(Checker *c) { } +void check_procedure_bodies(Checker *c) { + // TODO(bill): Make this an actual FIFO queue rather than this monstrosity + while (c->procs_to_check.count != 0) { + ProcInfo *pi = c->procs_to_check.data[0]; + + // Preparing to multithread the procedure checking code + #if 0 + gb_mutex_lock(&c->procs_to_check_mutex); + defer (gb_mutex_unlock(&c->procs_to_check_mutex)); + + array_ordered_remove(&c->procs_to_check, 0); + if (pi->decl->parent && pi->decl->parent->entity) { + Entity *parent = pi->decl->parent->entity; + if (parent->kind == Entity_Procedure && (parent->flags & EntityFlag_ProcBodyChecked) == 0) { + array_add(&c->procs_to_check, pi); + continue; + } + } + #else + array_ordered_remove(&c->procs_to_check, 0); + #endif + + check_proc_info(c, pi); + } +} + + void check_parsed_files(Checker *c) { #define TIME_SECTION(str) do { if (build_context.show_more_timings) timings_start_section(&global_timings, str_lit(str)); } while (0) @@ -4405,11 +4472,7 @@ void check_parsed_files(Checker *c) { c->builtin_ctx.decl = make_decl_info(nullptr, nullptr); TIME_SECTION("check procedure bodies"); - // NOTE(bill): Nested procedures bodies will be added to this "queue" - for_array(i, c->procs_to_check) { - ProcInfo pi = c->procs_to_check[i]; - check_proc_info(c, pi); - } + check_procedure_bodies(c); TIME_SECTION("check scope usage"); for_array(i, c->info.files.entries) { diff --git a/src/checker.hpp b/src/checker.hpp index b60440bf0..6f08159e9 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -290,6 +290,11 @@ struct CheckerInfo { // NOTE(bill): If the semantic checker (check_proc_body) is to ever to be multithreaded, // these variables will be of contention + gbMutex untyped_mutex; + gbMutex gen_procs_mutex; + gbMutex gen_types_mutex; + gbMutex type_info_mutex; + Map untyped; // Key: Ast * | Expression -> ExprInfo * // NOTE(bill): This needs to be a map and not on the Ast // as it needs to be iterated across @@ -346,8 +351,11 @@ struct Checker { CheckerContext builtin_ctx; - Array procs_to_check; - Array procs_with_deferred_to_check; + + gbMutex procs_to_check_mutex; + gbMutex procs_with_deferred_to_check_mutex; + Array procs_to_check; + Array procs_with_deferred_to_check; }; diff --git a/src/llvm_backend.cpp b/src/llvm_backend.cpp index ca1db269c..e1205b471 100644 --- a/src/llvm_backend.cpp +++ b/src/llvm_backend.cpp @@ -6356,7 +6356,7 @@ lbValue lb_find_value_from_entity(lbModule *m, Entity *e) { } } - GB_PANIC("\n\tError in: %s, missing value %.*s\n", token_pos_to_string(e->token.pos), LIT(e->token.string)); + GB_PANIC("\n\tError in: %s, missing value '%.*s'\n", token_pos_to_string(e->token.pos), LIT(e->token.string)); return {}; }