From 1ecbc16172649c5f11c2c5173a42f04a3c7e5d2b Mon Sep 17 00:00:00 2001 From: Matthew Lugg Date: Sun, 27 Sep 2026 11:42:31 +0100 Subject: [PATCH] link: delete a few unnecessary `tid` parameters --- src/Compilation.zig | 2 +- src/Zcu/PerThread.zig | 17 ++++++------ src/crash_report.zig | 15 +++++------ src/link.zig | 20 +++++++------- src/link/Coff.zig | 25 +++++++++-------- src/link/Elf2.zig | 40 ++++++++++++--------------- src/link/MachO2.zig | 20 +++++++------- src/link/Queue.zig | 63 ++++++++++++++++++++++--------------------- 8 files changed, 98 insertions(+), 104 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index 34aa59e063d05bf2b0676b6462ca470fb9b02407..06592b76ebcc7a2aa6e9b37b94b6970d8a3884da 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -4427,7 +4427,7 @@ fn performAllTheWork( comp.windows_libs_num_done = @intCast(comp.windows_libs.count()); try misc_group.await(io); - comp.link_queue.wait(io); + try comp.link_queue.wait(io); } fn dispatchPrelinkWork(comp: *Compilation, main_progress_node: std.Progress.Node) void { diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index 20cf31a063df84fcc173bd4f9929cd57f83976de..e8bf4fbc9967e6ef357c4e9dc9af47b017c274b9 100644 --- a/src/Zcu/PerThread.zig +++ b/src/Zcu/PerThread.zig @@ -138,6 +138,15 @@ pub fn update( const gpa = comp.gpa; const io = comp.io; + if (!zcu.backendSupportsFeature(.separate_thread)) { + // We may still be running the linker on a separate thread at the moment, but once prelink + // completes, we need that thread to exit so that we can process all ZCU link tasks on the + // main thread. We signal this condition by immediately closing the ZCU task queue. It's + // important that we do this before enqueueing *any* ZCU link tasks: otherwise those tasks + // will run on the linker thread which is meant to be for prelink only! + comp.link_queue.finishZcuQueue(comp); + } + { const tracy_trace = traceNamed(@src(), "astgen"); defer tracy_trace.end(); @@ -269,14 +278,6 @@ pub fn update( try zcu.flushRetryableFailures(); - if (!zcu.backendSupportsFeature(.separate_thread)) { - // Close the ZCU task queue. Prelink may still be running, but the closed - // queue will cause the linker task to exit once prelink finishes. The - // closed queue also communicates to `enqueueZcu` that it should wait for - // the linker task to finish and then run ZCU tasks serially. - comp.link_queue.finishZcuQueue(comp); - } - zcu.sema_prog_node = main_progress_node.start("Semantic Analysis", 0); if (comp.bin_file != null) { zcu.codegen_prog_node = main_progress_node.start("Code Generation", 0); diff --git a/src/crash_report.zig b/src/crash_report.zig index 87c8f8a6e9d08e27998be39b03b3b21f55b9ba4c..53cdf8535ecc511f06e36dec1e17b4937ceac94a 100644 --- a/src/crash_report.zig +++ b/src/crash_report.zig @@ -86,21 +86,20 @@ pub const CodegenFunc = if (enabled) struct { pub const LinkerOp = if (enabled) struct { lf: *link.File, - tid: Zcu.PerThread.Id, threadlocal var current: ?LinkerOp = null; - pub fn start(lf: *link.File, tid: Zcu.PerThread.Id) void { + pub fn start(lf: *link.File) void { std.debug.assert(current == null); - current = .{ .lf = lf, .tid = tid }; + current = .{ .lf = lf }; } - pub fn stop(lf: *link.File, tid: Zcu.PerThread.Id) void { - std.debug.assert(current.?.lf == lf and current.?.tid == tid); + pub fn stop(lf: *link.File) void { + std.debug.assert(current.?.lf == lf); current = null; } } else struct { const current: ?noreturn = null; // Dummy implementation - pub fn start(_: *link.File, _: Zcu.PerThread.Id) void {} - pub fn stop(_: *link.File, _: Zcu.PerThread.Id) void {} + pub fn start(_: *link.File) void {} + pub fn stop(_: *link.File) void {} }; fn dumpCrashContext() Io.Writer.Error!void { @@ -132,7 +131,7 @@ fn dumpCrashContext() Io.Writer.Error!void { try dumpCrashContextSema(anal, w, &S.crash_heap); } else if (LinkerOp.current) |linker_op| { try w.writeAll("Linker snapshot:\n"); - switch (try linker_op.lf.dump(w, linker_op.tid)) { + switch (try linker_op.lf.dump(w)) { .unimplemented => try w.writeAll("(backend does not support link snapshots)"), .needs_extensions => try w.writeAll("(build with -Ddebug-extensions to dump linker state)"), .disabled => try w.writeAll("(run with --debug-link-snapshot to dump linker state)"), diff --git a/src/link.zig b/src/link.zig index ab03353da50b05e487370c70bf3b564431f83893..a704a59a2819019b0d6906a1ea49a06966de1e30 100644 --- a/src/link.zig +++ b/src/link.zig @@ -950,12 +950,12 @@ pub const File = struct { } } - pub fn idle(base: *File, tid: Zcu.PerThread.Id) Error!bool { + fn idle(base: *File) Error!bool { switch (base.tag) { else => return false, inline .elf2, .coff, .macho2 => |tag| { dev.check(tag.devFeature()); - return @as(*tag.Type(), @fieldParentPtr("base", base)).idle(tid); + return @as(*tag.Type(), @fieldParentPtr("base", base)).idle(); }, } } @@ -974,8 +974,8 @@ pub const File = struct { /// Commit pending changes and write headers. Takes into account final output mode. /// `arena` has the lifetime of the call to `Compilation.update`. pub fn flush(base: *File, arena: Allocator, tid: Zcu.PerThread.Id, prog_node: std.Progress.Node) Error!void { - crash_report.LinkerOp.start(base, tid); - defer crash_report.LinkerOp.stop(base, tid); + crash_report.LinkerOp.start(base); + defer crash_report.LinkerOp.stop(base); const comp = base.comp; const io = comp.io; @@ -1026,8 +1026,8 @@ pub const File = struct { ) Error!void { assert(pt.zcu.llvm_object == null); - crash_report.LinkerOp.start(base, pt.tid); - defer crash_report.LinkerOp.stop(base, pt.tid); + crash_report.LinkerOp.start(base); + defer crash_report.LinkerOp.stop(base); switch (base.tag) { .lld => unreachable, @@ -1115,7 +1115,7 @@ pub const File = struct { enabled, }; - pub fn dump(base: *File, w: *Io.Writer, tid: Zcu.PerThread.Id) !DumpResult { + pub fn dump(base: *File, w: *Io.Writer) !DumpResult { if (!build_options.enable_debug_extensions) return .not_built; switch (base.tag) { .elf, @@ -1129,7 +1129,7 @@ pub const File = struct { => return .unimplemented, inline else => |tag| { dev.check(tag.devFeature()); - return @as(*tag.Type(), @fieldParentPtr("base", base)).dump(w, tid); + return @as(*tag.Type(), @fieldParentPtr("base", base)).dump(w); }, } } @@ -1910,8 +1910,8 @@ pub fn doZcuTask(comp: *Compilation, tid: Zcu.PerThread.Id, task: ZcuTask) void } } } -pub fn doIdleTask(comp: *Compilation, tid: Zcu.PerThread.Id) Error!bool { - return if (comp.bin_file) |lf| lf.idle(tid) else false; +pub fn doIdleTask(comp: *Compilation) Error!bool { + return if (comp.bin_file) |lf| lf.idle() else false; } /// After the main pipeline is done, but before flush, the compilation may need to link one final /// `Nav` into the binary: the `builtin.test_functions` value. Since the link thread isn't running diff --git a/src/link/Coff.zig b/src/link/Coff.zig index cf7e1daa91014276e230bf6cd840278498e5eb12..496ba9a76686a9ddd0014cb6aba3ecb642c101c2 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -5916,13 +5916,14 @@ pub fn flush( prog_node: std.Progress.Node, ) link.Error!void { _ = arena; + _ = tid; const sub_prog_node = prog_node.start("COFF Flush", 0); defer sub_prog_node.end(); const comp = coff.base.comp; while (try coff.resolve()) {} - while (try coff.idle(tid)) {} + while (try coff.idle()) {} // This has to occur after all other flushMoved / flushResized have resolved, // but it will also generate one more set of resizes and moves. @@ -5942,7 +5943,7 @@ pub fn flush( ), }; } - while (try coff.idle(tid)) {} + while (try coff.idle()) {} if (coff.isImage()) try coff.reportUndefs(); @@ -5957,7 +5958,7 @@ pub fn flush( }; if (coff.options.enable_link_snapshots) - coff.dumpStderr(tid) catch |err| + coff.dumpStderr() catch |err| return comp.link_diags.fail("dumping link snapshot failed: {t}", .{err}); } @@ -6069,8 +6070,7 @@ fn resolve(coff: *Coff) !bool { return false; } -pub fn idle(coff: *Coff, tid: Zcu.PerThread.Id) !bool { - _ = tid; +pub fn idle(coff: *Coff) !bool { // Idle tasks should not modify create / modify nodes, otherwise the output is not reproducible. coff.mf.nodes_lock.lock(); defer coff.mf.nodes_lock.unlock(); @@ -7457,7 +7457,7 @@ fn updateExportInner( try coff.genPending(pt); while (try coff.resolve()) {} - while (try coff.idle(pt.tid)) {} + while (try coff.idle()) {} const machine = coff.targetLoad(&coff.headerPtr().machine); const exported_ni = exported_si.node(coff); @@ -7595,21 +7595,21 @@ fn updateExportInner( } } -fn dumpStderr(coff: *Coff, tid: Zcu.PerThread.Id) Io.File.Writer.Error!void { +fn dumpStderr(coff: *Coff) Io.File.Writer.Error!void { const comp = coff.base.comp; const io = comp.io; var buffer: [512]u8 = undefined; const stderr = try io.lockStderr(&buffer, null); defer io.unlockStderr(); const w = &stderr.file_writer.interface; - _ = coff.dump(w, tid) catch |err| switch (err) { + _ = coff.dump(w) catch |err| switch (err) { error.WriteFailed => return stderr.file_writer.err.?, }; } -pub fn dump(coff: *Coff, w: *Io.Writer, tid: Zcu.PerThread.Id) Io.Writer.Error!link.File.DumpResult { +pub fn dump(coff: *Coff, w: *Io.Writer) Io.Writer.Error!link.File.DumpResult { if (coff.options.enable_link_snapshots) { - try coff.printNode(tid, w, .root, 0); + try coff.printNode(w, .root, 0); try w.writeAll("Section table:\n"); for (coff.section_table.keys(), coff.section_table.values()) |name, sec| try coff.printSection(w, name, sec.si); @@ -7762,9 +7762,8 @@ fn printNodeName( } } -pub fn printNode( +fn printNode( coff: *Coff, - tid: Zcu.PerThread.Id, w: *Io.Writer, ni: MappedFile.Node.Index, indent: usize, @@ -7792,7 +7791,7 @@ pub fn printNode( // non-leaf, just print children var child_ni = first_ni; while (true) { - try coff.printNode(tid, w, child_ni, indent + 1); + try coff.printNode(w, child_ni, indent + 1); child_ni = child_ni.next(&coff.mf).unwrap() orelse break; } return; diff --git a/src/link/Elf2.zig b/src/link/Elf2.zig index ddff228929c093864d7bf168e7909aa07d3e82e8..a08d5add1320da6a52a8bc63e75765aa97588a85 100644 --- a/src/link/Elf2.zig +++ b/src/link/Elf2.zig @@ -9726,8 +9726,9 @@ pub fn updateNav(elf: *Elf, pt: Zcu.PerThread, nav_index: InternPool.Nav.Index) }; } fn updateNavInner(elf: *Elf, pt: Zcu.PerThread, nav_index: InternPool.Nav.Index) Error!void { - const zcu = pt.zcu; - const gpa = zcu.gpa; + const comp = elf.base.comp; + const gpa = comp.gpa; + const zcu = comp.zcu.?; const ip = &zcu.intern_pool; const nav = ip.getNav(nav_index); @@ -9823,7 +9824,7 @@ pub fn updateContainerType( ), }; } -pub fn updateContainerTypeInner( +fn updateContainerTypeInner( elf: *Elf, pt: Zcu.PerThread, ty: InternPool.Index, @@ -10227,7 +10228,8 @@ pub fn flush( tid: Zcu.PerThread.Id, prog_node: std.Progress.Node, ) link.Error!void { - elf.flushInner(arena, tid, prog_node) catch |err| switch (err) { + _ = tid; + elf.flushInner(arena, prog_node) catch |err| switch (err) { else => |e| return e, error.MappedFileIo => return elf.base.comp.link_diags.fail( "failed to write output file: {t}", @@ -10238,7 +10240,6 @@ pub fn flush( fn flushInner( elf: *Elf, arena: std.mem.Allocator, - tid: Zcu.PerThread.Id, prog_node: std.Progress.Node, ) Error!void { const comp = elf.base.comp; @@ -10266,7 +10267,7 @@ fn flushInner( try elf.prepareDynamic(); - while (try elf.idle(tid)) {} + while (try elf.idle()) {} assert(elf.input_pending_index == elf.inputs.items.len); assert(elf.input_section_pending_index == elf.input_sections.items.len); @@ -10323,13 +10324,11 @@ fn flushInner( try elf.mf.flush(); if (elf.options.enable_link_snapshots) - elf.dumpStderr(tid) catch |err| + elf.dumpStderr() catch |err| return diags.fail("dumping link snapshot failed: {t}", .{err}); } -pub fn idle(elf: *Elf, tid: Zcu.PerThread.Id) link.Error!bool { - _ = tid; - +pub fn idle(elf: *Elf) link.Error!bool { // This function is called non-deterministically, and so must not affect the layout of any nodes. elf.mf.nodes_lock.lock(); defer elf.mf.nodes_lock.unlock(); @@ -12126,19 +12125,15 @@ pub fn updateExports( export_indices: []const Zcu.Export.Index, ) link.Error!void { for (export_indices) |export_index| { - elf.updateExportInner(pt, export_index) catch |err| switch (err) { + elf.updateExportInner(export_index) catch |err| switch (err) { else => |e| return e, error.MappedFileIo => return elf.base.comp.link_diags.fail("failed to write output file: {t}", .{elf.mf.io_err.?}), }; } try elf.genPending(pt); } -fn updateExportInner( - elf: *Elf, - pt: Zcu.PerThread, - export_index: Zcu.Export.Index, -) Error!void { - const zcu = pt.zcu; +fn updateExportInner(elf: *Elf, export_index: Zcu.Export.Index) Error!void { + const zcu = elf.base.comp.zcu.?; const ip = &zcu.intern_pool; const @"export" = export_index.ptr(zcu); @@ -12213,21 +12208,21 @@ fn updateExportInner( }; } -fn dumpStderr(elf: *Elf, tid: Zcu.PerThread.Id) Io.File.Writer.Error!void { +fn dumpStderr(elf: *Elf) Io.File.Writer.Error!void { const comp = elf.base.comp; const io = comp.io; var buffer: [512]u8 = undefined; const stderr = try io.lockStderr(&buffer, null); defer io.unlockStderr(); const w = &stderr.file_writer.interface; - _ = elf.dump(w, tid) catch |err| switch (err) { + _ = elf.dump(w) catch |err| switch (err) { error.WriteFailed => return stderr.file_writer.err.?, }; } -pub fn dump(elf: *Elf, w: *Io.Writer, tid: Zcu.PerThread.Id) Io.Writer.Error!link.File.DumpResult { +pub fn dump(elf: *Elf, w: *Io.Writer) Io.Writer.Error!link.File.DumpResult { if (elf.options.enable_link_snapshots) { - try elf.printNode(tid, w, .root, 0); + try elf.printNode(w, .root, 0); return .enabled; } return .disabled; @@ -12235,7 +12230,6 @@ pub fn dump(elf: *Elf, w: *Io.Writer, tid: Zcu.PerThread.Id) Io.Writer.Error!lin pub fn printNode( elf: *Elf, - tid: Zcu.PerThread.Id, w: *Io.Writer, ni: MappedFile.Node.Index, indent: usize, @@ -12366,7 +12360,7 @@ pub fn printNode( // non-leaf, just print children var child_ni = first_ni; while (true) { - try elf.printNode(tid, w, child_ni, indent + 1); + try elf.printNode(w, child_ni, indent + 1); child_ni = child_ni.next(&elf.mf).unwrap() orelse break; } return; diff --git a/src/link/MachO2.zig b/src/link/MachO2.zig index f5583cb7915a24b4a658636d35a77f76e7923cfb..04c3b0a235a64ad3725a5f98c025970b113adc3b 100644 --- a/src/link/MachO2.zig +++ b/src/link/MachO2.zig @@ -1100,10 +1100,11 @@ pub fn flush( ) link.Error!void { const diags = &macho.base.comp.link_diags; _ = arena; + _ = tid; _ = prog_node; try macho.updateSegmentLoadAddresses(); - while (try macho.idle(tid)) {} + while (try macho.idle()) {} if (macho.lc.entry_point) |entry_point_lc| { // TODO: actually find the entry point. @@ -1120,7 +1121,7 @@ pub fn flush( }; if (macho.options.enable_link_snapshots) - macho.dumpStderr(tid) catch |err| + macho.dumpStderr() catch |err| return diags.fail("dumping link snapshot failed: {t}", .{err}); } pub fn updateErrorData(macho: *MachO, pt: Zcu.PerThread) link.Error!void { @@ -1128,9 +1129,7 @@ pub fn updateErrorData(macho: *MachO, pt: Zcu.PerThread) link.Error!void { _ = pt; @panic("TODO"); } -pub fn idle(macho: *MachO, tid: Zcu.PerThread.Id) link.Error!bool { - _ = tid; - +pub fn idle(macho: *MachO) link.Error!bool { macho.mf.nodes_lock.lock(); defer macho.mf.nodes_lock.unlock(); @@ -1253,28 +1252,27 @@ pub fn updateExports( @panic("TODO"); } -fn dumpStderr(macho: *MachO, tid: Zcu.PerThread.Id) Io.File.Writer.Error!void { +fn dumpStderr(macho: *MachO) Io.File.Writer.Error!void { const comp = macho.base.comp; const io = comp.io; var buffer: [512]u8 = undefined; const stderr = try io.lockStderr(&buffer, null); defer io.unlockStderr(); const w = &stderr.file_writer.interface; - _ = macho.dump(w, tid) catch |err| switch (err) { + _ = macho.dump(w) catch |err| switch (err) { error.WriteFailed => return stderr.file_writer.err.?, }; } -pub fn dump(macho: *MachO, w: *Io.Writer, tid: Zcu.PerThread.Id) !link.File.DumpResult { +pub fn dump(macho: *MachO, w: *Io.Writer) !link.File.DumpResult { if (macho.options.enable_link_snapshots) { - try macho.printNode(tid, w, .root, 0); + try macho.printNode(w, .root, 0); return .enabled; } return .disabled; } fn printNode( macho: *const MachO, - tid: Zcu.PerThread.Id, w: *Io.Writer, ni: MappedFile.Node.Index, indent: usize, @@ -1310,7 +1308,7 @@ fn printNode( // non-leaf, just print children var child_ni = first_ni; while (true) { - try macho.printNode(tid, w, child_ni, indent + 1); + try macho.printNode(w, child_ni, indent + 1); child_ni = child_ni.next(&macho.mf).unwrap() orelse break; } return; diff --git a/src/link/Queue.zig b/src/link/Queue.zig index 2057245f012e32d25bd3005ec8f466430230d9bd..81585b91f377efd195b19a07a15cbf20241b1713 100644 --- a/src/link/Queue.zig +++ b/src/link/Queue.zig @@ -17,7 +17,7 @@ /// set to `null` by the main thread after it is canceled. It is not otherwise modified; as such, it /// may be checked non-atomically. If a task is being queued and this is `null`, tasks must be run /// eagerly. -future: ?std.Io.Future(void), +future: ?std.Io.Future(Io.Cancelable!void), /// This is only used if `future == null` during prelink. In that case, it is used to ensure that /// only one prelink task is run at a time. @@ -42,15 +42,15 @@ pub const empty: Queue = .{ pub fn cancel(q: *Queue, io: Io) void { if (q.future) |*f| { - f.cancel(io); - q.future = null; + defer q.future = null; + f.cancel(io) catch {}; } } -pub fn wait(q: *Queue, io: Io) void { +pub fn wait(q: *Queue, io: Io) Io.Cancelable!void { if (q.future) |*f| { - f.await(io); - q.future = null; + defer q.future = null; + try f.await(io); } } @@ -107,11 +107,13 @@ pub fn enqueueZcu( } else |err| switch (err) { error.Canceled => |e| return e, error.Closed => { - // The linker is still processing prelink tasks. Wait for those - // to finish, after which the linker task will exist, and ZCU - // tasks will be run non-concurrently. This logic exists for - // backends which do not support `Zcu.Feature.separate_thread`. - q.wait(io); + // This path is hit if the backend does not support `Zcu.Feature.separate_thread`. + // In that case, the linker processes *prelink* tasks on a thread, but once prelink + // completes we want to move processing to the main thread for all ZCU tasks. To + // achieve this, `Compilation.update` immmediately closes the ZCU task queue at the + // start of the update, causing `runLinkTasks` to exit as soon as prelink completes. + // So here we just need to wait for that, then fall back to the single-threaded path. + try q.wait(io); }, } } @@ -148,7 +150,7 @@ pub fn finishZcuQueue(q: *Queue, comp: *Compilation) void { } } -fn runLinkTasks(q: *Queue, comp: *Compilation) void { +fn runLinkTasks(q: *Queue, comp: *Compilation) Io.Cancelable!void { const io = comp.io; const tid: Zcu.PerThread.Id = .acquire(io); defer tid.release(io); @@ -159,12 +161,19 @@ fn runLinkTasks(q: *Queue, comp: *Compilation) void { var task_buf: [128]PrelinkTask = undefined; const limit: usize = if (have_idle_tasks) 0 else 1; const n = q.prelink_queue.get(io, &task_buf, limit) catch |err| switch (err) { - error.Canceled => return, + error.Canceled => |e| return e, error.Closed => break :prelink_tasks, }; if (n == 0) { assert(have_idle_tasks); - have_idle_tasks = runIdleTask(comp, tid); + have_idle_tasks = link.doIdleTask(comp) catch |err| switch (err) { + error.Canceled => |e| return e, + error.AlreadyReported => false, + error.OutOfMemory => more_idle_tasks: { + comp.link_diags.setAllocFailure(); + break :more_idle_tasks false; + }, + }; } else for (task_buf[0..n]) |task| { link.doPrelinkTask(comp, task); have_idle_tasks = true; @@ -181,8 +190,8 @@ fn runLinkTasks(q: *Queue, comp: *Compilation) void { break :prelink; } lf.prelink() catch |err| switch (err) { + error.Canceled => |e| return e, error.OutOfMemory => comp.link_diags.setAllocFailure(), - error.Canceled => @panic("TODO"), error.AlreadyReported => {}, }; } @@ -191,31 +200,25 @@ fn runLinkTasks(q: *Queue, comp: *Compilation) void { var task_buf: [128]ZcuTask = undefined; const limit: usize = if (have_idle_tasks) 0 else 1; const n = q.zcu_queue.get(io, &task_buf, limit) catch |err| switch (err) { - error.Canceled => return, + error.Canceled => |e| return e, error.Closed => break :zcu_tasks, }; if (n == 0) { assert(have_idle_tasks); - have_idle_tasks = runIdleTask(comp, tid); + have_idle_tasks = link.doIdleTask(comp) catch |err| switch (err) { + error.Canceled => |e| return e, + error.AlreadyReported => false, + error.OutOfMemory => more_idle_tasks: { + comp.link_diags.setAllocFailure(); + break :more_idle_tasks false; + }, + }; } else for (task_buf[0..n]) |task| { link.doZcuTask(comp, tid, task); have_idle_tasks = true; } } } -fn runIdleTask(comp: *Compilation, tid: Zcu.PerThread.Id) bool { - return link.doIdleTask(comp, tid) catch |err| switch (err) { - error.OutOfMemory => have_more: { - comp.link_diags.setAllocFailure(); - break :have_more false; - }, - error.AlreadyReported => false, - error.Canceled => { - comp.io.recancel(); - return false; - }, - }; -} const std = @import("std"); const assert = std.debug.assert; -- 2.54.0