commit 18a909f61d87d6b8e89147b7b46d8d0785b24b4d (tree)
parent 6cf8a49bb0e5a6df3154aa7c41602ef21057ce50
Author: Jakub Konka <kubkon@jakubkonka.com>
Date: Fri, 19 Nov 2021 20:30:20 +0100
Merge branch 'g-w1-callee-preserved'
Diffstat:
7 files changed, 167 insertions(+), 29 deletions(-)
diff --git a/src/arch/x86/bits.zig b/src/arch/x86/bits.zig
@@ -32,11 +32,9 @@ pub const Register = enum(u8) {
/// Returns the index into `callee_preserved_regs`.
pub fn allocIndex(self: Register) ?u4 {
return switch (self) {
- .eax, .ax, .al => 0,
- .ecx, .cx, .cl => 1,
- .edx, .dx, .dl => 2,
- .esi, .si => 3,
- .edi, .di => 4,
+ .ebx, .bx, .bl => 0,
+ .esi, .si => 1,
+ .edi, .di => 2,
else => null,
};
}
@@ -74,7 +72,11 @@ pub const Register = enum(u8) {
// zig fmt: on
-pub const callee_preserved_regs = [_]Register{ .eax, .ecx, .edx, .esi, .edi };
+/// These registers need to be preserved (saved on the stack) and restored by the callee before getting clobbered
+/// and when the callee returns.
+/// Note that .esp and .ebp also belong to this set, however, we never expect to use them
+/// for anything else but stack offset tracking therefore we exclude them from this set.
+pub const callee_preserved_regs = [_]Register{ .ebx, .esi, .edi };
// TODO add these to Register enum and corresponding dwarfLocOp
// // Return Address register. This is stored in `0(%esp, "")` and is not a physical register.
diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
@@ -159,6 +159,13 @@ pub const MCValue = union(enum) {
=> true,
};
}
+
+ fn isRegister(mcv: MCValue) bool {
+ return switch (mcv) {
+ .register => true,
+ else => false,
+ };
+ }
};
const Branch = struct {
@@ -349,6 +356,13 @@ pub fn addExtraAssumeCapacity(self: *Self, extra: anytype) u32 {
fn gen(self: *Self) InnerError!void {
const cc = self.fn_type.fnCallingConvention();
if (cc != .Naked) {
+ // push the callee_preserved_regs that were used
+ const backpatch_push_callee_preserved_regs_i = try self.addInst(.{
+ .tag = .push_regs_from_callee_preserved_regs,
+ .ops = undefined,
+ .data = .{ .regs_to_push_or_pop = undefined }, // to be backpatched
+ });
+
_ = try self.addInst(.{
.tag = .push,
.ops = (Mir.Ops{
@@ -423,6 +437,22 @@ fn gen(self: *Self) InnerError!void {
}).encode(),
.data = undefined,
});
+ // calculate the data for callee_preserved_regs to be pushed and popped
+ var callee_preserved_regs_push_data: u32 = 0x0;
+ inline for (callee_preserved_regs) |reg, i| {
+ if (self.register_manager.isRegAllocated(reg)) {
+ callee_preserved_regs_push_data |= 1 << @intCast(u5, i);
+ }
+ }
+ const data = self.mir_instructions.items(.data);
+ // backpatch the push instruction
+ data[backpatch_push_callee_preserved_regs_i].regs_to_push_or_pop = callee_preserved_regs_push_data;
+ // pop the callee_preserved_regs
+ _ = try self.addInst(.{
+ .tag = .pop_regs_from_callee_preserved_regs,
+ .ops = undefined,
+ .data = .{ .regs_to_push_or_pop = callee_preserved_regs_push_data },
+ });
_ = try self.addInst(.{
.tag = .ret,
.ops = (Mir.Ops{
@@ -737,6 +767,19 @@ fn copyToNewRegister(self: *Self, reg_owner: Air.Inst.Index, mcv: MCValue) !MCVa
return MCValue{ .register = reg };
}
+/// Like `copyToNewRegister` but allows to specify a list of excluded registers which
+/// will not be selected for allocation. This can be done via `exceptions` slice.
+fn copyToNewRegisterWithExceptions(
+ self: *Self,
+ reg_owner: Air.Inst.Index,
+ mcv: MCValue,
+ exceptions: []const Register,
+) !MCValue {
+ const reg = try self.register_manager.allocReg(reg_owner, exceptions);
+ try self.genSetReg(self.air.typeOfIndex(reg_owner), reg, mcv);
+ return MCValue{ .register = reg };
+}
+
fn airAlloc(self: *Self, inst: Air.Inst.Index) !void {
const stack_offset = try self.allocMemPtr(inst);
return self.finishAir(inst, .{ .ptr_stack_offset = stack_offset }, .{ .none, .none, .none });
@@ -1427,11 +1470,9 @@ fn genBinMathOp(self: *Self, inst: Air.Inst.Index, op_lhs: Air.Inst.Ref, op_rhs:
// as the result MCValue.
var dst_mcv: MCValue = undefined;
var src_mcv: MCValue = undefined;
- var src_inst: Air.Inst.Ref = undefined;
if (self.reuseOperand(inst, op_lhs, 0, lhs)) {
// LHS dies; use it as the destination.
// Both operands cannot be memory.
- src_inst = op_rhs;
if (lhs.isMemory() and rhs.isMemory()) {
dst_mcv = try self.copyToNewRegister(inst, lhs);
src_mcv = rhs;
@@ -1442,7 +1483,6 @@ fn genBinMathOp(self: *Self, inst: Air.Inst.Index, op_lhs: Air.Inst.Ref, op_rhs:
} else if (self.reuseOperand(inst, op_rhs, 1, rhs)) {
// RHS dies; use it as the destination.
// Both operands cannot be memory.
- src_inst = op_lhs;
if (lhs.isMemory() and rhs.isMemory()) {
dst_mcv = try self.copyToNewRegister(inst, rhs);
src_mcv = lhs;
@@ -1452,13 +1492,23 @@ fn genBinMathOp(self: *Self, inst: Air.Inst.Index, op_lhs: Air.Inst.Ref, op_rhs:
}
} else {
if (lhs.isMemory()) {
- dst_mcv = try self.copyToNewRegister(inst, lhs);
+ dst_mcv = if (rhs.isRegister())
+ // If the allocated register is the same as the rhs register, don't allocate that one
+ // and instead spill a subsequent one. Otherwise, this can result in a miscompilation
+ // in the presence of several binary operations performed in a single block.
+ try self.copyToNewRegisterWithExceptions(inst, lhs, &.{rhs.register})
+ else
+ try self.copyToNewRegister(inst, lhs);
src_mcv = rhs;
- src_inst = op_rhs;
} else {
- dst_mcv = try self.copyToNewRegister(inst, rhs);
+ dst_mcv = if (lhs.isRegister())
+ // If the allocated register is the same as the rhs register, don't allocate that one
+ // and instead spill a subsequent one. Otherwise, this can result in a miscompilation
+ // in the presence of several binary operations performed in a single block.
+ try self.copyToNewRegisterWithExceptions(inst, rhs, &.{lhs.register})
+ else
+ try self.copyToNewRegister(inst, rhs);
src_mcv = lhs;
- src_inst = op_lhs;
}
}
// This instruction supports only signed 32-bit immediates at most. If the immediate
@@ -1902,10 +1952,10 @@ fn airCall(self: *Self, inst: Air.Inst.Index) !void {
try self.register_manager.getReg(reg, null);
try self.genSetReg(arg_ty, reg, arg_mcv);
},
- .stack_offset => {
+ .stack_offset => |off| {
// Here we need to emit instructions like this:
// mov qword ptr [rsp + stack_offset], x
- return self.fail("TODO implement calling with parameters in memory", .{});
+ try self.genSetStack(arg_ty, off, arg_mcv);
},
.ptr_stack_offset => {
return self.fail("TODO implement calling with MCValue.ptr_stack_offset arg", .{});
diff --git a/src/arch/x86_64/Emit.zig b/src/arch/x86_64/Emit.zig
@@ -142,6 +142,9 @@ pub fn emitMir(emit: *Emit) InnerError!void {
.dbg_epilogue_begin => try emit.mirDbgEpilogueBegin(inst),
.arg_dbg_info => try emit.mirArgDbgInfo(inst),
+ .push_regs_from_callee_preserved_regs => try emit.mirPushPopRegsFromCalleePreservedRegs(.push, inst),
+ .pop_regs_from_callee_preserved_regs => try emit.mirPushPopRegsFromCalleePreservedRegs(.pop, inst),
+
else => {
return emit.fail("Implement MIR->Isel lowering for x86_64 for pseudo-inst: {s}", .{tag});
},
@@ -244,6 +247,39 @@ fn mirPushPop(emit: *Emit, tag: Mir.Inst.Tag, inst: Mir.Inst.Index) InnerError!v
0b11 => unreachable,
}
}
+fn mirPushPopRegsFromCalleePreservedRegs(emit: *Emit, tag: Mir.Inst.Tag, inst: Mir.Inst.Index) InnerError!void {
+ const callee_preserved_regs = bits.callee_preserved_regs;
+ // PUSH/POP reg
+ const opc: u8 = switch (tag) {
+ .push => 0x50,
+ .pop => 0x58,
+ else => unreachable,
+ };
+
+ const regs = emit.mir.instructions.items(.data)[inst].regs_to_push_or_pop;
+ if (tag == .push) {
+ for (callee_preserved_regs) |reg, i| {
+ if ((regs >> @intCast(u5, i)) & 1 == 0) continue;
+ const encoder = try Encoder.init(emit.code, 2);
+ encoder.rex(.{
+ .b = reg.isExtended(),
+ });
+ encoder.opcode_withReg(opc, reg.lowId());
+ }
+ } else {
+ // pop in the reverse direction
+ var i = callee_preserved_regs.len;
+ while (i > 0) : (i -= 1) {
+ const reg = callee_preserved_regs[i - 1];
+ if ((regs >> @intCast(u5, i - 1)) & 1 == 0) continue;
+ const encoder = try Encoder.init(emit.code, 2);
+ encoder.rex(.{
+ .b = reg.isExtended(),
+ });
+ encoder.opcode_withReg(opc, reg.lowId());
+ }
+ }
+}
fn mirJmpCall(emit: *Emit, tag: Mir.Inst.Tag, inst: Mir.Inst.Index) InnerError!void {
const ops = Mir.Ops.decode(emit.mir.instructions.items(.ops)[inst]);
diff --git a/src/arch/x86_64/Mir.zig b/src/arch/x86_64/Mir.zig
@@ -264,8 +264,21 @@ pub const Inst = struct {
/// arg debug info
arg_dbg_info,
- };
+ /// push registers from the callee_preserved_regs
+ /// data is the bitfield of which regs to push
+ /// for example on x86_64, the callee_preserved_regs are [_]Register{ .rcx, .rsi, .rdi, .r8, .r9, .r10, .r11 }; };
+ /// so to push rcx and r8 one would make data 0b00000000_00000000_00000000_00001001 (the first and fourth bits are set)
+ /// ops is unused
+ push_regs_from_callee_preserved_regs,
+
+ /// pop registers from the callee_preserved_regs
+ /// data is the bitfield of which regs to pop
+ /// for example on x86_64, the callee_preserved_regs are [_]Register{ .rcx, .rsi, .rdi, .r8, .r9, .r10, .r11 }; };
+ /// so to pop rcx and r8 one would make data 0b00000000_00000000_00000000_00001001 (the first and fourth bits are set)
+ /// ops is unused
+ pop_regs_from_callee_preserved_regs,
+ };
/// The position of an MIR instruction within the `Mir` instructions array.
pub const Index = u32;
@@ -284,6 +297,8 @@ pub const Inst = struct {
got_entry: u32,
/// Index into `extra`. Meaning of what can be found there is context-dependent.
payload: u32,
+ /// A bitfield of which callee_preserved_regs to push
+ regs_to_push_or_pop: u32,
};
// Make sure we don't accidentally make instructions bigger than expected.
diff --git a/src/arch/x86_64/bits.zig b/src/arch/x86_64/bits.zig
@@ -84,15 +84,11 @@ pub const Register = enum(u7) {
/// Returns the index into `callee_preserved_regs`.
pub fn allocIndex(self: Register) ?u4 {
return switch (self) {
- .rax, .eax, .ax, .al => 0,
- .rcx, .ecx, .cx, .cl => 1,
- .rdx, .edx, .dx, .dl => 2,
- .rsi, .esi, .si => 3,
- .rdi, .edi, .di => 4,
- .r8, .r8d, .r8w, .r8b => 5,
- .r9, .r9d, .r9w, .r9b => 6,
- .r10, .r10d, .r10w, .r10b => 7,
- .r11, .r11d, .r11w, .r11b => 8,
+ .rbx, .ebx, .bx, .bl => 0,
+ .r12, .r12d, .r12w, .r12b => 1,
+ .r13, .r13d, .r13w, .r13b => 2,
+ .r14, .r14d, .r14w, .r14b => 3,
+ .r15, .r15d, .r15w, .r15b => 4,
else => null,
};
}
@@ -144,8 +140,15 @@ pub const Register = enum(u7) {
// zig fmt: on
-/// These registers belong to the called function.
-pub const callee_preserved_regs = [_]Register{ .rax, .rcx, .rdx, .rsi, .rdi, .r8, .r9, .r10, .r11 };
+/// These registers need to be preserved (saved on the stack) and restored by the callee before getting clobbered
+/// and when the callee returns.
+/// Note that .rsp and .rbp also belong to this set, however, we never expect to use them
+/// for anything else but stack offset tracking therefore we exclude them from this set.
+pub const callee_preserved_regs = [_]Register{ .rbx, .r12, .r13, .r14, .r15 };
+/// These registers need to be preserved (saved on the stack) and restored by the caller before
+/// the caller relinquishes control to a subroutine via call instruction (or similar).
+/// In other words, these registers are free to use by the callee.
+pub const caller_preserved_regs = [_]Register{ .rax, .rcx, .rdx, .rsi, .rdi, .r8, .r9, .r10, .r11 };
pub const c_abi_int_param_regs = [_]Register{ .rdi, .rsi, .rdx, .rcx, .r8, .r9 };
pub const c_abi_int_return_regs = [_]Register{ .rax, .rdx };
diff --git a/src/link/Elf.zig b/src/link/Elf.zig
@@ -2604,12 +2604,12 @@ fn addDbgInfoType(self: *Elf, ty: Type, dbg_info_buffer: *std.ArrayList(u8)) !vo
// DW.AT.name, DW.FORM.string
try dbg_info_buffer.writer().print("{}\x00", .{ty});
} else {
- log.err("TODO implement .debug_info for type '{}'", .{ty});
+ log.debug("TODO implement .debug_info for type '{}'", .{ty});
try dbg_info_buffer.append(abbrev_pad1);
}
},
else => {
- log.err("TODO implement .debug_info for type '{}'", .{ty});
+ log.debug("TODO implement .debug_info for type '{}'", .{ty});
try dbg_info_buffer.append(abbrev_pad1);
},
}
diff --git a/test/cases.zig b/test/cases.zig
@@ -1819,4 +1819,36 @@ pub fn addCases(ctx: *TestContext) !void {
":2:28: error: cannot set address space of local variable 'foo'",
});
}
+ {
+ var case = ctx.exe("issue 10138: callee preserved regs working", linux_x64);
+ case.addCompareOutput(
+ \\pub fn main() void {
+ \\ const fd = open();
+ \\ _ = write(fd, "a", 1);
+ \\ _ = close(fd);
+ \\}
+ \\
+ \\fn open() usize {
+ \\ return 42;
+ \\}
+ \\
+ \\fn write(fd: usize, a: [*]const u8, len: usize) usize {
+ \\ return syscall4(.WRITE, fd, @ptrToInt(a), len);
+ \\}
+ \\
+ \\fn syscall4(n: enum { WRITE }, a: usize, b: usize, c: usize) usize {
+ \\ _ = n;
+ \\ _ = a;
+ \\ _ = b;
+ \\ _ = c;
+ \\ return 23;
+ \\}
+ \\
+ \\fn close(fd: usize) usize {
+ \\ if (fd != 42)
+ \\ unreachable;
+ \\ return 0;
+ \\}
+ , "");
+ }
}
