From dbe1b4a7e5731e4fb17d42b754faf1052aa78f32 Mon Sep 17 00:00:00 2001
From: Jacob Young <jacobly0@users.noreply.github.com>
Date: Thu, 23 Mar 2023 00:27:25 -0400
Subject: [PATCH 1/7] x86_64: fix value tracking bugs

---
 src/arch/x86_64/CodeGen.zig  | 310 ++++++++++++++++++++---------------
 src/arch/x86_64/abi.zig      |   2 +-
 src/register_manager.zig     |   8 +-
 test/behavior/array.zig      |   1 +
 test/behavior/bugs/10970.zig |   1 -
 test/behavior/for.zig        |   1 +
 test/behavior/if.zig         |   1 -
 test/behavior/union.zig      |   1 -
 8 files changed, 182 insertions(+), 143 deletions(-)

diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index f30be0e378..9dfa4f0502 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -265,12 +265,15 @@ pub fn generate(
     const fn_type = fn_owner_decl.ty;
 
     var branch_stack = std.ArrayList(Branch).init(bin_file.allocator);
+    try branch_stack.ensureUnusedCapacity(2);
+    // The outermost branch is used for constants only.
+    branch_stack.appendAssumeCapacity(.{});
+    branch_stack.appendAssumeCapacity(.{});
     defer {
-        assert(branch_stack.items.len == 1);
-        branch_stack.items[0].deinit(bin_file.allocator);
+        assert(branch_stack.items.len == 2);
+        for (branch_stack.items) |*branch| branch.deinit(bin_file.allocator);
         branch_stack.deinit();
     }
-    try branch_stack.append(.{});
 
     var function = Self{
         .gpa = bin_file.allocator,
@@ -1070,20 +1073,29 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
             if (self.air_bookkeeping < old_air_bookkeeping + 1) {
                 std.debug.panic("in codegen.zig, handling of AIR instruction %{d} ('{}') did not do proper bookkeeping. Look for a missing call to finishAir.", .{ inst, air_tags[inst] });
             }
+
+            { // check consistency of tracked registers
+                var it = self.register_manager.free_registers.iterator(.{ .kind = .unset });
+                while (it.next()) |index| {
+                    const tracked_inst = self.register_manager.registers[index];
+                    switch (air_tags[tracked_inst]) {
+                        .block => {},
+                        else => assert(RegisterManager.indexOfRegIntoTracked(
+                            switch (self.getResolvedInstValue(tracked_inst).?) {
+                                .register => |reg| reg,
+                                .register_overflow => |ro| ro.reg,
+                                else => unreachable,
+                            },
+                        ).? == index),
+                    }
+                }
+            }
         }
     }
 }
 
