authorgravatar for 81774659+gracefuu@users.noreply.github.comgracefu <81774659+gracefuu@users.noreply.github.com> 2021-04-09 14:05:53+08:00
committergravatar for 81774659+gracefuu@users.noreply.github.comgracefu <81774659+gracefuu@users.noreply.github.com> 2021-04-16 15:21:17+08:00
log5bd464e386df35bfe38b062190074ce3c2689001
treeb6f8a6f4b6e82814a8a20d7975cd9f1ce5f9b8de
parent36df1526da0e703a9f3d5bd6c8775d3f0e0f0a33
signature Commit is signed but in an unrecognized format.

stage2 x86_64: use abi size to determine 64-bit operation

From my very cursory reading, it seems that the register manager doesn't distinguish between registers that are physically the same but have different sizes. In that case, this means that during codegen, we can't rely on `reg.size()` when determining the width of the operations we have to perform. Instead, we must use some form of `ty.abiSize(self.target.*)` to determine the size of the type we're operating with. If this size is 64 bits, then we should enable 64-bit operation. This fixed a bug in the codegen for spilling instructions, which was overwriting the previous stack entry with zeroes. See the modified test case in this commit.

2 files changed, 28 insertions(+), 26 deletions(-)

src/codegen.zig+13-13
......@@ -1687,7 +1687,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
16871687 .register => |src_reg| {
16881688 // register, register use mr + 1 addressing mode: r/m16/32/64, r16/32/64
16891689 try self.encodeX8664Instruction(src, Instruction{
1690 .operand_size_64 = dst_reg.size() == 64,
1690 .operand_size_64 = dst_ty.abiSize(self.target.*) == 64,
16911691 .primary_opcode_1b = mr + 1,
16921692 // TODO: Explicit optional wrap due to stage 1 miscompilation :(
16931693 // https://github.com/ziglang/zig/issues/6515
......@@ -1705,7 +1705,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
17051705 const imm32 = @intCast(u31, imm); // This case must be handled before calling genX8664BinMathCode.
17061706 if (imm32 <= math.maxInt(u7)) {
17071707 try self.encodeX8664Instruction(src, Instruction{
1708 .operand_size_64 = dst_reg.size() == 64,
1708 .operand_size_64 = dst_ty.abiSize(self.target.*) == 64,
17091709 .primary_opcode_1b = 0x83,
17101710 .opcode_extension = opx,
17111711 // TODO: Explicit optional wrap due to stage 1 miscompilation :(
......@@ -1719,7 +1719,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
17191719 });
17201720 } else {
17211721 try self.encodeX8664Instruction(src, Instruction{
1722 .operand_size_64 = dst_reg.size() == 64,
1722 .operand_size_64 = dst_ty.abiSize(self.target.*) == 64,
17231723 .primary_opcode_1b = 0x81,
17241724 .opcode_extension = opx,
17251725 // TODO: Explicit optional wrap due to stage 1 miscompilation :(
......@@ -1743,7 +1743,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
17431743 return self.fail(src, "stack offset too large", .{});
17441744 }
17451745 try self.encodeX8664Instruction(src, Instruction{
1746 .operand_size_64 = dst_reg.size() == 64,
1746 .operand_size_64 = abi_size == 64,
17471747 .primary_opcode_1b = mr + 0x3,
17481748 .reg = dst_reg,
17491749 // TODO: Explicit optional wrap due to stage 1 miscompilation :(
......@@ -1802,7 +1802,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
18021802 return self.fail(src, "stack offset too large", .{});
18031803 }
18041804 try self.encodeX8664Instruction(src, Instruction{
1805 .operand_size_64 = reg.size() == 64,
1805 .operand_size_64 = abi_size == 64,
18061806 .primary_opcode_1b = opcode,
18071807 .reg = reg,
18081808 // TODO: Explicit optional wrap due to stage 1 miscompilation :(
......@@ -3707,7 +3707,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
37073707
37083708 // This is a variant of 8B /r.
37093709 try self.encodeX8664Instruction(src, Instruction{
3710 .operand_size_64 = reg.size() == 64,
3710 .operand_size_64 = ty.abiSize(self.target.*) == 64,
37113711
37123712 .primary_opcode_1b = 0x8B,
37133713
......@@ -3740,7 +3740,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
37403740 // LEA reg, [<offset>]
37413741 // manually do this instruction to make sure the offset into the disp32 field won't change.
37423742 try self.code.ensureCapacity(self.code.items.len + 7);
3743 self.rex(.{ .w = reg.size() == 64, .r = reg.isExtended() });
3743 self.rex(.{ .w = ty.abiSize(self.target.*) == 64, .r = reg.isExtended() });
37443744 self.code.appendSliceAssumeCapacity(&[_]u8{
37453745 0x8D,
37463746 0x05 | (@as(u8, reg.id() & 0b111) << 3),
......@@ -3749,7 +3749,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
37493749
37503750 // MOV reg, [reg]
37513751 try self.encodeX8664Instruction(src, Instruction{
3752 .operand_size_64 = reg.size() == 64,
3752 .operand_size_64 = ty.abiSize(self.target.*) == 64,
37533753
37543754 .primary_opcode_1b = 0x8B,
37553755
......@@ -3771,7 +3771,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
37713771 // 0b00RRR100, where RRR is the lower three bits of the register ID.
37723772 // The instruction is thus eight bytes; REX 0x8B 0b00RRR100 0x25 followed by a four-byte disp32.
37733773 try self.code.ensureCapacity(self.code.items.len + 8);
3774 self.rex(.{ .w = reg.size() == 64, .b = reg.isExtended() });
3774 self.rex(.{ .w = ty.abiSize(self.target.*) == 64, .r = reg.isExtended() });
37753775 self.code.appendSliceAssumeCapacity(&[_]u8{
37763776 0x8B,
37773777 0x04 | (@as(u8, reg.id() & 0b111) << 3), // R
......@@ -3809,7 +3809,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
38093809 // Currently, we're only allowing 64-bit registers, so we need the `REX.W 8B /r` variant.
38103810 // TODO: determine whether to allow other sized registers, and if so, handle them properly.
38113811 try self.encodeX8664Instruction(src, Instruction{
3812 .operand_size_64 = reg.size() == 64,
3812 .operand_size_64 = ty.abiSize(self.target.*) == 64,
38133813 .primary_opcode_1b = 0x8B,
38143814 .reg = reg,
38153815 // TODO: Explicit optional wrap due to stage 1 miscompilation :(
......@@ -3823,14 +3823,14 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
38233823 }
38243824 },
38253825 .stack_offset => |unadjusted_off| {
3826 const size_bytes = @divExact(reg.size(), 8);
3827 const off = unadjusted_off + size_bytes;
3826 const abi_size = ty.abiSize(self.target.*);
3827 const off = unadjusted_off + abi_size;
38283828 if (off < std.math.minInt(i32) or off > std.math.maxInt(i32)) {
38293829 return self.fail(src, "stack offset too large", .{});
38303830 }
38313831 const ioff = -@intCast(i32, off);
38323832 try self.encodeX8664Instruction(src, Instruction{
3833 .operand_size_64 = reg.size() == 64,
3833 .operand_size_64 = ty.abiSize(self.target.*) == 64,
38343834 .primary_opcode_1b = 0x8B,
38353835 .reg = reg,
38363836 // TODO: Explicit optional wrap due to stage 1 miscompilation :(
test/stage2/test.zig+15-13
......@@ -740,7 +740,7 @@ pub fn addCases(ctx: *TestContext) !void {
740740 // Spilling registers to the stack.
741741 case.addCompareOutput(
742742 \\export fn _start() noreturn {
743 \\ assert(add(3, 4) == 791);
743 \\ assert(add(3, 4) == 1221);
744744 \\
745745 \\ exit();
746746 \\}
......@@ -756,19 +756,21 @@ pub fn addCases(ctx: *TestContext) !void {
756756 \\ const i = g + h; // 100
757757 \\ const j = i + d; // 110
758758 \\ const k = i + j; // 210
759 \\ const l = k + c; // 217
760 \\ const m = l + d; // 227
761 \\ const n = m + e; // 241
762 \\ const o = n + f; // 265
763 \\ const p = o + g; // 303
764 \\ const q = p + h; // 365
765 \\ const r = q + i; // 465
766 \\ const s = r + j; // 575
767 \\ const t = s + k; // 785
768 \\ break :blk t;
759 \\ const l = j + k; // 320
760 \\ const m = l + c; // 327
761 \\ const n = m + d; // 337
762 \\ const o = n + e; // 351
763 \\ const p = o + f; // 375
764 \\ const q = p + g; // 413
765 \\ const r = q + h; // 475
766 \\ const s = r + i; // 575
767 \\ const t = s + j; // 685
768 \\ const u = t + k; // 895
769 \\ const v = u + l; // 1215
770 \\ break :blk v;
769771 \\ };
770 \\ const y = x + a; // 788
771 \\ const z = y + a; // 791
772 \\ const y = x + a; // 1218
773 \\ const z = y + a; // 1221
772774 \\ return z;
773775 \\}
774776 \\