Skip to content

Commit f1f05df

Browse files
committed
frontend: incremental progress
This commit makes more progress towards incremental compilation, fixing some crashes in the frontend. Notably, it fixes the regressions introduced by ziglang#20964. It also cleans up the "outdated file root" mechanism, by virtue of deleting it: we now detect outdated file roots just after updating ZIR refs, and re-scan their namespaces.
1 parent bb70501 commit f1f05df

File tree

9 files changed

+410
-305
lines changed

9 files changed

+410
-305
lines changed

src/Compilation.zig

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3081,7 +3081,7 @@ pub fn totalErrorCount(comp: *Compilation) u32 {
30813081
for (zcu.failed_analysis.keys()) |anal_unit| {
30823082
const file_index = switch (anal_unit.unwrap()) {
30833083
.cau => |cau| zcu.namespacePtr(ip.getCau(cau).namespace).file_scope,
3084-
.func => |ip_index| zcu.funcInfo(ip_index).zir_body_inst.resolveFull(ip).file,
3084+
.func => |ip_index| (zcu.funcInfo(ip_index).zir_body_inst.resolveFull(ip) orelse continue).file,
30853085
};
30863086
if (zcu.fileByIndex(file_index).okToReportErrors()) {
30873087
total += 1;
@@ -3091,11 +3091,13 @@ pub fn totalErrorCount(comp: *Compilation) u32 {
30913091
}
30923092
}
30933093

3094-
if (zcu.intern_pool.global_error_set.getNamesFromMainThread().len > zcu.error_limit) {
3095-
total += 1;
3094+
for (zcu.failed_codegen.keys()) |nav| {
3095+
if (zcu.navFileScope(nav).okToReportErrors()) {
3096+
total += 1;
3097+
}
30963098
}
30973099

3098-
for (zcu.failed_codegen.keys()) |_| {
3100+
if (zcu.intern_pool.global_error_set.getNamesFromMainThread().len > zcu.error_limit) {
30993101
total += 1;
31003102
}
31013103
}
@@ -3114,7 +3116,13 @@ pub fn totalErrorCount(comp: *Compilation) u32 {
31143116
}
31153117
}
31163118

3117-
return @as(u32, @intCast(total));
3119+
if (comp.module) |zcu| {
3120+
if (total == 0 and zcu.transitive_failed_analysis.count() > 0) {
3121+
@panic("Transitive analysis errors, but none actually emitted");
3122+
}
3123+
}
3124+
3125+
return @intCast(total);
31183126
}
31193127

31203128
/// This function is temporally single-threaded.
@@ -3214,7 +3222,7 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle {
32143222
for (zcu.failed_analysis.keys(), zcu.failed_analysis.values()) |anal_unit, error_msg| {
32153223
const file_index = switch (anal_unit.unwrap()) {
32163224
.cau => |cau| zcu.namespacePtr(ip.getCau(cau).namespace).file_scope,
3217-
.func => |ip_index| zcu.funcInfo(ip_index).zir_body_inst.resolveFull(ip).file,
3225+
.func => |ip_index| (zcu.funcInfo(ip_index).zir_body_inst.resolveFull(ip) orelse continue).file,
32183226
};
32193227

32203228
// Skip errors for AnalUnits within files that had a parse failure.
@@ -3243,7 +3251,8 @@ pub fn getAllErrorsAlloc(comp: *Compilation) !ErrorBundle {
32433251
}
32443252
}
32453253
}
3246-
for (zcu.failed_codegen.values()) |error_msg| {
3254+
for (zcu.failed_codegen.keys(), zcu.failed_codegen.values()) |nav, error_msg| {
3255+
if (!zcu.navFileScope(nav).okToReportErrors()) continue;
32473256
try addModuleErrorMsg(zcu, &bundle, error_msg.*, &all_references);
32483257
}
32493258
for (zcu.failed_exports.values()) |value| {
@@ -3608,10 +3617,9 @@ fn performAllTheWorkInner(
36083617
// Pre-load these things from our single-threaded context since they
36093618
// will be needed by the worker threads.
36103619
const path_digest = zcu.filePathDigest(file_index);
3611-
const old_root_type = zcu.fileRootType(file_index);
36123620
const file = zcu.fileByIndex(file_index);
36133621
comp.thread_pool.spawnWgId(&astgen_wait_group, workerAstGenFile, .{
3614-
comp, file, file_index, path_digest, old_root_type, zir_prog_node, &astgen_wait_group, .root,
3622+
comp, file, file_index, path_digest, zir_prog_node, &astgen_wait_group, .root,
36153623
});
36163624
}
36173625
}
@@ -3649,6 +3657,7 @@ fn performAllTheWorkInner(
36493657
}
36503658
try reportMultiModuleErrors(pt);
36513659
try zcu.flushRetryableFailures();
3660+
36523661
zcu.sema_prog_node = main_progress_node.start("Semantic Analysis", 0);
36533662
zcu.codegen_prog_node = main_progress_node.start("Code Generation", 0);
36543663
}
@@ -4283,7 +4292,6 @@ fn workerAstGenFile(
42834292
file: *Zcu.File,
42844293
file_index: Zcu.File.Index,
42854294
path_digest: Cache.BinDigest,
4286-
old_root_type: InternPool.Index,
42874295
prog_node: std.Progress.Node,
42884296
wg: *WaitGroup,
42894297
src: Zcu.AstGenSrc,
@@ -4292,7 +4300,7 @@ fn workerAstGenFile(
42924300
defer child_prog_node.end();
42934301

42944302
const pt: Zcu.PerThread = .{ .zcu = comp.module.?, .tid = @enumFromInt(tid) };
4295-
pt.astGenFile(file, path_digest, old_root_type) catch |err| switch (err) {
4303+
pt.astGenFile(file, path_digest) catch |err| switch (err) {
42964304
error.AnalysisFail => return,
42974305
else => {
42984306
file.status = .retryable_failure;
@@ -4323,7 +4331,7 @@ fn workerAstGenFile(
43234331
// `@import("builtin")` is handled specially.
43244332
if (mem.eql(u8, import_path, "builtin")) continue;
43254333

4326-
const import_result, const imported_path_digest, const imported_root_type = blk: {
4334+
const import_result, const imported_path_digest = blk: {
43274335
comp.mutex.lock();
43284336
defer comp.mutex.unlock();
43294337

@@ -4338,8 +4346,7 @@ fn workerAstGenFile(
43384346
comp.appendFileSystemInput(fsi, res.file.mod.root, res.file.sub_file_path) catch continue;
43394347
};
43404348
const imported_path_digest = pt.zcu.filePathDigest(res.file_index);
4341-
const imported_root_type = pt.zcu.fileRootType(res.file_index);
4342-
break :blk .{ res, imported_path_digest, imported_root_type };
4349+
break :blk .{ res, imported_path_digest };
43434350
};
43444351
if (import_result.is_new) {
43454352
log.debug("AstGen of {s} has import '{s}'; queuing AstGen of {s}", .{
@@ -4350,7 +4357,7 @@ fn workerAstGenFile(
43504357
.import_tok = item.data.token,
43514358
} };
43524359
comp.thread_pool.spawnWgId(wg, workerAstGenFile, .{
4353-
comp, import_result.file, import_result.file_index, imported_path_digest, imported_root_type, prog_node, wg, sub_src,
4360+
comp, import_result.file, import_result.file_index, imported_path_digest, prog_node, wg, sub_src,
43544361
});
43554362
}
43564363
}
@@ -6443,7 +6450,8 @@ fn buildOutputFromZig(
64436450

64446451
try comp.updateSubCompilation(sub_compilation, misc_task_tag, prog_node);
64456452

6446-
assert(out.* == null);
6453+
// Under incremental compilation, `out` may already be populated from a prior update.
6454+
assert(out.* == null or comp.incremental);
64476455
out.* = try sub_compilation.toCrtFile();
64486456
}
64496457

src/InternPool.zig

Lines changed: 135 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,19 +65,49 @@ pub const single_threaded = builtin.single_threaded or !want_multi_threaded;
6565
pub const TrackedInst = extern struct {
6666
file: FileIndex,
6767
inst: Zir.Inst.Index,
68-
comptime {
69-
// The fields should be tightly packed. See also serialiation logic in `Compilation.saveState`.
70-
assert(@sizeOf(@This()) == @sizeOf(FileIndex) + @sizeOf(Zir.Inst.Index));
71-
}
68+
69+
pub const MaybeLost = extern struct {
70+
file: FileIndex,
71+
inst: ZirIndex,
72+
pub const ZirIndex = enum(u32) {
73+
/// Tracking failed for this ZIR instruction. Uses of it should fail.
74+
lost = std.math.maxInt(u32),
75+
_,
76+
pub fn unwrap(inst: ZirIndex) ?Zir.Inst.Index {
77+
return switch (inst) {
78+
.lost => null,
79+
_ => @enumFromInt(@intFromEnum(inst)),
80+
};
81+
}
82+
pub fn wrap(inst: Zir.Inst.Index) ZirIndex {
83+
return @enumFromInt(@intFromEnum(inst));
84+
}
85+
};
86+
comptime {
87+
// The fields should be tightly packed. See also serialiation logic in `Compilation.saveState`.
88+
assert(@sizeOf(@This()) == @sizeOf(FileIndex) + @sizeOf(ZirIndex));
89+
}
90+
};
91+
7292
pub const Index = enum(u32) {
7393
_,
74-
pub fn resolveFull(tracked_inst_index: TrackedInst.Index, ip: *const InternPool) TrackedInst {
94+
pub fn resolveFull(tracked_inst_index: TrackedInst.Index, ip: *const InternPool) ?TrackedInst {
7595
const tracked_inst_unwrapped = tracked_inst_index.unwrap(ip);
7696
const tracked_insts = ip.getLocalShared(tracked_inst_unwrapped.tid).tracked_insts.acquire();
77-
return tracked_insts.view().items(.@"0")[tracked_inst_unwrapped.index];
97+
const maybe_lost = tracked_insts.view().items(.@"0")[tracked_inst_unwrapped.index];
98+
return .{
99+
.file = maybe_lost.file,
100+
.inst = maybe_lost.inst.unwrap() orelse return null,
101+
};
78102
}
79-
pub fn resolve(i: TrackedInst.Index, ip: *const InternPool) Zir.Inst.Index {
80-
return i.resolveFull(ip).inst;
103+
pub fn resolveFile(tracked_inst_index: TrackedInst.Index, ip: *const InternPool) FileIndex {
104+
const tracked_inst_unwrapped = tracked_inst_index.unwrap(ip);
105+
const tracked_insts = ip.getLocalShared(tracked_inst_unwrapped.tid).tracked_insts.acquire();
106+
const maybe_lost = tracked_insts.view().items(.@"0")[tracked_inst_unwrapped.index];
107+
return maybe_lost.file;
108+
}
109+
pub fn resolve(i: TrackedInst.Index, ip: *const InternPool) ?Zir.Inst.Index {
110+
return (i.resolveFull(ip) orelse return null).inst;
81111
}
82112

83113
pub fn toOptional(i: TrackedInst.Index) Optional {
@@ -120,7 +150,11 @@ pub fn trackZir(
120150
tid: Zcu.PerThread.Id,
121151
key: TrackedInst,
122152
) Allocator.Error!TrackedInst.Index {
123-
const full_hash = Hash.hash(0, std.mem.asBytes(&key));
153+
const maybe_lost_key: TrackedInst.MaybeLost = .{
154+
.file = key.file,
155+
.inst = TrackedInst.MaybeLost.ZirIndex.wrap(key.inst),
156+
};
157+
const full_hash = Hash.hash(0, std.mem.asBytes(&maybe_lost_key));
124158
const hash: u32 = @truncate(full_hash >> 32);
125159
const shard = &ip.shards[@intCast(full_hash & (ip.shards.len - 1))];
126160
var map = shard.shared.tracked_inst_map.acquire();
@@ -132,12 +166,11 @@ pub fn trackZir(
132166
const entry = &map.entries[map_index];
133167
const index = entry.acquire().unwrap() orelse break;
134168
if (entry.hash != hash) continue;
135-
if (std.meta.eql(index.resolveFull(ip), key)) return index;
169+
if (std.meta.eql(index.resolveFull(ip) orelse continue, key)) return index;
136170
}
137171
shard.mutate.tracked_inst_map.mutex.lock();
138172
defer shard.mutate.tracked_inst_map.mutex.unlock();
139173
if (map.entries != shard.shared.tracked_inst_map.entries) {
140-
shard.mutate.tracked_inst_map.len += 1;
141174
map = shard.shared.tracked_inst_map;
142175
map_mask = map.header().mask();
143176
map_index = hash;
@@ -147,7 +180,7 @@ pub fn trackZir(
147180
const entry = &map.entries[map_index];
148181
const index = entry.acquire().unwrap() orelse break;
149182
if (entry.hash != hash) continue;
150-
if (std.meta.eql(index.resolveFull(ip), key)) return index;
183+
if (std.meta.eql(index.resolveFull(ip) orelse continue, key)) return index;
151184
}
152185
defer shard.mutate.tracked_inst_map.len += 1;
153186
const local = ip.getLocal(tid);
@@ -161,7 +194,7 @@ pub fn trackZir(
161194
.tid = tid,
162195
.index = list.mutate.len,
163196
}).wrap(ip);
164-
list.appendAssumeCapacity(.{key});
197+
list.appendAssumeCapacity(.{maybe_lost_key});
165198
entry.release(index.toOptional());
166199
return index;
167200
}
@@ -205,12 +238,91 @@ pub fn trackZir(
205238
.tid = tid,
206239
.index = list.mutate.len,
207240
}).wrap(ip);
208-
list.appendAssumeCapacity(.{key});
241+
list.appendAssumeCapacity(.{maybe_lost_key});
209242
map.entries[map_index] = .{ .value = index.toOptional(), .hash = hash };
210243
shard.shared.tracked_inst_map.release(new_map);
211244
return index;
212245
}
213246

247+
pub fn rehashTrackedInsts(
248+
ip: *InternPool,
249+
gpa: Allocator,
250+
/// TODO: maybe don't take this? it doesn't actually matter, only one thread is running at this point
251+
tid: Zcu.PerThread.Id,
252+
) Allocator.Error!void {
253+
// TODO: this function doesn't handle OOM well. What should it do?
254+
// Indeed, what should anyone do when they run out of memory?
255+
256+
// We don't lock anything, as this function assumes that no other thread is
257+
// accessing `tracked_insts`. This is necessary because we're going to be
258+
// iterating the `TrackedInst`s in each `Local`, so we have to know that
259+
// none will be added as we work.
260+
261+
// Figure out how big each shard need to be and store it in its mutate `len`.
262+
for (ip.shards) |*shard| shard.mutate.tracked_inst_map.len = 0;
263+
for (ip.locals) |*local| {
264+
// `getMutableTrackedInsts` is okay only because no other thread is currently active.
265+
// We need the `mutate` for the len.
266+
for (local.getMutableTrackedInsts(gpa).viewAllowEmpty().items(.@"0")) |tracked_inst| {
267+
if (tracked_inst.inst == .lost) continue; // we can ignore this one!
268+
const full_hash = Hash.hash(0, std.mem.asBytes(&tracked_inst));
269+
const shard = &ip.shards[@intCast(full_hash & (ip.shards.len - 1))];
270+
shard.mutate.tracked_inst_map.len += 1;
271+
}
272+
}
273+
274+
const Map = Shard.Map(TrackedInst.Index.Optional);
275+
276+
const arena_state = &ip.getLocal(tid).mutate.arena;
277+
278+
// We know how big each shard must be, so ensure we have the capacity we need.
279+
for (ip.shards) |*shard| {
280+
const want_capacity = std.math.ceilPowerOfTwo(u32, shard.mutate.tracked_inst_map.len * 5 / 3) catch unreachable;
281+
const have_capacity = shard.shared.tracked_inst_map.header().capacity; // no acquire because we hold the mutex
282+
if (have_capacity >= want_capacity) {
283+
@memset(shard.shared.tracked_inst_map.entries[0..have_capacity], .{ .value = .none, .hash = undefined });
284+
continue;
285+
}
286+
var arena = arena_state.promote(gpa);
287+
defer arena_state.* = arena.state;
288+
const new_map_buf = try arena.allocator().alignedAlloc(
289+
u8,
290+
Map.alignment,
291+
Map.entries_offset + want_capacity * @sizeOf(Map.Entry),
292+
);
293+
const new_map: Map = .{ .entries = @ptrCast(new_map_buf[Map.entries_offset..].ptr) };
294+
new_map.header().* = .{ .capacity = want_capacity };
295+
@memset(new_map.entries[0..want_capacity], .{ .value = .none, .hash = undefined });
296+
shard.shared.tracked_inst_map.release(new_map);
297+
}
298+
299+
// Now, actually insert the items.
300+
for (ip.locals, 0..) |*local, local_tid| {
301+
// `getMutableTrackedInsts` is okay only because no other thread is currently active.
302+
// We need the `mutate` for the len.
303+
for (local.getMutableTrackedInsts(gpa).viewAllowEmpty().items(.@"0"), 0..) |tracked_inst, local_inst_index| {
304+
if (tracked_inst.inst == .lost) continue; // we can ignore this one!
305+
const full_hash = Hash.hash(0, std.mem.asBytes(&tracked_inst));
306+
const hash: u32 = @truncate(full_hash >> 32);
307+
const shard = &ip.shards[@intCast(full_hash & (ip.shards.len - 1))];
308+
const map = shard.shared.tracked_inst_map; // no acquire because we hold the mutex
309+
const map_mask = map.header().mask();
310+
var map_index = hash;
311+
const entry = while (true) : (map_index += 1) {
312+
map_index &= map_mask;
313+
const entry = &map.entries[map_index];
314+
if (entry.acquire() == .none) break entry;
315+
};
316+
const index = TrackedInst.Index.Unwrapped.wrap(.{
317+
.tid = @enumFromInt(local_tid),
318+
.index = @intCast(local_inst_index),
319+
}, ip);
320+
entry.hash = hash;
321+
entry.release(index.toOptional());
322+
}
323+
}
324+
}
325+
214326
/// Analysis Unit. Represents a single entity which undergoes semantic analysis.
215327
/// This is either a `Cau` or a runtime function.
216328
/// The LSB is used as a tag bit.
@@ -728,7 +840,7 @@ const Local = struct {
728840
else => @compileError("unsupported host"),
729841
};
730842
const Strings = List(struct { u8 });
731-
const TrackedInsts = List(struct { TrackedInst });
843+
const TrackedInsts = List(struct { TrackedInst.MaybeLost });
732844
const Maps = List(struct { FieldMap });
733845
const Caus = List(struct { Cau });
734846
const Navs = List(Nav.Repr);
@@ -959,6 +1071,14 @@ const Local = struct {
9591071
mutable.list.release(new_list);
9601072
}
9611073

1074+
pub fn viewAllowEmpty(mutable: Mutable) View {
1075+
const capacity = mutable.list.header().capacity;
1076+
return .{
1077+
.bytes = mutable.list.bytes,
1078+
.len = mutable.mutate.len,
1079+
.capacity = capacity,
1080+
};
1081+
}
9621082
pub fn view(mutable: Mutable) View {
9631083
const capacity = mutable.list.header().capacity;
9641084
assert(capacity > 0); // optimizes `MultiArrayList.Slice.items`
@@ -996,7 +1116,6 @@ const Local = struct {
9961116
fn header(list: ListSelf) *Header {
9971117
return @ptrFromInt(@intFromPtr(list.bytes) - bytes_offset);
9981118
}
999-
10001119
pub fn view(list: ListSelf) View {
10011120
const capacity = list.header().capacity;
10021121
assert(capacity > 0); // optimizes `MultiArrayList.Slice.items`
@@ -11000,7 +11119,6 @@ pub fn getOrPutTrailingString(
1100011119
shard.mutate.string_map.mutex.lock();
1100111120
defer shard.mutate.string_map.mutex.unlock();
1100211121
if (map.entries != shard.shared.string_map.entries) {
11003-
shard.mutate.string_map.len += 1;
1100411122
map = shard.shared.string_map;
1100511123
map_mask = map.header().mask();
1100611124
map_index = hash;

0 commit comments

Comments
 (0)