Commit b215241ef2153bf4b20f176923824e0e50d6da4c | zig

author	Jakub Konka <kubkon@jakubkonka.com> 2022-01-05 23:19:58+01:00
committer	Jakub Konka <kubkon@jakubkonka.com> 2022-01-06 09:54:33+01:00
log	b215241ef2153bf4b20f176923824e0e50d6da4c
tree	c4a68eb39e9086c687486205a2efdeda0dfe0789
parent	f4aa276a0c665633980ea49bccb29da9d02d2ae1

stage2: fix airSliceElemVal

Refactor codegen and fix a bug in Isel.

4 files changed, 180 insertions(+), 130 deletions(-)

src
- arch
  - x86_64
    CodeGen.zig
    Isel.zig
    Mir.zig
test
- stage2
  - x86_64.zig

src/arch/x86_64/CodeGen.zig+105-94

 @@ -1318,37 +1318,38 @@ fn airSliceElemVal(self: *Self, inst: Air.Inst.Index) !void {
         var buf: Type.SlicePtrFieldTypeBuffer = undefined;
         const slice_ptr_field_type = slice_ty.slicePtrFieldType(&buf);
-        const index_ty = self.air.typeOf(bin_op.rhs);
+        const offset_reg = blk: {
-        const index_mcv: MCValue = blk: {
+            const index_ty = self.air.typeOf(bin_op.rhs);
-            switch (try self.resolveInst(bin_op.rhs)) {
+            const index_mcv = try self.resolveInst(bin_op.rhs);
-                .register => |reg| {
+            const offset_reg = try self.register_manager.allocReg(null, &.{});
-                    if (reg.to64() != .rcx) {
+            try self.genSetReg(index_ty, offset_reg, index_mcv);
-                        try self.register_manager.getReg(.rcx, inst);
+            try self.genIMulOpMir(index_ty, .{ .register = offset_reg }, .{ .immediate = elem_size });
-                    }
+            break :blk offset_reg;
-                    break :blk MCValue{ .register = .rcx };
-                },
-                else => return self.fail("TODO move index mcv into a register", .{}),
-            }
         };
-        try self.genIMulOpMir(index_ty, index_mcv, .{ .immediate = elem_size });
         const dst_mcv = blk: {
             switch (slice_mcv) {
-                .stack_offset => |unadjusted_off| {
+                .stack_offset => |off| {
                     const dst_mcv = try self.allocRegOrMem(inst, false);
-                    const addr_reg = try self.register_manager.allocReg(null, &.{index_mcv.register});
+                    const addr_reg = try self.register_manager.allocReg(null, &.{offset_reg});
-                    const slice_ptr_abi_size = @intCast(u32, slice_ptr_field_type.abiSize(self.target.*));
+                    // mov reg, [rbp - 8]
-                    const off = unadjusted_off + elem_size;
-                    // lea reg, [rbp - 8 + rcx*1]
                     _ = try self.addInst(.{
-                        .tag = .lea,
+                        .tag = .mov,
                         .ops = (Mir.Ops{
-                            .reg1 = registerAlias(addr_reg, slice_ptr_abi_size),
+                            .reg1 = addr_reg.to64(),
                             .reg2 = .rbp,
-                            .flags = 0b11,
+                            .flags = 0b01,
+                        }).encode(),
+                        .data = .{ .imm = -@intCast(i32, off + 16) },
+                    });
+                    // add addr, offset
+                    _ = try self.addInst(.{
+                        .tag = .add,
+                        .ops = (Mir.Ops{
+                            .reg1 = addr_reg.to64(),
+                            .reg2 = offset_reg.to64(),
                         }).encode(),
-                        .data = .{ .imm = -@intCast(i32, off) },
+                        .data = undefined,
                     });
                     try self.load(dst_mcv, .{ .register = addr_reg }, slice_ptr_field_type);
                     break :blk dst_mcv;
 @@ -1528,7 +1529,7 @@ fn load(self: *Self, dst_mcv: MCValue, ptr: MCValue, ptr_ty: Type) InnerError!vo
                         &.{ reg, .rax, .rcx },
                     );
                     const addr_reg = regs[0];
-                    const len_reg = regs[1];
+                    const count_reg = regs[1];
                     const tmp_reg = regs[2];
                     _ = try self.addInst(.{
 @@ -1544,12 +1545,12 @@ fn load(self: *Self, dst_mcv: MCValue, ptr: MCValue, ptr_ty: Type) InnerError!vo
                     try self.register_manager.getReg(.rcx, null);
                     // TODO allow for abi size to be u64
-                    try self.genSetReg(Type.initTag(.u32), len_reg, .{ .immediate = @intCast(u32, abi_size) });
+                    try self.genSetReg(Type.initTag(.u32), count_reg, .{ .immediate = @intCast(u32, abi_size) });
                     return self.genInlineMemcpy(
                         -@intCast(i32, off + abi_size),
                         registerAlias(addr_reg, @divExact(reg.size(), 8)),
-                        len_reg.to64(),
+                        count_reg.to64(),
                         tmp_reg.to8(),
                     );
                 },
 @@ -1592,11 +1593,8 @@ fn airLoad(self: *Self, inst: Air.Inst.Index) !void {
     return self.finishAir(inst, result, .{ ty_op.operand, .none, .none });
 }
-fn airStore(self: *Self, inst: Air.Inst.Index) !void {
+fn store(self: *Self, ptr: MCValue, value: MCValue, ptr_ty: Type, value_ty: Type) InnerError!void {
-    const bin_op = self.air.instructions.items(.data)[inst].bin_op;
+    _ = ptr_ty;
-    const ptr = try self.resolveInst(bin_op.lhs);
-    const value = try self.resolveInst(bin_op.rhs);
-    const elem_ty = self.air.typeOf(bin_op.rhs);
     switch (ptr) {
         .none => unreachable,
         .undef => unreachable,
 @@ -1605,19 +1603,58 @@ fn airStore(self: *Self, inst: Air.Inst.Index) !void {
         .compare_flags_unsigned => unreachable,
         .compare_flags_signed => unreachable,
         .immediate => |imm| {
-            try self.setRegOrMem(elem_ty, .{ .memory = imm }, value);
+            try self.setRegOrMem(value_ty, .{ .memory = imm }, value);
         },
         .ptr_stack_offset => |off| {
-            try self.genSetStack(elem_ty, off, value);
+            try self.genSetStack(value_ty, off, value);
         },
         .ptr_embedded_in_code => |off| {
-            try self.setRegOrMem(elem_ty, .{ .embedded_in_code = off }, value);
+            try self.setRegOrMem(value_ty, .{ .embedded_in_code = off }, value);
         },
         .embedded_in_code => {
             return self.fail("TODO implement storing to MCValue.embedded_in_code", .{});
         },
         .register => |reg| {
-            try self.genSetPtrReg(elem_ty, reg, value);
+            switch (value) {
+                .none => unreachable,
+                .undef => unreachable,
+                .dead => unreachable,
+                .unreach => unreachable,
+                .compare_flags_unsigned => unreachable,
+                .compare_flags_signed => unreachable,
+                .immediate => |imm| {
+                    const abi_size = value_ty.abiSize(self.target.*);
+                    switch (abi_size) {
+                        1, 2, 4 => {
+                            // TODO this is wasteful!
+                            // introduce new MIR tag specifically for mov [reg + 0], imm
+                            const payload = try self.addExtra(Mir.ImmPair{
+                                .dest_off = 0,
+                                .operand = @bitCast(i32, @intCast(u32, imm)),
+                            });
+                            _ = try self.addInst(.{
+                                .tag = .mov_mem_imm,
+                                .ops = (Mir.Ops{
+                                    .reg1 = reg.to64(),
+                                    .flags = switch (abi_size) {
+                                        1 => 0b00,
+                                        2 => 0b01,
+                                        4 => 0b10,
+                                        else => unreachable,
+                                    },
+                                }).encode(),
+                                .data = .{ .payload = payload },
+                            });
+                        },
+                        else => {
+                            return self.fail("TODO implement set pointee with immediate of ABI size {d}", .{abi_size});
+                        },
+                    }
+                },
+                else => |other| {
+                    return self.fail("TODO implement set pointee with {}", .{other});
+                },
+            }
         },
         .memory => {
             return self.fail("TODO implement storing to MCValue.memory", .{});
 @@ -1626,6 +1663,15 @@ fn airStore(self: *Self, inst: Air.Inst.Index) !void {
             return self.fail("TODO implement storing to MCValue.stack_offset", .{});
         },
     }
+}
+fn airStore(self: *Self, inst: Air.Inst.Index) !void {
+    const bin_op = self.air.instructions.items(.data)[inst].bin_op;
+    const ptr = try self.resolveInst(bin_op.lhs);
+    const ptr_ty = self.air.typeOf(bin_op.lhs);
+    const value = try self.resolveInst(bin_op.rhs);
+    const value_ty = self.air.typeOf(bin_op.rhs);
+    try self.store(ptr, value, ptr_ty, value_ty);
     return self.finishAir(inst, .dead, .{ bin_op.lhs, bin_op.rhs, .none });
 }
 @@ -1653,9 +1699,8 @@ fn structFieldPtr(self: *Self, inst: Air.Inst.Index, operand: Air.Inst.Ref, inde
         switch (mcv) {
             .ptr_stack_offset => |off| {
-                break :result MCValue{
+                const ptr_stack_offset = off - struct_size + struct_field_offset + struct_field_size;
-                    .ptr_stack_offset = off + struct_size - struct_field_offset - struct_field_size,
+                break :result MCValue{ .ptr_stack_offset = ptr_stack_offset };
-                };
             },
             else => return self.fail("TODO implement codegen struct_field_ptr for {}", .{mcv}),
         }
 @@ -1677,9 +1722,8 @@ fn airStructFieldVal(self: *Self, inst: Air.Inst.Index) !void {
         switch (mcv) {
             .stack_offset => |off| {
-                break :result MCValue{
+                const stack_offset = off - struct_size + struct_field_offset + struct_field_size;
-                    .stack_offset = off + struct_size - struct_field_offset - struct_field_size,
+                break :result MCValue{ .stack_offset = stack_offset };
-                };
             },
             else => return self.fail("TODO implement codegen struct_field_val for {}", .{mcv}),
         }
 @@ -1956,7 +2000,7 @@ fn genIMulOpMir(self: *Self, dst_ty: Type, dst_mcv: MCValue, src_mcv: MCValue) !
                 .immediate => |imm| {
                     // TODO take into account the type's ABI size when selecting the register alias
                     // register, immediate
-                    if (imm <= math.maxInt(i32)) {
+                    if (math.minInt(i32) <= imm and imm <= math.maxInt(i32)) {
                         _ = try self.addInst(.{
                             .tag = .imul_complex,
                             .ops = (Mir.Ops{
 @@ -3166,7 +3210,7 @@ fn genSetStack(self: *Self, ty: Type, stack_offset: u32, mcv: MCValue) InnerErro
             const regs = try self.register_manager.allocRegs(3, .{ null, null, null }, &.{ .rax, .rcx });
             const addr_reg = regs[0];
-            const len_reg = regs[1];
+            const count_reg = regs[1];
             const tmp_reg = regs[2];
             try self.register_manager.getReg(.rax, null);
 @@ -3182,12 +3226,12 @@ fn genSetStack(self: *Self, ty: Type, stack_offset: u32, mcv: MCValue) InnerErro
             });
             // TODO allow for abi_size to be u64
-            try self.genSetReg(Type.initTag(.u32), len_reg, .{ .immediate = @intCast(u32, abi_size) });
+            try self.genSetReg(Type.initTag(.u32), count_reg, .{ .immediate = @intCast(u32, abi_size) });
             return self.genInlineMemcpy(
                 -@intCast(i32, stack_offset + abi_size),
                 addr_reg.to64(),
-                len_reg.to64(),
+                count_reg.to64(),
                 tmp_reg.to8(),
             );
         },
 @@ -3198,7 +3242,7 @@ fn genInlineMemcpy(
     self: *Self,
     stack_offset: i32,
     addr_reg: Register,
-    len_reg: Register,
+    count_reg: Register,
     tmp_reg: Register,
 ) InnerError!void {
     // mov rcx, 0
 @@ -3220,20 +3264,19 @@ fn genInlineMemcpy(
     });
     // loop:
-    // cmp rcx, len
+    // cmp count, 0
     const loop_start = try self.addInst(.{
         .tag = .cmp,
         .ops = (Mir.Ops{
-            .reg1 = .rcx,
+            .reg1 = count_reg,
-            .reg2 = len_reg,
         }).encode(),
-        .data = undefined,
+        .data = .{ .imm = 0 },
     });
-    // jge end
+    // je end
     const loop_reloc = try self.addInst(.{
-        .tag = .cond_jmp_above_below,
+        .tag = .cond_jmp_eq_ne,
-        .ops = (Mir.Ops{ .flags = 0b00 }).encode(),
+        .ops = (Mir.Ops{ .flags = 0b01 }).encode(),
         .data = .{ .inst = undefined },
     });
 @@ -3275,6 +3318,15 @@ fn genInlineMemcpy(
         .data = .{ .imm = 1 },
     });
+    // sub count, 1
+    _ = try self.addInst(.{
+        .tag = .sub,
+        .ops = (Mir.Ops{
+            .reg1 = count_reg,
+        }).encode(),
+        .data = .{ .imm = 1 },
+    });
     // jmp loop
     _ = try self.addInst(.{
         .tag = .jmp,
 @@ -3286,47 +3338,6 @@ fn genInlineMemcpy(
     try self.performReloc(loop_reloc);
 }
-/// Set pointee via pointer stored in a register.
-/// mov [reg], value
-fn genSetPtrReg(self: *Self, ty: Type, reg: Register, mcv: MCValue) InnerError!void {
-    switch (mcv) {
-        .dead => unreachable,
-        .unreach, .none => return, // Nothing to do.
-        .immediate => |imm| {
-            const abi_size = ty.abiSize(self.target.*);
-            switch (abi_size) {
-                1, 2, 4 => {
-                    // TODO this is wasteful!
-                    // introduce new MIR tag specifically for mov [reg + 0], imm
-                    const payload = try self.addExtra(Mir.ImmPair{
-                        .dest_off = 0,
-                        .operand = @bitCast(i32, @intCast(u32, imm)),
-                    });
-                    _ = try self.addInst(.{
-                        .tag = .mov_mem_imm,
-                        .ops = (Mir.Ops{
-                            .reg1 = reg.to64(),
-                            .flags = switch (abi_size) {
-                                1 => 0b00,
-                                2 => 0b01,
-                                4 => 0b10,
-                                else => unreachable,
-                            },
-                        }).encode(),
-                        .data = .{ .payload = payload },
-                    });
-                },
-                else => {
-                    return self.fail("TODO implement set pointee with immediate of ABI size {d}", .{abi_size});
-                },
-            }
-        },
-        else => |other| {
-            return self.fail("TODO implement set pointee with {}", .{other});
-        },
-    }
-}
 fn genSetReg(self: *Self, ty: Type, reg: Register, mcv: MCValue) InnerError!void {
     switch (mcv) {
         .dead => unreachable,
 @@ -3749,7 +3760,7 @@ fn genTypedValue(self: *Self, typed_value: TypedValue) InnerError!MCValue {
                 if (typed_value.val.tag() == .int_u64) {
                     return MCValue{ .immediate = typed_value.val.toUnsignedInt() };
                 }
-                return self.fail("TODO codegen more kinds of const pointers", .{});
+                return self.fail("TODO codegen more kinds of const pointers: {}", .{typed_value.val.tag()});
             },
         },
         .Int => {

src/arch/x86_64/Isel.zig+51-30

 @@ -265,32 +265,43 @@ fn mirPushPopRegsFromCalleePreservedRegs(isel: *Isel, tag: Tag, inst: Mir.Inst.I
 fn mirJmpCall(isel: *Isel, tag: Tag, inst: Mir.Inst.Index) InnerError!void {
     const ops = Mir.Ops.decode(isel.mir.instructions.items(.ops)[inst]);
-    const flag = @truncate(u1, ops.flags);
+    switch (ops.flags) {
-    if (flag == 0) {
+        0b00 => {
-        const target = isel.mir.instructions.items(.data)[inst].inst;
+            const target = isel.mir.instructions.items(.data)[inst].inst;
-        const source = isel.code.items.len;
+            const source = isel.code.items.len;
-        lowerToDEnc(tag, 0, isel.code) catch |err|
+            lowerToDEnc(tag, 0, isel.code) catch |err|
-            return isel.failWithLoweringError(err);
+                return isel.failWithLoweringError(err);
-        try isel.relocs.append(isel.bin_file.allocator, .{
+            try isel.relocs.append(isel.bin_file.allocator, .{
-            .source = source,
+                .source = source,
-            .target = target,
+                .target = target,
-            .offset = isel.code.items.len - 4,
+                .offset = isel.code.items.len - 4,
-            .length = 5,
+                .length = 5,
-        });
+            });
-        return;
+        },
-    }
+        0b01 => {
-    if (ops.reg1 == .none) {
+            if (ops.reg1 == .none) {
-        // JMP/CALL [imm]
+                // JMP/CALL [imm]
-        const imm = isel.mir.instructions.items(.data)[inst].imm;
+                const imm = isel.mir.instructions.items(.data)[inst].imm;
-        const ptr_size: Memory.PtrSize = switch (immOpSize(imm)) {
+                const ptr_size: Memory.PtrSize = switch (immOpSize(imm)) {
-            16 => .word_ptr,
+                    16 => .word_ptr,
-            else => .qword_ptr,
+                    else => .qword_ptr,
-        };
+                };
-        return lowerToMEnc(tag, RegisterOrMemory.mem(ptr_size, .{ .disp = imm }), isel.code) catch |err|
+                return lowerToMEnc(tag, RegisterOrMemory.mem(ptr_size, .{ .disp = imm }), isel.code) catch |err|
-            isel.failWithLoweringError(err);
+                    isel.failWithLoweringError(err);
+            }
+            // JMP/CALL reg
+            return lowerToMEnc(tag, RegisterOrMemory.reg(ops.reg1), isel.code) catch |err| isel.failWithLoweringError(err);
+        },
+        0b10 => {
+            // JMP/CALL r/m64
+            const imm = isel.mir.instructions.items(.data)[inst].imm;
+            return lowerToMEnc(tag, RegisterOrMemory.mem(Memory.PtrSize.fromBits(ops.reg1.size()), .{
+                .disp = imm,
+                .base = ops.reg1,
+            }), isel.code) catch |err| isel.failWithLoweringError(err);
+        },
+        0b11 => return isel.fail("TODO unused JMP/CALL variant 0b11", .{}),
     }
-    // JMP/CALL reg
-    return lowerToMEnc(tag, RegisterOrMemory.reg(ops.reg1), isel.code) catch |err| isel.failWithLoweringError(err);
 }
 fn mirCondJmp(isel: *Isel, mir_tag: Mir.Inst.Tag, inst: Mir.Inst.Index) InnerError!void {
 @@ -658,16 +669,17 @@ fn mirLea(isel: *Isel, inst: Mir.Inst.Index) InnerError!void {
             // lea reg, [rbp + rcx + imm32]
             const imm = isel.mir.instructions.items(.data)[inst].imm;
             const src_reg: ?Register = if (ops.reg2 == .none) null else ops.reg2;
+            const scale_index = ScaleIndex{
+                .scale = 0,
+                .index = .rcx,
+            };
             return lowerToRmEnc(
                 .lea,
                 ops.reg1,
                 RegisterOrMemory.mem(Memory.PtrSize.fromBits(ops.reg1.size()), .{
                     .disp = imm,
                     .base = src_reg,
-                    .scale_index = .{
+                    .scale_index = scale_index,
-                        .scale = 0,
-                        .index = .rcx,
-                    },
                 }),
                 isel.code,
             ) catch |err| isel.failWithLoweringError(err);
 @@ -1248,7 +1260,7 @@ const Memory = struct {
             const dst = base.lowId();
             const src = operand;
             if (dst == 4 or mem_op.scale_index != null) {
-                if (mem_op.disp == 0) {
+                if (mem_op.disp == 0 and dst != 5) {
                     encoder.modRm_SIBDisp0(src);
                     if (mem_op.scale_index) |si| {
                         encoder.sib_scaleIndexBase(si.scale, si.index.lowId(), dst);
 @@ -1916,6 +1928,15 @@ test "lower RM encoding" {
         },
     }), isel.code());
     try expectEqualHexStrings("\x44\x8A\x44\x0E\xE8", isel.lowered(), "mov r8b, byte ptr [rsi + rcx*1 - 24]");
+    try lowerToRmEnc(.lea, .rsi, RegisterOrMemory.mem(.qword_ptr, .{
+        .disp = 0,
+        .base = .rbp,
+        .scale_index = .{
+            .scale = 0,
+            .index = .rcx,
+        },
+    }), isel.code());
+    try expectEqualHexStrings("\x48\x8D\x74\x0D\x00", isel.lowered(), "lea rsi, qword ptr [rbp + rcx*1 + 0]");
 }
 test "lower MR encoding" {

src/arch/x86_64/Mir.zig+5-6

...	@@ -199,12 +199,11 @@ pub const Inst = struct {	...	@@ -199,12 +199,11 @@ pub const Inst = struct {
199	/// TODO handle scaling	199	/// TODO handle scaling
200	movabs,	200	movabs,
201		201
202	/// ops flags: 0bX0:	202	/// ops flags: form:
203	/// - Uses the `inst` Data tag as the jump target.	203	/// 0b00 inst
204	/// - reg1 and reg2 are ignored.	204	/// 0b01 reg1
205	/// ops flags: 0bX1:	205	/// 0b01 [imm32] if reg1 is none
206	/// - reg1 is the jump target, reg2 and data are ignored.	206	/// 0b10 [reg1 + imm32]
207	/// - if reg1 is none, [imm]
208	jmp,	207	jmp,
209	call,	208	call,
210		209

test/stage2/x86_64.zig+19

...	@@ -1761,6 +1761,25 @@ pub fn addCases(ctx: *TestContext) !void {	...	@@ -1761,6 +1761,25 @@ pub fn addCases(ctx: *TestContext) !void {
1761	\\}	1761	\\}
1762	, "");	1762	, "");
1763	}	1763	}
		1764
		1765	{
		1766	var case = ctx.exe("access slice element by index - slice_elem_val", target);
		1767	case.addCompareOutput(
		1768	\\var array = [_]usize{ 0, 42, 123, 34 };
		1769	\\var slice: []const usize = &array;
		1770	\\
		1771	\\pub fn main() void {
		1772	\\ assert(slice[0] == 0);
		1773	\\ assert(slice[1] == 42);
		1774	\\ assert(slice[2] == 123);
		1775	\\ assert(slice[3] == 34);
		1776	\\}
		1777	\\
		1778	\\fn assert(ok: bool) void {
		1779	\\ if (!ok) unreachable;
		1780	\\}
		1781	, "");
		1782	}
1764	}	1783	}
1765	}	1784	}
1766		1785