Commit 70bff2f4d5

Jakub Konka <kubkon@jakubkonka.com>
2021-12-15 23:07:09
stage2: fix MOV MIR -> Isel lowering logic
* ensure that every callsite of basic MOV MIR instruction follows the Intel syntax (dst <- src) * add extensive unit tests for MOV MIR -> Isel lowering * leave TODOs for cases that are currently not handled and/or missing * fix any ABI size mismatch between operands
1 parent ef0566d
Changed files (4)
src/arch/x86_64/CodeGen.zig
@@ -275,7 +275,9 @@ pub fn generate(
         .stack_align = undefined,
         .end_di_line = module_fn.rbrace_line,
         .end_di_column = module_fn.rbrace_column,
-        .mir_to_air_map = if (builtin.mode == .Debug) std.AutoHashMap(Mir.Inst.Index, Air.Inst.Index).init(bin_file.allocator) else {},
+        .mir_to_air_map = if (builtin.mode == .Debug)
+            std.AutoHashMap(Mir.Inst.Index, Air.Inst.Index).init(bin_file.allocator)
+        else {},
     };
     defer function.stack.deinit(bin_file.allocator);
     defer function.blocks.deinit(bin_file.allocator);
@@ -386,8 +388,8 @@ fn gen(self: *Self) InnerError!void {
         _ = try self.addInst(.{
             .tag = .mov,
             .ops = (Mir.Ops{
-                .reg1 = .rsp,
-                .reg2 = .rbp,
+                .reg1 = .rbp,
+                .reg2 = .rsp,
             }).encode(),
             .data = undefined,
         });
@@ -2838,8 +2840,8 @@ fn genSetStack(self: *Self, ty: Type, stack_offset: u32, mcv: MCValue) InnerErro
             _ = try self.addInst(.{
                 .tag = .mov,
                 .ops = (Mir.Ops{
-                    .reg1 = registerAlias(reg, @intCast(u32, abi_size)),
-                    .reg2 = registerAlias(.rbp, @intCast(u32, abi_size)),
+                    .reg1 = .rbp,
+                    .reg2 = registerAlias(reg, @intCast(u32, abi_size)),
                     .flags = 0b10,
                 }).encode(),
                 .data = .{ .imm = -@intCast(i32, adj_off) },
@@ -2925,7 +2927,7 @@ fn genSetReg(self: *Self, ty: Type, reg: Register, mcv: MCValue) InnerError!void
                 _ = try self.addInst(.{
                     .tag = .mov,
                     .ops = (Mir.Ops{
-                        .reg1 = reg,
+                        .reg1 = registerAlias(reg, 4),
                     }).encode(),
                     .data = .{ .imm = @intCast(i32, x) },
                 });
@@ -3055,15 +3057,14 @@ fn genSetReg(self: *Self, ty: Type, reg: Register, mcv: MCValue) InnerError!void
             if (off < std.math.minInt(i32) or off > std.math.maxInt(i32)) {
                 return self.fail("stack offset too large", .{});
             }
-            const ioff = -@intCast(i32, off);
             _ = try self.addInst(.{
                 .tag = .mov,
                 .ops = (Mir.Ops{
                     .reg1 = registerAlias(reg, @intCast(u32, abi_size)),
-                    .reg2 = registerAlias(.rbp, @intCast(u32, abi_size)),
+                    .reg2 = .rbp,
                     .flags = 0b01,
                 }).encode(),
-                .data = .{ .imm = ioff },
+                .data = .{ .imm = -@intCast(i32, off) },
             });
         },
     }
src/arch/x86_64/Emit.zig
@@ -11,8 +11,10 @@ const link = @import("../../link.zig");
 const log = std.log.scoped(.codegen);
 const math = std.math;
 const mem = std.mem;
+const testing = std.testing;
 
 const Air = @import("../../Air.zig");
+const Allocator = mem.Allocator;
 const DebugInfoOutput = @import("../../codegen.zig").DebugInfoOutput;
 const DW = std.dwarf;
 const Encoder = bits.Encoder;
@@ -46,6 +48,11 @@ const InnerError = error{
     EmitFail,
 };
 
+const EmitError = error{
+    OutOfMemory,
+    OperandSizeMismatch,
+};
+
 const Reloc = struct {
     /// Offset of the instruction.
     source: u64,
@@ -100,10 +107,7 @@ pub fn emitMir(emit: *Emit) InnerError!void {
             .sbb_scale_imm => try emit.mirArithScaleImm(.sbb, inst),
             .cmp_scale_imm => try emit.mirArithScaleImm(.cmp, inst),
 
-            // Even though MOV is technically not an arithmetic op,
-            // its structure can be represented using the same set of
-            // opcode primitives.
-            .mov => try emit.mirArith(.mov, inst),
+            .mov => try emit.mirMov(inst),
             .mov_scale_src => try emit.mirArithScaleSrc(.mov, inst),
             .mov_scale_dst => try emit.mirArithScaleDst(.mov, inst),
             .mov_scale_imm => try emit.mirArithScaleImm(.mov, inst),
@@ -566,7 +570,6 @@ inline fn getArithOpCode(tag: Mir.Inst.Tag, enc: EncType) OpCode {
             .@"or" => .{ .opc = 0x81, .modrm_ext = 0x1 },
             .sbb => .{ .opc = 0x81, .modrm_ext = 0x3 },
             .cmp => .{ .opc = 0x81, .modrm_ext = 0x7 },
-            .mov => .{ .opc = 0xc7, .modrm_ext = 0x0 },
             else => unreachable,
         },
         .mr => {
@@ -579,7 +582,6 @@ inline fn getArithOpCode(tag: Mir.Inst.Tag, enc: EncType) OpCode {
                 .@"or" => 0x09,
                 .sbb => 0x19,
                 .cmp => 0x39,
-                .mov => 0x89,
                 else => unreachable,
             };
             return .{ .opc = opc, .modrm_ext = undefined };
@@ -594,7 +596,6 @@ inline fn getArithOpCode(tag: Mir.Inst.Tag, enc: EncType) OpCode {
                 .@"or" => 0x0b,
                 .sbb => 0x1b,
                 .cmp => 0x3b,
-                .mov => 0x8b,
                 else => unreachable,
             };
             return .{ .opc = opc, .modrm_ext = undefined };
@@ -723,7 +724,7 @@ fn mirArith(emit: *Emit, tag: Mir.Inst.Tag, inst: Mir.Inst.Index) InnerError!voi
                 // OP [reg1 + imm32], imm32
                 // OP r/m64, imm32
                 const payload = emit.mir.instructions.items(.data)[inst].payload;
-                const imm_pair = emit.mir.extraData(Mir.ImmPair, payload).data;
+                const imm_pair = Mir.extraData(emit.mir.extra, Mir.ImmPair, payload).data;
                 const opcode = getArithOpCode(tag, .mi);
                 const opc = if (ops.reg1.size() == 8) opcode.opc - 1 else opcode.opc;
                 const encoder = try Encoder.init(emit.code, 11);
@@ -835,7 +836,7 @@ fn mirArithScaleImm(emit: *Emit, tag: Mir.Inst.Tag, inst: Mir.Inst.Index) InnerE
     const ops = Mir.Ops.decode(emit.mir.instructions.items(.ops)[inst]);
     const scale = ops.flags;
     const payload = emit.mir.instructions.items(.data)[inst].payload;
-    const imm_pair = emit.mir.extraData(Mir.ImmPair, payload).data;
+    const imm_pair = Mir.extraData(emit.mir.extra, Mir.ImmPair, payload).data;
     const opcode = getArithOpCode(tag, .mi);
     const opc = if (ops.reg1.size() == 8) opcode.opc - 1 else opcode.opc;
     const encoder = try Encoder.init(emit.code, 2);
@@ -856,6 +857,240 @@ fn mirArithScaleImm(emit: *Emit, tag: Mir.Inst.Tag, inst: Mir.Inst.Index) InnerE
     encoder.imm32(imm_pair.operand);
 }
 
+fn mirMov(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
+    return mirMovImpl(
+        emit.mir.instructions,
+        emit.mir.extra,
+        inst,
+        emit.code,
+    ) catch |err| switch (err) {
+        // TODO better formating of operands in case of an error
+        error.OperandSizeMismatch => emit.fail("operand size mismatch", .{}),
+        else => emit.fail("emit failed with error: {}", .{err}),
+    };
+}
+
+fn mirMovImpl(
+    mir_instructions: std.MultiArrayList(Mir.Inst).Slice,
+    mir_extra: []const u32,
+    inst: Mir.Inst.Index,
+    code: *std.ArrayList(u8),
+) EmitError!void {
+    const ops = Mir.Ops.decode(mir_instructions.items(.ops)[inst]);
+    switch (ops.flags) {
+        0b00 => blk: {
+            if (ops.reg2 == .none) {
+                // mov reg1, imm32
+                // MI
+                const imm = mir_instructions.items(.data)[inst].imm;
+                const opc: u8 = if (ops.reg1.size() == 8) 0xc6 else 0xc7;
+                const modrm_ext: u3 = 0x0;
+                const encoder = try Encoder.init(code, 7);
+                if (ops.reg1.size() == 16) {
+                    // 0x66 prefix switches to the non-default size; here we assume a switch from
+                    // the default 32bits to 16bits operand-size.
+                    // More info: https://www.cs.uni-potsdam.de/desn/lehre/ss15/64-ia-32-architectures-software-developer-instruction-set-reference-manual-325383.pdf#page=32&zoom=auto,-159,773
+                    encoder.opcode_1byte(0x66);
+                }
+                encoder.rex(.{
+                    .w = ops.reg1.size() == 64,
+                    .b = ops.reg1.isExtended(),
+                });
+                encoder.opcode_1byte(opc);
+                encoder.modRm_direct(modrm_ext, ops.reg1.lowId());
+                switch (ops.reg1.size()) {
+                    8 => {
+                        const imm8 = math.cast(i8, imm) catch return error.OperandSizeMismatch;
+                        encoder.imm8(imm8);
+                    },
+                    16 => {
+                        const imm16 = math.cast(i16, imm) catch return error.OperandSizeMismatch;
+                        encoder.imm16(imm16);
+                    },
+                    32, 64 => {
+                        encoder.imm32(imm);
+                    },
+                    else => unreachable,
+                }
+                break :blk;
+            }
+            // mov reg1, reg2
+            // MR
+            if (ops.reg1.size() != ops.reg2.size()) {
+                return error.OperandSizeMismatch;
+            }
+            const opc: u8 = if (ops.reg1.size() == 8) 0x88 else 0x89;
+            const encoder = try Encoder.init(code, 3);
+            encoder.rex(.{
+                .w = ops.reg1.size() == 64 and ops.reg2.size() == 64,
+                .r = ops.reg2.isExtended(),
+                .b = ops.reg1.isExtended(),
+            });
+            encoder.opcode_1byte(opc);
+            encoder.modRm_direct(ops.reg2.lowId(), ops.reg1.lowId());
+        },
+        0b01 => blk: {
+            const imm = mir_instructions.items(.data)[inst].imm;
+            const opc: u8 = if (ops.reg1.size() == 8) 0x8a else 0x8b;
+            if (ops.reg2 == .none) {
+                // mov reg1, [imm32]
+                // RM
+                const encoder = try Encoder.init(code, 9);
+                if (ops.reg1.size() == 16) {
+                    encoder.opcode_1byte(0x66);
+                }
+                encoder.rex(.{
+                    .w = ops.reg1.size() == 64,
+                    .r = ops.reg1.isExtended(),
+                });
+                encoder.opcode_1byte(opc);
+                encoder.modRm_SIBDisp0(ops.reg1.lowId());
+                encoder.sib_disp32();
+                encoder.disp32(imm);
+                break :blk;
+            }
+            // mov reg1, [reg2 + imm32]
+            // RM
+            // TODO handle 32-bit base register - requires prefix 0x67
+            // Intel Manual, Vol 1, chapter 3.6 and 3.6.1
+            if (ops.reg2.size() != 64) {
+                return error.OperandSizeMismatch;
+            }
+            const encoder = try Encoder.init(code, 8);
+            if (ops.reg1.size() == 16) {
+                encoder.opcode_1byte(0x66);
+            }
+            encoder.rex(.{
+                .w = ops.reg1.size() == 64,
+                .r = ops.reg1.isExtended(),
+                .b = ops.reg2.isExtended(),
+            });
+            encoder.opcode_1byte(opc);
+            if (immOpSize(imm) == 8) {
+                encoder.modRm_indirectDisp8(ops.reg1.lowId(), ops.reg2.lowId());
+                encoder.disp8(@intCast(i8, imm));
+            } else {
+                encoder.modRm_indirectDisp32(ops.reg1.lowId(), ops.reg2.lowId());
+                encoder.disp32(imm);
+            }
+        },
+        0b10 => blk: {
+            // TODO handle 32-bit base register - requires prefix 0x67
+            // Intel Manual, Vol 1, chapter 3.6 and 3.6.1
+            if (ops.reg1.size() != 64) {
+                return error.OperandSizeMismatch;
+            }
+            if (ops.reg2 == .none) {
+                // mov [reg1 + 0], imm32
+                // MI
+                // Base register reg1 can either be 64bit or 32bit in size.
+                // TODO for memory operand, immediate operand pair, we currently
+                // have no way of flagging whether the immediate can be 8-, 16- or
+                // 32-bit and whether the corresponding memory operand is respectively
+                // a byte, word or dword ptr.
+                // TODO we currently don't have a way to flag imm32 64bit sign extended
+                const imm = mir_instructions.items(.data)[inst].imm;
+                const opc: u8 = 0xc7;
+                const modrm_ext: u3 = 0x0;
+                const encoder = try Encoder.init(code, 7);
+                encoder.rex(.{
+                    .w = false,
+                    .b = ops.reg1.isExtended(),
+                });
+                encoder.opcode_1byte(opc);
+                encoder.modRm_indirectDisp0(modrm_ext, ops.reg1.lowId());
+                encoder.imm32(imm);
+                break :blk;
+            }
+            // mov [reg1 + imm32], reg2
+            // MR
+            // We use size of source register reg2 to work out which
+            // variant of memory ptr to pick:
+            // * reg2 is 64bit - qword ptr
+            // * reg2 is 32bit - dword ptr
+            // * reg2 is 16bit - word ptr
+            // * reg2 is 8bit - byte ptr
+            const imm = mir_instructions.items(.data)[inst].imm;
+            const opc: u8 = if (ops.reg2.size() == 8) 0x88 else 0x89;
+            const encoder = try Encoder.init(code, 5);
+            if (ops.reg2.size() == 16) {
+                encoder.opcode_1byte(0x66);
+            }
+            encoder.rex(.{
+                .w = ops.reg2.size() == 64,
+                .r = ops.reg2.isExtended(),
+                .b = ops.reg1.isExtended(),
+            });
+            encoder.opcode_1byte(opc);
+            if (immOpSize(imm) == 8) {
+                encoder.modRm_indirectDisp8(ops.reg2.lowId(), ops.reg1.lowId());
+                encoder.disp8(@intCast(i8, imm));
+            } else {
+                encoder.modRm_indirectDisp32(ops.reg2.lowId(), ops.reg1.lowId());
+                encoder.disp32(imm);
+            }
+        },
+        0b11 => blk: {
+            if (ops.reg2 == .none) {
+                // mov [reg1 + imm32], imm32
+                // MI
+                // Base register reg1 can either be 64bit or 32bit in size.
+                // TODO for memory operand, immediate operand pair, we currently
+                // have no way of flagging whether the immediate can be 8-, 16- or
+                // 32-bit and whether the corresponding memory operand is respectively
+                // a byte, word or dword ptr.
+                // TODO we currently don't have a way to flag imm32 64bit sign extended
+                if (ops.reg1.size() != 64) {
+                    return error.OperandSizeMismatch;
+                }
+                const payload = mir_instructions.items(.data)[inst].payload;
+                const imm_pair = Mir.extraData(mir_extra, Mir.ImmPair, payload).data;
+                const imm_op = imm_pair.operand;
+                const opc: u8 = 0xc7;
+                const modrm_ext: u3 = 0x0;
+                const encoder = try Encoder.init(code, 10);
+                encoder.rex(.{
+                    .w = false,
+                    .b = ops.reg1.isExtended(),
+                });
+                encoder.opcode_1byte(opc);
+                if (immOpSize(imm_pair.dest_off) == 8) {
+                    encoder.modRm_indirectDisp8(modrm_ext, ops.reg1.lowId());
+                    encoder.disp8(@intCast(i8, imm_pair.dest_off));
+                } else {
+                    encoder.modRm_indirectDisp32(modrm_ext, ops.reg1.lowId());
+                    encoder.disp32(imm_pair.dest_off);
+                }
+                encoder.imm32(imm_op);
+                break :blk;
+            }
+            // mov reg1, reg2
+            // RM
+            const opc: u8 = if (ops.reg1.size() == 8) 0x8a else 0x8b;
+            const encoder = try Encoder.init(code, 3);
+            encoder.rex(.{
+                .w = ops.reg1.size() == 64 and ops.reg2.size() == 64,
+                .r = ops.reg1.isExtended(),
+                .b = ops.reg2.isExtended(),
+            });
+            encoder.opcode_1byte(opc);
+            encoder.modRm_direct(ops.reg1.lowId(), ops.reg2.lowId());
+        },
+    }
+}
+
+fn immOpSize(imm: i32) u8 {
+    blk: {
+        _ = math.cast(i8, imm) catch break :blk;
+        return 8;
+    }
+    blk: {
+        _ = math.cast(i16, imm) catch break :blk;
+        return 16;
+    }
+    return 32;
+}
+
 fn mirMovabs(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
     const tag = emit.mir.instructions.items(.tag)[inst];
     assert(tag == .movabs);
@@ -897,7 +1132,7 @@ fn mirMovabs(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
 
     if (is_64) {
         const payload = emit.mir.instructions.items(.data)[inst].payload;
-        const imm64 = emit.mir.extraData(Mir.Imm64, payload).data;
+        const imm64 = Mir.extraData(emit.mir.extra, Mir.Imm64, payload).data;
         encoder.imm64(imm64.decode());
     } else {
         const imm = emit.mir.instructions.items(.data)[inst].imm;
@@ -1003,7 +1238,7 @@ fn mirLeaRip(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
     const end_offset = emit.code.items.len;
     if (@truncate(u1, ops.flags) == 0b0) {
         const payload = emit.mir.instructions.items(.data)[inst].payload;
-        const imm = emit.mir.extraData(Mir.Imm64, payload).data.decode();
+        const imm = Mir.extraData(emit.mir.extra, Mir.Imm64, payload).data.decode();
         encoder.disp32(@intCast(i32, @intCast(i64, imm) - @intCast(i64, end_offset - start_offset + 4)));
     } else {
         const got_entry = emit.mir.instructions.items(.data)[inst].got_entry;
@@ -1058,7 +1293,7 @@ fn mirDbgLine(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
     const tag = emit.mir.instructions.items(.tag)[inst];
     assert(tag == .dbg_line);
     const payload = emit.mir.instructions.items(.data)[inst].payload;
-    const dbg_line_column = emit.mir.extraData(Mir.DbgLineColumn, payload).data;
+    const dbg_line_column = Mir.extraData(emit.mir.extra, Mir.DbgLineColumn, payload).data;
     try emit.dbgAdvancePCAndLine(dbg_line_column.line, dbg_line_column.column);
 }
 
@@ -1143,7 +1378,7 @@ fn mirArgDbgInfo(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
     const tag = emit.mir.instructions.items(.tag)[inst];
     assert(tag == .arg_dbg_info);
     const payload = emit.mir.instructions.items(.data)[inst].payload;
-    const arg_dbg_info = emit.mir.extraData(Mir.ArgDbgInfo, payload).data;
+    const arg_dbg_info = Mir.extraData(emit.mir.extra, Mir.ArgDbgInfo, payload).data;
     const mcv = emit.mir.function.args[arg_dbg_info.arg_index];
     try emit.genArgDbgInfo(arg_dbg_info.air_inst, mcv);
 }
@@ -1206,3 +1441,384 @@ fn addDbgInfoTypeReloc(emit: *Emit, ty: Type) !void {
         .none => {},
     }
 }
+
+const Mock = struct {
+    const gpa = testing.allocator;
+
+    mir_instructions: std.MultiArrayList(Mir.Inst) = .{},
+    mir_extra: std.ArrayList(u32),
+    code: std.ArrayList(u8),
+
+    fn init() Mock {
+        return .{
+            .mir_extra = std.ArrayList(u32).init(gpa),
+            .code = std.ArrayList(u8).init(gpa),
+        };
+    }
+
+    fn deinit(self: *Mock) void {
+        self.mir_instructions.deinit(gpa);
+        self.mir_extra.deinit();
+        self.code.deinit();
+    }
+
+    fn addInst(self: *Mock, inst: Mir.Inst) error{OutOfMemory}!Mir.Inst.Index {
+        try self.mir_instructions.ensureUnusedCapacity(gpa, 1);
+        const result_index = @intCast(Air.Inst.Index, self.mir_instructions.len);
+        self.mir_instructions.appendAssumeCapacity(inst);
+        return result_index;
+    }
+
+    fn addExtra(self: *Mock, extra: anytype) Allocator.Error!u32 {
+        const fields = std.meta.fields(@TypeOf(extra));
+        try self.mir_extra.ensureUnusedCapacity(fields.len);
+        return self.addExtraAssumeCapacity(extra);
+    }
+
+    fn addExtraAssumeCapacity(self: *Mock, extra: anytype) u32 {
+        const fields = std.meta.fields(@TypeOf(extra));
+        const result = @intCast(u32, self.mir_extra.items.len);
+        inline for (fields) |field| {
+            self.mir_extra.appendAssumeCapacity(switch (field.field_type) {
+                u32 => @field(extra, field.name),
+                i32 => @bitCast(u32, @field(extra, field.name)),
+                else => @compileError("bad field type"),
+            });
+        }
+        return result;
+    }
+
+    fn testEmitSingleSuccess(
+        self: *Mock,
+        mir_inst: Mir.Inst,
+        expected_enc: []const u8,
+        assembly: []const u8,
+    ) !void {
+        const code_index = self.code.items.len;
+        const mir_index = try self.addInst(mir_inst);
+        switch (mir_inst.tag) {
+            .mov => try mirMovImpl(
+                self.mir_instructions.slice(),
+                self.mir_extra.items,
+                mir_index,
+                &self.code,
+            ),
+            else => unreachable,
+        }
+        const code_len = if (self.code.items[code_index..].len >= expected_enc.len)
+            expected_enc.len
+        else
+            self.code.items.len - code_index;
+        try expectEqualHexStrings(expected_enc, self.code.items[code_index..][0..code_len], assembly);
+    }
+
+    fn testEmitSingleError(self: *Mock, mir_inst: Mir.Inst, err: EmitError) !void {
+        const index = try self.addInst(mir_inst);
+        const res = switch (mir_inst.tag) {
+            .mov => mirMovImpl(
+                self.mir_instructions.slice(),
+                self.mir_extra.items,
+                index,
+                &self.code,
+            ),
+            else => unreachable,
+        };
+        try testing.expectError(err, res);
+    }
+};
+
+fn expectEqualHexStrings(expected: []const u8, given: []const u8, assembly: []const u8) !void {
+    assert(expected.len > 0);
+    if (mem.eql(u8, expected, given)) return;
+    const expected_fmt = try std.fmt.allocPrint(testing.allocator, "{x}", .{std.fmt.fmtSliceHexLower(expected)});
+    defer testing.allocator.free(expected_fmt);
+    const given_fmt = try std.fmt.allocPrint(testing.allocator, "{x}", .{std.fmt.fmtSliceHexLower(given)});
+    defer testing.allocator.free(given_fmt);
+    const idx = mem.indexOfDiff(u8, expected_fmt, given_fmt).?;
+    var padding = try testing.allocator.alloc(u8, idx + 5);
+    defer testing.allocator.free(padding);
+    mem.set(u8, padding, ' ');
+    std.debug.print("\nASM: {s}\nEXP: {s}\nGIV: {s}\n{s}^ -- first differing byte\n", .{
+        assembly,
+        expected_fmt,
+        given_fmt,
+        padding,
+    });
+    return error.TestFailed;
+}
+
+test "mov dst_reg, src_reg" {
+    var mock = Mock.init();
+    defer mock.deinit();
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rbp, .reg2 = .rsp }).encode(),
+        .data = undefined,
+    }, "\x48\x89\xe5", "mov rbp, rsp");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r12, .reg2 = .rax }).encode(),
+        .data = undefined,
+    }, "\x49\x89\xc4", "mov r12, rax");
+    try mock.testEmitSingleError(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r12, .reg2 = .eax }).encode(),
+        .data = undefined,
+    }, error.OperandSizeMismatch);
+    try mock.testEmitSingleError(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r12d, .reg2 = .rax }).encode(),
+        .data = undefined,
+    }, error.OperandSizeMismatch);
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r12d, .reg2 = .eax }).encode(),
+        .data = undefined,
+    }, "\x41\x89\xc4", "mov r12d, eax");
+
+    // TODO mov r12b, ah requires a codepath without REX prefix
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r12b, .reg2 = .al }).encode(),
+        .data = undefined,
+    }, "\x41\x88\xc4", "mov r12b, al");
+}
+
+test "mov dst_reg, imm" {
+    var mock = Mock.init();
+    defer mock.deinit();
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rcx }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x48\xc7\xc1\x10\x00\x00\x00", "mov rcx, 0x10");
+
+    // TODO we are wasting one byte here: this could be encoded as OI with the encoding opc + rd, imm8/16/32
+    // b9 10 00 00 00
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .ecx }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\xc7\xc1\x10\x00\x00\x00", "mov ecx, 0x10");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .cx }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x66\xc7\xc1\x10\x00", "mov cx, 0x10");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11w }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x66\x41\xC7\xC3\x10\x00", "mov r11w, 0x10");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .cl }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\xc6\xc1\x10", "mov cl, 0x10");
+    try mock.testEmitSingleError(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .cx }).encode(),
+        .data = .{ .imm = 0x10000000 },
+    }, error.OperandSizeMismatch);
+    try mock.testEmitSingleError(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .cl }).encode(),
+        .data = .{ .imm = 0x1000 },
+    }, error.OperandSizeMismatch);
+}
+
+test "mov dst_reg, [imm32]" {
+    var mock = Mock.init();
+    defer mock.deinit();
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rcx, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x48\x8B\x0C\x25\x10\x00\x00\x00", "mov rcx, [0x10]");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x4C\x8B\x1C\x25\x10\x00\x00\x00", "mov r11, [0x10]");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11d, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x44\x8B\x1C\x25\x10\x00\x00\x00", "mov r11d, [0x10]");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11w, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x66\x44\x8B\x1C\x25\x10\x00\x00\x00", "mov r11w, [0x10]");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11b, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x44\x8A\x1C\x25\x10\x00\x00\x00", "mov r11b, [0x10]");
+}
+
+test "mov dst_reg, [src_reg + imm]" {
+    var mock = Mock.init();
+    defer mock.deinit();
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rcx, .reg2 = .rbp, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x48\x8B\x4D\x10", "mov rcx, [rbp + 0x10]");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rcx, .reg2 = .rbp, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10000000 },
+    }, "\x48\x8B\x8D\x00\x00\x00\x10", "mov rcx, [rbp + 0x10000000]");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11b, .reg2 = .rbp, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x44\x8A\x5D\x10", "mov r11b, [rbp + 0x10]");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11w, .reg2 = .rbp, .flags = 0b01 }).encode(),
+        .data = .{ .imm = 0x10000000 },
+    }, "\x66\x44\x8B\x9D\x00\x00\x00\x10", "mov r11w, [rbp + 0x10000000]");
+}
+
+test "mov [dst_reg + 0], imm" {
+    var mock = Mock.init();
+    defer mock.deinit();
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x41\xC7\x03\x10\x00\x00\x00", "mov dword ptr [r11 + 0], 0x10");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rax, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10000000 },
+    }, "\xC7\x00\x00\x00\x00\x10", "mov dword ptr [rax + 0], 0x10000000");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rax, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x1000 },
+    }, "\xC7\x00\x00\x10\x00\x00", "mov dword ptr [rax + 0], 0x1000");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rax, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\xC7\x00\x10\x00\x00\x00", "mov dword ptr [rax + 0], 0x10");
+    try mock.testEmitSingleError(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .eax, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, error.OperandSizeMismatch);
+}
+
+test "mov [dst_reg + imm32], src_reg" {
+    var mock = Mock.init();
+    defer mock.deinit();
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rbp, .reg2 = .r11, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x4c\x89\x5d\x10", "mov qword ptr [rbp + 0x10], r11");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rbp, .reg2 = .r11d, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x44\x89\x5d\x10", "mov dword ptr [rbp + 0x10], r11d");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rbp, .reg2 = .r11w, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x66\x44\x89\x5d\x10", "mov word ptr [rbp + 0x10], r11w");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .rbp, .reg2 = .r11b, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x44\x88\x5d\x10", "mov byte ptr [rbp + 0x10], r11b");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11, .reg2 = .rax, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x49\x89\x43\x10", "mov qword ptr [r11 + 0x10], rax");
+    try mock.testEmitSingleSuccess(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11, .reg2 = .eax, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, "\x41\x89\x43\x10", "mov dword ptr [r11 + 0x10], eax");
+    try mock.testEmitSingleError(.{
+        .tag = .mov,
+        .ops = (Mir.Ops{ .reg1 = .r11w, .reg2 = .ax, .flags = 0b10 }).encode(),
+        .data = .{ .imm = 0x10 },
+    }, error.OperandSizeMismatch);
+}
+
+test "mov [dst_reg + imm32], imm32" {
+    var mock = Mock.init();
+    defer mock.deinit();
+    {
+        const payload = try mock.addExtra(Mir.ImmPair{
+            .dest_off = 0x10,
+            .operand = 0x20000000,
+        });
+        try mock.testEmitSingleSuccess(.{
+            .tag = .mov,
+            .ops = (Mir.Ops{ .reg1 = .rbp, .flags = 0b11 }).encode(),
+            .data = .{ .payload = payload },
+        }, "\xC7\x45\x10\x00\x00\x00\x20", "mov dword ptr [rbp + 0x10], 0x20000000");
+    }
+    {
+        const payload = try mock.addExtra(Mir.ImmPair{
+            .dest_off = 0x10,
+            .operand = 0x2000,
+        });
+        try mock.testEmitSingleSuccess(.{
+            .tag = .mov,
+            .ops = (Mir.Ops{ .reg1 = .rbp, .flags = 0b11 }).encode(),
+            .data = .{ .payload = payload },
+        }, "\xC7\x45\x10\x00\x20\x00\x00", "mov dword ptr [rbp + 0x10], 0x2000");
+    }
+    {
+        const payload = try mock.addExtra(Mir.ImmPair{
+            .dest_off = 0x10,
+            .operand = 0x20,
+        });
+        try mock.testEmitSingleSuccess(.{
+            .tag = .mov,
+            .ops = (Mir.Ops{ .reg1 = .rbp, .flags = 0b11 }).encode(),
+            .data = .{ .payload = payload },
+        }, "\xc7\x45\x10\x20\x00\x00\x00", "mov dword ptr [rbp + 0x10], 0x20");
+    }
+    {
+        const payload = try mock.addExtra(Mir.ImmPair{
+            .dest_off = 0x10,
+            .operand = 0x20000000,
+        });
+        try mock.testEmitSingleSuccess(.{
+            .tag = .mov,
+            .ops = (Mir.Ops{ .reg1 = .r11, .flags = 0b11 }).encode(),
+            .data = .{ .payload = payload },
+        }, "\x41\xC7\x43\x10\x00\x00\x00\x20", "mov dword ptr [r11 + 0x10], 0x20000000");
+    }
+    {
+        const payload = try mock.addExtra(Mir.ImmPair{
+            .dest_off = 0x10000000,
+            .operand = 0x20000000,
+        });
+        try mock.testEmitSingleSuccess(.{
+            .tag = .mov,
+            .ops = (Mir.Ops{ .reg1 = .r11, .flags = 0b11 }).encode(),
+            .data = .{ .payload = payload },
+        }, "\x41\xC7\x83\x00\x00\x00\x10\x00\x00\x00\x20", "mov dword ptr [r11 + 0x10], 0x20000000");
+    }
+    {
+        const payload = try mock.addExtra(Mir.ImmPair{
+            .dest_off = 0x10,
+            .operand = 0x20,
+        });
+        try mock.testEmitSingleError(.{
+            .tag = .mov,
+            .ops = (Mir.Ops{ .reg1 = .r11d, .flags = 0b11 }).encode(),
+            .data = .{ .payload = payload },
+        }, error.OperandSizeMismatch);
+    }
+}
src/arch/x86_64/Mir.zig
@@ -136,10 +136,24 @@ pub const Inst = struct {
         cmp_scale_src,
         cmp_scale_dst,
         cmp_scale_imm,
+
+        /// ops flags:  form:
+        ///       0b00  reg1, reg2 (MR)
+        ///       0b00  reg1, imm32
+        ///       0b01  reg1, [reg2 + imm32]
+        ///       0b01  reg1, [ds:imm32]
+        ///       0b10  [reg1 + imm32], reg2
+        ///       0b10  [reg1 + 0], imm32
+        ///       0b11  [reg1 + imm32], imm32
+        ///       0b11  reg1, reg2 (RM)
+        /// Notes:
+        ///  * If reg2 is `none` then it means Data field `imm` is used as the immediate.
+        ///  * When two imm32 values are required, Data field `payload` points at `ImmPair`.
         mov,
         mov_scale_src,
         mov_scale_dst,
         mov_scale_imm,
+
         lea,
         lea_scale_src,
         lea_scale_dst,
@@ -292,6 +306,7 @@ pub const Inst = struct {
         /// Another instruction.
         inst: Index,
         /// A 32-bit immediate value.
+        /// TODO we should add support for 16- and 8-bit immediate values.
         imm: i32,
         /// An extern function.
         /// Index into the linker's string table.
@@ -378,14 +393,14 @@ pub const Ops = struct {
     }
 };
 
-pub fn extraData(mir: Mir, comptime T: type, index: usize) struct { data: T, end: usize } {
+pub fn extraData(mir_extra: []const u32, comptime T: type, index: usize) struct { data: T, end: usize } {
     const fields = std.meta.fields(T);
     var i: usize = index;
     var result: T = undefined;
     inline for (fields) |field| {
         @field(result, field.name) = switch (field.field_type) {
-            u32 => mir.extra[i],
-            i32 => @bitCast(i32, mir.extra[i]),
+            u32 => mir_extra[i],
+            i32 => @bitCast(i32, mir_extra[i]),
             else => @compileError("bad field type"),
         };
         i += 1;
src/arch/x86_64/PrintMir.zig
@@ -481,7 +481,7 @@ fn mirArith(print: *const Print, tag: Mir.Inst.Tag, inst: Mir.Inst.Index, w: any
         0b11 => {
             if (ops.reg2 == .none) {
                 const payload = print.mir.instructions.items(.data)[inst].payload;
-                const imm_pair = print.mir.extraData(Mir.ImmPair, payload).data;
+                const imm_pair = Mir.extraData(print.mir.extra, Mir.ImmPair, payload).data;
                 try w.print("[{s} + {d}], {d}", .{ @tagName(ops.reg1), imm_pair.dest_off, imm_pair.operand });
             }
             try w.writeAll("TODO");
@@ -516,7 +516,7 @@ fn mirArithScaleImm(print: *const Print, tag: Mir.Inst.Tag, inst: Mir.Inst.Index
     const ops = Mir.Ops.decode(print.mir.instructions.items(.ops)[inst]);
     const scale = ops.flags;
     const payload = print.mir.instructions.items(.data)[inst].payload;
-    const imm_pair = print.mir.extraData(Mir.ImmPair, payload).data;
+    const imm_pair = Mir.extraData(print.mir.extra, Mir.ImmPair, payload).data;
     try w.print("{s} [{s} + {d}*rcx + {d}], {d}\n", .{ @tagName(tag), @tagName(ops.reg1), scale, imm_pair.dest_off, imm_pair.operand });
 }
 
@@ -528,7 +528,7 @@ fn mirMovabs(print: *const Print, inst: Mir.Inst.Index, w: anytype) !void {
     const is_64 = ops.reg1.size() == 64;
     const imm: i128 = if (is_64) blk: {
         const payload = print.mir.instructions.items(.data)[inst].payload;
-        const imm64 = print.mir.extraData(Mir.Imm64, payload).data;
+        const imm64 = Mir.extraData(print.mir.extra, Mir.Imm64, payload).data;
         break :blk imm64.decode();
     } else print.mir.instructions.items(.data)[inst].imm;
     if (ops.flags == 0b00) {