From 81d5104e228dc30184b31158c1b36ec0ec371b0b Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 7 May 2021 18:52:11 -0700 Subject: [PATCH] stage2: implement global variables * Sema: implement global variables - Improved global constants to stop needlessly creating a Var structure; they can just store the value directly. - This required making memory management a bit more sophisticated to detect when a Decl owns the Namespace associated with it, for the purposes of deinitialization. * Decl.name and Namespace decl table keys no longer directly reference ZIR; instead they have heap-duped names, so that deleted decls, which no longer have any ZIR to reference for their names, can be removed from the parent Namespace table. - In the future I would like to explore going a different direction with this, where the strings would still point to the ZIR however they would be removed from their owner Namespace objects during the update detection. The design principle here is that the existence of incremental compilation as a feature should not incur any cost for the use case when it is not used. In this example Decl names could simply point to ZIR string table memory, and it is only because of incremental compilation that we duplicate their names. * AstGen: implement threadlocal variables * CLI: call cleanExit after building a compilation so that in release modes we don't bother freeing memory or closing file descriptors, allowing the OS to do it more efficiently. * Avoid calling `freeDecl` in the linker for unreferenced Decl objects. * Fix CBE test case expecting the compile error to point to the wrong column. --- BRANCH_TODO | 5 +- src/AstGen.zig | 4 + src/Module.zig | 179 ++++++++++++++++++++++---------------------- src/Sema.zig | 57 +++++++++++++- src/Zir.zig | 3 +- src/main.zig | 2 + src/value.zig | 8 -- test/stage2/cbe.zig | 2 +- 8 files changed, 157 insertions(+), 103 deletions(-) diff --git a/BRANCH_TODO b/BRANCH_TODO index 19d43daacb..54153185e2 100644 --- a/BRANCH_TODO +++ b/BRANCH_TODO @@ -58,5 +58,8 @@ natural alignment for fields and do not have any comptime fields. this will save 16 bytes per struct field in the compilation. - * AstGen threadlocal * extern "foo" for vars + + * use ZIR memory for decl names where possible and also for keys + - this will require more sophisticated changelist detection which does some + pre-emptive deletion of decls from the parent namespace diff --git a/src/AstGen.zig b/src/AstGen.zig index 77846c4732..32220948a3 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -3009,6 +3009,7 @@ fn globalVarDecl( .align_inst = .none, // passed via the decls data .init = init_inst, .is_extern = false, + .is_threadlocal = is_threadlocal, }); break :vi var_inst; } else { @@ -3026,6 +3027,7 @@ fn globalVarDecl( .align_inst = .none, // passed via the decls data .init = .none, .is_extern = true, + .is_threadlocal = is_threadlocal, }); break :vi var_inst; } else { @@ -8100,6 +8102,7 @@ const GenZir = struct { var_type: Zir.Inst.Ref, init: Zir.Inst.Ref, is_extern: bool, + is_threadlocal: bool, }) !Zir.Inst.Ref { const astgen = gz.astgen; const gpa = astgen.gpa; @@ -8137,6 +8140,7 @@ const GenZir = struct { .has_align = args.align_inst != .none, .has_init = args.init != .none, .is_extern = args.is_extern, + .is_threadlocal = args.is_threadlocal, }), .operand = payload_index, } }, diff --git a/src/Module.zig b/src/Module.zig index 3d0f22b46c..0601245db3 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -154,9 +154,7 @@ pub const DeclPlusEmitH = struct { }; pub const Decl = struct { - /// For declarations that have corresponding source code, this is identical to - /// `getName().?`. For anonymous declarations this is allocated with Module's - /// allocator. + /// Allocated with Module's allocator; outlives the ZIR code. name: [*:0]const u8, /// The most recent Type of the Decl after a successful semantic analysis. /// Populated when `has_tv`. @@ -270,13 +268,7 @@ pub const Decl = struct { ); pub fn clearName(decl: *Decl, gpa: *Allocator) void { - // name could be allocated in the ZIR or it could be owned by gpa. - const file = decl.namespace.file_scope; - const string_table_start = @ptrToInt(file.zir.string_bytes.ptr); - const string_table_end = string_table_start + file.zir.string_bytes.len; - if (@ptrToInt(decl.name) < string_table_start or @ptrToInt(decl.name) >= string_table_end) { - gpa.free(mem.spanZ(decl.name)); - } + gpa.free(mem.spanZ(decl.name)); decl.name = undefined; } @@ -285,7 +277,7 @@ pub const Decl = struct { log.debug("destroy {*} ({s})", .{ decl, decl.name }); decl.clearName(gpa); if (decl.has_tv) { - if (decl.val.getTypeNamespace()) |namespace| { + if (decl.getInnerNamespace()) |namespace| { if (namespace.getDecl() == decl) { namespace.clearDecls(module); } @@ -308,6 +300,9 @@ pub const Decl = struct { func.deinit(gpa); gpa.destroy(func); } + if (decl.getVariable()) |variable| { + gpa.destroy(variable); + } if (decl.value_arena) |arena_state| { arena_state.promote(gpa).deinit(); decl.value_arena = null; @@ -472,6 +467,47 @@ pub const Decl = struct { return func; } + pub fn getVariable(decl: *Decl) ?*Var { + if (!decl.has_tv) return null; + const variable = (decl.val.castTag(.variable) orelse return null).data; + if (variable.owner_decl != decl) return null; + return variable; + } + + /// Gets the namespace that this Decl creates by being a struct, union, + /// 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; + 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; + return &struct_obj.namespace; + }, + .enum_full => { + const enum_obj = ty.castTag(.enum_full).?.data; + if (enum_obj.owner_decl != decl) return null; + 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"); + }, + .@"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; + return &union_obj.namespace; + }, + + else => return null, + } + } + pub fn dump(decl: *Decl) void { const loc = std.zig.findLineColumn(decl.scope.source.bytes, decl.src); std.debug.print("{s}:{d}:{d} name={s} status={s}", .{ @@ -504,6 +540,23 @@ pub const Decl = struct { fn removeDependency(decl: *Decl, other: *Decl) void { decl.dependencies.removeAssertDiscard(other); } + + fn hasLinkAllocation(decl: Decl) bool { + return switch (decl.analysis) { + .unreferenced, + .in_progress, + .dependency_failure, + .sema_failure, + .sema_failure_retryable, + .codegen_failure, + .codegen_failure_retryable, + => false, + + .complete, + .outdated, + => true, + }; + } }; /// This state is attached to every Decl when Module emit_h is non-null. @@ -831,9 +884,8 @@ pub const Scope = struct { /// Direct children of the namespace. Used during an update to detect /// which decls have been added/removed from source. /// Declaration order is preserved via entry order. - /// Key memory references the string table of the containing `File` ZIR. + /// Key memory is owned by `decl.name`. /// TODO save memory with https://github.com/ziglang/zig/issues/8619. - /// Does not contain anonymous decls. decls: std.StringArrayHashMapUnmanaged(*Decl) = .{}, pub fn deinit(ns: *Namespace, mod: *Module) void { @@ -2468,8 +2520,6 @@ pub fn astGenFile(mod: *Module, file: *Scope.File, prog_node: *std.Progress.Node /// * Decl.zir_index /// * Fn.zir_body_inst /// * Decl.zir_decl_index -/// * Decl.name -/// * Namespace.decl keys fn updateZirRefs(gpa: *Allocator, file: *Scope.File, old_zir: Zir) !void { const new_zir = file.zir; @@ -2484,18 +2534,6 @@ fn updateZirRefs(gpa: *Allocator, file: *Scope.File, old_zir: Zir) !void { try mapOldZirToNew(gpa, old_zir, new_zir, &inst_map, &extra_map); - // Build string table for new ZIR. - var string_table: std.StringHashMapUnmanaged(u32) = .{}; - defer string_table.deinit(gpa); - { - var i: usize = 2; - while (i < new_zir.string_bytes.len) { - const string = new_zir.nullTerminatedString(i); - try string_table.put(gpa, string, @intCast(u32, i)); - i += string.len + 1; - } - } - // Walk the Decl graph, updating ZIR indexes, strings, and populating // the deleted and outdated lists. @@ -2523,12 +2561,6 @@ fn updateZirRefs(gpa: *Allocator, file: *Scope.File, old_zir: Zir) !void { try file.deleted_decls.append(gpa, decl); continue; }; - const new_name_index = string_table.get(mem.spanZ(decl.name)) orelse { - try file.deleted_decls.append(gpa, decl); - continue; - }; - decl.name = new_zir.nullTerminatedString(new_name_index).ptr; - const new_hash = decl.contentsHashZir(new_zir); if (!std.zig.srcHashEql(old_hash, new_hash)) { try file.outdated_decls.append(gpa, decl); @@ -2558,16 +2590,9 @@ fn updateZirRefs(gpa: *Allocator, file: *Scope.File, old_zir: Zir) !void { }; } - if (decl.val.getTypeNamespace()) |namespace| { + if (decl.getInnerNamespace()) |namespace| { for (namespace.decls.items()) |*entry| { const sub_decl = entry.value; - if (sub_decl.zir_decl_index != 0) { - const new_key_index = string_table.get(entry.key) orelse { - try file.deleted_decls.append(gpa, sub_decl); - continue; - }; - entry.key = new_zir.nullTerminatedString(new_key_index); - } try decl_stack.append(gpa, sub_decl); } } @@ -2936,46 +2961,14 @@ fn semaDecl(mod: *Module, decl: *Decl) !bool { } return type_changed or is_inline != prev_is_inline; } else { - const is_mutable = decl_tv.val.tag() == .variable; - - var is_threadlocal = false; // TODO implement threadlocal variables - var is_extern = false; // TODO implement extern variables - - if (is_mutable and !decl_tv.ty.isValidVarType(is_extern)) { - return mod.fail( - &block_scope.base, - src, // TODO point at the mut token - "variable of type '{}' must be const", - .{decl_tv.ty}, - ); - } - var type_changed = true; if (decl.has_tv) { type_changed = !decl.ty.eql(decl_tv.ty); decl.clearValues(gpa); } - const copied_val = try decl_tv.val.copy(&decl_arena.allocator); - const is_extern_fn = copied_val.tag() == .extern_fn; - - // TODO: also avoid allocating this Var structure if `!is_mutable`. - // I think this will require adjusting Sema to copy the value or something - // like that; otherwise it causes use of undefined value when freeing resources. - const decl_val: Value = if (is_extern_fn) copied_val else blk: { - const new_variable = try decl_arena.allocator.create(Var); - new_variable.* = .{ - .owner_decl = decl, - .init = copied_val, - .is_extern = is_extern, - .is_mutable = is_mutable, - .is_threadlocal = is_threadlocal, - }; - break :blk try Value.Tag.variable.create(&decl_arena.allocator, new_variable); - }; - decl.ty = try decl_tv.ty.copy(&decl_arena.allocator); - decl.val = decl_val; + decl.val = try decl_tv.val.copy(&decl_arena.allocator); decl.align_val = try align_val.copy(&decl_arena.allocator); decl.linksection_val = try linksection_val.copy(&decl_arena.allocator); decl.has_tv = true; @@ -3211,7 +3204,8 @@ fn scanDecl(iter: *ScanDeclIter, decl_sub_index: usize, flags: u4) InnerError!vo const decl_node = iter.parent_decl.relativeToNodeIndex(decl_block_inst_data.src_node); // Every Decl needs a name. - const raw_decl_name: [:0]const u8 = switch (decl_name_index) { + var is_named_test = false; + const decl_name: [:0]const u8 = switch (decl_name_index) { 0 => name: { if (is_exported) { const i = iter.usingnamespace_index; @@ -3228,24 +3222,28 @@ fn scanDecl(iter: *ScanDeclIter, decl_sub_index: usize, flags: u4) InnerError!vo iter.unnamed_test_index += 1; break :name try std.fmt.allocPrintZ(gpa, "test_{d}", .{i}); }, - else => zir.nullTerminatedString(decl_name_index), - }; - const decl_name = if (raw_decl_name.len != 0) raw_decl_name else name: { - const test_name = zir.nullTerminatedString(decl_name_index + 1); - break :name try std.fmt.allocPrintZ(gpa, "test.{s}", .{test_name}); + else => name: { + const raw_name = zir.nullTerminatedString(decl_name_index); + if (raw_name.len == 0) { + is_named_test = true; + const test_name = zir.nullTerminatedString(decl_name_index + 1); + break :name try std.fmt.allocPrintZ(gpa, "test.{s}", .{test_name}); + } else { + break :name try gpa.dupeZ(u8, raw_name); + } + }, }; // We create a Decl for it regardless of analysis status. const gop = try namespace.decls.getOrPut(gpa, decl_name); if (!gop.found_existing) { const new_decl = try mod.allocateNewDecl(namespace, decl_node); - log.debug("scan new decl {*} ({s}) into {*}", .{ new_decl, decl_name, namespace }); + log.debug("scan new {*} ({s}) into {*}", .{ new_decl, decl_name, namespace }); new_decl.src_line = line; new_decl.name = decl_name; gop.entry.value = new_decl; // Exported decls, comptime decls, usingnamespace decls, and // test decls if in test mode, get analyzed. - const is_named_test = raw_decl_name.len == 0; const want_analysis = is_exported or switch (decl_name_index) { 0 => true, // comptime decl 1 => mod.comp.bin_file.options.is_test, // test decl @@ -3261,17 +3259,15 @@ fn scanDecl(iter: *ScanDeclIter, decl_sub_index: usize, flags: u4) InnerError!vo new_decl.zir_decl_index = @intCast(u32, decl_sub_index); return; } + gpa.free(decl_name); const decl = gop.entry.value; - log.debug("scan existing decl {*} ({s}) of {*}", .{ decl, decl_name, namespace }); + log.debug("scan existing {*} ({s}) of {*}", .{ decl, decl_name, namespace }); // Update the AST node of the decl; even if its contents are unchanged, it may // have been re-ordered. const prev_src_node = decl.src_node; decl.src_node = decl_node; decl.src_line = line; - decl.clearName(gpa); - decl.name = decl_name; - decl.is_pub = is_pub; decl.is_exported = is_exported; decl.has_align = has_align; @@ -3305,14 +3301,13 @@ pub fn deleteDecl( const tracy = trace(@src()); defer tracy.end(); - log.debug("deleting decl '{s}'", .{decl.name}); + log.debug("deleting {*} ({s})", .{ decl, decl.name }); if (outdated_decls) |map| { _ = map.swapRemove(decl); - try map.ensureCapacity(map.count() + decl.dependants.count()); + try map.ensureUnusedCapacity(decl.dependants.count()); } - try mod.deletion_set.ensureCapacity(mod.gpa, mod.deletion_set.count() + - decl.dependencies.count()); + try mod.deletion_set.ensureUnusedCapacity(mod.gpa, decl.dependencies.count()); // Remove from the namespace it resides in. decl.namespace.removeDecl(decl); @@ -3354,7 +3349,9 @@ pub fn deleteDecl( } _ = mod.compile_log_decls.swapRemove(decl); mod.deleteDeclExports(decl); - mod.comp.bin_file.freeDecl(decl); + if (decl.hasLinkAllocation()) { + mod.comp.bin_file.freeDecl(decl); + } decl.destroy(mod); } diff --git a/src/Sema.zig b/src/Sema.zig index 3e6ab6b588..2645a6b2a0 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -5742,8 +5742,63 @@ fn zirVarExtended( ) InnerError!*Inst { const extra = sema.code.extraData(Zir.Inst.ExtendedVar, extended.operand); const src = sema.src; + const align_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at align + const ty_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at type + const mut_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at mut token + const init_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at init expr + const small = @bitCast(Zir.Inst.ExtendedVar.Small, extended.small); + const var_ty = try sema.resolveType(block, ty_src, extra.data.var_type); - return sema.mod.fail(&block.base, src, "TODO implement Sema.zirVarExtended", .{}); + var extra_index: usize = extra.end; + + const lib_name: ?[]const u8 = if (small.has_lib_name) blk: { + const lib_name = sema.code.nullTerminatedString(sema.code.extra[extra_index]); + extra_index += 1; + break :blk lib_name; + } else null; + + // ZIR supports encoding this information but it is not used; the information + // is encoded via the Decl entry. + assert(!small.has_align); + //const align_val: Value = if (small.has_align) blk: { + // const align_ref = @intToEnum(Zir.Inst.Ref, sema.code.extra[extra_index]); + // extra_index += 1; + // const align_tv = try sema.resolveInstConst(block, align_src, align_ref); + // break :blk align_tv.val; + //} else Value.initTag(.null_value); + + const init_val: Value = if (small.has_init) blk: { + const init_ref = @intToEnum(Zir.Inst.Ref, sema.code.extra[extra_index]); + extra_index += 1; + const init_tv = try sema.resolveInstConst(block, init_src, init_ref); + break :blk init_tv.val; + } else Value.initTag(.null_value); + + if (!var_ty.isValidVarType(small.is_extern)) { + return sema.mod.fail(&block.base, mut_src, "variable of type '{}' must be const", .{ + var_ty, + }); + } + + if (lib_name != null) { + // Look at the sema code for functions which has this logic, it just needs to + // be extracted and shared by both var and func + return sema.mod.fail(&block.base, src, "TODO: handle var with lib_name in Sema", .{}); + } + + const new_var = try sema.gpa.create(Module.Var); + new_var.* = .{ + .owner_decl = sema.owner_decl, + .init = init_val, + .is_extern = small.is_extern, + .is_mutable = true, // TODO get rid of this unused field + .is_threadlocal = small.is_threadlocal, + }; + const result = try sema.mod.constInst(sema.arena, src, .{ + .ty = var_ty, + .val = try Value.Tag.variable.create(sema.arena, new_var), + }); + return result; } fn zirFuncExtended( diff --git a/src/Zir.zig b/src/Zir.zig index b44386afea..ea703ddb74 100644 --- a/src/Zir.zig +++ b/src/Zir.zig @@ -2235,7 +2235,8 @@ pub const Inst = struct { has_align: bool, has_init: bool, is_extern: bool, - _: u12 = undefined, + is_threadlocal: bool, + _: u11 = undefined, }; }; diff --git a/src/main.zig b/src/main.zig index 1e1e7f32ed..01be971602 100644 --- a/src/main.zig +++ b/src/main.zig @@ -2086,6 +2086,8 @@ fn buildOutputType( break; } } + // Skip resource deallocation in release builds; let the OS do it. + return cleanExit(); } fn runOrTest( diff --git a/src/value.zig b/src/value.zig index 65d8a8f6fa..9b87de3821 100644 --- a/src/value.zig +++ b/src/value.zig @@ -626,14 +626,6 @@ pub const Value = extern union { unreachable; } - /// Returns null if not a type or if the type has no namespace. - pub fn getTypeNamespace(self: Value) ?*Module.Scope.Namespace { - return switch (self.tag()) { - .ty => self.castTag(.ty).?.data.getNamespace(), - else => null, - }; - } - /// Asserts that the value is representable as a type. pub fn toType(self: Value, allocator: *Allocator) !Type { return switch (self.tag()) { diff --git a/test/stage2/cbe.zig b/test/stage2/cbe.zig index 332a55e608..f9f86bf8ed 100644 --- a/test/stage2/cbe.zig +++ b/test/stage2/cbe.zig @@ -51,7 +51,7 @@ pub fn addCases(ctx: *TestContext) !void { \\} \\var y: i32 = 1234; , &.{ - ":2:18: error: unable to resolve comptime value", + ":2:22: error: unable to resolve comptime value", ":5:26: error: unable to resolve comptime value", }); }