| author |  Alex Rønne Petersen <alex@alexrp.com> 2024-10-31 09:00:02+01:00 |
| committer | Alex Rønne Petersen <alex@alexrp.com> 2025-05-12 17:07:49+02:00 |
| log | 4c36a403a895bd3e7dc7b4dd308f2b82a3dab60a |
| tree | 50763d16076bdc4d5c636f0aa39654fb8b631e39 |
| parent | d4ca9804f8a546c45451a58c8bde20bc2299d4a6 |
| signature | Signed by SSH key SHA256:7B/LJ7bpR1eX8aCXSr4mtd5M45VMPKcx9zY8e95b5QM |
Air: Fix mustLower() for atomic_load with inter-thread ordering.
3 files changed, 29 insertions(+), 18 deletions(-)
src
- Air.zig
arch
riscv64
x86_64
src/Air.zig+4-1
| ... | @@ -1889,7 +1889,10 @@ pub fn mustLower(air: Air, inst: Air.Inst.Index, ip: *const InternPool) bool { | ... | @@ -1889,7 +1889,10 @@ pub fn mustLower(air: Air, inst: Air.Inst.Index, ip: *const InternPool) bool { |
| 1889 | }, | 1889 | }, |
| 1890 | .load => air.typeOf(data.ty_op.operand, ip).isVolatilePtrIp(ip), | 1890 | .load => air.typeOf(data.ty_op.operand, ip).isVolatilePtrIp(ip), |
| 1891 | .slice_elem_val, .ptr_elem_val => air.typeOf(data.bin_op.lhs, ip).isVolatilePtrIp(ip), | 1891 | .slice_elem_val, .ptr_elem_val => air.typeOf(data.bin_op.lhs, ip).isVolatilePtrIp(ip), |
| 1892 | .atomic_load => air.typeOf(data.atomic_load.ptr, ip).isVolatilePtrIp(ip), | 1892 | .atomic_load => switch (data.atomic_load.order) { |
| 1893 | .unordered, .monotonic => air.typeOf(data.atomic_load.ptr, ip).isVolatilePtrIp(ip), | ||
| 1894 | else => true, // Stronger memory orderings have inter-thread side effects. | ||
| 1895 | }, | ||
| 1893 | }; | 1896 | }; |
| 1894 | } | 1897 | } |
| 1895 | 1898 |
src/arch/riscv64/CodeGen.zig+8-6
| ... | @@ -7738,7 +7738,10 @@ fn airAtomicLoad(func: *Func, inst: Air.Inst.Index) !void { | ... | @@ -7738,7 +7738,10 @@ fn airAtomicLoad(func: *Func, inst: Air.Inst.Index) !void { |
| 7738 | const bit_size = elem_ty.bitSize(zcu); | 7738 | const bit_size = elem_ty.bitSize(zcu); |
| 7739 | if (bit_size > 64) return func.fail("TODO: airAtomicLoad > 64 bits", .{}); | 7739 | if (bit_size > 64) return func.fail("TODO: airAtomicLoad > 64 bits", .{}); |
| 7740 | 7740 | ||
| 7741 | const result_mcv = try func.allocRegOrMem(elem_ty, inst, true); | 7741 | const result_mcv: MCValue = if (func.liveness.isUnused(inst)) |
| 7742 | .{ .register = .zero } | ||
| 7743 | else | ||
| 7744 | try func.allocRegOrMem(elem_ty, inst, true); | ||
| 7742 | assert(result_mcv == .register); // should be less than 8 bytes | 7745 | assert(result_mcv == .register); // should be less than 8 bytes |
| 7743 | 7746 | ||
| 7744 | if (order == .seq_cst) { | 7747 | if (order == .seq_cst) { |
| ... | @@ -7754,11 +7757,10 @@ fn airAtomicLoad(func: *Func, inst: Air.Inst.Index) !void { | ... | @@ -7754,11 +7757,10 @@ fn airAtomicLoad(func: *Func, inst: Air.Inst.Index) !void { |
| 7754 | try func.load(result_mcv, ptr_mcv, ptr_ty); | 7757 | try func.load(result_mcv, ptr_mcv, ptr_ty); |
| 7755 | 7758 | ||
| 7756 | switch (order) { | 7759 | switch (order) { |
| 7757 | // Don't guarnetee other memory operations to be ordered after the load. | 7760 | // Don't guarantee other memory operations to be ordered after the load. |
| 7758 | .unordered => {}, | 7761 | .unordered, .monotonic => {}, |
| 7759 | .monotonic => {}, | 7762 | // Make sure all previous reads happen before any reading or writing occurs. |
| 7760 | // Make sure all previous reads happen before any reading or writing accurs. | 7763 | .acquire, .seq_cst => { |
| 7761 | .seq_cst, .acquire => { | ||
| 7762 | _ = try func.addInst(.{ | 7764 | _ = try func.addInst(.{ |
| 7763 | .tag = .fence, | 7765 | .tag = .fence, |
| 7764 | .data = .{ .fence = .{ | 7766 | .data = .{ .fence = .{ |
src/arch/x86_64/CodeGen.zig+17-11
| ... | @@ -106219,23 +106219,29 @@ fn airAtomicRmw(self: *CodeGen, inst: Air.Inst.Index) !void { | ... | @@ -106219,23 +106219,29 @@ fn airAtomicRmw(self: *CodeGen, inst: Air.Inst.Index) !void { |
| 106219 | 106219 | ||
| 106220 | fn airAtomicLoad(self: *CodeGen, inst: Air.Inst.Index) !void { | 106220 | fn airAtomicLoad(self: *CodeGen, inst: Air.Inst.Index) !void { |
| 106221 | const atomic_load = self.air.instructions.items(.data)[@intFromEnum(inst)].atomic_load; | 106221 | const atomic_load = self.air.instructions.items(.data)[@intFromEnum(inst)].atomic_load; |
| 106222 | const result: MCValue = result: { | ||
| 106223 | const ptr_ty = self.typeOf(atomic_load.ptr); | ||
| 106224 | const ptr_mcv = try self.resolveInst(atomic_load.ptr); | ||
| 106225 | const ptr_lock = switch (ptr_mcv) { | ||
| 106226 | .register => |reg| self.register_manager.lockRegAssumeUnused(reg), | ||
| 106227 | else => null, | ||
| 106228 | }; | ||
| 106229 | defer if (ptr_lock) |lock| self.register_manager.unlockReg(lock); | ||
| 106222 | 106230 | ||
| 106223 | const ptr_ty = self.typeOf(atomic_load.ptr); | 106231 | const unused = self.liveness.isUnused(inst); |
| 106224 | const ptr_mcv = try self.resolveInst(atomic_load.ptr); | ||
| 106225 | const ptr_lock = switch (ptr_mcv) { | ||
| 106226 | .register => |reg| self.register_manager.lockRegAssumeUnused(reg), | ||
| 106227 | else => null, | ||
| 106228 | }; | ||
| 106229 | defer if (ptr_lock) |lock| self.register_manager.unlockReg(lock); | ||
| 106230 | 106232 | ||
| 106231 | const dst_mcv = | 106233 | const dst_mcv: MCValue = if (unused) |
| 106232 | if (self.reuseOperand(inst, atomic_load.ptr, 0, ptr_mcv)) | 106234 | .{ .register = try self.register_manager.allocReg(null, self.regSetForType(ptr_ty.childType(self.pt.zcu))) } |
| 106235 | else if (self.reuseOperand(inst, atomic_load.ptr, 0, ptr_mcv)) | ||
| 106233 | ptr_mcv | 106236 | ptr_mcv |
| 106234 | else | 106237 | else |
| 106235 | try self.allocRegOrMem(inst, true); | 106238 | try self.allocRegOrMem(inst, true); |
| 106236 | 106239 | ||
| 106237 | try self.load(dst_mcv, ptr_ty, ptr_mcv); | 106240 | try self.load(dst_mcv, ptr_ty, ptr_mcv); |
| 106238 | return self.finishAir(inst, dst_mcv, .{ atomic_load.ptr, .none, .none }); | 106241 | |
| 106242 | break :result if (unused) .unreach else dst_mcv; | ||
| 106243 | }; | ||
| 106244 | return self.finishAir(inst, result, .{ atomic_load.ptr, .none, .none }); | ||
| 106239 | } | 106245 | } |
| 106240 | 106246 | ||
| 106241 | fn airAtomicStore(self: *CodeGen, inst: Air.Inst.Index, order: std.builtin.AtomicOrder) !void { | 106247 | fn airAtomicStore(self: *CodeGen, inst: Air.Inst.Index, order: std.builtin.AtomicOrder) !void { |