From bcf15e39e2d4e2243f475852aca7749e40a70fbd Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 11 May 2021 14:17:52 -0700 Subject: [PATCH] stage2: add `owns_tv` flag to `Module.Decl` Decl objects need to know whether they are the owner of the Type/Value associated with them, in order to decide whether to destroy the associated Namespace, Fn, or Var when cleaning up. --- BRANCH_TODO | 10 ------ src/Module.zig | 86 ++++++++++++++++++++++++++++++--------------- src/Sema.zig | 2 +- src/codegen/c.zig | 55 ++++++++++++++++++++++++++--- src/link/C/zig.h | 32 +++++++++-------- test/stage2/cbe.zig | 2 +- 6 files changed, 128 insertions(+), 59 deletions(-) diff --git a/BRANCH_TODO b/BRANCH_TODO index 4df72d8315..ac1ad75b7e 100644 --- a/BRANCH_TODO +++ b/BRANCH_TODO @@ -1,13 +1,3 @@ - * The next problem is that when trying to deinitialize everything, when we - deinit a Decl that is the owner of a Namespace, there may still be other Decl - objects that reference that Namespace. They want to check if they are the owner - in order to find out if they should destroy it. But they can't check if they are - the owner because the owner_decl field is destroyed. - So there's a memory management problem to solve. We could easily solve this with - ref counting or whatever but the goal is to not introduce extra overhead / unnecessary - fields just to help figure out how to free stuff. So come up with some way to make - this sound, and easily debuggable when something goes wrong. - * get stage2 tests passing * modify stage2 tests so that only 1 uses _start and the rest use pub fn main diff --git a/src/Module.zig b/src/Module.zig index 7860f9141c..d483a4fd4b 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -227,6 +227,10 @@ pub const Decl = struct { }, /// Whether `typed_value`, `align_val`, and `linksection_val` are populated. has_tv: bool, + /// If `true` it means the `Decl` is the resource owner of the type/value associated + /// with it. That means when `Decl` is destroyed, the cleanup code should additionally + /// check if the value owns a `Namespace`, and destroy that too. + owns_tv: bool, /// This flag is set when this Decl is added to `Module.deletion_set`, and cleared /// when removed. deletion_flag: bool, @@ -278,9 +282,7 @@ pub const Decl = struct { decl.clearName(gpa); if (decl.has_tv) { if (decl.getInnerNamespace()) |namespace| { - if (namespace.getDecl() == decl) { - namespace.clearDecls(module); - } + namespace.clearDecls(module); } decl.clearValues(gpa); } @@ -307,6 +309,7 @@ pub const Decl = struct { arena_state.promote(gpa).deinit(); decl.value_arena = null; decl.has_tv = false; + decl.owns_tv = false; } } @@ -441,36 +444,36 @@ pub const Decl = struct { /// If the Decl has a value and it is a struct, return it, /// otherwise null. pub fn getStruct(decl: *Decl) ?*Struct { - if (!decl.has_tv) return null; + if (!decl.owns_tv) return null; const ty = (decl.val.castTag(.ty) orelse return null).data; const struct_obj = (ty.castTag(.@"struct") orelse return null).data; - if (struct_obj.owner_decl != decl) return null; + assert(struct_obj.owner_decl == decl); return struct_obj; } /// If the Decl has a value and it is a union, return it, /// otherwise null. pub fn getUnion(decl: *Decl) ?*Union { - if (!decl.has_tv) return null; + if (!decl.owns_tv) return null; const ty = (decl.val.castTag(.ty) orelse return null).data; const union_obj = (ty.cast(Type.Payload.Union) orelse return null).data; - if (union_obj.owner_decl != decl) return null; + assert(union_obj.owner_decl == decl); return union_obj; } /// If the Decl has a value and it is a function, return it, /// otherwise null. pub fn getFunction(decl: *Decl) ?*Fn { - if (!decl.has_tv) return null; + if (!decl.owns_tv) return null; const func = (decl.val.castTag(.function) orelse return null).data; - if (func.owner_decl != decl) return null; + assert(func.owner_decl == decl); return func; } pub fn getVariable(decl: *Decl) ?*Var { - if (!decl.has_tv) return null; + if (!decl.owns_tv) return null; const variable = (decl.val.castTag(.variable) orelse return null).data; - if (variable.owner_decl != decl) return null; + assert(variable.owner_decl == decl); return variable; } @@ -478,29 +481,28 @@ pub const Decl = struct { /// enum, or opaque. /// Only returns it if the Decl is the owner. pub fn getInnerNamespace(decl: *Decl) ?*Scope.Namespace { - if (!decl.has_tv) return null; + if (!decl.owns_tv) return null; const ty = (decl.val.castTag(.ty) orelse return null).data; switch (ty.tag()) { .@"struct" => { const struct_obj = ty.castTag(.@"struct").?.data; - if (struct_obj.owner_decl != decl) return null; + assert(struct_obj.owner_decl == decl); return &struct_obj.namespace; }, .enum_full => { const enum_obj = ty.castTag(.enum_full).?.data; - if (enum_obj.owner_decl != decl) return null; + assert(enum_obj.owner_decl == decl); return &enum_obj.namespace; }, .empty_struct => { - // design flaw, can't verify the owner is this decl - @panic("TODO can't implement getInnerNamespace for this type"); + return ty.castTag(.empty_struct).?.data; }, .@"opaque" => { @panic("TODO opaque types"); }, .@"union", .union_tagged => { const union_obj = ty.cast(Type.Payload.Union).?.data; - if (union_obj.owner_decl != decl) return null; + assert(union_obj.owner_decl == decl); return &union_obj.namespace; }, @@ -554,7 +556,11 @@ pub const Decl = struct { .complete, .outdated, - => true, + => { + if (!decl.owns_tv) + return false; + return decl.ty.hasCodeGenBits(); + }, }; } }; @@ -2838,6 +2844,7 @@ pub fn semaFile(mod: *Module, file: *Scope.File) InnerError!void { new_decl.ty = struct_ty; new_decl.val = struct_val; new_decl.has_tv = true; + new_decl.owns_tv = true; new_decl.analysis = .complete; new_decl.generation = mod.generation; @@ -2948,7 +2955,7 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool { errdefer decl_arena.deinit(); const decl_arena_state = try decl_arena.allocator.create(std.heap.ArenaAllocator.State); - if (decl_tv.val.tag() == .function) { + if (decl_tv.val.castTag(.function)) |fn_payload| { var prev_type_has_bits = false; var prev_is_inline = false; var type_changed = true; @@ -2956,10 +2963,8 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool { if (decl.has_tv) { prev_type_has_bits = decl.ty.hasCodeGenBits(); type_changed = !decl.ty.eql(decl_tv.ty); - if (decl.val.castTag(.function)) |payload| { - const prev_func = payload.data; + if (decl.getFunction()) |prev_func| { prev_is_inline = prev_func.state == .inline_only; - prev_func.deinit(gpa); } decl.clearValues(gpa); } @@ -2969,6 +2974,7 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool { decl.align_val = try align_val.copy(&decl_arena.allocator); decl.linksection_val = try linksection_val.copy(&decl_arena.allocator); decl.has_tv = true; + decl.owns_tv = fn_payload.data.owner_decl == decl; decl_arena_state.* = decl_arena.state; decl.value_arena = decl_arena_state; decl.analysis = .complete; @@ -3004,6 +3010,25 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool { decl.clearValues(gpa); } + decl.owns_tv = false; + var queue_linker_work = false; + if (decl_tv.val.castTag(.variable)) |payload| { + const variable = payload.data; + if (variable.owner_decl == decl) { + decl.owns_tv = true; + queue_linker_work = true; + + const copied_init = try variable.init.copy(&decl_arena.allocator); + variable.init = copied_init; + } + } else if (decl_tv.val.castTag(.extern_fn)) |payload| { + const owner_decl = payload.data; + if (decl == owner_decl) { + decl.owns_tv = true; + queue_linker_work = true; + } + } + decl.ty = try decl_tv.ty.copy(&decl_arena.allocator); decl.val = try decl_tv.val.copy(&decl_arena.allocator); decl.align_val = try align_val.copy(&decl_arena.allocator); @@ -3014,13 +3039,7 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool { decl.analysis = .complete; decl.generation = mod.generation; - if (decl.is_exported) { - const export_src = src; // TODO point to the export token - // The scope needs to have the decl in it. - try mod.analyzeExport(&block_scope.base, export_src, mem.spanZ(decl.name), decl); - } - - if (decl.val.tag() == .extern_fn) { + if (queue_linker_work and decl.ty.hasCodeGenBits()) { try mod.comp.bin_file.allocateDeclIndexes(decl); try mod.comp.work_queue.writeItem(.{ .codegen_decl = decl }); @@ -3029,6 +3048,13 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool { } } + if (decl.is_exported) { + const export_src = src; // TODO point to the export token + // The scope needs to have the decl in it. + try mod.analyzeExport(&block_scope.base, export_src, mem.spanZ(decl.name), decl); + } + + return type_changed; } } @@ -3531,6 +3557,7 @@ fn allocateNewDecl(mod: *Module, namespace: *Scope.Namespace, src_node: ast.Node .src_node = src_node, .src_line = undefined, .has_tv = false, + .owns_tv = false, .ty = undefined, .val = undefined, .align_val = undefined, @@ -3779,6 +3806,7 @@ pub fn createAnonymousDeclNamed( new_decl.ty = typed_value.ty; new_decl.val = typed_value.val; new_decl.has_tv = true; + new_decl.owns_tv = true; new_decl.analysis = .complete; new_decl.generation = mod.generation; diff --git a/src/Sema.zig b/src/Sema.zig index 35248d8f8b..43b18d0032 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -5867,7 +5867,7 @@ fn zirVarExtended( extra_index += 1; const init_tv = try sema.resolveInstConst(block, init_src, init_ref); break :blk init_tv.val; - } else Value.initTag(.null_value); + } else Value.initTag(.unreachable_value); if (!var_ty.isValidVarType(small.is_extern)) { return sema.mod.fail(&block.base, mut_src, "variable of type '{}' must be const", .{ diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 43b851019b..6f80ac6154 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -30,6 +30,7 @@ pub const CValue = union(enum) { arg: usize, /// By-value decl: *Decl, + decl_ref: *Decl, }; pub const CValueMap = std.AutoHashMap(*Inst, CValue); @@ -117,6 +118,7 @@ pub const Object = struct { .constant => |inst| return o.dg.renderValue(w, inst.ty, inst.value().?), .arg => |i| return w.print("a{d}", .{i}), .decl => |decl| return w.writeAll(mem.span(decl.name)), + .decl_ref => |decl| return w.print("&{s}", .{decl.name}), } } @@ -528,13 +530,17 @@ pub const DeclGen = struct { } } - fn functionIsGlobal(dg: *DeclGen, tv: TypedValue) bool { + fn declIsGlobal(dg: *DeclGen, tv: TypedValue) bool { switch (tv.val.tag()) { .extern_fn => return true, .function => { const func = tv.val.castTag(.function).?.data; return dg.module.decl_exports.contains(func.owner_decl); }, + .variable => { + const variable = tv.val.castTag(.variable).?.data; + return dg.module.decl_exports.contains(variable.owner_decl); + }, else => unreachable, } } @@ -549,7 +555,7 @@ pub fn genDecl(o: *Object) !void { .val = o.dg.decl.val, }; if (tv.val.castTag(.function)) |func_payload| { - const is_global = o.dg.functionIsGlobal(tv); + const is_global = o.dg.declIsGlobal(tv); const fwd_decl_writer = o.dg.fwd_decl.writer(); if (is_global) { try fwd_decl_writer.writeAll("ZIG_EXTERN_C "); @@ -570,6 +576,30 @@ pub fn genDecl(o: *Object) !void { try writer.writeAll("ZIG_EXTERN_C "); try o.dg.renderFunctionSignature(writer, true); try writer.writeAll(";\n"); + } else if (tv.val.castTag(.variable)) |var_payload| { + const variable: *Module.Var = var_payload.data; + const is_global = o.dg.declIsGlobal(tv); + const fwd_decl_writer = o.dg.fwd_decl.writer(); + if (is_global or variable.is_extern) { + try fwd_decl_writer.writeAll("ZIG_EXTERN_C "); + } + if (variable.is_threadlocal) { + try fwd_decl_writer.writeAll("zig_threadlocal "); + } + try o.dg.renderType(fwd_decl_writer, o.dg.decl.ty); + const decl_name = mem.span(o.dg.decl.name); + try fwd_decl_writer.print(" {s};\n", .{decl_name}); + + try o.indent_writer.insertNewline(); + const w = o.writer(); + try o.dg.renderType(w, o.dg.decl.ty); + try w.print(" {s} = ", .{decl_name}); + if (variable.init.tag() != .unreachable_value) { + try o.dg.renderValue(w, tv.ty, variable.init); + } + try w.writeAll(";"); + try o.indent_writer.insertNewline(); + } else { const writer = o.writer(); try writer.writeAll("static "); @@ -598,7 +628,7 @@ pub fn genHeader(dg: *DeclGen) error{ AnalysisFail, OutOfMemory }!void { switch (tv.ty.zigTypeTag()) { .Fn => { - const is_global = dg.functionIsGlobal(tv); + const is_global = dg.declIsGlobal(tv); if (is_global) { try writer.writeAll("ZIG_EXTERN_C "); } @@ -696,7 +726,7 @@ pub fn genBody(o: *Object, body: ir.Body) error{ AnalysisFail, OutOfMemory }!voi .wrap_errunion_err => try genWrapErrUnionErr(o, inst.castTag(.wrap_errunion_err).?), .br_block_flat => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for br_block_flat", .{}), .ptrtoint => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for ptrtoint", .{}), - .varptr => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for varptr", .{}), + .varptr => try genVarPtr(o, inst.castTag(.varptr).?), .floatcast => return o.dg.fail(.{ .node_offset = 0 }, "TODO: C backend: implement codegen for floatcast", .{}), }; switch (result_value) { @@ -709,6 +739,10 @@ pub fn genBody(o: *Object, body: ir.Body) error{ AnalysisFail, OutOfMemory }!voi try writer.writeAll("}"); } +fn genVarPtr(o: *Object, inst: *Inst.VarPtr) !CValue { + return CValue{ .decl_ref = inst.variable.owner_decl }; +} + fn genAlloc(o: *Object, alloc: *Inst.NoOp) !CValue { const writer = o.writer(); @@ -743,6 +777,12 @@ fn genLoad(o: *Object, inst: *Inst.UnOp) !CValue { try o.writeCValue(writer, wrapped); try writer.writeAll(";\n"); }, + .decl_ref => |decl| { + const wrapped: CValue = .{ .decl = decl }; + try writer.writeAll(" = "); + try o.writeCValue(writer, wrapped); + try writer.writeAll(";\n"); + }, else => { try writer.writeAll(" = *"); try o.writeCValue(writer, operand); @@ -791,6 +831,13 @@ fn genStore(o: *Object, inst: *Inst.BinOp) !CValue { try o.writeCValue(writer, src_val); try writer.writeAll(";\n"); }, + .decl_ref => |decl| { + const dest: CValue = .{ .decl = decl }; + try o.writeCValue(writer, dest); + try writer.writeAll(" = "); + try o.writeCValue(writer, src_val); + try writer.writeAll(";\n"); + }, else => { try writer.writeAll("*"); try o.writeCValue(writer, dest_ptr); diff --git a/src/link/C/zig.h b/src/link/C/zig.h index 640dfb2345..ad2b5d4498 100644 --- a/src/link/C/zig.h +++ b/src/link/C/zig.h @@ -1,25 +1,15 @@ -#if __STDC_VERSION__ >= 199901L -#include -#else -#define bool unsigned char -#define true 1 -#define false 0 -#endif - #if __STDC_VERSION__ >= 201112L #define zig_noreturn _Noreturn +#define zig_threadlocal thread_local #elif __GNUC__ #define zig_noreturn __attribute__ ((noreturn)) +#define zig_threadlocal __thread #elif _MSC_VER #define zig_noreturn __declspec(noreturn) +#define zig_threadlocal __declspec(thread) #else #define zig_noreturn -#endif - -#if defined(__GNUC__) -#define zig_unreachable() __builtin_unreachable() -#else -#define zig_unreachable() +#define zig_threadlocal zig_threadlocal_unavailable #endif #if __STDC_VERSION__ >= 199901L @@ -30,6 +20,20 @@ #define ZIG_RESTRICT #endif +#if __STDC_VERSION__ >= 199901L +#include +#else +#define bool unsigned char +#define true 1 +#define false 0 +#endif + +#if defined(__GNUC__) +#define zig_unreachable() __builtin_unreachable() +#else +#define zig_unreachable() +#endif + #ifdef __cplusplus #define ZIG_EXTERN_C extern "C" #else diff --git a/test/stage2/cbe.zig b/test/stage2/cbe.zig index 5ffb67f731..0e5b398f59 100644 --- a/test/stage2/cbe.zig +++ b/test/stage2/cbe.zig @@ -517,7 +517,7 @@ pub fn addCases(ctx: *TestContext) !void { \\} , &.{ ":3:21: error: missing struct field: x", - ":1:15: note: struct 'Point' declared here", + ":1:15: note: struct 'test_case.Point' declared here", }); case.addError( \\const Point = struct { x: i32, y: i32 };