authorgravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2022-05-07 10:31:08+02:00
committergravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2022-05-07 10:31:08+02:00
log43a627927f989034ae64846abea73bc8bdb545be
tree013d5461f93704047072e6b6849a4028577394f1
parentac954eb539ec242af568fe3565094cea12c4beea

x64: fix misused register locks

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

src/arch/x86_64/CodeGen.zig+19-26
...@@ -211,6 +211,16 @@ pub const MCValue = union(enum) {...@@ -211,6 +211,16 @@ pub const MCValue = union(enum) {
211 else => false,211 else => false,
212 };212 };
213 }213 }
214
215 fn asRegister(mcv: MCValue) ?Register {
216 return switch (mcv) {
217 .register,
218 .register_overflow_unsigned,
219 .register_overflow_signed,
220 => |reg| reg,
221 else => null,
222 };
223 }
214};224};
215225
216const Branch = struct {226const Branch = struct {
...@@ -842,20 +852,15 @@ fn finishAir(self: *Self, inst: Air.Inst.Index, result: MCValue, operands: [Live...@@ -842,20 +852,15 @@ fn finishAir(self: *Self, inst: Air.Inst.Index, result: MCValue, operands: [Live
842 const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];852 const branch = &self.branch_stack.items[self.branch_stack.items.len - 1];
843 branch.inst_table.putAssumeCapacityNoClobber(inst, result);853 branch.inst_table.putAssumeCapacityNoClobber(inst, result);
844854
845 switch (result) {855 if (result.asRegister()) |reg| {
846 .register,856 // In some cases (such as bitcast), an operand
847 .register_overflow_signed,857 // may be the same MCValue as the result. If
848 .register_overflow_unsigned,858 // that operand died and was a register, it
849 => |reg| {859 // was freed by processDeath. We have to
850 // In some cases (such as bitcast), an operand860 // "re-allocate" the register.
851 // may be the same MCValue as the result. If861 if (self.register_manager.isRegFree(reg)) {
852 // that operand died and was a register, it862 self.register_manager.getRegAssumeFree(reg, inst);
853 // was freed by processDeath. We have to863 }
854 // "re-allocate" the register.
855 if (self.register_manager.isRegFree(reg)) {
856 self.register_manager.getRegAssumeFree(reg, inst);
857 }
858 },
859 }864 }
860 }865 }
861 self.finishAirBookkeeping();866 self.finishAirBookkeeping();
...@@ -4011,12 +4016,6 @@ fn airRet(self: *Self, inst: Air.Inst.Index) !void {...@@ -4011,12 +4016,6 @@ fn airRet(self: *Self, inst: Air.Inst.Index) !void {
4011 const ret_ty = self.fn_type.fnReturnType();4016 const ret_ty = self.fn_type.fnReturnType();
4012 switch (self.ret_mcv) {4017 switch (self.ret_mcv) {
4013 .stack_offset => {4018 .stack_offset => {
4014 var reg_locks: [2]RegisterLock = undefined;
4015 self.register_manager.freezeRegsAssumeUnused(2, .{ .rax, .rcx }, &reg_locks);
4016 defer for (reg_locks) |reg| {
4017 self.register_manager.unfreezeReg(reg);
4018 };
4019
4020 const reg = try self.copyToTmpRegister(Type.usize, self.ret_mcv);4019 const reg = try self.copyToTmpRegister(Type.usize, self.ret_mcv);
4021 const reg_lock = self.register_manager.freezeRegAssumeUnused(reg);4020 const reg_lock = self.register_manager.freezeRegAssumeUnused(reg);
4022 defer self.register_manager.unfreezeReg(reg_lock);4021 defer self.register_manager.unfreezeReg(reg_lock);
...@@ -4051,12 +4050,6 @@ fn airRetLoad(self: *Self, inst: Air.Inst.Index) !void {...@@ -4051,12 +4050,6 @@ fn airRetLoad(self: *Self, inst: Air.Inst.Index) !void {
4051 const elem_ty = ptr_ty.elemType();4050 const elem_ty = ptr_ty.elemType();
4052 switch (self.ret_mcv) {4051 switch (self.ret_mcv) {
4053 .stack_offset => {4052 .stack_offset => {
4054 var reg_locks: [2]RegisterLock = undefined;
4055 self.register_manager.freezeRegsAssumeUnused(2, .{ .rax, .rcx }, &reg_locks);
4056 defer for (reg_locks) |reg| {
4057 self.register_manager.unfreezeReg(reg);
4058 };
4059
4060 const reg = try self.copyToTmpRegister(Type.usize, self.ret_mcv);4053 const reg = try self.copyToTmpRegister(Type.usize, self.ret_mcv);
4061 const reg_lock = self.register_manager.freezeRegAssumeUnused(reg);4054 const reg_lock = self.register_manager.freezeRegAssumeUnused(reg);
4062 defer self.register_manager.unfreezeReg(reg_lock);4055 defer self.register_manager.unfreezeReg(reg_lock);
src/register_manager.zig+7-1
...@@ -127,7 +127,11 @@ pub fn RegisterManager(...@@ -127,7 +127,11 @@ pub fn RegisterManager(
127 /// Only the owner of the `RegisterLock` can unfreeze the127 /// Only the owner of the `RegisterLock` can unfreeze the
128 /// register later.128 /// register later.
129 pub fn freezeReg(self: *Self, reg: Register) ?RegisterLock {129 pub fn freezeReg(self: *Self, reg: Register) ?RegisterLock {
130 if (self.isRegFrozen(reg)) return null;130 log.debug("freezing {}", .{reg});
131 if (self.isRegFrozen(reg)) {
132 log.debug(" register already locked", .{});
133 return null;
134 }
131 const mask = getRegisterMask(reg) orelse return null;135 const mask = getRegisterMask(reg) orelse return null;
132 self.frozen_registers |= mask;136 self.frozen_registers |= mask;
133 return RegisterLock{ .register = reg };137 return RegisterLock{ .register = reg };
...@@ -136,6 +140,7 @@ pub fn RegisterManager(...@@ -136,6 +140,7 @@ pub fn RegisterManager(
136 /// Like `freezeReg` but asserts the register was unused always140 /// Like `freezeReg` but asserts the register was unused always
137 /// returning a valid lock.141 /// returning a valid lock.
138 pub fn freezeRegAssumeUnused(self: *Self, reg: Register) RegisterLock {142 pub fn freezeRegAssumeUnused(self: *Self, reg: Register) RegisterLock {
143 log.debug("freezing asserting free {}", .{reg});
139 assert(!self.isRegFrozen(reg));144 assert(!self.isRegFrozen(reg));
140 const mask = getRegisterMask(reg) orelse unreachable;145 const mask = getRegisterMask(reg) orelse unreachable;
141 self.frozen_registers |= mask;146 self.frozen_registers |= mask;
...@@ -158,6 +163,7 @@ pub fn RegisterManager(...@@ -158,6 +163,7 @@ pub fn RegisterManager(
158 /// Requires `RegisterLock` to unfreeze a register.163 /// Requires `RegisterLock` to unfreeze a register.
159 /// Call `freezeReg` to obtain the lock first.164 /// Call `freezeReg` to obtain the lock first.
160 pub fn unfreezeReg(self: *Self, lock: RegisterLock) void {165 pub fn unfreezeReg(self: *Self, lock: RegisterLock) void {
166 log.debug("unfreezing {}", .{lock.register});
161 const mask = getRegisterMask(lock.register) orelse return;167 const mask = getRegisterMask(lock.register) orelse return;
162 self.frozen_registers &= ~mask;168 self.frozen_registers &= ~mask;
163 }169 }