-/// Asserts there is already capacity to insert into top branch inst_table.
-fn processDeath(self: *Self, inst: Air.Inst.Index) void {
-    const air_tags = self.air.instructions.items(.tag);
-    if (air_tags[inst] == .constant) return; // Constants are immortal.
-    const prev_value = self.getResolvedInstValue(inst) orelse return;
-    log.debug("%{d} => {}", .{ inst, MCValue.dead });
-    // When editing this function, note that the logic must synchronize with `reuseOperand`.
-    const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
-    branch.inst_table.putAssumeCapacity(inst, .dead);
-    switch (prev_value) {
+fn freeValue(self: *Self, value: MCValue) void {
+    switch (value) {
         .register => |reg| {
             self.register_manager.freeReg(reg);
         },
@@ -1098,6 +1110,18 @@ fn processDeath(self: *Self, inst: Air.Inst.Index) void {
     }
 }
 
+/// Asserts there is already capacity to insert into top branch inst_table.
+fn processDeath(self: *Self, inst: Air.Inst.Index) void {
+    const air_tags = self.air.instructions.items(.tag);
+    if (air_tags[inst] == .constant) return; // Constants are immortal.
+    const prev_value = self.getResolvedInstValue(inst) orelse return;
+    log.debug("%{d} => {}", .{ inst, MCValue.dead });
+    // When editing this function, note that the logic must synchronize with `reuseOperand`.
+    const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
+    branch.inst_table.putAssumeCapacity(inst, .dead);
+    self.freeValue(prev_value);
+}
+
 /// Called when there are no operands, and the instruction is always unreferenced.
 fn finishAirBookkeeping(self: *Self) void {
     if (std.debug.runtime_safety) {
@@ -1140,13 +1164,17 @@ fn finishAir(self: *Self, inst: Air.Inst.Index, result: MCValue, operands: [Live
             },
             else => {},
         }
+    } else switch (result) {
+        .none, .dead, .unreach => {},
+        else => unreachable, // Why didn't the result die?
     }
     self.finishAirBookkeeping();
 }
 
 fn ensureProcessDeathCapacity(self: *Self, additional_count: usize) !void {
+    // In addition to the caller's needs, we need enough space to spill every register and eflags.
     const table = &self.branch_stack.items[self.branch_stack.items.len - 1].inst_table;
-    try table.ensureUnusedCapacity(self.gpa, additional_count);
+    try table.ensureUnusedCapacity(self.gpa, additional_count + self.register_manager.registers.len + 1);
 }
 
 fn allocMem(self: *Self, inst: ?Air.Inst.Index, abi_size: u32, abi_align: u32) !u32 {
@@ -1252,12 +1280,15 @@ fn captureState(self: *Self) !State {
     };
 }
 
-fn revertState(self: *Self, state: State) void {
+fn revertState(self: *Self, state: State) !void {
+    var stack = try state.stack.clone(self.gpa);
+    errdefer stack.deinit(self.gpa);
+
     self.register_manager.registers = state.registers;
     self.eflags_inst = state.eflags_inst;
 
     self.stack.deinit(self.gpa);
-    self.stack = state.stack;
+    self.stack = stack;
 
     self.next_stack_offset = state.next_stack_offset;
     self.register_manager.free_registers = state.free_registers;
@@ -1277,7 +1308,7 @@ pub fn spillInstruction(self: *Self, reg: Register, inst: Air.Inst.Index) !void
         else => {},
     }
     const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
-    try branch.inst_table.put(self.gpa, inst, stack_mcv);
+    branch.inst_table.putAssumeCapacity(inst, stack_mcv);
     try self.genSetStack(self.air.typeOfIndex(inst), stack_mcv.stack_offset, reg_mcv, .{});
 }
 
@@ -1294,7 +1325,7 @@ pub fn spillEflagsIfOccupied(self: *Self) !void {
         log.debug("spilling %{d} to mcv {any}", .{ inst_to_save, new_mcv });
 
         const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
-        try branch.inst_table.put(self.gpa, inst_to_save, new_mcv);
+        branch.inst_table.putAssumeCapacity(inst_to_save, new_mcv);
 
         self.eflags_inst = null;
 
@@ -1347,13 +1378,23 @@ fn copyToRegisterWithInstTracking(self: *Self, reg_owner: Air.Inst.Index, ty: Ty
 }
 
 fn airAlloc(self: *Self, inst: Air.Inst.Index) !void {
-    const stack_offset = try self.allocMemPtr(inst);
-    return self.finishAir(inst, .{ .ptr_stack_offset = @intCast(i32, stack_offset) }, .{ .none, .none, .none });
+    const result: MCValue = result: {
+        if (self.liveness.isUnused(inst)) break :result .dead;
+
+        const stack_offset = try self.allocMemPtr(inst);
+        break :result .{ .ptr_stack_offset = @intCast(i32, stack_offset) };
+    };
+    return self.finishAir(inst, result, .{ .none, .none, .none });
 }
 
 fn airRetPtr(self: *Self, inst: Air.Inst.Index) !void {
-    const stack_offset = try self.allocMemPtr(inst);
-    return self.finishAir(inst, .{ .ptr_stack_offset = @intCast(i32, stack_offset) }, .{ .none, .none, .none });
+    const result: MCValue = result: {
+        if (self.liveness.isUnused(inst)) break :result .dead;
+
+        const stack_offset = try self.allocMemPtr(inst);
+        break :result .{ .ptr_stack_offset = @intCast(i32, stack_offset) };
+    };
+    return self.finishAir(inst, result, .{ .none, .none, .none });
 }
 
 fn airFptrunc(self: *Self, inst: Air.Inst.Index) !void {
@@ -1992,11 +2033,6 @@ fn airUnwrapErrUnionErr(self: *Self, inst: Air.Inst.Index) !void {
             },
             .register => |reg| {
                 // TODO reuse operand
-                self.register_manager.getRegAssumeFree(.rcx, null);
-                const rcx_lock =
-                    if (err_off > 0) self.register_manager.lockRegAssumeUnused(.rcx) else null;
-                defer if (rcx_lock) |lock| self.register_manager.unlockReg(lock);
-
                 const eu_lock = self.register_manager.lockReg(reg);
                 defer if (eu_lock) |lock| self.register_manager.unlockReg(lock);
 
@@ -2047,11 +2083,6 @@ fn genUnwrapErrorUnionPayloadMir(
             },
             .register => |reg| {
                 // TODO reuse operand
-                self.register_manager.getRegAssumeFree(.rcx, null);
-                const rcx_lock =
-                    if (payload_off > 0) self.register_manager.lockRegAssumeUnused(.rcx) else null;
-                defer if (rcx_lock) |lock| self.register_manager.unlockReg(lock);
-
                 const eu_lock = self.register_manager.lockReg(reg);
                 defer if (eu_lock) |lock| self.register_manager.unlockReg(lock);
 
@@ -2877,17 +2908,18 @@ fn airPopcount(self: *Self, inst: Air.Inst.Index) !void {
         const imm_0000_1111 = Immediate.u(mask / 0b0001_0001);
         const imm_0000_0001 = Immediate.u(mask / 0b1111_1111);
 
-        const tmp_reg = if (src_mcv.isRegister() and self.reuseOperand(inst, ty_op.operand, 0, src_mcv))
-            src_mcv.register
+        const dst_mcv = if (src_mcv.isRegister() and self.reuseOperand(inst, ty_op.operand, 0, src_mcv))
+            src_mcv
         else
-            try self.copyToTmpRegister(src_ty, src_mcv);
-        const tmp_lock = self.register_manager.lockRegAssumeUnused(tmp_reg);
-        defer self.register_manager.unlockReg(tmp_lock);
-
-        const dst_reg = try self.register_manager.allocReg(inst, gp);
+            try self.copyToRegisterWithInstTracking(inst, src_ty, src_mcv);
+        const dst_reg = dst_mcv.register;
         const dst_lock = self.register_manager.lockRegAssumeUnused(dst_reg);
         defer self.register_manager.unlockReg(dst_lock);
 
+        const tmp_reg = try self.register_manager.allocReg(null, gp);
+        const tmp_lock = self.register_manager.lockRegAssumeUnused(tmp_reg);
+        defer self.register_manager.unlockReg(tmp_lock);
+
         {
             const dst = registerAlias(dst_reg, src_abi_size);
             const tmp = registerAlias(tmp_reg, src_abi_size);
@@ -2896,9 +2928,9 @@ fn airPopcount(self: *Self, inst: Air.Inst.Index) !void {
             else
                 undefined;
 
-            // tmp = operand
-            try self.asmRegisterRegister(.mov, dst, tmp);
             // dst = operand
+            try self.asmRegisterRegister(.mov, tmp, dst);
+            // tmp = operand
             try self.asmRegisterImmediate(.shr, tmp, Immediate.u(1));
             // tmp = operand >> 1
             if (src_abi_size > 4) {
@@ -2948,7 +2980,7 @@ fn airPopcount(self: *Self, inst: Air.Inst.Index) !void {
             }
             // dst = (temp3 * 0x01...01) >> (bits - 8)
         }
-        break :result .{ .register = dst_reg };
+        break :result dst_mcv;
     };
     return self.finishAir(inst, result, .{ ty_op.operand, .none, .none });
 }
@@ -3796,8 +3828,6 @@ fn genUnOpMir(self: *Self, mir_tag: Mir.Inst.Tag, dst_ty: Type, dst_mcv: MCValue
 
 /// Clobbers .rcx for non-immediate shift value.
 fn genShiftBinOpMir(self: *Self, tag: Mir.Inst.Tag, ty: Type, reg: Register, shift: MCValue) !void {
-    assert(reg.to64() != .rcx);
-
     switch (tag) {
         .sal, .sar, .shl, .shr => {},
         else => unreachable,
@@ -4612,23 +4642,24 @@ fn airArg(self: *Self, inst: Air.Inst.Index) !void {
     const src_index = self.air.instructions.items(.data)[inst].arg.src_index;
     const name = self.mod_fn.getParamName(self.bin_file.options.module.?, src_index);
 
-    if (self.liveness.isUnused(inst))
-        return self.finishAirBookkeeping();
+    const result: MCValue = result: {
+        if (self.liveness.isUnused(inst)) break :result .dead;
 
-    const dst_mcv: MCValue = switch (mcv) {
-        .register => |reg| blk: {
-            self.register_manager.getRegAssumeFree(reg.to64(), inst);
-            break :blk MCValue{ .register = reg };
-        },
-        .stack_offset => |off| blk: {
-            const offset = @intCast(i32, self.max_end_stack) - off + 16;
-            break :blk MCValue{ .stack_offset = -offset };
-        },
-        else => return self.fail("TODO implement arg for {}", .{mcv}),
+        const dst_mcv: MCValue = switch (mcv) {
+            .register => |reg| blk: {
+                self.register_manager.getRegAssumeFree(reg.to64(), inst);
+                break :blk MCValue{ .register = reg };
+            },
+            .stack_offset => |off| blk: {
+                const offset = @intCast(i32, self.max_end_stack) - off + 16;
+                break :blk MCValue{ .stack_offset = -offset };
+            },
+            else => return self.fail("TODO implement arg for {}", .{mcv}),
+        };
+        try self.genArgDbgInfo(ty, name, dst_mcv);
+        break :result dst_mcv;
     };
-    try self.genArgDbgInfo(ty, name, dst_mcv);
-
-    return self.finishAir(inst, dst_mcv, .{ .none, .none, .none });
+    return self.finishAir(inst, result, .{ .none, .none, .none });
 }
 
 fn genArgDbgInfo(self: Self, ty: Type, name: [:0]const u8, mcv: MCValue) !void {
@@ -4924,6 +4955,8 @@ fn airCall(self: *Self, inst: Air.Inst.Index, modifier: std.builtin.CallModifier
     }
 
     const result: MCValue = result: {
+        if (self.liveness.isUnused(inst)) break :result .dead;
+
         switch (info.return_value) {
             .register => {
                 // Save function return value in a new register
@@ -5137,7 +5170,10 @@ fn genTry(
     const reloc = try self.genCondBrMir(Type.anyerror, is_err_mcv);
     try self.genBody(body);
     try self.performReloc(reloc);
-    const result = try self.genUnwrapErrorUnionPayloadMir(inst, err_union_ty, err_union);
+    const result = if (self.liveness.isUnused(inst))
+        .dead
+    else
+        try self.genUnwrapErrorUnionPayloadMir(inst, err_union_ty, err_union);
     return result;
 }
 
@@ -5234,7 +5270,8 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
     }
 
     // Capture the state of register and stack allocation state so that we can revert to it.
-    const saved_state = try self.captureState();
+    var saved_state = try self.captureState();
+    defer saved_state.deinit(self.gpa);
 
     {
         try self.branch_stack.append(.{});
@@ -5252,7 +5289,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
     var then_branch = self.branch_stack.pop();
     defer then_branch.deinit(self.gpa);
 
-    self.revertState(saved_state);
+    try self.revertState(saved_state);
 
     try self.performReloc(reloc);
 
@@ -5286,9 +5323,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
 
     log.debug("Then branch: {}", .{then_branch.fmtDebug()});
     log.debug("Else branch: {}", .{else_branch.fmtDebug()});
-
-    const parent_branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
-    try self.canonicaliseBranches(parent_branch, &then_branch, &else_branch);
+    try self.canonicaliseBranches(true, &then_branch, &else_branch);
 
     // We already took care of pl_op.operand earlier, so we're going
     // to pass .none here
@@ -5423,10 +5458,6 @@ fn isErr(self: *Self, maybe_inst: ?Air.Inst.Index, ty: Type, operand: MCValue) !
             try self.genBinOpMir(.cmp, Type.anyerror, .{ .stack_offset = offset }, .{ .immediate = 0 });
         },
         .register => |reg| {
-            self.register_manager.getRegAssumeFree(.rcx, null);
-            const rcx_lock = if (err_off > 0) self.register_manager.lockRegAssumeUnused(.rcx) else null;
-            defer if (rcx_lock) |lock| self.register_manager.unlockReg(lock);
-
             const eu_lock = self.register_manager.lockReg(reg);
             defer if (eu_lock) |lock| self.register_manager.unlockReg(lock);
 
@@ -5606,7 +5637,7 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
         // break instruction will choose a MCValue for the block result and overwrite
         // this field. Following break instructions will use that MCValue to put their
         // block results.
-        .mcv = .none,
+        .mcv = if (self.liveness.isUnused(inst)) .dead else .none,
     });
     defer self.blocks.getPtr(inst).?.relocs.deinit(self.gpa);
 
@@ -5646,21 +5677,29 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
         }
     }
 
-    var branch_stack = std.ArrayList(Branch).init(self.gpa);
-    defer {
-        for (branch_stack.items) |*bs| {
-            bs.deinit(self.gpa);
-        }
-        branch_stack.deinit();
+    log.debug("airSwitch: %{d}", .{inst});
+    log.debug("Upper branches:", .{});
+    for (self.branch_stack.items) |bs| {
+        log.debug("{}", .{bs.fmtDebug()});
     }
-    try branch_stack.ensureTotalCapacityPrecise(switch_br.data.cases_len + 1);
 
+    var prev_branch: ?Branch = null;
+    defer if (prev_branch) |*branch| branch.deinit(self.gpa);
+
+    // Capture the state of register and stack allocation state so that we can revert to it.
+    var saved_state = try self.captureState();
+    defer saved_state.deinit(self.gpa);
+
+    const cases_len = switch_br.data.cases_len + @boolToInt(switch_br.data.else_body_len > 0);
     while (case_i < switch_br.data.cases_len) : (case_i += 1) {
         const case = self.air.extraData(Air.SwitchBr.Case, extra_index);
         const items = @ptrCast([]const Air.Inst.Ref, self.air.extra[case.end..][0..case.data.items_len]);
         const case_body = self.air.extra[case.end + items.len ..][0..case.data.body_len];
         extra_index = case.end + items.len + case_body.len;
 
+        // Revert to the previous register and stack allocation state.
+        if (prev_branch) |_| try self.revertState(saved_state);
+
         var relocs = try self.gpa.alloc(u32, items.len);
         defer self.gpa.free(relocs);
 
@@ -5671,12 +5710,9 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
             reloc.* = try self.asmJccReloc(undefined, .ne);
         }
 
-        // Capture the state of register and stack allocation state so that we can revert to it.
-        const saved_state = try self.captureState();
-
         {
-            try self.branch_stack.append(.{});
-            errdefer _ = self.branch_stack.pop();
+            if (cases_len > 1) try self.branch_stack.append(.{});
+            errdefer _ = if (cases_len > 1) self.branch_stack.pop();
 
             try self.ensureProcessDeathCapacity(liveness.deaths[case_i].len);
             for (liveness.deaths[case_i]) |operand| {
@@ -5686,25 +5722,31 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
             try self.genBody(case_body);
         }
 
-        branch_stack.appendAssumeCapacity(self.branch_stack.pop());
+        // Consolidate returned MCValues between prongs like we do in airCondBr.
+        if (cases_len > 1) {
+            var case_branch = self.branch_stack.pop();
+            errdefer case_branch.deinit(self.gpa);
 
-        // Revert to the previous register and stack allocation state.
-        self.revertState(saved_state);
-
-        for (relocs) |reloc| {
-            try self.performReloc(reloc);
+            log.debug("Case-{d} branch: {}", .{ case_i, case_branch.fmtDebug() });
+            if (prev_branch) |*canon_branch| {
+                try self.canonicaliseBranches(case_i == cases_len - 1, canon_branch, &case_branch);
+                canon_branch.deinit(self.gpa);
+            }
+            prev_branch = case_branch;
         }
+
+        for (relocs) |reloc| try self.performReloc(reloc);
     }
 
     if (switch_br.data.else_body_len > 0) {
         const else_body = self.air.extra[extra_index..][0..switch_br.data.else_body_len];
 
-        // Capture the state of register and stack allocation state so that we can revert to it.
-        const saved_state = try self.captureState();
+        // Revert to the previous register and stack allocation state.
+        if (prev_branch) |_| try self.revertState(saved_state);
 
         {
-            try self.branch_stack.append(.{});
-            errdefer _ = self.branch_stack.pop();
+            if (cases_len > 1) try self.branch_stack.append(.{});
+            errdefer _ = if (cases_len > 1) self.branch_stack.pop();
 
             const else_deaths = liveness.deaths.len - 1;
             try self.ensureProcessDeathCapacity(liveness.deaths[else_deaths].len);
@@ -5715,53 +5757,48 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
             try self.genBody(else_body);
         }
 
-        branch_stack.appendAssumeCapacity(self.branch_stack.pop());
+        // Consolidate returned MCValues between a prong and the else branch like we do in airCondBr.
+        if (cases_len > 1) {
+            var else_branch = self.branch_stack.pop();
+            errdefer else_branch.deinit(self.gpa);
 
-        // Revert to the previous register and stack allocation state.
-        self.revertState(saved_state);
+            log.debug("Else branch: {}", .{else_branch.fmtDebug()});
+            if (prev_branch) |*canon_branch| {
+                try self.canonicaliseBranches(true, canon_branch, &else_branch);
+                canon_branch.deinit(self.gpa);
+            }
+            prev_branch = else_branch;
+        }
     }
 
-    // Consolidate returned MCValues between prongs and else branch like we do
-    // in airCondBr.
-    log.debug("airSwitch: %{d}", .{inst});
-    log.debug("Upper branches:", .{});
-    for (self.branch_stack.items) |bs| {
-        log.debug("{}", .{bs.fmtDebug()});
-    }
-    for (branch_stack.items, 0..) |bs, i| {
-        log.debug("Case-{d} branch: {}", .{ i, bs.fmtDebug() });
-    }
-
-    // TODO: can we reduce the complexity of this algorithm?
-    const parent_branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
-    var i: usize = branch_stack.items.len;
-    while (i > 1) : (i -= 1) {
-        const canon_branch = &branch_stack.items[i - 2];
-        const target_branch = &branch_stack.items[i - 1];
-        try self.canonicaliseBranches(parent_branch, canon_branch, target_branch);
-    }
-
-    // We already took care of pl_op.operand earlier, so we're going
-    // to pass .none here
+    // We already took care of pl_op.operand earlier, so we're going to pass .none here
     return self.finishAir(inst, .unreach, .{ .none, .none, .none });
 }
 
-fn canonicaliseBranches(self: *Self, parent_branch: *Branch, canon_branch: *Branch, target_branch: *Branch) !void {
-    try parent_branch.inst_table.ensureUnusedCapacity(self.gpa, target_branch.inst_table.count());
+fn canonicaliseBranches(
+    self: *Self,
+    update_parent: bool,
+    canon_branch: *Branch,
+    target_branch: *const Branch,
+) !void {
+    const parent_branch =
+        if (update_parent) &self.branch_stack.items[self.branch_stack.items.len - 1] else undefined;
+    if (update_parent) try self.ensureProcessDeathCapacity(target_branch.inst_table.count());
 
     const target_slice = target_branch.inst_table.entries.slice();
     for (target_slice.items(.key), target_slice.items(.value)) |target_key, target_value| {
         const canon_mcv = if (canon_branch.inst_table.fetchSwapRemove(target_key)) |canon_entry| blk: {
             // The instruction's MCValue is overridden in both branches.
-            parent_branch.inst_table.putAssumeCapacity(target_key, canon_entry.value);
+            if (update_parent) {
+                parent_branch.inst_table.putAssumeCapacity(target_key, canon_entry.value);
+            }
             if (target_value == .dead) {
                 assert(canon_entry.value == .dead);
                 continue;
             }
             break :blk canon_entry.value;
         } else blk: {
-            if (target_value == .dead)
-                continue;
+            if (target_value == .dead) continue;
             // The instruction is only overridden in the else branch.
             // If integer overflows occurs, the question is: why wasn't the instruction marked dead?
             break :blk self.getResolvedInstValue(target_key).?;
@@ -5770,22 +5807,25 @@ fn canonicaliseBranches(self: *Self, parent_branch: *Branch, canon_branch: *Bran
         // TODO make sure the destination stack offset / register does not already have something
         // going on there.
         try self.setRegOrMem(self.air.typeOfIndex(target_key), canon_mcv, target_value);
+        self.freeValue(target_value);
         // TODO track the new register / stack allocation
     }
-    try parent_branch.inst_table.ensureUnusedCapacity(self.gpa, canon_branch.inst_table.count());
+    if (update_parent) try self.ensureProcessDeathCapacity(canon_branch.inst_table.count());
     const canon_slice = canon_branch.inst_table.entries.slice();
     for (canon_slice.items(.key), canon_slice.items(.value)) |canon_key, canon_value| {
         // We already deleted the items from this table that matched the target_branch.
         // So these are all instructions that are only overridden in the canon branch.
-        parent_branch.inst_table.putAssumeCapacity(canon_key, canon_value);
-        log.debug("canon_value = {}", .{canon_value});
-        if (canon_value == .dead)
-            continue;
-        const parent_mcv = self.getResolvedInstValue(canon_key).?;
+        const parent_mcv =
+            if (canon_value != .dead) self.getResolvedInstValue(canon_key).? else undefined;
+        if (update_parent) {
+            parent_branch.inst_table.putAssumeCapacity(canon_key, canon_value);
+        }
+        if (canon_value == .dead) continue;
         log.debug("consolidating canon_entry {d} {}=>{}", .{ canon_key, parent_mcv, canon_value });
         // TODO make sure the destination stack offset / register does not already have something
         // going on there.
-        try self.setRegOrMem(self.air.typeOfIndex(canon_key), parent_mcv, canon_value);
+        try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);
+        self.freeValue(parent_mcv);
         // TODO track the new register / stack allocation
     }
 }
@@ -5811,11 +5851,9 @@ fn airBr(self: *Self, inst: Air.Inst.Index) !void {
 
 fn br(self: *Self, block: Air.Inst.Index, operand: Air.Inst.Ref) !void {
     const block_data = self.blocks.getPtr(block).?;
-
-    if (self.air.typeOf(operand).hasRuntimeBits()) {
+    if (block_data.mcv != .dead and self.air.typeOf(operand).hasRuntimeBits()) {
         const operand_mcv = try self.resolveInst(operand);
-        const block_mcv = block_data.mcv;
-        if (block_mcv == .none) {
+        if (block_data.mcv == .none) {
             block_data.mcv = switch (operand_mcv) {
                 .none, .dead, .unreach => unreachable,
                 .register, .stack_offset, .memory => operand_mcv,
@@ -5827,7 +5865,7 @@ fn br(self: *Self, block: Air.Inst.Index, operand: Air.Inst.Ref) !void {
                 else => return self.fail("TODO implement block_data.mcv = operand_mcv for {}", .{operand_mcv}),
             };
         } else {
-            try self.setRegOrMem(self.air.typeOfIndex(block), block_mcv, operand_mcv);
+            try self.setRegOrMem(self.air.typeOfIndex(block), block_data.mcv, operand_mcv);
         }
     }
     return self.brVoid(block);
@@ -6916,7 +6954,8 @@ fn airAtomicRmw(self: *Self, inst: Air.Inst.Index) !void {
     const pl_op = self.air.instructions.items(.data)[inst].pl_op;
     const extra = self.air.extraData(Air.AtomicRmw, pl_op.payload).data;
 
-    const dst_reg = try self.register_manager.allocReg(inst, gp);
+    const unused = self.liveness.isUnused(inst);
+    const dst_reg = try self.register_manager.allocReg(if (unused) null else inst, gp);
 
     const ptr_ty = self.air.typeOf(pl_op.operand);
     const ptr_mcv = try self.resolveInst(pl_op.operand);
@@ -6924,7 +6963,6 @@ fn airAtomicRmw(self: *Self, inst: Air.Inst.Index) !void {
     const val_ty = self.air.typeOf(extra.operand);
     const val_mcv = try self.resolveInst(extra.operand);
 
-    const unused = self.liveness.isUnused(inst);
     try self.atomicOp(dst_reg, ptr_mcv, val_mcv, ptr_ty, val_ty, unused, extra.op(), extra.ordering());
     const result: MCValue = if (unused) .dead else .{ .register = dst_reg };
     return self.finishAir(inst, result, .{ pl_op.operand, extra.operand, .none });
diff --git a/src/arch/x86_64/abi.zig b/src/arch/x86_64/abi.zig
index 193efa6dc4..e9da09b999 100644
--- a/src/arch/x86_64/abi.zig
+++ b/src/arch/x86_64/abi.zig
@@ -523,7 +523,7 @@ pub fn getCAbiIntReturnRegs(target: Target) []const Register {
 }
 
 const gp_regs = [_]Register{
-    .rbx, .r12, .r13, .r14, .r15, .rax, .rcx, .rdx, .rsi, .rdi, .r8, .r9, .r10, .r11,
+    .rax, .rcx, .rdx, .rbx, .rsi, .rdi, .r8, .r9, .r10, .r11, .r12, .r13, .r14, .r15,
 };
 const sse_avx_regs = [_]Register{
     .ymm0, .ymm1, .ymm2,  .ymm3,  .ymm4,  .ymm5,  .ymm6,  .ymm7,
diff --git a/src/register_manager.zig b/src/register_manager.zig
index 713b669b06..1a5d2fd501 100644
--- a/src/register_manager.zig
+++ b/src/register_manager.zig
@@ -210,13 +210,14 @@ pub fn RegisterManager(
             }
             assert(i == count);
 
-            for (regs, 0..) |reg, j| {
+            for (regs, insts) |reg, inst| {
+                log.debug("tryAllocReg {} for inst {?}", .{ reg, inst });
                 self.markRegAllocated(reg);
 
-                if (insts[j]) |inst| {
+                if (inst) |tracked_inst| {
                     // Track the register
                     const index = indexOfRegIntoTracked(reg).?; // indexOfReg() on a callee-preserved reg should never return null
-                    self.registers[index] = inst;
+                    self.registers[index] = tracked_inst;
                     self.markRegUsed(reg);
                 }
             }
@@ -258,6 +259,7 @@ pub fn RegisterManager(
                     if (excludeRegister(reg, register_class)) break;
                     if (self.isRegLocked(reg)) continue;
 
+                    log.debug("allocReg {} for inst {?}", .{ reg, insts[i] });
                     regs[i] = reg;
                     self.markRegAllocated(reg);
                     const index = indexOfRegIntoTracked(reg).?; // indexOfReg() on a callee-preserved reg should never return null
diff --git a/test/behavior/array.zig b/test/behavior/array.zig
index 484cab4722..96b1be1778 100644
--- a/test/behavior/array.zig
+++ b/test/behavior/array.zig
@@ -191,6 +191,7 @@ test "nested arrays of strings" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
 
     const array_of_strings = [_][]const u8{ "hello", "this", "is", "my", "thing" };
     for (array_of_strings, 0..) |s, i| {
diff --git a/test/behavior/bugs/10970.zig b/test/behavior/bugs/10970.zig
index e04680c443..539dfaff71 100644
--- a/test/behavior/bugs/10970.zig
+++ b/test/behavior/bugs/10970.zig
@@ -6,7 +6,6 @@ fn retOpt() ?u32 {
 test "breaking from a loop in an if statement" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
 
     var cond = true;
diff --git a/test/behavior/for.zig b/test/behavior/for.zig
index 98ffff85a3..8ab378c6d4 100644
--- a/test/behavior/for.zig
+++ b/test/behavior/for.zig
@@ -275,6 +275,7 @@ test "two counters" {
 test "1-based counter and ptr to array" {
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
 
     var ok: usize = 0;
 
diff --git a/test/behavior/if.zig b/test/behavior/if.zig
index 948629038b..730c0713c6 100644
--- a/test/behavior/if.zig
+++ b/test/behavior/if.zig
@@ -112,7 +112,6 @@ test "if prongs cast to expected type instead of peer type resolution" {
 }
 
 test "if peer expressions inferred optional type" {
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
diff --git a/test/behavior/union.zig b/test/behavior/union.zig
index f247bf6fa2..b78bac5c3e 100644
--- a/test/behavior/union.zig
+++ b/test/behavior/union.zig
@@ -1514,7 +1514,6 @@ test "packed union with zero-bit field" {
 }
 
 test "reinterpreting enum value inside packed union" {
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO

From 12c07fcf20aba9e986b5b2131515b33e3d27176a Mon Sep 17 00:00:00 2001
From: Jacob Young <jacobly0@users.noreply.github.com>
Date: Thu, 23 Mar 2023 02:03:43 -0400
Subject: [PATCH 2/7] x86_64: fix more value tracking bugs

---
 src/arch/x86_64/CodeGen.zig | 170 +++++++++++++++++-------------------
 test/behavior/cast.zig      |   7 +-
 test/behavior/enum.zig      |   1 +
 test/behavior/if.zig        |   1 +
 test/behavior/switch.zig    |   1 -
 5 files changed, 85 insertions(+), 95 deletions(-)

diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index 9dfa4f0502..852a8b47be 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -214,11 +214,6 @@ const StackAllocation = struct {
 
 const BlockData = struct {
     relocs: std.ArrayListUnmanaged(Mir.Inst.Index),
-    /// The first break instruction encounters `null` here and chooses a
-    /// machine code value for the block result, populating this field.
-    /// Following break instructions encounter that value and use it for
-    /// the location to store their block results.
-    mcv: MCValue,
 };
 
 const BigTomb = struct {
@@ -1078,16 +1073,12 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
                 var it = self.register_manager.free_registers.iterator(.{ .kind = .unset });
                 while (it.next()) |index| {
                     const tracked_inst = self.register_manager.registers[index];
-                    switch (air_tags[tracked_inst]) {
-                        .block => {},
-                        else => assert(RegisterManager.indexOfRegIntoTracked(
-                            switch (self.getResolvedInstValue(tracked_inst).?) {
-                                .register => |reg| reg,
-                                .register_overflow => |ro| ro.reg,
-                                else => unreachable,
-                            },
-                        ).? == index),
-                    }
+                    const tracked_mcv = self.getResolvedInstValue(tracked_inst).?.*;
+                    assert(RegisterManager.indexOfRegIntoTracked(switch (tracked_mcv) {
+                        .register => |reg| reg,
+                        .register_overflow => |ro| ro.reg,
+                        else => unreachable,
+                    }).? == index);
                 }
             }
         }
@@ -1114,7 +1105,7 @@ fn freeValue(self: *Self, value: MCValue) void {
 fn processDeath(self: *Self, inst: Air.Inst.Index) void {
     const air_tags = self.air.instructions.items(.tag);
     if (air_tags[inst] == .constant) return; // Constants are immortal.
-    const prev_value = self.getResolvedInstValue(inst) orelse return;
+    const prev_value = (self.getResolvedInstValue(inst) orelse return).*;
     log.debug("%{d} => {}", .{ inst, MCValue.dead });
     // When editing this function, note that the logic must synchronize with `reuseOperand`.
     const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
@@ -1259,45 +1250,29 @@ fn allocRegOrMemAdvanced(self: *Self, elem_ty: Type, inst: ?Air.Inst.Index, reg_
 }
 
 const State = struct {
-    next_stack_offset: u32,
     registers: abi.RegisterManager.TrackedRegisters,
     free_registers: abi.RegisterManager.RegisterBitSet,
     eflags_inst: ?Air.Inst.Index,
-    stack: std.AutoHashMapUnmanaged(u32, StackAllocation),
-
-    fn deinit(state: *State, gpa: Allocator) void {
-        state.stack.deinit(gpa);
-    }
 };
 
-fn captureState(self: *Self) !State {
+fn captureState(self: *Self) State {
     return State{
-        .next_stack_offset = self.next_stack_offset,
         .registers = self.register_manager.registers,
         .free_registers = self.register_manager.free_registers,
         .eflags_inst = self.eflags_inst,
-        .stack = try self.stack.clone(self.gpa),
     };
 }
 
-fn revertState(self: *Self, state: State) !void {
-    var stack = try state.stack.clone(self.gpa);
-    errdefer stack.deinit(self.gpa);
-
-    self.register_manager.registers = state.registers;
+fn revertState(self: *Self, state: State) void {
     self.eflags_inst = state.eflags_inst;
-
-    self.stack.deinit(self.gpa);
-    self.stack = stack;
-
-    self.next_stack_offset = state.next_stack_offset;
     self.register_manager.free_registers = state.free_registers;
+    self.register_manager.registers = state.registers;
 }
 
 pub fn spillInstruction(self: *Self, reg: Register, inst: Air.Inst.Index) !void {
     const stack_mcv = try self.allocRegOrMem(inst, false);
     log.debug("spilling %{d} to stack mcv {any}", .{ inst, stack_mcv });
-    const reg_mcv = self.getResolvedInstValue(inst).?;
+    const reg_mcv = self.getResolvedInstValue(inst).?.*;
     switch (reg_mcv) {
         .register => |other| {
             assert(reg.to64() == other.to64());
@@ -1314,7 +1289,7 @@ pub fn spillInstruction(self: *Self, reg: Register, inst: Air.Inst.Index) !void
 
 pub fn spillEflagsIfOccupied(self: *Self) !void {
     if (self.eflags_inst) |inst_to_save| {
-        const mcv = self.getResolvedInstValue(inst_to_save).?;
+        const mcv = self.getResolvedInstValue(inst_to_save).?.*;
         const new_mcv = switch (mcv) {
             .register_overflow => try self.allocRegOrMem(inst_to_save, false),
             .eflags => try self.allocRegOrMem(inst_to_save, true),
@@ -2607,7 +2582,7 @@ fn airPtrElemVal(self: *Self, inst: Air.Inst.Index) !void {
     const index_ty = self.air.typeOf(bin_op.rhs);
     const index = try self.resolveInst(bin_op.rhs);
     const index_lock: ?RegisterLock = switch (index) {
-        .register => |reg| self.register_manager.lockRegAssumeUnused(reg),
+        .register => |reg| self.register_manager.lockReg(reg),
         else => null,
     };
     defer if (index_lock) |lock| self.register_manager.unlockReg(lock);
@@ -2656,7 +2631,7 @@ fn airPtrElemPtr(self: *Self, inst: Air.Inst.Index) !void {
     const index_ty = self.air.typeOf(extra.rhs);
     const index = try self.resolveInst(extra.rhs);
     const index_lock: ?RegisterLock = switch (index) {
-        .register => |reg| self.register_manager.lockRegAssumeUnused(reg),
+        .register => |reg| self.register_manager.lockReg(reg),
         else => null,
     };
     defer if (index_lock) |lock| self.register_manager.unlockReg(lock);
@@ -3202,8 +3177,8 @@ fn reuseOperand(
         .register => |reg| {
             // If it's in the registers table, need to associate the register with the
             // new instruction.
-            if (RegisterManager.indexOfRegIntoTracked(reg)) |index| {
-                if (!self.register_manager.isRegFree(reg)) {
+            if (!self.register_manager.isRegFree(reg)) {
+                if (RegisterManager.indexOfRegIntoTracked(reg)) |index| {
                     self.register_manager.registers[index] = inst;
                 }
             }
@@ -3542,7 +3517,7 @@ fn airStore(self: *Self, inst: Air.Inst.Index) !void {
     const value_ty = self.air.typeOf(bin_op.rhs);
     log.debug("airStore(%{d}): {} <- {}", .{ inst, ptr, value });
     try self.store(ptr, value, ptr_ty, value_ty);
-    return self.finishAir(inst, .dead, .{ bin_op.lhs, bin_op.rhs, .none });
+    return self.finishAir(inst, .none, .{ bin_op.lhs, bin_op.rhs, .none });
 }
 
 fn airStructFieldPtr(self: *Self, inst: Air.Inst.Index) !void {
@@ -5270,8 +5245,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
     }
 
     // Capture the state of register and stack allocation state so that we can revert to it.
-    var saved_state = try self.captureState();
-    defer saved_state.deinit(self.gpa);
+    const saved_state = self.captureState();
 
     {
         try self.branch_stack.append(.{});
@@ -5289,7 +5263,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
     var then_branch = self.branch_stack.pop();
     defer then_branch.deinit(self.gpa);
 
-    try self.revertState(saved_state);
+    self.revertState(saved_state);
 
     try self.performReloc(reloc);
 
@@ -5632,24 +5606,28 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
     try self.blocks.putNoClobber(self.gpa, inst, .{
         // A block is a setup to be able to jump to the end.
         .relocs = .{},
-        // It also acts as a receptacle for break operands.
-        // Here we use `MCValue.none` to represent a null value so that the first
-        // break instruction will choose a MCValue for the block result and overwrite
-        // this field. Following break instructions will use that MCValue to put their
-        // block results.
-        .mcv = if (self.liveness.isUnused(inst)) .dead else .none,
     });
     defer self.blocks.getPtr(inst).?.relocs.deinit(self.gpa);
 
+    {
+        // Here we use `.none` to represent a null value so that the first break
+        // instruction will choose a MCValue for the block result and overwrite
+        // this field. Following break instructions will use that MCValue to put
+        // their block results.
+        const ty = self.air.typeOfIndex(inst);
+        const result: MCValue =
+            if (!ty.hasRuntimeBitsIgnoreComptime() or self.liveness.isUnused(inst)) .dead else .none;
+        const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
+        branch.inst_table.putAssumeCapacityNoClobber(inst, result);
+    }
+
     const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
     const extra = self.air.extraData(Air.Block, ty_pl.payload);
     const body = self.air.extra[extra.end..][0..extra.data.body_len];
     try self.genBody(body);
 
     for (self.blocks.getPtr(inst).?.relocs.items) |reloc| try self.performReloc(reloc);
-
-    const result = self.blocks.getPtr(inst).?.mcv;
-    return self.finishAir(inst, result, .{ .none, .none, .none });
+    self.finishAirBookkeeping();
 }
 
 fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
@@ -5687,8 +5665,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
     defer if (prev_branch) |*branch| branch.deinit(self.gpa);
 
     // Capture the state of register and stack allocation state so that we can revert to it.
-    var saved_state = try self.captureState();
-    defer saved_state.deinit(self.gpa);
+    const saved_state = self.captureState();
 
     const cases_len = switch_br.data.cases_len + @boolToInt(switch_br.data.else_body_len > 0);
     while (case_i < switch_br.data.cases_len) : (case_i += 1) {
@@ -5698,7 +5675,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
         extra_index = case.end + items.len + case_body.len;
 
         // Revert to the previous register and stack allocation state.
-        if (prev_branch) |_| try self.revertState(saved_state);
+        if (prev_branch) |_| self.revertState(saved_state);
 
         var relocs = try self.gpa.alloc(u32, items.len);
         defer self.gpa.free(relocs);
@@ -5742,7 +5719,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
         const else_body = self.air.extra[extra_index..][0..switch_br.data.else_body_len];
 
         // Revert to the previous register and stack allocation state.
-        if (prev_branch) |_| try self.revertState(saved_state);
+        if (prev_branch) |_| self.revertState(saved_state);
 
         {
             if (cases_len > 1) try self.branch_stack.append(.{});
@@ -5801,7 +5778,7 @@ fn canonicaliseBranches(
             if (target_value == .dead) continue;
             // The instruction is only overridden in the else branch.
             // If integer overflows occurs, the question is: why wasn't the instruction marked dead?
-            break :blk self.getResolvedInstValue(target_key).?;
+            break :blk self.getResolvedInstValue(target_key).?.*;
         };
         log.debug("consolidating target_entry {d} {}=>{}", .{ target_key, target_value, canon_mcv });
         // TODO make sure the destination stack offset / register does not already have something
@@ -5816,17 +5793,18 @@ fn canonicaliseBranches(
         // We already deleted the items from this table that matched the target_branch.
         // So these are all instructions that are only overridden in the canon branch.
         const parent_mcv =
-            if (canon_value != .dead) self.getResolvedInstValue(canon_key).? else undefined;
+            if (canon_value != .dead) self.getResolvedInstValue(canon_key).?.* else undefined;
+        if (canon_value != .dead) {
+            log.debug("consolidating canon_entry {d} {}=>{}", .{ canon_key, parent_mcv, canon_value });
+            // TODO make sure the destination stack offset / register does not already have something
+            // going on there.
+            try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);
+            self.freeValue(parent_mcv);
+            // TODO track the new register / stack allocation
+        }
         if (update_parent) {
             parent_branch.inst_table.putAssumeCapacity(canon_key, canon_value);
         }
-        if (canon_value == .dead) continue;
-        log.debug("consolidating canon_entry {d} {}=>{}", .{ canon_key, parent_mcv, canon_value });
-        // TODO make sure the destination stack offset / register does not already have something
-        // going on there.
-        try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);
-        self.freeValue(parent_mcv);
-        // TODO track the new register / stack allocation
     }
 }
 
@@ -5845,27 +5823,41 @@ fn performReloc(self: *Self, reloc: Mir.Inst.Index) !void {
 
 fn airBr(self: *Self, inst: Air.Inst.Index) !void {
     const branch = self.air.instructions.items(.data)[inst].br;
-    try self.br(branch.block_inst, branch.operand);
+    try self.br(inst, branch.block_inst, branch.operand);
     return self.finishAir(inst, .dead, .{ branch.operand, .none, .none });
 }
 
-fn br(self: *Self, block: Air.Inst.Index, operand: Air.Inst.Ref) !void {
-    const block_data = self.blocks.getPtr(block).?;
-    if (block_data.mcv != .dead and self.air.typeOf(operand).hasRuntimeBits()) {
-        const operand_mcv = try self.resolveInst(operand);
-        if (block_data.mcv == .none) {
-            block_data.mcv = switch (operand_mcv) {
-                .none, .dead, .unreach => unreachable,
-                .register, .stack_offset, .memory => operand_mcv,
-                .eflags, .immediate, .ptr_stack_offset => blk: {
-                    const new_mcv = try self.allocRegOrMem(block, true);
-                    try self.setRegOrMem(self.air.typeOfIndex(block), new_mcv, operand_mcv);
-                    break :blk new_mcv;
-                },
-                else => return self.fail("TODO implement block_data.mcv = operand_mcv for {}", .{operand_mcv}),
-            };
-        } else {
-            try self.setRegOrMem(self.air.typeOfIndex(block), block_data.mcv, operand_mcv);
+fn br(self: *Self, inst: Air.Inst.Index, block: Air.Inst.Index, operand: Air.Inst.Ref) !void {
+    // The first break instruction encounters `.none` here and chooses a
+    // machine code value for the block result, populating this field.
+    // Following break instructions encounter that value and use it for
+    // the location to store their block results.
+    if (self.getResolvedInstValue(block)) |dst_mcv| {
+        const src_mcv = try self.resolveInst(operand);
+        switch (dst_mcv.*) {
+            .none => {
+                const result = result: {
+                    if (!self.reuseOperand(inst, operand, 0, src_mcv)) {
+                        const new_mcv = try self.allocRegOrMem(block, true);
+                        try self.setRegOrMem(self.air.typeOfIndex(block), new_mcv, src_mcv);
+                        break :result new_mcv;
+                    }
+
+                    // the value is actually tracked with block, not inst
+                    switch (src_mcv) {
+                        .register => |reg| if (!self.register_manager.isRegFree(reg)) {
+                            if (RegisterManager.indexOfRegIntoTracked(reg)) |index| {
+                                self.register_manager.registers[index] = block;
+                            }
+                        },
+                        .stack_offset => {},
+                        else => unreachable,
+                    }
+                    break :result src_mcv;
+                };
+                dst_mcv.* = result;
+            },
+            else => try self.setRegOrMem(self.air.typeOfIndex(block), dst_mcv.*, src_mcv),
         }
     }
     return self.brVoid(block);
@@ -7243,17 +7235,17 @@ fn resolveInst(self: *Self, inst: Air.Inst.Ref) InnerError!MCValue {
             return gop.value_ptr.*;
         },
         .const_ty => unreachable,
-        else => return self.getResolvedInstValue(inst_index).?,
+        else => return self.getResolvedInstValue(inst_index).?.*,
     }
 }
 
-fn getResolvedInstValue(self: *Self, inst: Air.Inst.Index) ?MCValue {
+fn getResolvedInstValue(self: *Self, inst: Air.Inst.Index) ?*MCValue {
     // Treat each stack item as a "layer" on top of the previous one.
     var i: usize = self.branch_stack.items.len;
     while (true) {
         i -= 1;
-        if (self.branch_stack.items[i].inst_table.get(inst)) |mcv| {
-            return if (mcv != .dead) mcv else null;
+        if (self.branch_stack.items[i].inst_table.getPtr(inst)) |mcv| {
+            return if (mcv.* != .dead) mcv else null;
         }
     }
 }
diff --git a/test/behavior/cast.zig b/test/behavior/cast.zig
index 7d27138ded..978c75c92f 100644
--- a/test/behavior/cast.zig
+++ b/test/behavior/cast.zig
@@ -38,6 +38,8 @@ fn peerTypeTAndOptionalT(c: bool, b: bool) ?usize {
 }
 
 test "resolve undefined with integer" {
+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
+
     try testResolveUndefWithInt(true, 1234);
     comptime try testResolveUndefWithInt(true, 1234);
 }
@@ -419,7 +421,6 @@ fn testCastIntToErr(err: anyerror) !void {
 test "peer resolve array and const slice" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
 
     try testPeerResolveArrayConstSlice(true);
@@ -818,7 +819,6 @@ test "peer type resolution: error union after non-error" {
 test "peer cast *[0]T to E![]const T" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
 
     var buffer: [5]u8 = "abcde".*;
@@ -833,7 +833,6 @@ test "peer cast *[0]T to E![]const T" {
 test "peer cast *[0]T to []const T" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
 
     var buffer: [5]u8 = "abcde".*;
@@ -855,7 +854,6 @@ test "peer cast *[N]T to [*]T" {
 test "peer resolution of string literals" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
 
     const S = struct {
@@ -1360,7 +1358,6 @@ test "cast f128 to narrower types" {
 test "peer type resolution: unreachable, null, slice" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
 
     const S = struct {
diff --git a/test/behavior/enum.zig b/test/behavior/enum.zig
index 9076f9f9ac..72784d9abc 100644
--- a/test/behavior/enum.zig
+++ b/test/behavior/enum.zig
@@ -904,6 +904,7 @@ test "enum literal casting to tagged union" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
 
     const Arch = union(enum) {
         x86_64,
diff --git a/test/behavior/if.zig b/test/behavior/if.zig
index 730c0713c6..223d73ac85 100644
--- a/test/behavior/if.zig
+++ b/test/behavior/if.zig
@@ -115,6 +115,7 @@ test "if peer expressions inferred optional type" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
 
     var self: []const u8 = "abcdef";
     var index: usize = 0;
diff --git a/test/behavior/switch.zig b/test/behavior/switch.zig
index 1643a2f697..132cef5c1e 100644
--- a/test/behavior/switch.zig
+++ b/test/behavior/switch.zig
@@ -509,7 +509,6 @@ test "return result loc and then switch with range implicit casted to error unio
 }
 
 test "switch with null and T peer types and inferred result location type" {
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO

From c604111e22cb2f41e3151b1062e19035f35a6dec Mon Sep 17 00:00:00 2001
From: Jacob Young <jacobly0@users.noreply.github.com>
Date: Fri, 24 Mar 2023 00:03:39 -0400
Subject: [PATCH 3/7] x86_64: fix block result value tracking

---
 src/arch/x86_64/CodeGen.zig | 64 +++++++++++++++----------------------
 1 file changed, 26 insertions(+), 38 deletions(-)

diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index 852a8b47be..45cb671e93 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -1085,6 +1085,17 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
     }
 }
 
+fn getValue(self: *Self, value: MCValue, inst: ?Air.Inst.Index) void {
+    const reg = switch (value) {
+        .register => |reg| reg,
+        .register_overflow => |ro| ro.reg,
+        else => return,
+    };
+    if (self.register_manager.isRegFree(reg)) {
+        self.register_manager.getRegAssumeFree(reg, inst);
+    }
+}
+
 fn freeValue(self: *Self, value: MCValue) void {
     switch (value) {
         .register => |reg| {
@@ -1136,25 +1147,10 @@ fn finishAir(self: *Self, inst: Air.Inst.Index, result: MCValue, operands: [Live
         log.debug("%{d} => {}", .{ inst, result });
         const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
         branch.inst_table.putAssumeCapacityNoClobber(inst, result);
-
-        // In some cases (such as bitcast), an operand
-        // may be the same MCValue as the result. If
-        // that operand died and was a register, it
-        // was freed by processDeath. We have to
-        // "re-allocate" the register.
-        switch (result) {
-            .register => |reg| {
-                if (self.register_manager.isRegFree(reg)) {
-                    self.register_manager.getRegAssumeFree(reg, inst);
-                }
-            },
-            .register_overflow => |ro| {
-                if (self.register_manager.isRegFree(ro.reg)) {
-                    self.register_manager.getRegAssumeFree(ro.reg, inst);
-                }
-            },
-            else => {},
-        }
+        // In some cases, an operand may be reused as the result.
+        // If that operand died and was a register, it was freed by
+        // processDeath, so we have to "re-allocate" the register.
+        self.getValue(result, inst);
     } else switch (result) {
         .none, .dead, .unreach => {},
         else => unreachable, // Why didn't the result die?
@@ -5609,14 +5605,14 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
     });
     defer self.blocks.getPtr(inst).?.relocs.deinit(self.gpa);
 
+    const ty = self.air.typeOfIndex(inst);
+    const unused = !ty.hasRuntimeBitsIgnoreComptime() or self.liveness.isUnused(inst);
     {
         // Here we use `.none` to represent a null value so that the first break
         // instruction will choose a MCValue for the block result and overwrite
         // this field. Following break instructions will use that MCValue to put
         // their block results.
-        const ty = self.air.typeOfIndex(inst);
-        const result: MCValue =
-            if (!ty.hasRuntimeBitsIgnoreComptime() or self.liveness.isUnused(inst)) .dead else .none;
+        const result: MCValue = if (unused) .dead else .none;
         const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
         branch.inst_table.putAssumeCapacityNoClobber(inst, result);
     }
@@ -5627,6 +5623,9 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
     try self.genBody(body);
 
     for (self.blocks.getPtr(inst).?.relocs.items) |reloc| try self.performReloc(reloc);
+
+    const result = if (unused) .dead else self.getResolvedInstValue(inst).?.*;
+    self.getValue(result, inst);
     self.finishAirBookkeeping();
 }
 
@@ -5837,25 +5836,14 @@ fn br(self: *Self, inst: Air.Inst.Index, block: Air.Inst.Index, operand: Air.Ins
         switch (dst_mcv.*) {
             .none => {
                 const result = result: {
-                    if (!self.reuseOperand(inst, operand, 0, src_mcv)) {
-                        const new_mcv = try self.allocRegOrMem(block, true);
-                        try self.setRegOrMem(self.air.typeOfIndex(block), new_mcv, src_mcv);
-                        break :result new_mcv;
-                    }
+                    if (self.reuseOperand(inst, operand, 0, src_mcv)) break :result src_mcv;
 
-                    // the value is actually tracked with block, not inst
-                    switch (src_mcv) {
-                        .register => |reg| if (!self.register_manager.isRegFree(reg)) {
-                            if (RegisterManager.indexOfRegIntoTracked(reg)) |index| {
-                                self.register_manager.registers[index] = block;
-                            }
-                        },
-                        .stack_offset => {},
-                        else => unreachable,
-                    }
-                    break :result src_mcv;
+                    const new_mcv = try self.allocRegOrMem(block, true);
+                    try self.setRegOrMem(self.air.typeOfIndex(block), new_mcv, src_mcv);
+                    break :result new_mcv;
                 };
                 dst_mcv.* = result;
+                self.freeValue(result);
             },
             else => try self.setRegOrMem(self.air.typeOfIndex(block), dst_mcv.*, src_mcv),
         }

From 935ec9ec6a571010ddc11a9212ff0c93f82d0d74 Mon Sep 17 00:00:00 2001
From: Jacob Young <jacobly0@users.noreply.github.com>
Date: Fri, 24 Mar 2023 04:40:45 -0400
Subject: [PATCH 4/7] x86_64: canonicalize each br of a block

---
 src/arch/x86_64/CodeGen.zig   | 139 ++++++++++++++++++++++++----------
 test/behavior/cast.zig        |   2 -
 test/behavior/enum.zig        |   1 -
 test/behavior/if.zig          |   1 -
 tools/lldb_pretty_printers.py |  15 ++++
 5 files changed, 114 insertions(+), 44 deletions(-)

diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index 45cb671e93..b1efcca1ac 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -213,7 +213,15 @@ const StackAllocation = struct {
 };
 
 const BlockData = struct {
-    relocs: std.ArrayListUnmanaged(Mir.Inst.Index),
+    relocs: std.ArrayListUnmanaged(Mir.Inst.Index) = .{},
+    branch: ?Branch = null,
+    branch_depth: u32,
+
+    fn deinit(self: *BlockData, gpa: Allocator) void {
+        if (self.branch) |*branch| branch.deinit(gpa);
+        self.relocs.deinit(gpa);
+        self.* = undefined;
+    }
 };
 
 const BigTomb = struct {
@@ -5233,11 +5241,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
     // that death now instead of later as this has an effect on
     // whether it needs to be spilled in the branches
     if (self.liveness.operandDies(inst, 0)) {
-        const op_int = @enumToInt(pl_op.operand);
-        if (op_int >= Air.Inst.Ref.typed_value_map.len) {
-            const op_index = @intCast(Air.Inst.Index, op_int - Air.Inst.Ref.typed_value_map.len);
-            self.processDeath(op_index);
-        }
+        if (Air.refToIndex(pl_op.operand)) |op_inst| self.processDeath(op_inst);
     }
 
     // Capture the state of register and stack allocation state so that we can revert to it.
@@ -5290,10 +5294,10 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
     for (self.branch_stack.items) |bs| {
         log.debug("{}", .{bs.fmtDebug()});
     }
-
     log.debug("Then branch: {}", .{then_branch.fmtDebug()});
     log.debug("Else branch: {}", .{else_branch.fmtDebug()});
-    try self.canonicaliseBranches(true, &then_branch, &else_branch);
+
+    try self.canonicaliseBranches(true, &then_branch, &else_branch, true);
 
     // We already took care of pl_op.operand earlier, so we're going
     // to pass .none here
@@ -5599,11 +5603,16 @@ fn airLoop(self: *Self, inst: Air.Inst.Index) !void {
 }
 
 fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
-    try self.blocks.putNoClobber(self.gpa, inst, .{
-        // A block is a setup to be able to jump to the end.
-        .relocs = .{},
-    });
-    defer self.blocks.getPtr(inst).?.relocs.deinit(self.gpa);
+    // A block is a setup to be able to jump to the end.
+    const branch_depth = @intCast(u32, self.branch_stack.items.len);
+    try self.blocks.putNoClobber(self.gpa, inst, .{ .branch_depth = branch_depth });
+    defer {
+        var block_data = self.blocks.fetchRemove(inst).?.value;
+        block_data.deinit(self.gpa);
+    }
+
+    try self.branch_stack.append(.{});
+    defer _ = self.branch_stack.pop();
 
     const ty = self.air.typeOfIndex(inst);
     const unused = !ty.hasRuntimeBitsIgnoreComptime() or self.liveness.isUnused(inst);
@@ -5613,8 +5622,8 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
         // this field. Following break instructions will use that MCValue to put
         // their block results.
         const result: MCValue = if (unused) .dead else .none;
-        const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
-        branch.inst_table.putAssumeCapacityNoClobber(inst, result);
+        const branch = &self.branch_stack.items[branch_depth];
+        try branch.inst_table.putNoClobber(self.gpa, inst, result);
     }
 
     const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
@@ -5622,7 +5631,23 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
     const body = self.air.extra[extra.end..][0..extra.data.body_len];
     try self.genBody(body);
 
-    for (self.blocks.getPtr(inst).?.relocs.items) |reloc| try self.performReloc(reloc);
+    const block_data = self.blocks.getPtr(inst).?;
+    {
+        const src_branch = block_data.branch orelse self.branch_stack.items[branch_depth];
+        const dst_branch = &self.branch_stack.items[branch_depth - 1];
+        try dst_branch.inst_table.ensureUnusedCapacity(self.gpa, src_branch.inst_table.count());
+        var it = src_branch.inst_table.iterator();
+        while (it.next()) |entry| {
+            const tracked_inst = entry.key_ptr.*;
+            const tracked_value = entry.value_ptr.*;
+            if (dst_branch.inst_table.fetchPutAssumeCapacity(tracked_inst, tracked_value)) |old_entry| {
+                self.freeValue(old_entry.value);
+            }
+            self.getValue(tracked_value, tracked_inst);
+        }
+    }
+
+    for (block_data.relocs.items) |reloc| try self.performReloc(reloc);
 
     const result = if (unused) .dead else self.getResolvedInstValue(inst).?.*;
     self.getValue(result, inst);
@@ -5647,11 +5672,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
     // that death now instead of later as this has an effect on
     // whether it needs to be spilled in the branches
     if (self.liveness.operandDies(inst, 0)) {
-        const op_int = @enumToInt(pl_op.operand);
-        if (op_int >= Air.Inst.Ref.typed_value_map.len) {
-            const op_index = @intCast(Air.Inst.Index, op_int - Air.Inst.Ref.typed_value_map.len);
-            self.processDeath(op_index);
-        }
+        if (Air.refToIndex(pl_op.operand)) |op_inst| self.processDeath(op_inst);
     }
 
     log.debug("airSwitch: %{d}", .{inst});
@@ -5704,8 +5725,9 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
             errdefer case_branch.deinit(self.gpa);
 
             log.debug("Case-{d} branch: {}", .{ case_i, case_branch.fmtDebug() });
+            const final = case_i == cases_len - 1;
             if (prev_branch) |*canon_branch| {
-                try self.canonicaliseBranches(case_i == cases_len - 1, canon_branch, &case_branch);
+                try self.canonicaliseBranches(final, canon_branch, &case_branch, true);
                 canon_branch.deinit(self.gpa);
             }
             prev_branch = case_branch;
@@ -5740,7 +5762,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
 
             log.debug("Else branch: {}", .{else_branch.fmtDebug()});
             if (prev_branch) |*canon_branch| {
-                try self.canonicaliseBranches(true, canon_branch, &else_branch);
+                try self.canonicaliseBranches(true, canon_branch, &else_branch, true);
                 canon_branch.deinit(self.gpa);
             }
             prev_branch = else_branch;
@@ -5756,27 +5778,30 @@ fn canonicaliseBranches(
     update_parent: bool,
     canon_branch: *Branch,
     target_branch: *const Branch,
+    comptime assert_same_deaths: bool,
 ) !void {
     const parent_branch =
         if (update_parent) &self.branch_stack.items[self.branch_stack.items.len - 1] else undefined;
-    if (update_parent) try self.ensureProcessDeathCapacity(target_branch.inst_table.count());
 
-    const target_slice = target_branch.inst_table.entries.slice();
-    for (target_slice.items(.key), target_slice.items(.value)) |target_key, target_value| {
+    if (update_parent) try self.ensureProcessDeathCapacity(target_branch.inst_table.count());
+    var target_it = target_branch.inst_table.iterator();
+    while (target_it.next()) |target_entry| {
+        const target_key = target_entry.key_ptr.*;
+        const target_value = target_entry.value_ptr.*;
         const canon_mcv = if (canon_branch.inst_table.fetchSwapRemove(target_key)) |canon_entry| blk: {
             // The instruction's MCValue is overridden in both branches.
             if (update_parent) {
                 parent_branch.inst_table.putAssumeCapacity(target_key, canon_entry.value);
             }
             if (target_value == .dead) {
-                assert(canon_entry.value == .dead);
+                if (assert_same_deaths) assert(canon_entry.value == .dead);
                 continue;
             }
             break :blk canon_entry.value;
         } else blk: {
             if (target_value == .dead) continue;
             // The instruction is only overridden in the else branch.
-            // If integer overflows occurs, the question is: why wasn't the instruction marked dead?
+            // If integer overflow occurs, the question is: why wasn't the instruction marked dead?
             break :blk self.getResolvedInstValue(target_key).?.*;
         };
         log.debug("consolidating target_entry {d} {}=>{}", .{ target_key, target_value, canon_mcv });
@@ -5786,9 +5811,12 @@ fn canonicaliseBranches(
         self.freeValue(target_value);
         // TODO track the new register / stack allocation
     }
+
     if (update_parent) try self.ensureProcessDeathCapacity(canon_branch.inst_table.count());
-    const canon_slice = canon_branch.inst_table.entries.slice();
-    for (canon_slice.items(.key), canon_slice.items(.value)) |canon_key, canon_value| {
+    var canon_it = canon_branch.inst_table.iterator();
+    while (canon_it.next()) |canon_entry| {
+        const canon_key = canon_entry.key_ptr.*;
+        const canon_value = canon_entry.value_ptr.*;
         // We already deleted the items from this table that matched the target_branch.
         // So these are all instructions that are only overridden in the canon branch.
         const parent_mcv =
@@ -5821,22 +5849,19 @@ fn performReloc(self: *Self, reloc: Mir.Inst.Index) !void {
 }
 
 fn airBr(self: *Self, inst: Air.Inst.Index) !void {
-    const branch = self.air.instructions.items(.data)[inst].br;
-    try self.br(inst, branch.block_inst, branch.operand);
-    return self.finishAir(inst, .dead, .{ branch.operand, .none, .none });
-}
+    const br = self.air.instructions.items(.data)[inst].br;
+    const block = br.block_inst;
 
-fn br(self: *Self, inst: Air.Inst.Index, block: Air.Inst.Index, operand: Air.Inst.Ref) !void {
     // The first break instruction encounters `.none` here and chooses a
     // machine code value for the block result, populating this field.
     // Following break instructions encounter that value and use it for
     // the location to store their block results.
     if (self.getResolvedInstValue(block)) |dst_mcv| {
-        const src_mcv = try self.resolveInst(operand);
+        const src_mcv = try self.resolveInst(br.operand);
         switch (dst_mcv.*) {
             .none => {
                 const result = result: {
-                    if (self.reuseOperand(inst, operand, 0, src_mcv)) break :result src_mcv;
+                    if (self.reuseOperand(inst, br.operand, 0, src_mcv)) break :result src_mcv;
 
                     const new_mcv = try self.allocRegOrMem(block, true);
                     try self.setRegOrMem(self.air.typeOfIndex(block), new_mcv, src_mcv);
@@ -5848,16 +5873,50 @@ fn br(self: *Self, inst: Air.Inst.Index, block: Air.Inst.Index, operand: Air.Ins
             else => try self.setRegOrMem(self.air.typeOfIndex(block), dst_mcv.*, src_mcv),
         }
     }
-    return self.brVoid(block);
-}
 
-fn brVoid(self: *Self, block: Air.Inst.Index) !void {
+    // Process operand death early so that it is properly accounted for in the Branch below.
+    if (self.liveness.operandDies(inst, 0)) {
+        if (Air.refToIndex(br.operand)) |op_inst| self.processDeath(op_inst);
+    }
+
     const block_data = self.blocks.getPtr(block).?;
+    {
+        var branch = Branch{};
+        errdefer branch.deinit(self.gpa);
+
+        var branch_i = self.branch_stack.items.len - 1;
+        while (branch_i >= block_data.branch_depth) : (branch_i -= 1) {
+            const table = &self.branch_stack.items[branch_i].inst_table;
+            try branch.inst_table.ensureUnusedCapacity(self.gpa, table.count());
+            var it = table.iterator();
+            while (it.next()) |entry| {
+                const gop = branch.inst_table.getOrPutAssumeCapacity(entry.key_ptr.*);
+                if (!gop.found_existing) gop.value_ptr.* = entry.value_ptr.*;
+            }
+        }
+
+        if (block_data.branch) |*prev_branch| {
+            log.debug("brVoid: %{d}", .{inst});
+            log.debug("Upper branches:", .{});
+            for (self.branch_stack.items) |bs| {
+                log.debug("{}", .{bs.fmtDebug()});
+            }
+            log.debug("Prev branch: {}", .{prev_branch.fmtDebug()});
+            log.debug("Cur branch: {}", .{branch.fmtDebug()});
+
+            try self.canonicaliseBranches(false, prev_branch, &branch, false);
+            prev_branch.deinit(self.gpa);
+        }
+        block_data.branch = branch;
+    }
+
     // Emit a jump with a relocation. It will be patched up after the block ends.
     try block_data.relocs.ensureUnusedCapacity(self.gpa, 1);
     // Leave the jump offset undefined
     const jmp_reloc = try self.asmJmpReloc(undefined);
     block_data.relocs.appendAssumeCapacity(jmp_reloc);
+
+    self.finishAirBookkeeping();
 }
 
 fn airAsm(self: *Self, inst: Air.Inst.Index) !void {
diff --git a/test/behavior/cast.zig b/test/behavior/cast.zig
index 978c75c92f..9ba91c64f3 100644
--- a/test/behavior/cast.zig
+++ b/test/behavior/cast.zig
@@ -38,8 +38,6 @@ fn peerTypeTAndOptionalT(c: bool, b: bool) ?usize {
 }
 
 test "resolve undefined with integer" {
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
-
     try testResolveUndefWithInt(true, 1234);
     comptime try testResolveUndefWithInt(true, 1234);
 }
diff --git a/test/behavior/enum.zig b/test/behavior/enum.zig
index 72784d9abc..9076f9f9ac 100644
--- a/test/behavior/enum.zig
+++ b/test/behavior/enum.zig
@@ -904,7 +904,6 @@ test "enum literal casting to tagged union" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
 
     const Arch = union(enum) {
         x86_64,
diff --git a/test/behavior/if.zig b/test/behavior/if.zig
index 223d73ac85..730c0713c6 100644
--- a/test/behavior/if.zig
+++ b/test/behavior/if.zig
@@ -115,7 +115,6 @@ test "if peer expressions inferred optional type" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
 
     var self: []const u8 = "abcdef";
     var index: usize = 0;
diff --git a/tools/lldb_pretty_printers.py b/tools/lldb_pretty_printers.py
index b72b6f9760..e0b84e1b41 100644
--- a/tools/lldb_pretty_printers.py
+++ b/tools/lldb_pretty_printers.py
@@ -164,6 +164,20 @@ class zig_ErrorUnion_SynthProvider:
     def get_child_index(self, name): return 0 if name == ('payload' if self.payload else 'error_set') else -1
     def get_child_at_index(self, index): return self.payload or self.error_set if index == 0 else None
 
+class zig_TaggedUnion_SynthProvider:
+    def __init__(self, value, _=None): self.value = value
+    def update(self):
+        try:
+            self.tag = self.value.GetChildMemberWithName('tag')
+            self.payload = self.value.GetChildMemberWithName('payload').GetChildMemberWithName(self.tag.value)
+        except: pass
+    def has_children(self): return True
+    def num_children(self): return 1 + (self.payload is not None)
+    def get_child_index(self, name):
+        try: return ('tag', 'payload').index(name)
+        except: return -1
+    def get_child_at_index(self, index): return (self.tag, self.payload)[index] if index >= 0 and index < 2 else None
+
 # Define Zig Standard Library
 
 class std_SegmentedList_SynthProvider:
@@ -606,3 +620,4 @@ def __lldb_init_module(debugger, _=None):
     add(debugger, category='zig.stage2', type='type.Type', summary=True)
     add(debugger, category='zig.stage2', type='value.Value', identifier='TagOrPayloadPtr', synth=True)
     add(debugger, category='zig.stage2', type='value.Value', summary=True)
+    add(debugger, category='zig.stage2', type='arch.x86_64.CodeGen.MCValue', identifier='zig_TaggedUnion', synth=True, inline_children=True, summary=True)

From 5e0f09168452bb099d487fd4e2fb6b9f37de5640 Mon Sep 17 00:00:00 2001
From: Jacob Young <jacobly0@users.noreply.github.com>
Date: Fri, 24 Mar 2023 17:44:26 -0400
Subject: [PATCH 5/7] x86_64: try to fix br canonicalization

---
 src/arch/x86_64/CodeGen.zig | 106 +++++++++++++++++++-----------------
 1 file changed, 57 insertions(+), 49 deletions(-)

diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index b1efcca1ac..e5028b17fc 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -214,11 +214,11 @@ const StackAllocation = struct {
 
 const BlockData = struct {
     relocs: std.ArrayListUnmanaged(Mir.Inst.Index) = .{},
-    branch: ?Branch = null,
+    branch: Branch = .{},
     branch_depth: u32,
 
     fn deinit(self: *BlockData, gpa: Allocator) void {
-        if (self.branch) |*branch| branch.deinit(gpa);
+        self.branch.deinit(gpa);
         self.relocs.deinit(gpa);
         self.* = undefined;
     }
@@ -2759,7 +2759,7 @@ fn airClz(self: *Self, inst: Air.Inst.Index) !void {
         };
         defer if (mat_src_lock) |lock| self.register_manager.unlockReg(lock);
 
-        const dst_reg = try self.register_manager.allocReg(inst, gp);
+        const dst_reg = try self.register_manager.allocReg(null, gp);
         const dst_mcv = MCValue{ .register = dst_reg };
         const dst_lock = self.register_manager.lockReg(dst_reg);
         defer if (dst_lock) |lock| self.register_manager.unlockReg(lock);
@@ -2774,14 +2774,14 @@ fn airClz(self: *Self, inst: Air.Inst.Index) !void {
         }
 
         const src_bits = src_ty.bitSize(self.target.*);
-        const width_reg = try self.copyToTmpRegister(dst_ty, .{ .immediate = src_bits });
-        const width_mcv = MCValue{ .register = width_reg };
+        const width_mcv =
+            try self.copyToRegisterWithInstTracking(inst, dst_ty, .{ .immediate = src_bits });
         try self.genBinOpMir(.bsr, src_ty, dst_mcv, mat_src_mcv);
 
         const dst_abi_size = @intCast(u32, @max(dst_ty.abiSize(self.target.*), 2));
         try self.asmCmovccRegisterRegister(
             registerAlias(dst_reg, dst_abi_size),
-            registerAlias(width_reg, dst_abi_size),
+            registerAlias(width_mcv.register, dst_abi_size),
             .z,
         );
 
@@ -2845,7 +2845,6 @@ fn airCtz(self: *Self, inst: Air.Inst.Index) !void {
             registerAlias(width_reg, abi_size),
             .z,
         );
-
         break :result dst_mcv;
     };
     return self.finishAir(inst, result, .{ ty_op.operand, .none, .none });
@@ -5297,7 +5296,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
     log.debug("Then branch: {}", .{then_branch.fmtDebug()});
     log.debug("Else branch: {}", .{else_branch.fmtDebug()});
 
-    try self.canonicaliseBranches(true, &then_branch, &else_branch, true);
+    try self.canonicaliseBranches(true, &then_branch, &else_branch, true, true);
 
     // We already took care of pl_op.operand earlier, so we're going
     // to pass .none here
@@ -5611,41 +5610,32 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
         block_data.deinit(self.gpa);
     }
 
-    try self.branch_stack.append(.{});
-    defer _ = self.branch_stack.pop();
-
     const ty = self.air.typeOfIndex(inst);
     const unused = !ty.hasRuntimeBitsIgnoreComptime() or self.liveness.isUnused(inst);
+
     {
         // Here we use `.none` to represent a null value so that the first break
         // instruction will choose a MCValue for the block result and overwrite
         // this field. Following break instructions will use that MCValue to put
         // their block results.
         const result: MCValue = if (unused) .dead else .none;
-        const branch = &self.branch_stack.items[branch_depth];
+        const branch = &self.branch_stack.items[branch_depth - 1];
         try branch.inst_table.putNoClobber(self.gpa, inst, result);
     }
 
-    const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
-    const extra = self.air.extraData(Air.Block, ty_pl.payload);
-    const body = self.air.extra[extra.end..][0..extra.data.body_len];
-    try self.genBody(body);
+    {
+        try self.branch_stack.append(.{});
+        errdefer _ = self.branch_stack.pop();
+
+        const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
+        const extra = self.air.extraData(Air.Block, ty_pl.payload);
+        const body = self.air.extra[extra.end..][0..extra.data.body_len];
+        try self.genBody(body);
+    }
 
     const block_data = self.blocks.getPtr(inst).?;
-    {
-        const src_branch = block_data.branch orelse self.branch_stack.items[branch_depth];
-        const dst_branch = &self.branch_stack.items[branch_depth - 1];
-        try dst_branch.inst_table.ensureUnusedCapacity(self.gpa, src_branch.inst_table.count());
-        var it = src_branch.inst_table.iterator();
-        while (it.next()) |entry| {
-            const tracked_inst = entry.key_ptr.*;
-            const tracked_value = entry.value_ptr.*;
-            if (dst_branch.inst_table.fetchPutAssumeCapacity(tracked_inst, tracked_value)) |old_entry| {
-                self.freeValue(old_entry.value);
-            }
-            self.getValue(tracked_value, tracked_inst);
-        }
-    }
+    const target_branch = self.branch_stack.pop();
+    try self.canonicaliseBranches(true, &block_data.branch, &target_branch, false, false);
 
     for (block_data.relocs.items) |reloc| try self.performReloc(reloc);
 
@@ -5727,7 +5717,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
             log.debug("Case-{d} branch: {}", .{ case_i, case_branch.fmtDebug() });
             const final = case_i == cases_len - 1;
             if (prev_branch) |*canon_branch| {
-                try self.canonicaliseBranches(final, canon_branch, &case_branch, true);
+                try self.canonicaliseBranches(final, canon_branch, &case_branch, true, true);
                 canon_branch.deinit(self.gpa);
             }
             prev_branch = case_branch;
@@ -5762,7 +5752,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
 
             log.debug("Else branch: {}", .{else_branch.fmtDebug()});
             if (prev_branch) |*canon_branch| {
-                try self.canonicaliseBranches(true, canon_branch, &else_branch, true);
+                try self.canonicaliseBranches(true, canon_branch, &else_branch, true, true);
                 canon_branch.deinit(self.gpa);
             }
             prev_branch = else_branch;
@@ -5778,6 +5768,7 @@ fn canonicaliseBranches(
     update_parent: bool,
     canon_branch: *Branch,
     target_branch: *const Branch,
+    comptime set_values: bool,
     comptime assert_same_deaths: bool,
 ) !void {
     const parent_branch =
@@ -5790,16 +5781,24 @@ fn canonicaliseBranches(
         const target_value = target_entry.value_ptr.*;
         const canon_mcv = if (canon_branch.inst_table.fetchSwapRemove(target_key)) |canon_entry| blk: {
             // The instruction's MCValue is overridden in both branches.
-            if (update_parent) {
-                parent_branch.inst_table.putAssumeCapacity(target_key, canon_entry.value);
-            }
             if (target_value == .dead) {
+                if (update_parent) {
+                    parent_branch.inst_table.putAssumeCapacity(target_key, .dead);
+                }
                 if (assert_same_deaths) assert(canon_entry.value == .dead);
                 continue;
             }
+            if (update_parent) {
+                parent_branch.inst_table.putAssumeCapacity(target_key, canon_entry.value);
+            }
             break :blk canon_entry.value;
         } else blk: {
-            if (target_value == .dead) continue;
+            if (target_value == .dead) {
+                if (update_parent) {
+                    parent_branch.inst_table.putAssumeCapacity(target_key, .dead);
+                }
+                continue;
+            }
             // The instruction is only overridden in the else branch.
             // If integer overflow occurs, the question is: why wasn't the instruction marked dead?
             break :blk self.getResolvedInstValue(target_key).?.*;
@@ -5807,7 +5806,9 @@ fn canonicaliseBranches(
         log.debug("consolidating target_entry {d} {}=>{}", .{ target_key, target_value, canon_mcv });
         // TODO make sure the destination stack offset / register does not already have something
         // going on there.
-        try self.setRegOrMem(self.air.typeOfIndex(target_key), canon_mcv, target_value);
+        if (set_values) {
+            try self.setRegOrMem(self.air.typeOfIndex(target_key), canon_mcv, target_value);
+        } else self.getValue(canon_mcv, target_key);
         self.freeValue(target_value);
         // TODO track the new register / stack allocation
     }
@@ -5825,7 +5826,9 @@ fn canonicaliseBranches(
             log.debug("consolidating canon_entry {d} {}=>{}", .{ canon_key, parent_mcv, canon_value });
             // TODO make sure the destination stack offset / register does not already have something
             // going on there.
-            try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);
+            if (set_values) {
+                try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);
+            } else self.getValue(canon_value, canon_key);
             self.freeValue(parent_mcv);
             // TODO track the new register / stack allocation
         }
@@ -5890,23 +5893,28 @@ fn airBr(self: *Self, inst: Air.Inst.Index) !void {
             try branch.inst_table.ensureUnusedCapacity(self.gpa, table.count());
             var it = table.iterator();
             while (it.next()) |entry| {
+                // This loop could be avoided by tracking inst depth, which
+                // will be needed later anyway for reusing loop deaths.
+                var parent_branch_i = block_data.branch_depth - 1;
+                while (parent_branch_i > 0) : (parent_branch_i -= 1) {
+                    const parent_table = &self.branch_stack.items[parent_branch_i].inst_table;
+                    if (parent_table.contains(entry.key_ptr.*)) break;
+                } else continue;
                 const gop = branch.inst_table.getOrPutAssumeCapacity(entry.key_ptr.*);
                 if (!gop.found_existing) gop.value_ptr.* = entry.value_ptr.*;
             }
         }
 
-        if (block_data.branch) |*prev_branch| {
-            log.debug("brVoid: %{d}", .{inst});
-            log.debug("Upper branches:", .{});
-            for (self.branch_stack.items) |bs| {
-                log.debug("{}", .{bs.fmtDebug()});
-            }
-            log.debug("Prev branch: {}", .{prev_branch.fmtDebug()});
-            log.debug("Cur branch: {}", .{branch.fmtDebug()});
-
-            try self.canonicaliseBranches(false, prev_branch, &branch, false);
-            prev_branch.deinit(self.gpa);
+        log.debug("airBr: %{d}", .{inst});
+        log.debug("Upper branches:", .{});
+        for (self.branch_stack.items) |bs| {
+            log.debug("{}", .{bs.fmtDebug()});
         }
+        log.debug("Prev branch: {}", .{block_data.branch.fmtDebug()});
+        log.debug("Cur branch: {}", .{branch.fmtDebug()});
+
+        try self.canonicaliseBranches(false, &block_data.branch, &branch, true, false);
+        block_data.branch.deinit(self.gpa);
         block_data.branch = branch;
     }
 

From 0987ed1970bc0332844a0e398b3c981b38627ed6 Mon Sep 17 00:00:00 2001
From: Jacob Young <jacobly0@users.noreply.github.com>
Date: Fri, 24 Mar 2023 17:55:47 -0400
Subject: [PATCH 6/7] x86_64: detect canonicalisation hazards

---
 src/arch/x86_64/CodeGen.zig | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index e5028b17fc..7335d3ac94 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -5771,6 +5771,9 @@ fn canonicaliseBranches(
     comptime set_values: bool,
     comptime assert_same_deaths: bool,
 ) !void {
+    var hazard_map = std.AutoHashMap(MCValue, void).init(self.gpa);
+    defer hazard_map.deinit();
+
     const parent_branch =
         if (update_parent) &self.branch_stack.items[self.branch_stack.items.len - 1] else undefined;
 
@@ -5804,8 +5807,10 @@ fn canonicaliseBranches(
             break :blk self.getResolvedInstValue(target_key).?.*;
         };
         log.debug("consolidating target_entry {d} {}=>{}", .{ target_key, target_value, canon_mcv });
-        // TODO make sure the destination stack offset / register does not already have something
+        // TODO handle the case where the destination stack offset / register has something
         // going on there.
+        assert(!hazard_map.contains(target_value));
+        try hazard_map.putNoClobber(canon_mcv, {});
         if (set_values) {
             try self.setRegOrMem(self.air.typeOfIndex(target_key), canon_mcv, target_value);
         } else self.getValue(canon_mcv, target_key);
@@ -5824,8 +5829,10 @@ fn canonicaliseBranches(
             if (canon_value != .dead) self.getResolvedInstValue(canon_key).?.* else undefined;
         if (canon_value != .dead) {
             log.debug("consolidating canon_entry {d} {}=>{}", .{ canon_key, parent_mcv, canon_value });
-            // TODO make sure the destination stack offset / register does not already have something
+            // TODO handle the case where the destination stack offset / register has something
             // going on there.
+            assert(!hazard_map.contains(parent_mcv));
+            try hazard_map.putNoClobber(canon_value, {});
             if (set_values) {
                 try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);
             } else self.getValue(canon_value, canon_key);

From 4ab4bd04fe8f4308d67b757eaa88f5a356aea688 Mon Sep 17 00:00:00 2001
From: Jacob Young <jacobly0@users.noreply.github.com>
Date: Fri, 24 Mar 2023 21:33:17 -0400
Subject: [PATCH 7/7] x86_64: add back assume unused

This seems to have been asserting due to a value tracking bug that has
since been fixed.
---
 src/arch/x86_64/CodeGen.zig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index 7335d3ac94..66e1904420 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -2586,7 +2586,7 @@ fn airPtrElemVal(self: *Self, inst: Air.Inst.Index) !void {
     const index_ty = self.air.typeOf(bin_op.rhs);
     const index = try self.resolveInst(bin_op.rhs);
     const index_lock: ?RegisterLock = switch (index) {
-        .register => |reg| self.register_manager.lockReg(reg),
+        .register => |reg| self.register_manager.lockRegAssumeUnused(reg),
         else => null,
     };
     defer if (index_lock) |lock| self.register_manager.unlockReg(lock);
@@ -2635,7 +2635,7 @@ fn airPtrElemPtr(self: *Self, inst: Air.Inst.Index) !void {
     const index_ty = self.air.typeOf(extra.rhs);
     const index = try self.resolveInst(extra.rhs);
     const index_lock: ?RegisterLock = switch (index) {
-        .register => |reg| self.register_manager.lockReg(reg),
+        .register => |reg| self.register_manager.lockRegAssumeUnused(reg),
         else => null,
     };
     defer if (index_lock) |lock| self.register_manager.unlockReg(lock);