| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2025-06-13 19:05:44+01:00 |
| committer | Matthew Lugg <mlugg@mlugg.co.uk> 2025-06-13 19:05:44+01:00 |
| log | 121d62044399da0957fe3bf80c87ef23d7cdb1a3 |
| tree | 92e671cfe9021a01debe208ff0633bfec506a6dd |
| parent | dd75e7bcb1fe142f4d60dc2d83e6feee53e580f3 |
| signature |  Commit is signed but in an unrecognized format. |
compiler: fix atomic orderings
I messed up atomic orderings on this variable because they changed in a
local refactor at some point. We need to always release on the store and
acquire on the loads so that a linker thread observing `.ready` sees the
stored MIR.3 files changed, 23 insertions(+), 28 deletions(-)
src/Zcu/PerThread.zig+17-22
| ... | @@ -4373,33 +4373,28 @@ pub fn addDependency(pt: Zcu.PerThread, unit: AnalUnit, dependee: InternPool.Dep | ... | @@ -4373,33 +4373,28 @@ pub fn addDependency(pt: Zcu.PerThread, unit: AnalUnit, dependee: InternPool.Dep |
| 4373 | /// codegen thread, depending on whether the backend supports `Zcu.Feature.separate_thread`. | 4373 | /// codegen thread, depending on whether the backend supports `Zcu.Feature.separate_thread`. |
| 4374 | pub fn runCodegen(pt: Zcu.PerThread, func_index: InternPool.Index, air: *Air, out: *@import("../link.zig").ZcuTask.LinkFunc.SharedMir) void { | 4374 | pub fn runCodegen(pt: Zcu.PerThread, func_index: InternPool.Index, air: *Air, out: *@import("../link.zig").ZcuTask.LinkFunc.SharedMir) void { |
| 4375 | const zcu = pt.zcu; | 4375 | const zcu = pt.zcu; |
| 4376 | if (runCodegenInner(pt, func_index, air)) |mir| { | 4376 | const success: bool = if (runCodegenInner(pt, func_index, air)) |mir| success: { |
| 4377 | out.value = mir; | 4377 | out.value = mir; |
| 4378 | out.status.store(.ready, .release); | 4378 | break :success true; |
| 4379 | } else |err| switch (err) { | 4379 | } else |err| success: { |
| 4380 | error.OutOfMemory => { | 4380 | switch (err) { |
| 4381 | zcu.comp.setAllocFailure(); | 4381 | error.OutOfMemory => zcu.comp.setAllocFailure(), |
| 4382 | out.status.store(.failed, .monotonic); | 4382 | error.CodegenFail => zcu.assertCodegenFailed(zcu.funcInfo(func_index).owner_nav), |
| 4383 | }, | 4383 | error.NoLinkFile => assert(zcu.comp.bin_file == null), |
| 4384 | error.CodegenFail => { | 4384 | error.BackendDoesNotProduceMir => switch (target_util.zigBackend( |
| 4385 | zcu.assertCodegenFailed(zcu.funcInfo(func_index).owner_nav); | 4385 | zcu.root_mod.resolved_target.result, |
| 4386 | out.status.store(.failed, .monotonic); | 4386 | zcu.comp.config.use_llvm, |
| 4387 | }, | 4387 | )) { |
| 4388 | error.NoLinkFile => { | ||
| 4389 | assert(zcu.comp.bin_file == null); | ||
| 4390 | out.status.store(.failed, .monotonic); | ||
| 4391 | }, | ||
| 4392 | error.BackendDoesNotProduceMir => { | ||
| 4393 | const backend = target_util.zigBackend(zcu.root_mod.resolved_target.result, zcu.comp.config.use_llvm); | ||
| 4394 | switch (backend) { | ||
| 4395 | else => unreachable, // assertion failure | 4388 | else => unreachable, // assertion failure |
| 4396 | .stage2_spirv64, | 4389 | .stage2_spirv64, |
| 4397 | .stage2_llvm, | 4390 | .stage2_llvm, |
| 4398 | => {}, | 4391 | => {}, |
| 4399 | } | 4392 | }, |
| 4400 | out.status.store(.failed, .monotonic); | 4393 | } |
| 4401 | }, | 4394 | break :success false; |
| 4402 | } | 4395 | }; |
| 4396 | // release `out.value` with this store; synchronizes with acquire loads in `link` | ||
| 4397 | out.status.store(if (success) .ready else .failed, .release); | ||
| 4403 | zcu.comp.link_task_queue.mirReady(zcu.comp, out); | 4398 | zcu.comp.link_task_queue.mirReady(zcu.comp, out); |
| 4404 | if (zcu.pending_codegen_jobs.rmw(.Sub, 1, .monotonic) == 1) { | 4399 | if (zcu.pending_codegen_jobs.rmw(.Sub, 1, .monotonic) == 1) { |
| 4405 | // Decremented to 0, so all done. | 4400 | // Decremented to 0, so all done. |
src/link.zig+2-2
| ... | @@ -1249,7 +1249,7 @@ pub const ZcuTask = union(enum) { | ... | @@ -1249,7 +1249,7 @@ pub const ZcuTask = union(enum) { |
| 1249 | .update_line_number, | 1249 | .update_line_number, |
| 1250 | => {}, | 1250 | => {}, |
| 1251 | .link_func => |link_func| { | 1251 | .link_func => |link_func| { |
| 1252 | switch (link_func.mir.status.load(.monotonic)) { | 1252 | switch (link_func.mir.status.load(.acquire)) { |
| 1253 | .pending => unreachable, // cannot deinit until MIR done | 1253 | .pending => unreachable, // cannot deinit until MIR done |
| 1254 | .failed => {}, // MIR not populated so doesn't need freeing | 1254 | .failed => {}, // MIR not populated so doesn't need freeing |
| 1255 | .ready => link_func.mir.value.deinit(zcu), | 1255 | .ready => link_func.mir.value.deinit(zcu), |
| ... | @@ -1453,7 +1453,7 @@ pub fn doZcuTask(comp: *Compilation, tid: usize, task: ZcuTask) void { | ... | @@ -1453,7 +1453,7 @@ pub fn doZcuTask(comp: *Compilation, tid: usize, task: ZcuTask) void { |
| 1453 | const fqn_slice = ip.getNav(nav).fqn.toSlice(ip); | 1453 | const fqn_slice = ip.getNav(nav).fqn.toSlice(ip); |
| 1454 | const nav_prog_node = comp.link_prog_node.start(fqn_slice, 0); | 1454 | const nav_prog_node = comp.link_prog_node.start(fqn_slice, 0); |
| 1455 | defer nav_prog_node.end(); | 1455 | defer nav_prog_node.end(); |
| 1456 | switch (func.mir.status.load(.monotonic)) { | 1456 | switch (func.mir.status.load(.acquire)) { |
| 1457 | .pending => unreachable, | 1457 | .pending => unreachable, |
| 1458 | .ready => {}, | 1458 | .ready => {}, |
| 1459 | .failed => return, | 1459 | .failed => return, |
src/link/Queue.zig+4-4
| ... | @@ -126,7 +126,7 @@ pub fn mirReady(q: *Queue, comp: *Compilation, mir: *ZcuTask.LinkFunc.SharedMir) | ... | @@ -126,7 +126,7 @@ pub fn mirReady(q: *Queue, comp: *Compilation, mir: *ZcuTask.LinkFunc.SharedMir) |
| 126 | // We were waiting for `mir`, so we will restart the linker thread. | 126 | // We were waiting for `mir`, so we will restart the linker thread. |
| 127 | q.state = .running; | 127 | q.state = .running; |
| 128 | } | 128 | } |
| 129 | assert(mir.status.load(.monotonic) != .pending); | 129 | assert(mir.status.load(.acquire) != .pending); |
| 130 | comp.thread_pool.spawnWgId(&comp.link_task_wait_group, flushTaskQueue, .{ q, comp }); | 130 | comp.thread_pool.spawnWgId(&comp.link_task_wait_group, flushTaskQueue, .{ q, comp }); |
| 131 | } | 131 | } |
| 132 | 132 | ||
| ... | @@ -170,7 +170,7 @@ pub fn enqueueZcu(q: *Queue, comp: *Compilation, task: ZcuTask) Allocator.Error! | ... | @@ -170,7 +170,7 @@ pub fn enqueueZcu(q: *Queue, comp: *Compilation, task: ZcuTask) Allocator.Error! |
| 170 | .finished => if (q.pending_prelink_tasks != 0) return, | 170 | .finished => if (q.pending_prelink_tasks != 0) return, |
| 171 | } | 171 | } |
| 172 | // Restart the linker thread, unless it would immediately be blocked | 172 | // Restart the linker thread, unless it would immediately be blocked |
| 173 | if (task == .link_func and task.link_func.mir.status.load(.monotonic) == .pending) { | 173 | if (task == .link_func and task.link_func.mir.status.load(.acquire) == .pending) { |
| 174 | q.state = .{ .wait_for_mir = task.link_func.mir }; | 174 | q.state = .{ .wait_for_mir = task.link_func.mir }; |
| 175 | return; | 175 | return; |
| 176 | } | 176 | } |
| ... | @@ -243,10 +243,10 @@ fn flushTaskQueue(tid: usize, q: *Queue, comp: *Compilation) void { | ... | @@ -243,10 +243,10 @@ fn flushTaskQueue(tid: usize, q: *Queue, comp: *Compilation) void { |
| 243 | if (task != .link_func) break :pending; | 243 | if (task != .link_func) break :pending; |
| 244 | const status_ptr = &task.link_func.mir.status; | 244 | const status_ptr = &task.link_func.mir.status; |
| 245 | // First check without the mutex to optimize for the common case where MIR is ready. | 245 | // First check without the mutex to optimize for the common case where MIR is ready. |
| 246 | if (status_ptr.load(.monotonic) != .pending) break :pending; | 246 | if (status_ptr.load(.acquire) != .pending) break :pending; |
| 247 | q.mutex.lock(); | 247 | q.mutex.lock(); |
| 248 | defer q.mutex.unlock(); | 248 | defer q.mutex.unlock(); |
| 249 | if (status_ptr.load(.monotonic) != .pending) break :pending; | 249 | if (status_ptr.load(.acquire) != .pending) break :pending; |
| 250 | // We will stop for now, and get restarted once this MIR is ready. | 250 | // We will stop for now, and get restarted once this MIR is ready. |
| 251 | q.state = .{ .wait_for_mir = task.link_func.mir }; | 251 | q.state = .{ .wait_for_mir = task.link_func.mir }; |
| 252 | q.flush_safety.unlock(); | 252 | q.flush_safety.unlock(); |