From 8c6175c1343a00278efc029a0be4091ff505dc3d Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 6 Jan 2022 00:52:10 -0700 Subject: [PATCH] Sema: const inferred alloc infers comptime-ness const locals now detect if the value ends up being comptime known. In such case, it replaces the runtime AIR instructions with a decl_ref const. In the backends, some more sophisticated logic for marking decls as alive was needed to prevent Decls incorrectly being garbage collected that were indirectly referenced in such manner. --- src/Sema.zig | 50 +++++++++++++++++++++++++++++++++ src/arch/wasm/CodeGen.zig | 17 +++++++++-- src/codegen.zig | 15 +++++++++- src/codegen/c.zig | 15 +++++++++- src/codegen/llvm.zig | 27 ++++++++++++++---- test/behavior/struct_llvm.zig | 20 +++++++++++++ test/behavior/struct_stage1.zig | 19 ------------- 7 files changed, 135 insertions(+), 28 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 234898b8fc..80231ac3cb 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -2427,7 +2427,57 @@ fn zirResolveInferredAlloc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Com if (var_is_mut) { try sema.validateVarType(block, ty_src, final_elem_ty, false); + } else ct: { + // Detect if the value is comptime known. In such case, the + // last 3 AIR instructions of the block will look like this: + // + // %a = constant + // %b = bitcast(%a) + // %c = store(%b, %d) + // + // If `%d` is comptime-known, then we want to store the value + // inside an anonymous Decl and then erase these three AIR + // instructions from the block, replacing the inst_map entry + // corresponding to the ZIR alloc instruction with a constant + // decl_ref pointing at our new Decl. + if (block.instructions.items.len < 3) break :ct; + // zig fmt: off + const const_inst = block.instructions.items[block.instructions.items.len - 3]; + const bitcast_inst = block.instructions.items[block.instructions.items.len - 2]; + const store_inst = block.instructions.items[block.instructions.items.len - 1]; + const air_tags = sema.air_instructions.items(.tag); + const air_datas = sema.air_instructions.items(.data); + if (air_tags[const_inst] != .constant) break :ct; + if (air_tags[bitcast_inst] != .bitcast ) break :ct; + if (air_tags[store_inst] != .store ) break :ct; + // zig fmt: on + const store_op = air_datas[store_inst].bin_op; + const store_val = (try sema.resolveMaybeUndefVal(block, src, store_op.rhs)) orelse break :ct; + if (store_op.lhs != Air.indexToRef(bitcast_inst)) break :ct; + if (air_datas[bitcast_inst].ty_op.operand != Air.indexToRef(const_inst)) break :ct; + + const bitcast_ty_ref = air_datas[bitcast_inst].ty_op.ty; + + const new_decl = d: { + var anon_decl = try block.startAnonDecl(); + defer anon_decl.deinit(); + const new_decl = try anon_decl.finish( + try final_elem_ty.copy(anon_decl.arena()), + try store_val.copy(anon_decl.arena()), + ); + break :d new_decl; + }; + try sema.mod.declareDeclDependency(sema.owner_decl, new_decl); + + // Even though we reuse the constant instruction, we still remove it from the + // block so that codegen does not see it. + block.instructions.shrinkRetainingCapacity(block.instructions.items.len - 3); + sema.air_values.items[value_index] = try Value.Tag.decl_ref.create(sema.arena, new_decl); + air_datas[ptr_inst].ty_pl.ty = bitcast_ty_ref; + + return; } + // Change it to a normal alloc. const final_ptr_ty = try Type.ptr(sema.arena, .{ .pointee_type = final_elem_ty, diff --git a/src/arch/wasm/CodeGen.zig b/src/arch/wasm/CodeGen.zig index de283306dd..fc93eb9dc8 100644 --- a/src/arch/wasm/CodeGen.zig +++ b/src/arch/wasm/CodeGen.zig @@ -1047,7 +1047,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; - decl.alive = true; + markDeclAlive(decl); 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, @@ -1876,7 +1876,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; - decl.alive = true; + markDeclAlive(decl); // 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; @@ -1985,6 +1985,19 @@ 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 9de13ee657..e385158ba6 100644 --- a/src/codegen.zig +++ b/src/codegen.zig @@ -464,7 +464,7 @@ fn lowerDeclRef( } if (decl.analysis != .complete) return error.AnalysisFail; - decl.alive = true; + markDeclAlive(decl); // TODO handle the dependency of this symbol on the decl's vaddr. // If the decl changes vaddr, then this symbol needs to get regenerated. const vaddr = bin_file.getDeclVAddr(decl); @@ -478,3 +478,16 @@ 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 191be36494..922e1d9c3e 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -195,7 +195,7 @@ pub const DeclGen = struct { val: Value, decl: *Decl, ) error{ OutOfMemory, AnalysisFail }!void { - decl.alive = true; + markDeclAlive(decl); if (ty.isSlice()) { try writer.writeByte('('); @@ -227,6 +227,19 @@ 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 070c667e6b..4bf719f4c0 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -749,7 +749,6 @@ pub const DeclGen = struct { fn llvmType(dg: *DeclGen, t: Type) Error!*const llvm.Type { const gpa = dg.gpa; - log.debug("llvmType for {}", .{t}); switch (t.zigTypeTag()) { .Void, .NoReturn => return dg.context.voidType(), .Int => { @@ -1168,7 +1167,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; - decl.alive = true; + dg.markDeclAlive(decl); const val = try dg.resolveGlobalDecl(decl); const llvm_var_type = try dg.llvmType(tv.ty); const llvm_addrspace = dg.llvmAddressSpace(decl.@"addrspace"); @@ -1317,7 +1316,7 @@ pub const DeclGen = struct { .function => tv.val.castTag(.function).?.data.owner_decl, else => unreachable, }; - fn_decl.alive = true; + dg.markDeclAlive(fn_decl); return dg.resolveLlvmFunction(fn_decl); }, .ErrorSet => { @@ -1625,7 +1624,7 @@ pub const DeclGen = struct { ptr_val: Value, decl: *Module.Decl, ) Error!ParentPtr { - decl.alive = true; + dg.markDeclAlive(decl); var ptr_ty_payload: Type.Payload.ElemType = .{ .base = .{ .tag = .single_mut_pointer }, .data = decl.ty, @@ -1707,7 +1706,7 @@ pub const DeclGen = struct { return self.lowerPtrToVoid(tv.ty); } - decl.alive = true; + self.markDeclAlive(decl); const llvm_val = if (decl.ty.zigTypeTag() == .Fn) try self.resolveLlvmFunction(decl) @@ -1718,6 +1717,24 @@ 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/test/behavior/struct_llvm.zig b/test/behavior/struct_llvm.zig index 36310d797b..a6bd9170f3 100644 --- a/test/behavior/struct_llvm.zig +++ b/test/behavior/struct_llvm.zig @@ -285,3 +285,23 @@ fn getB(data: *const BitField1) u3 { fn getC(data: *const BitField1) u2 { return data.c; } + +test "default struct initialization fields" { + const S = struct { + a: i32 = 1234, + b: i32, + }; + const x = S{ + .b = 5, + }; + var five: i32 = 5; + const y = S{ + .b = five, + }; + if (x.a + x.b != 1239) { + @compileError("it should be comptime known"); + } + try expect(y.a == x.a); + try expect(y.b == x.b); + try expect(1239 == x.a + x.b); +} diff --git a/test/behavior/struct_stage1.zig b/test/behavior/struct_stage1.zig index 66549d1767..f079302f43 100644 --- a/test/behavior/struct_stage1.zig +++ b/test/behavior/struct_stage1.zig @@ -166,25 +166,6 @@ test "packed struct with fp fields" { try expectEqual(@as(f32, 20.0), s.data[2]); } -test "default struct initialization fields" { - const S = struct { - a: i32 = 1234, - b: i32, - }; - const x = S{ - .b = 5, - }; - var five: i32 = 5; - const y = S{ - .b = five, - }; - if (x.a + x.b != 1239) { - @compileError("it should be comptime known"); - } - try expectEqual(y, x); - try expectEqual(1239, x.a + x.b); -} - test "fn with C calling convention returns struct by value" { const S = struct { fn entry() !void {