From 9b48fc2833be409902f4d3256d2d921f4f924e0d Mon Sep 17 00:00:00 2001 From: mlugg Date: Thu, 1 Jun 2023 08:10:45 +0100 Subject: [PATCH] Allocate capture scopes in gpa instead of Decl.value_arena This eliminates the last major use of value_arena, in preparation to remove it. --- src/Module.zig | 76 ++++++++++++++++++++++++----------------- src/Sema.zig | 91 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 101 insertions(+), 66 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index cf0d222a2e..06e8c53eb7 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -277,61 +277,75 @@ pub const Export = struct { }; pub const CaptureScope = struct { + refs: u32, parent: ?*CaptureScope, /// Values from this decl's evaluation that will be closed over in - /// child decls. Values stored in the value_arena of the linked decl. - /// During sema, this map is backed by the gpa. Once sema completes, - /// it is reallocated using the value_arena. - captures: std.AutoHashMapUnmanaged(Zir.Inst.Index, TypedValue) = .{}, + /// child decls. This map is backed by the gpa, and deinited when + /// the refcount reaches 0. + captures: std.AutoHashMapUnmanaged(Zir.Inst.Index, Capture) = .{}, - pub fn failed(noalias self: *const @This()) bool { + pub const Capture = union(enum) { + comptime_val: InternPool.Index, // index of value + runtime_val: InternPool.Index, // index of type + }; + + pub fn failed(noalias self: *const CaptureScope) bool { return self.captures.available == 0 and self.captures.size == std.math.maxInt(u32); } - pub fn fail(noalias self: *@This()) void { + pub fn fail(noalias self: *CaptureScope, gpa: Allocator) void { + self.captures.deinit(gpa); self.captures.available = 0; self.captures.size = std.math.maxInt(u32); } + + pub fn incRef(self: *CaptureScope) void { + self.refs += 1; + } + + pub fn decRef(self: *CaptureScope, gpa: Allocator) void { + self.refs -= 1; + if (self.refs > 0) return; + if (self.parent) |p| p.decRef(gpa); + if (!self.failed()) { + self.captures.deinit(gpa); + } + } }; pub const WipCaptureScope = struct { scope: *CaptureScope, finalized: bool, gpa: Allocator, - perm_arena: Allocator, - pub fn init(gpa: Allocator, perm_arena: Allocator, parent: ?*CaptureScope) !@This() { - const scope = try perm_arena.create(CaptureScope); - scope.* = .{ .parent = parent }; - return @This(){ + pub fn init(gpa: Allocator, parent: ?*CaptureScope) !WipCaptureScope { + const scope = try gpa.create(CaptureScope); + if (parent) |p| p.incRef(); + scope.* = .{ .refs = 1, .parent = parent }; + return .{ .scope = scope, .finalized = false, .gpa = gpa, - .perm_arena = perm_arena, }; } - pub fn finalize(noalias self: *@This()) !void { - assert(!self.finalized); - // use a temp to avoid unintentional aliasing due to RLS - const tmp = try self.scope.captures.clone(self.perm_arena); - self.scope.captures.deinit(self.gpa); - self.scope.captures = tmp; + pub fn finalize(noalias self: *WipCaptureScope) !void { self.finalized = true; } - pub fn reset(noalias self: *@This(), parent: ?*CaptureScope) !void { - if (!self.finalized) try self.finalize(); - self.scope = try self.perm_arena.create(CaptureScope); - self.scope.* = .{ .parent = parent }; - self.finalized = false; + pub fn reset(noalias self: *WipCaptureScope, parent: ?*CaptureScope) !void { + self.scope.decRef(self.gpa); + self.scope = try self.gpa.create(CaptureScope); + if (parent) |p| p.incRef(); + self.scope.* = .{ .refs = 1, .parent = parent }; } - pub fn deinit(noalias self: *@This()) void { - if (!self.finalized) { - self.scope.captures.deinit(self.gpa); - self.scope.fail(); + pub fn deinit(noalias self: *WipCaptureScope) void { + if (self.finalized) { + self.scope.decRef(self.gpa); + } else { + self.scope.fail(self.gpa); } self.* = undefined; } @@ -3311,6 +3325,7 @@ pub fn destroyDecl(mod: *Module, decl_index: Decl.Index) void { mod.destroyNamespace(i); } } + if (decl.src_scope) |scope| scope.decRef(gpa); decl.clearValues(mod); decl.dependants.deinit(gpa); decl.dependencies.deinit(gpa); @@ -4425,7 +4440,7 @@ pub fn semaFile(mod: *Module, file: *File) SemaError!void { }; defer sema.deinit(); - var wip_captures = try WipCaptureScope.init(gpa, new_decl_arena_allocator, null); + var wip_captures = try WipCaptureScope.init(gpa, null); defer wip_captures.deinit(); if (sema.analyzeStructDecl(new_decl, main_struct_inst, struct_index)) |_| { @@ -4538,7 +4553,7 @@ fn semaDecl(mod: *Module, decl_index: Decl.Index) !bool { } log.debug("semaDecl {*} ({s})", .{ decl, decl.name }); - var wip_captures = try WipCaptureScope.init(gpa, decl_arena_allocator, decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, decl.src_scope); defer wip_captures.deinit(); var block_scope: Sema.Block = .{ @@ -5499,7 +5514,7 @@ pub fn analyzeFnBody(mod: *Module, func_index: Fn.Index, arena: Allocator) SemaE try sema.air_extra.ensureTotalCapacity(gpa, reserved_count); sema.air_extra.items.len += reserved_count; - var wip_captures = try WipCaptureScope.init(gpa, decl_arena_allocator, decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, decl.src_scope); defer wip_captures.deinit(); var inner_block: Sema.Block = .{ @@ -5771,6 +5786,7 @@ pub fn allocateNewDecl( }; }; + if (src_scope) |scope| scope.incRef(); decl_and_index.new_decl.* = .{ .name = undefined, .src_namespace = namespace, diff --git a/src/Sema.zig b/src/Sema.zig index a0d7c483d9..cecf59f2c3 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -889,17 +889,18 @@ fn analyzeBodyInner( try sema.inst_map.ensureSpaceForInstructions(sema.gpa, body); + // Most of the time, we don't need to construct a new capture scope for a + // block. However, successive iterations of comptime loops can capture + // different values for the same Zir.Inst.Index, so in those cases, we will + // have to create nested capture scopes; see the `.repeat` case below. const parent_capture_scope = block.wip_capture_scope; - - var wip_captures = WipCaptureScope{ - .finalized = true, + parent_capture_scope.incRef(); + var wip_captures: WipCaptureScope = .{ .scope = parent_capture_scope, - .perm_arena = sema.perm_arena, .gpa = sema.gpa, + .finalized = true, // don't finalize the parent scope }; - defer if (wip_captures.scope != parent_capture_scope) { - wip_captures.deinit(); - }; + defer wip_captures.deinit(); const mod = sema.mod; const map = &sema.inst_map; @@ -1452,6 +1453,11 @@ fn analyzeBodyInner( const src = LazySrcLoc.nodeOffset(datas[inst].node); try sema.emitBackwardBranch(block, src); if (wip_captures.scope.captures.count() != orig_captures) { + // We need to construct new capture scopes for the next loop iteration so it + // can capture values without clobbering the earlier iteration's captures. + // At first, we reused the parent capture scope as an optimization, but for + // successive scopes we have to create new ones as children of the parent + // scope. try wip_captures.reset(parent_capture_scope); block.wip_capture_scope = wip_captures.scope; orig_captures = 0; @@ -1467,6 +1473,11 @@ fn analyzeBodyInner( const src = LazySrcLoc.nodeOffset(datas[inst].node); try sema.emitBackwardBranch(block, src); if (wip_captures.scope.captures.count() != orig_captures) { + // We need to construct new capture scopes for the next loop iteration so it + // can capture values without clobbering the earlier iteration's captures. + // At first, we reused the parent capture scope as an optimization, but for + // successive scopes we have to create new ones as children of the parent + // scope. try wip_captures.reset(parent_capture_scope); block.wip_capture_scope = wip_captures.scope; orig_captures = 0; @@ -1749,6 +1760,8 @@ fn analyzeBodyInner( if (noreturn_inst) |some| try block.instructions.append(sema.gpa, some); if (!wip_captures.finalized) { + // We've updated the capture scope due to a `repeat` instruction where + // the body had a capture; finalize our child scope and reset try wip_captures.finalize(); block.wip_capture_scope = parent_capture_scope; } @@ -3007,7 +3020,7 @@ fn zirEnumDecl( defer sema.func = prev_func; defer sema.func_index = prev_func_index; - var wip_captures = try WipCaptureScope.init(gpa, sema.perm_arena, new_decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, new_decl.src_scope); defer wip_captures.deinit(); var enum_block: Block = .{ @@ -6888,7 +6901,7 @@ fn analyzeCall( sema.error_return_trace_index_on_fn_entry = block.error_return_trace_index; defer sema.error_return_trace_index_on_fn_entry = parent_err_ret_index; - var wip_captures = try WipCaptureScope.init(gpa, sema.perm_arena, fn_owner_decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, fn_owner_decl.src_scope); defer wip_captures.deinit(); var child_block: Block = .{ @@ -7751,7 +7764,7 @@ fn resolveGenericInstantiationType( }; defer child_sema.deinit(); - var wip_captures = try WipCaptureScope.init(gpa, sema.perm_arena, new_decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, new_decl.src_scope); defer wip_captures.deinit(); var child_block: Block = .{ @@ -11151,7 +11164,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError const body = sema.code.extra[extra_index..][0..body_len]; extra_index += body_len; - var wip_captures = try WipCaptureScope.init(gpa, sema.perm_arena, child_block.wip_capture_scope); + var wip_captures = try WipCaptureScope.init(gpa, child_block.wip_capture_scope); defer wip_captures.deinit(); case_block.instructions.shrinkRetainingCapacity(0); @@ -11396,7 +11409,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError var cond_body = try case_block.instructions.toOwnedSlice(gpa); defer gpa.free(cond_body); - var wip_captures = try WipCaptureScope.init(gpa, sema.perm_arena, child_block.wip_capture_scope); + var wip_captures = try WipCaptureScope.init(gpa, child_block.wip_capture_scope); defer wip_captures.deinit(); case_block.instructions.shrinkRetainingCapacity(0); @@ -11577,7 +11590,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError }), }; - var wip_captures = try WipCaptureScope.init(gpa, sema.perm_arena, child_block.wip_capture_scope); + var wip_captures = try WipCaptureScope.init(gpa, child_block.wip_capture_scope); defer wip_captures.deinit(); case_block.instructions.shrinkRetainingCapacity(0); @@ -15720,13 +15733,15 @@ fn zirClosureCapture( // ...in which case the closure_capture instruction has access to a runtime // value only. In such case we preserve the type and use a dummy runtime value. const operand = try sema.resolveInst(inst_data.operand); - const val = (try sema.resolveMaybeUndefValAllowVariables(operand)) orelse - Value.@"unreachable"; - - try block.wip_capture_scope.captures.putNoClobber(sema.gpa, inst, .{ - .ty = sema.typeOf(operand), - .val = try val.copy(sema.perm_arena), - }); + const ty = sema.typeOf(operand); + const capture: CaptureScope.Capture = blk: { + if (try sema.resolveMaybeUndefValAllowVariables(operand)) |val| { + const ip_index = try val.intern(ty, sema.mod); + break :blk .{ .comptime_val = ip_index }; + } + break :blk .{ .runtime_val = ty.toIntern() }; + }; + try block.wip_capture_scope.captures.putNoClobber(sema.gpa, inst, capture); } fn zirClosureGet( @@ -15740,7 +15755,7 @@ fn zirClosureGet( var scope: *CaptureScope = mod.declPtr(block.src_decl).src_scope.?; // Note: The target closure must be in this scope list. // If it's not here, the zir is invalid, or the list is broken. - const tv = while (true) { + const capture = while (true) { // Note: We don't need to add a dependency here, because // decls always depend on their lexical parents. @@ -15753,13 +15768,13 @@ fn zirClosureGet( } return error.AnalysisFail; } - if (scope.captures.getPtr(inst_data.inst)) |tv| { - break tv; + if (scope.captures.get(inst_data.inst)) |capture| { + break capture; } scope = scope.parent.?; }; - if (tv.val.toIntern() == .unreachable_value and !block.is_typeof and sema.func_index == .none) { + if (capture == .runtime_val and !block.is_typeof and sema.func_index == .none) { const msg = msg: { const name = name: { const file = sema.owner_decl.getFileScope(mod); @@ -15787,7 +15802,7 @@ fn zirClosureGet( return sema.failWithOwnedErrorMsg(msg); } - if (tv.val.toIntern() == .unreachable_value and !block.is_typeof and !block.is_comptime and sema.func_index != .none) { + if (capture == .runtime_val and !block.is_typeof and !block.is_comptime and sema.func_index != .none) { const msg = msg: { const name = name: { const file = sema.owner_decl.getFileScope(mod); @@ -15817,13 +15832,17 @@ fn zirClosureGet( return sema.failWithOwnedErrorMsg(msg); } - if (tv.val.toIntern() == .unreachable_value) { - assert(block.is_typeof); - // We need a dummy runtime instruction with the correct type. - return block.addTy(.alloc, tv.ty); + switch (capture) { + .runtime_val => |ty_ip_index| { + assert(block.is_typeof); + // We need a dummy runtime instruction with the correct type. + return block.addTy(.alloc, ty_ip_index.toType()); + }, + .comptime_val => |val_ip_index| { + const ty = mod.intern_pool.typeOf(val_ip_index).toType(); + return sema.addConstant(ty, val_ip_index.toValue()); + }, } - - return sema.addConstant(tv.ty, tv.val); } fn zirRetAddr( @@ -31838,7 +31857,7 @@ fn semaBackingIntType(mod: *Module, struct_obj: *Module.Struct) CompileError!voi var sema: Sema = .{ .mod = mod, .gpa = gpa, .arena = analysis_arena.allocator(), .perm_arena = decl_arena_allocator, .code = zir, .owner_decl = decl, .owner_decl_index = decl_index, .func = null, .func_index = .none, .fn_ret_ty = Type.void, .owner_func = null, .owner_func_index = .none }; defer sema.deinit(); - var wip_captures = try WipCaptureScope.init(gpa, decl_arena_allocator, decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, decl.src_scope); defer wip_captures.deinit(); var block: Block = .{ @@ -32591,7 +32610,7 @@ fn semaStructFields(mod: *Module, struct_obj: *Module.Struct) CompileError!void }; defer sema.deinit(); - var wip_captures = try WipCaptureScope.init(gpa, decl_arena_allocator, decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, decl.src_scope); defer wip_captures.deinit(); var block_scope: Block = .{ @@ -32933,7 +32952,7 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void { }; defer sema.deinit(); - var wip_captures = try WipCaptureScope.init(gpa, decl_arena_allocator, decl.src_scope); + var wip_captures = try WipCaptureScope.init(gpa, decl.src_scope); defer wip_captures.deinit(); var block_scope: Block = .{ @@ -33360,7 +33379,7 @@ fn generateUnionTagTypeSimple( } fn getBuiltin(sema: *Sema, name: []const u8) CompileError!Air.Inst.Ref { - var wip_captures = try WipCaptureScope.init(sema.gpa, sema.perm_arena, sema.owner_decl.src_scope); + var wip_captures = try WipCaptureScope.init(sema.gpa, sema.owner_decl.src_scope); defer wip_captures.deinit(); var block: Block = .{ @@ -33405,7 +33424,7 @@ fn getBuiltin(sema: *Sema, name: []const u8) CompileError!Air.Inst.Ref { fn getBuiltinType(sema: *Sema, name: []const u8) CompileError!Type { const ty_inst = try sema.getBuiltin(name); - var wip_captures = try WipCaptureScope.init(sema.gpa, sema.perm_arena, sema.owner_decl.src_scope); + var wip_captures = try WipCaptureScope.init(sema.gpa, sema.owner_decl.src_scope); defer wip_captures.deinit(); var block: Block = .{