From a5c7742ba6fc793608b8bb7ba058e33eccd9cfec Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 15 Jan 2022 00:17:25 -0700 Subject: [PATCH] stage2: fix Decl garbage collection not marking enough It is the job of codegen backends to mark Decls that are referenced as alive so that the frontend does not sweep them with the garbage. This commit unifies the code between the backends with an added method on Decl. The implementation is more complete than before, switching on the Decl val tag and recursing into sub-values. As a result, two more array tests are passing. --- src/Module.zig | 10 ++++ src/arch/wasm/CodeGen.zig | 17 +------ src/codegen.zig | 15 +----- src/codegen/c.zig | 15 +----- src/codegen/llvm.zig | 26 ++-------- src/value.zig | 49 ++++++++++++++++++ test/behavior/array_llvm.zig | 79 +++++++++++++++++++++++++++++ test/behavior/array_stage1.zig | 90 +++------------------------------- 8 files changed, 152 insertions(+), 149 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index e93fe2549c..a464ac83de 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -783,6 +783,16 @@ pub const Decl = struct { return decl.ty.abiAlignment(target); } } + + pub fn markAlive(decl: *Decl) void { + if (decl.alive) return; + decl.alive = true; + + // This is the first time we are marking this Decl alive. We must + // therefore recurse into its value and mark any Decl it references + // as also alive, so that any Decl referenced does not get garbage collected. + decl.val.markReferencedDeclsAlive(); + } }; /// This state is attached to every Decl when Module emit_h is non-null. diff --git a/src/arch/wasm/CodeGen.zig b/src/arch/wasm/CodeGen.zig index 97b43b3067..7a8efdfcc2 100644 --- a/src/arch/wasm/CodeGen.zig +++ b/src/arch/wasm/CodeGen.zig @@ -1037,7 +1037,7 @@ fn lowerDeclRef(self: *Self, ty: Type, val: Value, decl: *Module.Decl) InnerErro const offset = @intCast(u32, self.code.items.len); const atom = &self.decl.link.wasm; const target_sym_index = decl.link.wasm.sym_index; - markDeclAlive(decl); + decl.markAlive(); if (decl.ty.zigTypeTag() == .Fn) { // We found a function pointer, so add it to our table, // as function pointers are not allowed to be stored inside the data section, @@ -1935,7 +1935,7 @@ fn emitConstant(self: *Self, val: Value, ty: Type) InnerError!void { try self.emitConstant(slice.data.len, Type.usize); } else if (val.castTag(.decl_ref)) |payload| { const decl = payload.data; - markDeclAlive(decl); + decl.markAlive(); // Function pointers use a table index, rather than a memory address if (decl.ty.zigTypeTag() == .Fn) { const target_sym_index = decl.link.wasm.sym_index; @@ -2101,19 +2101,6 @@ fn emitConstant(self: *Self, val: Value, ty: Type) InnerError!void { } } -fn markDeclAlive(decl: *Decl) void { - if (decl.alive) return; - decl.alive = true; - - // This is the first time we are marking this Decl alive. We must - // therefore recurse into its value and mark any Decl it references - // as also alive, so that any Decl referenced does not get garbage collected. - - if (decl.val.pointerDecl()) |pointee| { - return markDeclAlive(pointee); - } -} - fn emitUndefined(self: *Self, ty: Type) InnerError!void { switch (ty.zigTypeTag()) { .Int => switch (ty.intInfo(self.target).bits) { diff --git a/src/codegen.zig b/src/codegen.zig index 1d20c4bc74..65b0318e9e 100644 --- a/src/codegen.zig +++ b/src/codegen.zig @@ -464,7 +464,7 @@ fn lowerDeclRef( } if (decl.analysis != .complete) return error.AnalysisFail; - markDeclAlive(decl); + decl.markAlive(); const vaddr = vaddr: { if (bin_file.cast(link.File.MachO)) |macho_file| { break :vaddr try macho_file.getDeclVAddrWithReloc(decl, code.items.len); @@ -484,16 +484,3 @@ fn lowerDeclRef( return Result{ .appended = {} }; } - -fn markDeclAlive(decl: *Module.Decl) void { - if (decl.alive) return; - decl.alive = true; - - // This is the first time we are marking this Decl alive. We must - // therefore recurse into its value and mark any Decl it references - // as also alive, so that any Decl referenced does not get garbage collected. - - if (decl.val.pointerDecl()) |pointee| { - return markDeclAlive(pointee); - } -} diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 58bf919b1f..3f3147ca80 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -215,7 +215,7 @@ pub const DeclGen = struct { val: Value, decl: *Decl, ) error{ OutOfMemory, AnalysisFail }!void { - markDeclAlive(decl); + decl.markAlive(); if (ty.isSlice()) { try writer.writeByte('('); @@ -253,19 +253,6 @@ pub const DeclGen = struct { try dg.renderDeclName(decl, writer); } - fn markDeclAlive(decl: *Decl) void { - if (decl.alive) return; - decl.alive = true; - - // This is the first time we are marking this Decl alive. We must - // therefore recurse into its value and mark any Decl it references - // as also alive, so that any Decl referenced does not get garbage collected. - - if (decl.val.pointerDecl()) |pointee| { - return markDeclAlive(pointee); - } - } - fn renderInt128( writer: anytype, int_val: anytype, diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 7858fa76fa..d8c9589213 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -1224,7 +1224,7 @@ pub const DeclGen = struct { .decl_ref => return lowerDeclRefValue(dg, tv, tv.val.castTag(.decl_ref).?.data), .variable => { const decl = tv.val.castTag(.variable).?.data.owner_decl; - dg.markDeclAlive(decl); + decl.markAlive(); const val = try dg.resolveGlobalDecl(decl); const llvm_var_type = try dg.llvmType(tv.ty); const llvm_addrspace = dg.llvmAddressSpace(decl.@"addrspace"); @@ -1373,7 +1373,7 @@ pub const DeclGen = struct { .function => tv.val.castTag(.function).?.data.owner_decl, else => unreachable, }; - dg.markDeclAlive(fn_decl); + fn_decl.markAlive(); return dg.resolveLlvmFunction(fn_decl); }, .ErrorSet => { @@ -1681,7 +1681,7 @@ pub const DeclGen = struct { ptr_val: Value, decl: *Module.Decl, ) Error!ParentPtr { - dg.markDeclAlive(decl); + decl.markAlive(); var ptr_ty_payload: Type.Payload.ElemType = .{ .base = .{ .tag = .single_mut_pointer }, .data = decl.ty, @@ -1763,7 +1763,7 @@ pub const DeclGen = struct { return self.lowerPtrToVoid(tv.ty); } - self.markDeclAlive(decl); + decl.markAlive(); const llvm_val = if (decl.ty.zigTypeTag() == .Fn) try self.resolveLlvmFunction(decl) @@ -1774,24 +1774,6 @@ pub const DeclGen = struct { return llvm_val.constBitCast(llvm_type); } - fn markDeclAlive(dg: *DeclGen, decl: *Module.Decl) void { - if (decl.alive) return; - decl.alive = true; - - log.debug("{*} ({s}) marked alive by {*} ({s})", .{ - decl, decl.name, - dg.decl, dg.decl.name, - }); - - // This is the first time we are marking this Decl alive. We must - // therefore recurse into its value and mark any Decl it references - // as also alive, so that any Decl referenced does not get garbage collected. - - if (decl.val.pointerDecl()) |pointee| { - return dg.markDeclAlive(pointee); - } - } - fn lowerPtrToVoid(dg: *DeclGen, ptr_ty: Type) !*const llvm.Value { const target = dg.module.getTarget(); const alignment = ptr_ty.ptrAlignment(target); diff --git a/src/value.zig b/src/value.zig index 1993a853cd..1836a14ce0 100644 --- a/src/value.zig +++ b/src/value.zig @@ -1746,6 +1746,55 @@ pub const Value = extern union { }; } + pub fn markReferencedDeclsAlive(val: Value) void { + switch (val.tag()) { + .decl_ref_mut => return val.castTag(.decl_ref_mut).?.data.decl.markAlive(), + .extern_fn, .decl_ref => return val.cast(Payload.Decl).?.data.markAlive(), + .function => return val.castTag(.function).?.data.owner_decl.markAlive(), + .variable => return val.castTag(.variable).?.data.owner_decl.markAlive(), + + .repeated, + .eu_payload, + .eu_payload_ptr, + .opt_payload, + .opt_payload_ptr, + .empty_array_sentinel, + => return markReferencedDeclsAlive(val.cast(Payload.SubValue).?.data), + + .array => { + for (val.cast(Payload.Array).?.data) |elem_val| { + markReferencedDeclsAlive(elem_val); + } + }, + .slice => { + const slice = val.cast(Payload.Slice).?.data; + markReferencedDeclsAlive(slice.ptr); + markReferencedDeclsAlive(slice.len); + }, + + .elem_ptr => { + const elem_ptr = val.cast(Payload.ElemPtr).?.data; + return markReferencedDeclsAlive(elem_ptr.array_ptr); + }, + .field_ptr => { + const field_ptr = val.cast(Payload.FieldPtr).?.data; + return markReferencedDeclsAlive(field_ptr.container_ptr); + }, + .@"struct" => { + for (val.cast(Payload.Struct).?.data) |field_val| { + markReferencedDeclsAlive(field_val); + } + }, + .@"union" => { + const data = val.cast(Payload.Union).?.data; + markReferencedDeclsAlive(data.tag); + markReferencedDeclsAlive(data.val); + }, + + else => {}, + } + } + pub fn slicePtr(val: Value) Value { return switch (val.tag()) { .slice => val.castTag(.slice).?.data.ptr, diff --git a/test/behavior/array_llvm.zig b/test/behavior/array_llvm.zig index a828954b73..f1a57d9c29 100644 --- a/test/behavior/array_llvm.zig +++ b/test/behavior/array_llvm.zig @@ -1,6 +1,7 @@ const std = @import("std"); const testing = std.testing; const expect = testing.expect; +const mem = std.mem; var s_array: [8]Sub = undefined; const Sub = struct { b: u8 }; @@ -87,3 +88,81 @@ test "array literal as argument to function" { try S.entry(2); comptime try S.entry(2); } + +test "double nested array to const slice cast in array literal" { + const S = struct { + fn entry(two: i32) !void { + const cases = [_][]const []const i32{ + &[_][]const i32{&[_]i32{1}}, + &[_][]const i32{&[_]i32{ 2, 3 }}, + &[_][]const i32{ + &[_]i32{4}, + &[_]i32{ 5, 6, 7 }, + }, + }; + try check(&cases); + + const cases2 = [_][]const i32{ + &[_]i32{1}, + &[_]i32{ two, 3 }, + }; + try expect(cases2.len == 2); + try expect(cases2[0].len == 1); + try expect(cases2[0][0] == 1); + try expect(cases2[1].len == 2); + try expect(cases2[1][0] == 2); + try expect(cases2[1][1] == 3); + + const cases3 = [_][]const []const i32{ + &[_][]const i32{&[_]i32{1}}, + &[_][]const i32{&[_]i32{ two, 3 }}, + &[_][]const i32{ + &[_]i32{4}, + &[_]i32{ 5, 6, 7 }, + }, + }; + try check(&cases3); + } + + fn check(cases: []const []const []const i32) !void { + try expect(cases.len == 3); + try expect(cases[0].len == 1); + try expect(cases[0][0].len == 1); + try expect(cases[0][0][0] == 1); + try expect(cases[1].len == 1); + try expect(cases[1][0].len == 2); + try expect(cases[1][0][0] == 2); + try expect(cases[1][0][1] == 3); + try expect(cases[2].len == 2); + try expect(cases[2][0].len == 1); + try expect(cases[2][0][0] == 4); + try expect(cases[2][1].len == 3); + try expect(cases[2][1][0] == 5); + try expect(cases[2][1][1] == 6); + try expect(cases[2][1][2] == 7); + } + }; + try S.entry(2); + comptime try S.entry(2); +} + +test "anonymous literal in array" { + const S = struct { + const Foo = struct { + a: usize = 2, + b: usize = 4, + }; + fn doTheTest() !void { + var array: [2]Foo = .{ + .{ .a = 3 }, + .{ .b = 3 }, + }; + try expect(array[0].a == 3); + try expect(array[0].b == 4); + try expect(array[1].a == 2); + try expect(array[1].b == 3); + } + }; + try S.doTheTest(); + comptime try S.doTheTest(); +} diff --git a/test/behavior/array_stage1.zig b/test/behavior/array_stage1.zig index daef20ffb5..5c37f34de6 100644 --- a/test/behavior/array_stage1.zig +++ b/test/behavior/array_stage1.zig @@ -4,84 +4,6 @@ const mem = std.mem; const expect = testing.expect; const expectEqual = testing.expectEqual; -test "double nested array to const slice cast in array literal" { - const S = struct { - fn entry(two: i32) !void { - const cases = [_][]const []const i32{ - &[_][]const i32{&[_]i32{1}}, - &[_][]const i32{&[_]i32{ 2, 3 }}, - &[_][]const i32{ - &[_]i32{4}, - &[_]i32{ 5, 6, 7 }, - }, - }; - try check(&cases); - - const cases2 = [_][]const i32{ - &[_]i32{1}, - &[_]i32{ two, 3 }, - }; - try expect(cases2.len == 2); - try expect(cases2[0].len == 1); - try expect(cases2[0][0] == 1); - try expect(cases2[1].len == 2); - try expect(cases2[1][0] == 2); - try expect(cases2[1][1] == 3); - - const cases3 = [_][]const []const i32{ - &[_][]const i32{&[_]i32{1}}, - &[_][]const i32{&[_]i32{ two, 3 }}, - &[_][]const i32{ - &[_]i32{4}, - &[_]i32{ 5, 6, 7 }, - }, - }; - try check(&cases3); - } - - fn check(cases: []const []const []const i32) !void { - try expect(cases.len == 3); - try expect(cases[0].len == 1); - try expect(cases[0][0].len == 1); - try expect(cases[0][0][0] == 1); - try expect(cases[1].len == 1); - try expect(cases[1][0].len == 2); - try expect(cases[1][0][0] == 2); - try expect(cases[1][0][1] == 3); - try expect(cases[2].len == 2); - try expect(cases[2][0].len == 1); - try expect(cases[2][0][0] == 4); - try expect(cases[2][1].len == 3); - try expect(cases[2][1][0] == 5); - try expect(cases[2][1][1] == 6); - try expect(cases[2][1][2] == 7); - } - }; - try S.entry(2); - comptime try S.entry(2); -} - -test "anonymous literal in array" { - const S = struct { - const Foo = struct { - a: usize = 2, - b: usize = 4, - }; - fn doTheTest() !void { - var array: [2]Foo = .{ - .{ .a = 3 }, - .{ .b = 3 }, - }; - try expect(array[0].a == 3); - try expect(array[0].b == 4); - try expect(array[1].a == 2); - try expect(array[1].b == 3); - } - }; - try S.doTheTest(); - comptime try S.doTheTest(); -} - test "access the null element of a null terminated array" { const S = struct { fn doTheTest() !void { @@ -100,9 +22,9 @@ test "type deduction for array subscript expression" { fn doTheTest() !void { var array = [_]u8{ 0x55, 0xAA }; var v0 = true; - try expectEqual(@as(u8, 0xAA), array[if (v0) 1 else 0]); + try expect(@as(u8, 0xAA) == array[if (v0) 1 else 0]); var v1 = false; - try expectEqual(@as(u8, 0x55), array[if (v1) 1 else 0]); + try expect(@as(u8, 0x55) == array[if (v1) 1 else 0]); } }; try S.doTheTest(); @@ -119,9 +41,9 @@ test "sentinel element count towards the ABI size calculation" { }; var x = T{}; var as_slice = mem.asBytes(&x); - try expectEqual(@as(usize, 3), as_slice.len); - try expectEqual(@as(u8, 0x55), as_slice[0]); - try expectEqual(@as(u8, 0xAA), as_slice[2]); + try expect(@as(usize, 3) == as_slice.len); + try expect(@as(u8, 0x55) == as_slice[0]); + try expect(@as(u8, 0xAA) == as_slice[2]); } }; @@ -144,7 +66,7 @@ test "zero-sized array with recursive type definition" { }; var t: S = .{ .list = .{ .s = undefined } }; - try expectEqual(@as(usize, 0), t.list.x); + try expect(@as(usize, 0) == t.list.x); } test "type coercion of anon struct literal to array" {