Commit 32edb9b55d

Andrew Kelley <andrew@ziglang.org>
2022-02-18 06:20:49
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.
1 parent dee96e2
Changed files (7)
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| {
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();
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| {
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,
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;
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);
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);
 }