Browse Source

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
gingerBill 4 years ago
parent
commit
9f7154a039
6 changed files with 144 additions and 56 deletions
  1. 3 0
      src/check_decl.cpp
  2. 24 15
      src/check_expr.cpp
  3. 9 4
      src/check_type.cpp
  4. 97 34
      src/checker.cpp
  5. 10 2
      src/checker.hpp
  6. 1 1
      src/llvm_backend.cpp

+ 3 - 0
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) {

+ 24 - 15
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<Entity *>(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);
 

+ 9 - 4
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<Operand> 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<Entity *>(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);

+ 97 - 34
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) {

+ 10 - 2
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<ExprInfo *>       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<ProcInfo> procs_to_check;
-	Array<Entity *> procs_with_deferred_to_check;
+
+	gbMutex procs_to_check_mutex;
+	gbMutex procs_with_deferred_to_check_mutex;
+	Array<ProcInfo *> procs_to_check;
+	Array<Entity *>   procs_with_deferred_to_check;
 };
 
 

+ 1 - 1
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 {};
 }