From 32edb9b55d0e41a58a11e0437149c4a9705c4699 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 17 Feb 2022 22:20:49 -0700 Subject: [PATCH] stage2: eliminate ZIR arg instruction references to ZIR Prior to this commit, the AIR arg instruction kept a reference to a ZIR string index for the corresponding parameter name. This is used by DWARF emitting code. However, this is a design flaw because we want AIR objects to be independent from ZIR. This commit saves the parameter names into memory managed by `Module.Fn`. This is sub-optimal because we should be able to get the parameter names from the ZIR for a function without having them redundantly stored along with `Fn` memory. However the current way that ZIR param instructions are encoded does not support this case. They appear in the same ZIR body as the function instruction, just before it. Instead, they should be embedded within the function instruction, which will allow this TODO to be solved. That improvement is too big for this commit, however. After this there is one last dependency to untangle, which is for inline assembly. The issue for that is #10784. --- src/Air.zig | 11 ++--------- src/Module.zig | 26 +++++++++++++++++++++----- src/Sema.zig | 28 ++++++++++++++-------------- src/arch/arm/Emit.zig | 6 ++---- src/arch/riscv64/CodeGen.zig | 10 ++++------ src/arch/x86_64/Emit.zig | 10 ++++------ src/print_air.zig | 9 +-------- 7 files changed, 48 insertions(+), 52 deletions(-) diff --git a/src/Air.zig b/src/Air.zig index 623da26255433e5ac661427485a9a11fce05d0cc..586fe65c677c20fdbbe7f667427d4cc02b52ab8d 100644 --- a/src/Air.zig +++ b/src/Air.zig @@ -32,8 +32,7 @@ pub const Inst = struct { /// The first N instructions in the main block must be one arg instruction per /// function parameter. This makes function parameters participate in /// liveness analysis without any special handling. - /// Uses the `ty_str` field. - /// The string is the parameter name. + /// Uses the `ty` field. arg, /// Float or integer addition. For integers, wrapping is undefined behavior. /// Both operands are guaranteed to be the same type, and the result type @@ -615,11 +614,6 @@ pub const Inst = struct { // Index into a different array. payload: u32, }, - ty_str: struct { - ty: Ref, - // ZIR string table index. - str: u32, - }, br: struct { block_inst: Index, operand: Ref, @@ -759,8 +753,6 @@ pub fn typeOf(air: Air, inst: Air.Inst.Ref) Type { pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type { const datas = air.instructions.items(.data); switch (air.instructions.items(.tag)[inst]) { - .arg => return air.getRefType(datas[inst].ty_str.ty), - .add, .addwrap, .add_sat, @@ -827,6 +819,7 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type { .alloc, .ret_ptr, + .arg, => return datas[inst].ty, .assembly, diff --git a/src/Module.zig b/src/Module.zig index 524e8402cdc51154a8754701a6e4378a8cdb57c0..15e90ab068e221ca1d46ab72fa470ec05a9657e0 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -1370,6 +1370,14 @@ pub const Fn = struct { /// ZIR instruction. zir_body_inst: Zir.Inst.Index, + /// Prefer to use `getParamName` to access this because of the future improvement + /// we want to do mentioned in the TODO below. + /// Stored in gpa. + /// TODO: change param ZIR instructions to be embedded inside the function + /// ZIR instruction instead of before it, so that `zir_body_inst` can be used to + /// determine param names rather than redundantly storing them here. + param_names: []const [:0]const u8, + /// Relative to owner Decl. lbrace_line: u32, /// Relative to owner Decl. @@ -1466,6 +1474,18 @@ pub const Fn = struct { gpa.destroy(node); it = next; } + + for (func.param_names) |param_name| { + gpa.free(param_name); + } + gpa.free(func.param_names); + } + + pub fn getParamName(func: Fn, index: u32) [:0]const u8 { + // TODO rework ZIR of parameters so that this function looks up + // param names in ZIR instead of redundantly saving them into Fn. + // const zir = func.owner_decl.getFileScope().zir; + return func.param_names[index]; } }; @@ -4606,15 +4626,11 @@ pub fn analyzeFnBody(mod: *Module, decl: *Decl, func: *Fn, arena: Allocator) Sem runtime_param_index += 1; continue; } - const ty_ref = try sema.addType(param_type); const arg_index = @intCast(u32, sema.air_instructions.len); inner_block.instructions.appendAssumeCapacity(arg_index); sema.air_instructions.appendAssumeCapacity(.{ .tag = .arg, - .data = .{ .ty_str = .{ - .ty = ty_ref, - .str = param.name, - } }, + .data = .{ .ty = param_type }, }); sema.inst_map.putAssumeCapacityNoClobber(inst, Air.indexToRef(arg_index)); total_param_index += 1; diff --git a/src/Sema.zig b/src/Sema.zig index a153be51194a4778dce18910b0691898a529ceac..7bf1b88260b5a8f22f14c19a1aa927c5cc606a8f 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -134,6 +134,7 @@ pub const Block = struct { /// `noreturn` means `anytype`. ty: Type, is_comptime: bool, + name: []const u8, }; /// This `Block` maps a block ZIR instruction to the corresponding @@ -284,13 +285,10 @@ pub const Block = struct { }); } - fn addArg(block: *Block, ty: Type, name: u32) error{OutOfMemory}!Air.Inst.Ref { + fn addArg(block: *Block, ty: Type) error{OutOfMemory}!Air.Inst.Ref { return block.addInst(.{ .tag = .arg, - .data = .{ .ty_str = .{ - .ty = try block.sema.addType(ty), - .str = name, - } }, + .data = .{ .ty = ty }, }); } @@ -4645,7 +4643,7 @@ fn analyzeCall( } else { // We insert into the map an instruction which is runtime-known // but has the type of the argument. - const child_arg = try child_block.addArg(arg_ty, 0); + const child_arg = try child_block.addArg(arg_ty); child_sema.inst_map.putAssumeCapacityNoClobber(inst, child_arg); } } @@ -5712,6 +5710,11 @@ fn funcCommon( break :blk if (sema.comptime_args.len == 0) null else sema.comptime_args.ptr; } else null; + const param_names = try sema.gpa.alloc([:0]const u8, block.params.items.len); + for (param_names) |*param_name, i| { + param_name.* = try sema.gpa.dupeZ(u8, block.params.items[i].name); + } + const fn_payload = try sema.arena.create(Value.Payload.Function); new_func.* = .{ .state = anal_state, @@ -5722,6 +5725,7 @@ fn funcCommon( .rbrace_line = src_locs.rbrace_line, .lbrace_column = @truncate(u16, src_locs.columns), .rbrace_column = @truncate(u16, src_locs.columns >> 16), + .param_names = param_names, }; if (maybe_inferred_error_set_node) |node| { new_func.inferred_error_sets.prepend(node); @@ -5746,10 +5750,6 @@ fn zirParam( const param_name = sema.code.nullTerminatedString(extra.data.name); const body = sema.code.extra[extra.end..][0..extra.data.body_len]; - // TODO check if param_name shadows a Decl. This only needs to be done if - // usingnamespace is implemented. - _ = param_name; - // We could be in a generic function instantiation, or we could be evaluating a generic // function without any comptime args provided. const param_ty = param_ty: { @@ -5776,6 +5776,7 @@ fn zirParam( try block.params.append(sema.gpa, .{ .ty = Type.initTag(.generic_poison), .is_comptime = comptime_syntax, + .name = param_name, }); try sema.inst_map.putNoClobber(sema.gpa, inst, .generic_poison); return; @@ -5801,6 +5802,7 @@ fn zirParam( try block.params.append(sema.gpa, .{ .ty = param_ty, .is_comptime = is_comptime, + .name = param_name, }); const result = try sema.addConstant(param_ty, Value.initTag(.generic_poison)); try sema.inst_map.putNoClobber(sema.gpa, inst, result); @@ -5816,10 +5818,6 @@ fn zirParamAnytype( const src = inst_data.src(); const param_name = inst_data.get(sema.code); - // TODO check if param_name shadows a Decl. This only needs to be done if - // usingnamespace is implemented. - _ = param_name; - if (sema.inst_map.get(inst)) |air_ref| { const param_ty = sema.typeOf(air_ref); if (comptime_syntax or try sema.typeRequiresComptime(block, src, param_ty)) { @@ -5831,6 +5829,7 @@ fn zirParamAnytype( try block.params.append(sema.gpa, .{ .ty = param_ty, .is_comptime = false, + .name = param_name, }); return; } @@ -5840,6 +5839,7 @@ fn zirParamAnytype( try block.params.append(sema.gpa, .{ .ty = Type.initTag(.generic_poison), .is_comptime = comptime_syntax, + .name = param_name, }); try sema.inst_map.put(sema.gpa, inst, .generic_poison); } diff --git a/src/arch/arm/Emit.zig b/src/arch/arm/Emit.zig index 72eb4307696d3580836815abe438968886d21383..3ee70583d06f7804ebe3a270679f50281cff5922 100644 --- a/src/arch/arm/Emit.zig +++ b/src/arch/arm/Emit.zig @@ -393,11 +393,9 @@ fn addDbgInfoTypeReloc(self: *Emit, ty: Type) !void { fn genArgDbgInfo(self: *Emit, inst: Air.Inst.Index, arg_index: u32) !void { const mcv = self.function.args[arg_index]; - const ty_str = self.function.air.instructions.items(.data)[inst].ty_str; - const zir = &self.function.mod_fn.owner_decl.getFileScope().zir; - const name = zir.nullTerminatedString(ty_str.str); + const ty = self.function.air.instructions.items(.data)[inst].ty; + const name = self.function.mod_fn.getParamName(arg_index); const name_with_null = name.ptr[0 .. name.len + 1]; - const ty = self.function.air.getRefType(ty_str.ty); switch (mcv) { .register => |reg| { diff --git a/src/arch/riscv64/CodeGen.zig b/src/arch/riscv64/CodeGen.zig index 09ca92f2291003133e96542626c02f718c7e45fb..a6e15dd76c2cd188aa3c28d69625f1ee9ecc873c 100644 --- a/src/arch/riscv64/CodeGen.zig +++ b/src/arch/riscv64/CodeGen.zig @@ -1340,12 +1340,10 @@ fn airStructFieldVal(self: *Self, inst: Air.Inst.Index) !void { //return self.finishAir(inst, result, .{ extra.struct_ptr, .none, .none }); } -fn genArgDbgInfo(self: *Self, inst: Air.Inst.Index, mcv: MCValue) !void { - const ty_str = self.air.instructions.items(.data)[inst].ty_str; - const zir = &self.mod_fn.owner_decl.getFileScope().zir; - const name = zir.nullTerminatedString(ty_str.str); +fn genArgDbgInfo(self: *Self, inst: Air.Inst.Index, mcv: MCValue, arg_index: u32) !void { + const ty = self.air.instructions.items(.data)[inst].ty; + const name = self.mod_fn.getParamName(arg_index); const name_with_null = name.ptr[0 .. name.len + 1]; - const ty = self.air.getRefType(ty_str.ty); switch (mcv) { .register => |reg| { @@ -1388,7 +1386,7 @@ fn airArg(self: *Self, inst: Air.Inst.Index) !void { // TODO support stack-only arguments // TODO Copy registers to the stack const mcv = result; - try self.genArgDbgInfo(inst, mcv); + try self.genArgDbgInfo(inst, mcv, @intCast(u32, arg_index)); if (self.liveness.isUnused(inst)) return self.finishAirBookkeeping(); diff --git a/src/arch/x86_64/Emit.zig b/src/arch/x86_64/Emit.zig index 5ad3508aa2fb188cd0abbe447231a668aecbb12b..5d15b28ee8d5234120a60f06f6de8cec5b706a8c 100644 --- a/src/arch/x86_64/Emit.zig +++ b/src/arch/x86_64/Emit.zig @@ -946,15 +946,13 @@ fn mirArgDbgInfo(emit: *Emit, inst: Mir.Inst.Index) InnerError!void { const payload = emit.mir.instructions.items(.data)[inst].payload; const arg_dbg_info = emit.mir.extraData(Mir.ArgDbgInfo, payload).data; const mcv = emit.mir.function.args[arg_dbg_info.arg_index]; - try emit.genArgDbgInfo(arg_dbg_info.air_inst, mcv, arg_dbg_info.max_stack); + try emit.genArgDbgInfo(arg_dbg_info.air_inst, mcv, arg_dbg_info.max_stack, arg_dbg_info.arg_index); } -fn genArgDbgInfo(emit: *Emit, inst: Air.Inst.Index, mcv: MCValue, max_stack: u32) !void { - const ty_str = emit.mir.function.air.instructions.items(.data)[inst].ty_str; - const zir = &emit.mir.function.mod_fn.owner_decl.getFileScope().zir; - const name = zir.nullTerminatedString(ty_str.str); +fn genArgDbgInfo(emit: *Emit, inst: Air.Inst.Index, mcv: MCValue, max_stack: u32, arg_index: u32) !void { + const ty = emit.mir.function.air.instructions.items(.data)[inst].ty; + const name = emit.mir.function.mod_fn.getParamName(arg_index); const name_with_null = name.ptr[0 .. name.len + 1]; - const ty = emit.mir.function.air.getRefType(ty_str.ty); switch (mcv) { .register => |reg| { diff --git a/src/print_air.zig b/src/print_air.zig index bcadf31e74666872d92413d40d539c8ca578605a..7270f626021a3fa1fbd9182a4d62aa2bffd0aeb2 100644 --- a/src/print_air.zig +++ b/src/print_air.zig @@ -100,8 +100,6 @@ const Writer = struct { const tag = tags[inst]; try s.print("= {s}(", .{@tagName(tags[inst])}); switch (tag) { - .arg => try w.writeTyStr(s, inst), - .add, .addwrap, .add_sat, @@ -181,6 +179,7 @@ const Writer = struct { .const_ty, .alloc, .ret_ptr, + .arg, => try w.writeTy(s, inst), .not, @@ -257,12 +256,6 @@ const Writer = struct { } } - fn writeTyStr(w: *Writer, s: anytype, inst: Air.Inst.Index) @TypeOf(s).Error!void { - const ty_str = w.air.instructions.items(.data)[inst].ty_str; - const name = w.zir.nullTerminatedString(ty_str.str); - try s.print("\"{}\", {}", .{ std.zig.fmtEscapes(name), w.air.getRefType(ty_str.ty) }); - } - fn writeBinOp(w: *Writer, s: anytype, inst: Air.Inst.Index) @TypeOf(s).Error!void { const bin_op = w.air.instructions.items(.data)[inst].bin_op; try w.writeOperand(s, inst, 0, bin_op.lhs); -- 2.54.0