From 935ec9ec6a571010ddc11a9212ff0c93f82d0d74 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Fri, 24 Mar 2023 04:40:45 -0400 Subject: [PATCH] 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)