authorgravatar for jacobly@ziglang.orgJacob Young <jacobly@ziglang.org> 2023-03-24 17:44:26-04:00
committergravatar for jacobly@ziglang.orgJacob Young <jacobly@ziglang.org> 2023-03-24 17:57:58-04:00
log5e0f09168452bb099d487fd4e2fb6b9f37de5640
tree7a865fefb21d32e13f93ff66f05ef4eea48f6cba
parent935ec9ec6a571010ddc11a9212ff0c93f82d0d74

x86_64: try to fix br canonicalization


1 files changed, 57 insertions(+), 49 deletions(-)

src/arch/x86_64/CodeGen.zig+57-49
...@@ -214,11 +214,11 @@ const StackAllocation = struct {...@@ -214,11 +214,11 @@ const StackAllocation = struct {
214214
215const BlockData = struct {215const BlockData = struct {
216 relocs: std.ArrayListUnmanaged(Mir.Inst.Index) = .{},216 relocs: std.ArrayListUnmanaged(Mir.Inst.Index) = .{},
217 branch: ?Branch = null,217 branch: Branch = .{},
218 branch_depth: u32,218 branch_depth: u32,
219219
220 fn deinit(self: *BlockData, gpa: Allocator) void {220 fn deinit(self: *BlockData, gpa: Allocator) void {
221 if (self.branch) |*branch| branch.deinit(gpa);221 self.branch.deinit(gpa);
222 self.relocs.deinit(gpa);222 self.relocs.deinit(gpa);
223 self.* = undefined;223 self.* = undefined;
224 }224 }
...@@ -2759,7 +2759,7 @@ fn airClz(self: *Self, inst: Air.Inst.Index) !void {...@@ -2759,7 +2759,7 @@ fn airClz(self: *Self, inst: Air.Inst.Index) !void {
2759 };2759 };
2760 defer if (mat_src_lock) |lock| self.register_manager.unlockReg(lock);2760 defer if (mat_src_lock) |lock| self.register_manager.unlockReg(lock);
27612761
2762 const dst_reg = try self.register_manager.allocReg(inst, gp);2762 const dst_reg = try self.register_manager.allocReg(null, gp);
2763 const dst_mcv = MCValue{ .register = dst_reg };2763 const dst_mcv = MCValue{ .register = dst_reg };
2764 const dst_lock = self.register_manager.lockReg(dst_reg);2764 const dst_lock = self.register_manager.lockReg(dst_reg);
2765 defer if (dst_lock) |lock| self.register_manager.unlockReg(lock);2765 defer if (dst_lock) |lock| self.register_manager.unlockReg(lock);
...@@ -2774,14 +2774,14 @@ fn airClz(self: *Self, inst: Air.Inst.Index) !void {...@@ -2774,14 +2774,14 @@ fn airClz(self: *Self, inst: Air.Inst.Index) !void {
2774 }2774 }
27752775
2776 const src_bits = src_ty.bitSize(self.target.*);2776 const src_bits = src_ty.bitSize(self.target.*);
2777 const width_reg = try self.copyToTmpRegister(dst_ty, .{ .immediate = src_bits });2777 const width_mcv =
2778 const width_mcv = MCValue{ .register = width_reg };2778 try self.copyToRegisterWithInstTracking(inst, dst_ty, .{ .immediate = src_bits });
2779 try self.genBinOpMir(.bsr, src_ty, dst_mcv, mat_src_mcv);2779 try self.genBinOpMir(.bsr, src_ty, dst_mcv, mat_src_mcv);
27802780
2781 const dst_abi_size = @intCast(u32, @max(dst_ty.abiSize(self.target.*), 2));2781 const dst_abi_size = @intCast(u32, @max(dst_ty.abiSize(self.target.*), 2));
2782 try self.asmCmovccRegisterRegister(2782 try self.asmCmovccRegisterRegister(
2783 registerAlias(dst_reg, dst_abi_size),2783 registerAlias(dst_reg, dst_abi_size),
2784 registerAlias(width_reg, dst_abi_size),2784 registerAlias(width_mcv.register, dst_abi_size),
2785 .z,2785 .z,
2786 );2786 );
27872787
...@@ -2845,7 +2845,6 @@ fn airCtz(self: *Self, inst: Air.Inst.Index) !void {...@@ -2845,7 +2845,6 @@ fn airCtz(self: *Self, inst: Air.Inst.Index) !void {
2845 registerAlias(width_reg, abi_size),2845 registerAlias(width_reg, abi_size),
2846 .z,2846 .z,
2847 );2847 );
2848
2849 break :result dst_mcv;2848 break :result dst_mcv;
2850 };2849 };
2851 return self.finishAir(inst, result, .{ ty_op.operand, .none, .none });2850 return self.finishAir(inst, result, .{ ty_op.operand, .none, .none });
...@@ -5297,7 +5296,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {...@@ -5297,7 +5296,7 @@ fn airCondBr(self: *Self, inst: Air.Inst.Index) !void {
5297 log.debug("Then branch: {}", .{then_branch.fmtDebug()});5296 log.debug("Then branch: {}", .{then_branch.fmtDebug()});
5298 log.debug("Else branch: {}", .{else_branch.fmtDebug()});5297 log.debug("Else branch: {}", .{else_branch.fmtDebug()});
52995298
5300 try self.canonicaliseBranches(true, &then_branch, &else_branch, true);5299 try self.canonicaliseBranches(true, &then_branch, &else_branch, true, true);
53015300
5302 // We already took care of pl_op.operand earlier, so we're going5301 // We already took care of pl_op.operand earlier, so we're going
5303 // to pass .none here5302 // to pass .none here
...@@ -5611,42 +5610,33 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {...@@ -5611,42 +5610,33 @@ fn airBlock(self: *Self, inst: Air.Inst.Index) !void {
5611 block_data.deinit(self.gpa);5610 block_data.deinit(self.gpa);
5612 }5611 }
56135612
5614 try self.branch_stack.append(.{});
5615 defer _ = self.branch_stack.pop();
5616
5617 const ty = self.air.typeOfIndex(inst);5613 const ty = self.air.typeOfIndex(inst);
5618 const unused = !ty.hasRuntimeBitsIgnoreComptime() or self.liveness.isUnused(inst);5614 const unused = !ty.hasRuntimeBitsIgnoreComptime() or self.liveness.isUnused(inst);
5615
5619 {5616 {
5620 // Here we use `.none` to represent a null value so that the first break5617 // Here we use `.none` to represent a null value so that the first break
5621 // instruction will choose a MCValue for the block result and overwrite5618 // instruction will choose a MCValue for the block result and overwrite
5622 // this field. Following break instructions will use that MCValue to put5619 // this field. Following break instructions will use that MCValue to put
5623 // their block results.5620 // their block results.
5624 const result: MCValue = if (unused) .dead else .none;5621 const result: MCValue = if (unused) .dead else .none;
5625 const branch = &self.branch_stack.items[branch_depth];5622 const branch = &self.branch_stack.items[branch_depth - 1];
5626 try branch.inst_table.putNoClobber(self.gpa, inst, result);5623 try branch.inst_table.putNoClobber(self.gpa, inst, result);
5627 }5624 }
56285625
5629 const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
5630 const extra = self.air.extraData(Air.Block, ty_pl.payload);
5631 const body = self.air.extra[extra.end..][0..extra.data.body_len];
5632 try self.genBody(body);
5633
5634 const block_data = self.blocks.getPtr(inst).?;
5635 {5626 {
5636 const src_branch = block_data.branch orelse self.branch_stack.items[branch_depth];5627 try self.branch_stack.append(.{});
5637 const dst_branch = &self.branch_stack.items[branch_depth - 1];5628 errdefer _ = self.branch_stack.pop();
5638 try dst_branch.inst_table.ensureUnusedCapacity(self.gpa, src_branch.inst_table.count());5629
5639 var it = src_branch.inst_table.iterator();5630 const ty_pl = self.air.instructions.items(.data)[inst].ty_pl;
5640 while (it.next()) |entry| {5631 const extra = self.air.extraData(Air.Block, ty_pl.payload);
5641 const tracked_inst = entry.key_ptr.*;5632 const body = self.air.extra[extra.end..][0..extra.data.body_len];
5642 const tracked_value = entry.value_ptr.*;5633 try self.genBody(body);
5643 if (dst_branch.inst_table.fetchPutAssumeCapacity(tracked_inst, tracked_value)) |old_entry| {
5644 self.freeValue(old_entry.value);
5645 }
5646 self.getValue(tracked_value, tracked_inst);
5647 }
5648 }5634 }
56495635
5636 const block_data = self.blocks.getPtr(inst).?;
5637 const target_branch = self.branch_stack.pop();
5638 try self.canonicaliseBranches(true, &block_data.branch, &target_branch, false, false);
5639
5650 for (block_data.relocs.items) |reloc| try self.performReloc(reloc);5640 for (block_data.relocs.items) |reloc| try self.performReloc(reloc);
56515641
5652 const result = if (unused) .dead else self.getResolvedInstValue(inst).?.*;5642 const result = if (unused) .dead else self.getResolvedInstValue(inst).?.*;
...@@ -5727,7 +5717,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {...@@ -5727,7 +5717,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
5727 log.debug("Case-{d} branch: {}", .{ case_i, case_branch.fmtDebug() });5717 log.debug("Case-{d} branch: {}", .{ case_i, case_branch.fmtDebug() });
5728 const final = case_i == cases_len - 1;5718 const final = case_i == cases_len - 1;
5729 if (prev_branch) |*canon_branch| {5719 if (prev_branch) |*canon_branch| {
5730 try self.canonicaliseBranches(final, canon_branch, &case_branch, true);5720 try self.canonicaliseBranches(final, canon_branch, &case_branch, true, true);
5731 canon_branch.deinit(self.gpa);5721 canon_branch.deinit(self.gpa);
5732 }5722 }
5733 prev_branch = case_branch;5723 prev_branch = case_branch;
...@@ -5762,7 +5752,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {...@@ -5762,7 +5752,7 @@ fn airSwitch(self: *Self, inst: Air.Inst.Index) !void {
57625752
5763 log.debug("Else branch: {}", .{else_branch.fmtDebug()});5753 log.debug("Else branch: {}", .{else_branch.fmtDebug()});
5764 if (prev_branch) |*canon_branch| {5754 if (prev_branch) |*canon_branch| {
5765 try self.canonicaliseBranches(true, canon_branch, &else_branch, true);5755 try self.canonicaliseBranches(true, canon_branch, &else_branch, true, true);
5766 canon_branch.deinit(self.gpa);5756 canon_branch.deinit(self.gpa);
5767 }5757 }
5768 prev_branch = else_branch;5758 prev_branch = else_branch;
...@@ -5778,6 +5768,7 @@ fn canonicaliseBranches(...@@ -5778,6 +5768,7 @@ fn canonicaliseBranches(
5778 update_parent: bool,5768 update_parent: bool,
5779 canon_branch: *Branch,5769 canon_branch: *Branch,
5780 target_branch: *const Branch,5770 target_branch: *const Branch,
5771 comptime set_values: bool,
5781 comptime assert_same_deaths: bool,5772 comptime assert_same_deaths: bool,
5782) !void {5773) !void {
5783 const parent_branch =5774 const parent_branch =
...@@ -5790,16 +5781,24 @@ fn canonicaliseBranches(...@@ -5790,16 +5781,24 @@ fn canonicaliseBranches(
5790 const target_value = target_entry.value_ptr.*;5781 const target_value = target_entry.value_ptr.*;
5791 const canon_mcv = if (canon_branch.inst_table.fetchSwapRemove(target_key)) |canon_entry| blk: {5782 const canon_mcv = if (canon_branch.inst_table.fetchSwapRemove(target_key)) |canon_entry| blk: {
5792 // The instruction's MCValue is overridden in both branches.5783 // The instruction's MCValue is overridden in both branches.
5793 if (update_parent) {
5794 parent_branch.inst_table.putAssumeCapacity(target_key, canon_entry.value);
5795 }
5796 if (target_value == .dead) {5784 if (target_value == .dead) {
5785 if (update_parent) {
5786 parent_branch.inst_table.putAssumeCapacity(target_key, .dead);
5787 }
5797 if (assert_same_deaths) assert(canon_entry.value == .dead);5788 if (assert_same_deaths) assert(canon_entry.value == .dead);
5798 continue;5789 continue;
5799 }5790 }
5791 if (update_parent) {
5792 parent_branch.inst_table.putAssumeCapacity(target_key, canon_entry.value);
5793 }
5800 break :blk canon_entry.value;5794 break :blk canon_entry.value;
5801 } else blk: {5795 } else blk: {
5802 if (target_value == .dead) continue;5796 if (target_value == .dead) {
5797 if (update_parent) {
5798 parent_branch.inst_table.putAssumeCapacity(target_key, .dead);
5799 }
5800 continue;
5801 }
5803 // The instruction is only overridden in the else branch.5802 // The instruction is only overridden in the else branch.
5804 // If integer overflow occurs, the question is: why wasn't the instruction marked dead?5803 // If integer overflow occurs, the question is: why wasn't the instruction marked dead?
5805 break :blk self.getResolvedInstValue(target_key).?.*;5804 break :blk self.getResolvedInstValue(target_key).?.*;
...@@ -5807,7 +5806,9 @@ fn canonicaliseBranches(...@@ -5807,7 +5806,9 @@ fn canonicaliseBranches(
5807 log.debug("consolidating target_entry {d} {}=>{}", .{ target_key, target_value, canon_mcv });5806 log.debug("consolidating target_entry {d} {}=>{}", .{ target_key, target_value, canon_mcv });
5808 // TODO make sure the destination stack offset / register does not already have something5807 // TODO make sure the destination stack offset / register does not already have something
5809 // going on there.5808 // going on there.
5810 try self.setRegOrMem(self.air.typeOfIndex(target_key), canon_mcv, target_value);5809 if (set_values) {
5810 try self.setRegOrMem(self.air.typeOfIndex(target_key), canon_mcv, target_value);
5811 } else self.getValue(canon_mcv, target_key);
5811 self.freeValue(target_value);5812 self.freeValue(target_value);
5812 // TODO track the new register / stack allocation5813 // TODO track the new register / stack allocation
5813 }5814 }
...@@ -5825,7 +5826,9 @@ fn canonicaliseBranches(...@@ -5825,7 +5826,9 @@ fn canonicaliseBranches(
5825 log.debug("consolidating canon_entry {d} {}=>{}", .{ canon_key, parent_mcv, canon_value });5826 log.debug("consolidating canon_entry {d} {}=>{}", .{ canon_key, parent_mcv, canon_value });
5826 // TODO make sure the destination stack offset / register does not already have something5827 // TODO make sure the destination stack offset / register does not already have something
5827 // going on there.5828 // going on there.
5828 try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);5829 if (set_values) {
5830 try self.setRegOrMem(self.air.typeOfIndex(canon_key), canon_value, parent_mcv);
5831 } else self.getValue(canon_value, canon_key);
5829 self.freeValue(parent_mcv);5832 self.freeValue(parent_mcv);
5830 // TODO track the new register / stack allocation5833 // TODO track the new register / stack allocation
5831 }5834 }
...@@ -5890,23 +5893,28 @@ fn airBr(self: *Self, inst: Air.Inst.Index) !void {...@@ -5890,23 +5893,28 @@ fn airBr(self: *Self, inst: Air.Inst.Index) !void {
5890 try branch.inst_table.ensureUnusedCapacity(self.gpa, table.count());5893 try branch.inst_table.ensureUnusedCapacity(self.gpa, table.count());
5891 var it = table.iterator();5894 var it = table.iterator();
5892 while (it.next()) |entry| {5895 while (it.next()) |entry| {
5896 // This loop could be avoided by tracking inst depth, which
5897 // will be needed later anyway for reusing loop deaths.
5898 var parent_branch_i = block_data.branch_depth - 1;
5899 while (parent_branch_i > 0) : (parent_branch_i -= 1) {
5900 const parent_table = &self.branch_stack.items[parent_branch_i].inst_table;
5901 if (parent_table.contains(entry.key_ptr.*)) break;
5902 } else continue;
5893 const gop = branch.inst_table.getOrPutAssumeCapacity(entry.key_ptr.*);5903 const gop = branch.inst_table.getOrPutAssumeCapacity(entry.key_ptr.*);
5894 if (!gop.found_existing) gop.value_ptr.* = entry.value_ptr.*;5904 if (!gop.found_existing) gop.value_ptr.* = entry.value_ptr.*;
5895 }5905 }
5896 }5906 }
58975907
5898 if (block_data.branch) |*prev_branch| {5908 log.debug("airBr: %{d}", .{inst});
5899 log.debug("brVoid: %{d}", .{inst});5909 log.debug("Upper branches:", .{});
5900 log.debug("Upper branches:", .{});5910 for (self.branch_stack.items) |bs| {
5901 for (self.branch_stack.items) |bs| {5911 log.debug("{}", .{bs.fmtDebug()});
5902 log.debug("{}", .{bs.fmtDebug()});
5903 }
5904 log.debug("Prev branch: {}", .{prev_branch.fmtDebug()});
5905 log.debug("Cur branch: {}", .{branch.fmtDebug()});
5906
5907 try self.canonicaliseBranches(false, prev_branch, &branch, false);
5908 prev_branch.deinit(self.gpa);
5909 }5912 }
5913 log.debug("Prev branch: {}", .{block_data.branch.fmtDebug()});
5914 log.debug("Cur branch: {}", .{branch.fmtDebug()});
5915
5916 try self.canonicaliseBranches(false, &block_data.branch, &branch, true, false);
5917 block_data.branch.deinit(self.gpa);
5910 block_data.branch = branch;5918 block_data.branch = branch;
5911 }5919 }
59125920