From 9cf8a7661f34e764f785583a3028429e2a700d20 Mon Sep 17 00:00:00 2001 From: mlugg Date: Mon, 19 Aug 2024 07:50:57 +0100 Subject: [PATCH 1/7] compiler: handle eval branch quota in memoized calls In a `memoized_call`, store how many backwards braches the call performs. Add this to `sema.branch_count` when using a memoized call. If this exceeds the quota, perform a non-memoized call to get a correct "exceeded X backwards branches" error. Also, do not memoize calls which do `@setEvalBranchQuota` or similar, as this affects global state which must apply to the caller. Change some eval branch quotas so that the compiler itself still builds correctly. This commit manually changes a file in Aro which is automatically generated. The sources which generate the file are not in this repo. Upstream Aro should make the suitable changes on their end before the next sync of Aro sources into the Zig repo. --- lib/compiler/aro/aro/Builtins/Builtin.zig | 2 +- lib/compiler/aro/aro/Parser.zig | 2 + lib/std/crypto/pcurves/p384.zig | 2 +- src/InternPool.zig | 4 ++ src/Sema.zig | 49 ++++++++++++++++++----- src/register_manager.zig | 2 + 6 files changed, 50 insertions(+), 11 deletions(-) diff --git a/lib/compiler/aro/aro/Builtins/Builtin.zig b/lib/compiler/aro/aro/Builtins/Builtin.zig index 9564bfecb5..c5cf98608b 100644 --- a/lib/compiler/aro/aro/Builtins/Builtin.zig +++ b/lib/compiler/aro/aro/Builtins/Builtin.zig @@ -5165,7 +5165,7 @@ const dafsa = [_]Node{ .{ .char = 'e', .end_of_word = false, .end_of_list = true, .number = 1, .child_index = 4913 }, }; pub const data = blk: { - @setEvalBranchQuota(3986); + @setEvalBranchQuota(30_000); break :blk [_]@This(){ // _Block_object_assign .{ .tag = @enumFromInt(0), .properties = .{ .param_str = "vv*vC*iC", .header = .blocks, .attributes = .{ .lib_function_without_prefix = true } } }, diff --git a/lib/compiler/aro/aro/Parser.zig b/lib/compiler/aro/aro/Parser.zig index 956dc5e114..1cb5e18934 100644 --- a/lib/compiler/aro/aro/Parser.zig +++ b/lib/compiler/aro/aro/Parser.zig @@ -4802,6 +4802,7 @@ const CallExpr = union(enum) { } fn shouldPromoteVarArg(self: CallExpr, arg_idx: u32) bool { + @setEvalBranchQuota(2000); return switch (self) { .standard => true, .builtin => |builtin| switch (builtin.tag) { @@ -4902,6 +4903,7 @@ const CallExpr = union(enum) { } fn returnType(self: CallExpr, p: *Parser, callable_ty: Type) Type { + @setEvalBranchQuota(6000); return switch (self) { .standard => callable_ty.returnType(), .builtin => |builtin| switch (builtin.tag) { diff --git a/lib/std/crypto/pcurves/p384.zig b/lib/std/crypto/pcurves/p384.zig index f8c5713209..3ab95da5e8 100644 --- a/lib/std/crypto/pcurves/p384.zig +++ b/lib/std/crypto/pcurves/p384.zig @@ -393,7 +393,7 @@ pub const P384 = struct { } const basePointPc = pc: { - @setEvalBranchQuota(50000); + @setEvalBranchQuota(70000); break :pc precompute(P384.basePoint, 15); }; diff --git a/src/InternPool.zig b/src/InternPool.zig index d953755987..83732a29f6 100644 --- a/src/InternPool.zig +++ b/src/InternPool.zig @@ -2391,6 +2391,7 @@ pub const Key = union(enum) { func: Index, arg_values: []const Index, result: Index, + branch_count: u32, }; pub fn hash32(key: Key, ip: *const InternPool) u32 { @@ -6157,6 +6158,7 @@ pub const MemoizedCall = struct { func: Index, args_len: u32, result: Index, + branch_count: u32, }; pub fn init(ip: *InternPool, gpa: Allocator, available_threads: usize) !void { @@ -6785,6 +6787,7 @@ pub fn indexToKey(ip: *const InternPool, index: Index) Key { .func = extra.data.func, .arg_values = @ptrCast(extra_list.view().items(.@"0")[extra.end..][0..extra.data.args_len]), .result = extra.data.result, + .branch_count = extra.data.branch_count, } }; }, }; @@ -7955,6 +7958,7 @@ pub fn get(ip: *InternPool, gpa: Allocator, tid: Zcu.PerThread.Id, key: Key) All .func = memoized_call.func, .args_len = @intCast(memoized_call.arg_values.len), .result = memoized_call.result, + .branch_count = memoized_call.branch_count, }), }); extra.appendSliceAssumeCapacity(.{@ptrCast(memoized_call.arg_values)}); diff --git a/src/Sema.zig b/src/Sema.zig index 9feda2c3f1..8e7e403a52 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -113,6 +113,11 @@ type_references: std.AutoArrayHashMapUnmanaged(InternPool.Index, void) = .{}, /// `AnalUnit` multiple times. dependencies: std.AutoArrayHashMapUnmanaged(InternPool.Dependee, void) = .{}, +/// Whether memoization of this call is permitted. Operations with side effects global +/// to the `Sema`, such as `@setEvalBranchQuota`, set this to `false`. It is observed +/// by `analyzeCall`. +allow_memoize: bool = true, + const MaybeComptimeAlloc = struct { /// The runtime index of the `alloc` instruction. runtime_index: Value.RuntimeIndex, @@ -5524,6 +5529,7 @@ fn zirSetEvalBranchQuota(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Compi .needed_comptime_reason = "eval branch quota must be comptime-known", })); sema.branch_quota = @max(sema.branch_quota, quota); + sema.allow_memoize = false; } fn zirStoreNode(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void { @@ -6416,6 +6422,7 @@ fn zirSetAlignStack(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.Inst } zcu.intern_pool.funcMaxStackAlignment(sema.func_index, alignment); + sema.allow_memoize = false; } fn zirSetCold(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstData) CompileError!void { @@ -6434,6 +6441,7 @@ fn zirSetCold(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstData) .cau => return, // does nothing outside a function }; ip.funcSetCold(func, is_cold); + sema.allow_memoize = false; } fn zirDisableInstrumentation(sema: *Sema) CompileError!void { @@ -6445,6 +6453,7 @@ fn zirDisableInstrumentation(sema: *Sema) CompileError!void { .cau => return, // does nothing outside a function }; ip.funcSetDisableInstrumentation(func); + sema.allow_memoize = false; } fn zirSetFloatMode(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstData) CompileError!void { @@ -7728,15 +7737,25 @@ fn analyzeCall( // This `res2` is here instead of directly breaking from `res` due to a stage1 // bug generating invalid LLVM IR. const res2: Air.Inst.Ref = res2: { - if (should_memoize and is_comptime_call) { - if (zcu.intern_pool.getIfExists(.{ .memoized_call = .{ - .func = module_fn_index, - .arg_values = memoized_arg_values, - .result = .none, - } })) |memoized_call_index| { - const memoized_call = zcu.intern_pool.indexToKey(memoized_call_index).memoized_call; - break :res2 Air.internedToRef(memoized_call.result); + memoize: { + if (!should_memoize) break :memoize; + if (!is_comptime_call) break :memoize; + const memoized_call_index = ip.getIfExists(.{ + .memoized_call = .{ + .func = module_fn_index, + .arg_values = memoized_arg_values, + .result = undefined, // ignored by hash+eql + .branch_count = undefined, // ignored by hash+eql + }, + }) orelse break :memoize; + const memoized_call = ip.indexToKey(memoized_call_index).memoized_call; + if (sema.branch_count + memoized_call.branch_count > sema.branch_quota) { + // Let the call play out se we get the correct source location for the + // "evaluation exceeded X backwards branches" error. + break :memoize; } + sema.branch_count += memoized_call.branch_count; + break :res2 Air.internedToRef(memoized_call.result); } new_fn_info.return_type = sema.fn_ret_ty.toIntern(); @@ -7774,6 +7793,17 @@ fn analyzeCall( child_block.error_return_trace_index = error_return_trace_index; } + // We temporarily set `allow_memoize` to `true` to track this comptime call. + // It is restored after this call finishes analysis, so that a caller may + // know whether an in-progress call (containing this call) may be memoized. + const old_allow_memoize = sema.allow_memoize; + defer sema.allow_memoize = old_allow_memoize and sema.allow_memoize; + sema.allow_memoize = true; + + // Store the current eval branch count so we can find out how many eval branches + // the comptime call caused. + const old_branch_count = sema.branch_count; + const result = result: { sema.analyzeFnBody(&child_block, fn_info.body) catch |err| switch (err) { error.ComptimeReturn => break :result inlining.comptime_result, @@ -7793,11 +7823,12 @@ fn analyzeCall( // a reference to `comptime_allocs` so is not stable across instances of `Sema`. // TODO: check whether any external comptime memory was mutated by the // comptime function call. If so, then do not memoize the call here. - if (should_memoize and !Value.fromInterned(result_interned).canMutateComptimeVarState(zcu)) { + if (should_memoize and sema.allow_memoize and !Value.fromInterned(result_interned).canMutateComptimeVarState(zcu)) { _ = try pt.intern(.{ .memoized_call = .{ .func = module_fn_index, .arg_values = memoized_arg_values, .result = result_transformed, + .branch_count = sema.branch_count - old_branch_count, } }); } diff --git a/src/register_manager.zig b/src/register_manager.zig index 751aed988a..7ca117be0c 100644 --- a/src/register_manager.zig +++ b/src/register_manager.zig @@ -93,6 +93,8 @@ pub fn RegisterManager( comptime set: []const Register, reg: Register, ) ?std.math.IntFittingRange(0, set.len - 1) { + @setEvalBranchQuota(3000); + const Id = @TypeOf(reg.id()); comptime var min_id: Id = std.math.maxInt(Id); comptime var max_id: Id = std.math.minInt(Id); From 95fbfde9daa165ca74a4d7267720fd32b42af45e Mon Sep 17 00:00:00 2001 From: mlugg Date: Mon, 19 Aug 2024 07:55:18 +0100 Subject: [PATCH 2/7] Sema: delete bootstrap compiler bug workaround --- src/Sema.zig | 204 +++++++++++++++++++++++++-------------------------- 1 file changed, 99 insertions(+), 105 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 8e7e403a52..b0f2623652 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -7734,123 +7734,117 @@ fn analyzeCall( } })); } - // This `res2` is here instead of directly breaking from `res` due to a stage1 - // bug generating invalid LLVM IR. - const res2: Air.Inst.Ref = res2: { - memoize: { - if (!should_memoize) break :memoize; - if (!is_comptime_call) break :memoize; - const memoized_call_index = ip.getIfExists(.{ - .memoized_call = .{ - .func = module_fn_index, - .arg_values = memoized_arg_values, - .result = undefined, // ignored by hash+eql - .branch_count = undefined, // ignored by hash+eql - }, - }) orelse break :memoize; - const memoized_call = ip.indexToKey(memoized_call_index).memoized_call; - if (sema.branch_count + memoized_call.branch_count > sema.branch_quota) { - // Let the call play out se we get the correct source location for the - // "evaluation exceeded X backwards branches" error. - break :memoize; - } - sema.branch_count += memoized_call.branch_count; - break :res2 Air.internedToRef(memoized_call.result); + memoize: { + if (!should_memoize) break :memoize; + if (!is_comptime_call) break :memoize; + const memoized_call_index = ip.getIfExists(.{ + .memoized_call = .{ + .func = module_fn_index, + .arg_values = memoized_arg_values, + .result = undefined, // ignored by hash+eql + .branch_count = undefined, // ignored by hash+eql + }, + }) orelse break :memoize; + const memoized_call = ip.indexToKey(memoized_call_index).memoized_call; + if (sema.branch_count + memoized_call.branch_count > sema.branch_quota) { + // Let the call play out se we get the correct source location for the + // "evaluation exceeded X backwards branches" error. + break :memoize; } + sema.branch_count += memoized_call.branch_count; + break :res Air.internedToRef(memoized_call.result); + } - new_fn_info.return_type = sema.fn_ret_ty.toIntern(); - if (!is_comptime_call and !block.is_typeof) { - const zir_tags = sema.code.instructions.items(.tag); - for (fn_info.param_body) |param| switch (zir_tags[@intFromEnum(param)]) { - .param, .param_comptime => { - const inst_data = sema.code.instructions.items(.data)[@intFromEnum(param)].pl_tok; - const extra = sema.code.extraData(Zir.Inst.Param, inst_data.payload_index); - const param_name = sema.code.nullTerminatedString(extra.data.name); - const inst = sema.inst_map.get(param).?; + new_fn_info.return_type = sema.fn_ret_ty.toIntern(); + if (!is_comptime_call and !block.is_typeof) { + const zir_tags = sema.code.instructions.items(.tag); + for (fn_info.param_body) |param| switch (zir_tags[@intFromEnum(param)]) { + .param, .param_comptime => { + const inst_data = sema.code.instructions.items(.data)[@intFromEnum(param)].pl_tok; + const extra = sema.code.extraData(Zir.Inst.Param, inst_data.payload_index); + const param_name = sema.code.nullTerminatedString(extra.data.name); + const inst = sema.inst_map.get(param).?; - try sema.addDbgVar(&child_block, inst, .dbg_arg_inline, param_name); - }, - .param_anytype, .param_anytype_comptime => { - const inst_data = sema.code.instructions.items(.data)[@intFromEnum(param)].str_tok; - const param_name = inst_data.get(sema.code); - const inst = sema.inst_map.get(param).?; + try sema.addDbgVar(&child_block, inst, .dbg_arg_inline, param_name); + }, + .param_anytype, .param_anytype_comptime => { + const inst_data = sema.code.instructions.items(.data)[@intFromEnum(param)].str_tok; + const param_name = inst_data.get(sema.code); + const inst = sema.inst_map.get(param).?; - try sema.addDbgVar(&child_block, inst, .dbg_arg_inline, param_name); - }, - else => continue, - }; - } - - if (is_comptime_call and ensure_result_used) { - try sema.ensureResultUsed(block, sema.fn_ret_ty, call_src); - } - - if (is_comptime_call or block.is_typeof) { - // Save the error trace as our first action in the function - // to match the behavior of runtime function calls. - const error_return_trace_index = try sema.analyzeSaveErrRetIndex(&child_block); - sema.error_return_trace_index_on_fn_entry = error_return_trace_index; - child_block.error_return_trace_index = error_return_trace_index; - } - - // We temporarily set `allow_memoize` to `true` to track this comptime call. - // It is restored after this call finishes analysis, so that a caller may - // know whether an in-progress call (containing this call) may be memoized. - const old_allow_memoize = sema.allow_memoize; - defer sema.allow_memoize = old_allow_memoize and sema.allow_memoize; - sema.allow_memoize = true; - - // Store the current eval branch count so we can find out how many eval branches - // the comptime call caused. - const old_branch_count = sema.branch_count; - - const result = result: { - sema.analyzeFnBody(&child_block, fn_info.body) catch |err| switch (err) { - error.ComptimeReturn => break :result inlining.comptime_result, - else => |e| return e, - }; - break :result try sema.resolveAnalyzedBlock(block, call_src, &child_block, merges, need_debug_scope); + try sema.addDbgVar(&child_block, inst, .dbg_arg_inline, param_name); + }, + else => continue, }; + } - if (is_comptime_call) { - const result_val = try sema.resolveConstValue(block, LazySrcLoc.unneeded, result, undefined); - const result_interned = result_val.toIntern(); + if (is_comptime_call and ensure_result_used) { + try sema.ensureResultUsed(block, sema.fn_ret_ty, call_src); + } - // Transform ad-hoc inferred error set types into concrete error sets. - const result_transformed = try sema.resolveAdHocInferredErrorSet(block, call_src, result_interned); + if (is_comptime_call or block.is_typeof) { + // Save the error trace as our first action in the function + // to match the behavior of runtime function calls. + const error_return_trace_index = try sema.analyzeSaveErrRetIndex(&child_block); + sema.error_return_trace_index_on_fn_entry = error_return_trace_index; + child_block.error_return_trace_index = error_return_trace_index; + } - // If the result can mutate comptime vars, we must not memoize it, as it contains - // a reference to `comptime_allocs` so is not stable across instances of `Sema`. - // TODO: check whether any external comptime memory was mutated by the - // comptime function call. If so, then do not memoize the call here. - if (should_memoize and sema.allow_memoize and !Value.fromInterned(result_interned).canMutateComptimeVarState(zcu)) { - _ = try pt.intern(.{ .memoized_call = .{ - .func = module_fn_index, - .arg_values = memoized_arg_values, - .result = result_transformed, - .branch_count = sema.branch_count - old_branch_count, - } }); - } + // We temporarily set `allow_memoize` to `true` to track this comptime call. + // It is restored after this call finishes analysis, so that a caller may + // know whether an in-progress call (containing this call) may be memoized. + const old_allow_memoize = sema.allow_memoize; + defer sema.allow_memoize = old_allow_memoize and sema.allow_memoize; + sema.allow_memoize = true; - break :res2 Air.internedToRef(result_transformed); - } + // Store the current eval branch count so we can find out how many eval branches + // the comptime call caused. + const old_branch_count = sema.branch_count; - if (try sema.resolveValue(result)) |result_val| { - const result_transformed = try sema.resolveAdHocInferredErrorSet(block, call_src, result_val.toIntern()); - break :res2 Air.internedToRef(result_transformed); - } - - const new_ty = try sema.resolveAdHocInferredErrorSetTy(block, call_src, sema.typeOf(result).toIntern()); - if (new_ty != .none) { - // TODO: mutate in place the previous instruction if possible - // rather than adding a bitcast instruction. - break :res2 try block.addBitCast(Type.fromInterned(new_ty), result); - } - - break :res2 result; + const result = result: { + sema.analyzeFnBody(&child_block, fn_info.body) catch |err| switch (err) { + error.ComptimeReturn => break :result inlining.comptime_result, + else => |e| return e, + }; + break :result try sema.resolveAnalyzedBlock(block, call_src, &child_block, merges, need_debug_scope); }; - break :res res2; + if (is_comptime_call) { + const result_val = try sema.resolveConstValue(block, LazySrcLoc.unneeded, result, undefined); + const result_interned = result_val.toIntern(); + + // Transform ad-hoc inferred error set types into concrete error sets. + const result_transformed = try sema.resolveAdHocInferredErrorSet(block, call_src, result_interned); + + // If the result can mutate comptime vars, we must not memoize it, as it contains + // a reference to `comptime_allocs` so is not stable across instances of `Sema`. + // TODO: check whether any external comptime memory was mutated by the + // comptime function call. If so, then do not memoize the call here. + if (should_memoize and sema.allow_memoize and !Value.fromInterned(result_interned).canMutateComptimeVarState(zcu)) { + _ = try pt.intern(.{ .memoized_call = .{ + .func = module_fn_index, + .arg_values = memoized_arg_values, + .result = result_transformed, + .branch_count = sema.branch_count - old_branch_count, + } }); + } + + break :res Air.internedToRef(result_transformed); + } + + if (try sema.resolveValue(result)) |result_val| { + const result_transformed = try sema.resolveAdHocInferredErrorSet(block, call_src, result_val.toIntern()); + break :res Air.internedToRef(result_transformed); + } + + const new_ty = try sema.resolveAdHocInferredErrorSetTy(block, call_src, sema.typeOf(result).toIntern()); + if (new_ty != .none) { + // TODO: mutate in place the previous instruction if possible + // rather than adding a bitcast instruction. + break :res try block.addBitCast(Type.fromInterned(new_ty), result); + } + + break :res result; } else res: { assert(!func_ty_info.is_generic); From 43fdd061f7430794c142283101f9c97a0829d446 Mon Sep 17 00:00:00 2001 From: mlugg Date: Mon, 19 Aug 2024 08:45:30 +0100 Subject: [PATCH 3/7] AstGen: incorporate extra information into source hashes * Indices of referenced captures * Line and column of `@src()` The second point aligns with a reversal of the "incremental compilation" section of https://github.com/ziglang/zig/issues/2029#issuecomment-645793168. This reversal was already done as #17688 (46a6d50), with the idea to push incremental compilation down the line. My proposal is to keep it as comptime-known, and simply re-analyze uses of `@src()` whenever their line/column change. I think this decision is reasonable for a few reasons: * The Zig compiler is quite fast. Occasionally re-analyzing a few functions containing `@src()` calls is perfectly acceptable and won't noticably impact update times. * The system described by Andrew in #2029 is currently vaporware. * The system described by Andrew in #2029 is non-trivial to implement. In particular, it requires some way to have backends update a single global in certain cases, without re-doing semantic analysis. There is no other part of incremental compilation which requires this. * Having `@src().line` be comptime-known is useful. For instance, #17688 was justified by broken Tracy integration because the source line couldn't be comptime-known. --- lib/std/zig/AstGen.zig | 137 ++++++++++++++++++++++++++++++++--------- 1 file changed, 107 insertions(+), 30 deletions(-) diff --git a/lib/std/zig/AstGen.zig b/lib/std/zig/AstGen.zig index d0f1c73fc8..3450e39cc6 100644 --- a/lib/std/zig/AstGen.zig +++ b/lib/std/zig/AstGen.zig @@ -66,6 +66,10 @@ scratch: std.ArrayListUnmanaged(u32) = .{}, /// of ZIR. /// The key is the ref operand; the value is the ref instruction. ref_table: std.AutoHashMapUnmanaged(Zir.Inst.Index, Zir.Inst.Index) = .{}, +/// Any information which should trigger invalidation of incremental compilation +/// data should be used to update this hasher. The result is the final source +/// hash of the enclosing declaration/etc. +src_hasher: std.zig.SrcHasher, const InnerError = error{ OutOfMemory, AnalysisFail }; @@ -137,6 +141,7 @@ pub fn generate(gpa: Allocator, tree: Ast) Allocator.Error!Zir { .arena = arena.allocator(), .tree = &tree, .nodes_need_rl = &nodes_need_rl, + .src_hasher = undefined, // `structDeclInner` for the root struct will set this }; defer astgen.deinit(gpa); @@ -1422,6 +1427,8 @@ fn fnProtoExpr( .is_extern = false, .is_noinline = false, .noalias_bits = noalias_bits, + + .proto_hash = undefined, // ignored for `body_gz == null` }); _ = try block_scope.addBreak(.break_inline, block_inst, result); @@ -4007,6 +4014,13 @@ fn fnDecl( const tree = astgen.tree; const token_tags = tree.tokens.items(.tag); + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); + // We don't add the full source yet, because we also need the prototype hash! + // The source slice is added towards the *end* of this function. + astgen.src_hasher.update(std.mem.asBytes(&astgen.source_column)); + // missing function name already happened in scanDecls() const fn_name_token = fn_proto.name_token orelse return error.AnalysisFail; @@ -4300,11 +4314,21 @@ fn fnDecl( .is_extern = true, .is_noinline = is_noinline, .noalias_bits = noalias_bits, + .proto_hash = undefined, // ignored for `body_gz == null` }); } else func: { // as a scope, fn_gz encloses ret_gz, but for instruction list, fn_gz stacks on ret_gz fn_gz.instructions_top = ret_gz.instructions.items.len; + // Construct the prototype hash. + // Leave `astgen.src_hasher` unmodified; this will be used for hashing + // the *whole* function declaration, including its body. + var proto_hasher = astgen.src_hasher; + const proto_node = tree.nodes.items(.data)[decl_node].lhs; + proto_hasher.update(tree.getNodeSource(proto_node)); + var proto_hash: std.zig.SrcHash = undefined; + proto_hasher.final(&proto_hash); + const prev_fn_block = astgen.fn_block; const prev_fn_ret_ty = astgen.fn_ret_ty; defer { @@ -4362,16 +4386,22 @@ fn fnDecl( .is_extern = false, .is_noinline = is_noinline, .noalias_bits = noalias_bits, + .proto_hash = proto_hash, }); }; + // *Now* we can incorporate the full source code into the hasher. + astgen.src_hasher.update(tree.getNodeSource(decl_node)); + // We add this at the end so that its instruction index marks the end range // of the top level declaration. addFunc already unstacked fn_gz and ret_gz. _ = try decl_gz.addBreak(.break_inline, decl_inst, func_inst); + var hash: std.zig.SrcHash = undefined; + astgen.src_hasher.final(&hash); try setDeclaration( decl_inst, - std.zig.hashSrc(tree.getNodeSource(decl_node)), + hash, .{ .named = fn_name_token }, decl_gz.decl_line, is_pub, @@ -4395,6 +4425,12 @@ fn globalVarDecl( const tree = astgen.tree; const token_tags = tree.tokens.items(.tag); + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); + astgen.src_hasher.update(tree.getNodeSource(node)); + astgen.src_hasher.update(std.mem.asBytes(&astgen.source_column)); + const is_mutable = token_tags[var_decl.ast.mut_token] == .keyword_var; // We do this at the beginning so that the instruction index marks the range start // of the top level declaration. @@ -4534,9 +4570,11 @@ fn globalVarDecl( _ = try addrspace_gz.addBreakWithSrcNode(.break_inline, decl_inst, addrspace_inst, node); } + var hash: std.zig.SrcHash = undefined; + astgen.src_hasher.final(&hash); try setDeclaration( decl_inst, - std.zig.hashSrc(tree.getNodeSource(node)), + hash, .{ .named = name_token }, block_scope.decl_line, is_pub, @@ -4562,6 +4600,12 @@ fn comptimeDecl( const node_datas = tree.nodes.items(.data); const body_node = node_datas[node].lhs; + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); + astgen.src_hasher.update(tree.getNodeSource(node)); + astgen.src_hasher.update(std.mem.asBytes(&astgen.source_column)); + // Up top so the ZIR instruction index marks the start range of this // top-level declaration. const decl_inst = try gz.makeDeclaration(node); @@ -4584,9 +4628,11 @@ fn comptimeDecl( _ = try decl_block.addBreak(.break_inline, decl_inst, .void_value); } + var hash: std.zig.SrcHash = undefined; + astgen.src_hasher.final(&hash); try setDeclaration( decl_inst, - std.zig.hashSrc(tree.getNodeSource(node)), + hash, .@"comptime", decl_block.decl_line, false, @@ -4607,6 +4653,12 @@ fn usingnamespaceDecl( const tree = astgen.tree; const node_datas = tree.nodes.items(.data); + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); + astgen.src_hasher.update(tree.getNodeSource(node)); + astgen.src_hasher.update(std.mem.asBytes(&astgen.source_column)); + const type_expr = node_datas[node].lhs; const is_pub = blk: { const main_tokens = tree.nodes.items(.main_token); @@ -4634,9 +4686,11 @@ fn usingnamespaceDecl( const namespace_inst = try typeExpr(&decl_block, &decl_block.base, type_expr); _ = try decl_block.addBreak(.break_inline, decl_inst, namespace_inst); + var hash: std.zig.SrcHash = undefined; + astgen.src_hasher.final(&hash); try setDeclaration( decl_inst, - std.zig.hashSrc(tree.getNodeSource(node)), + hash, .@"usingnamespace", decl_block.decl_line, is_pub, @@ -4658,6 +4712,12 @@ fn testDecl( const node_datas = tree.nodes.items(.data); const body_node = node_datas[node].rhs; + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); + astgen.src_hasher.update(tree.getNodeSource(node)); + astgen.src_hasher.update(std.mem.asBytes(&astgen.source_column)); + // Up top so the ZIR instruction index marks the start range of this // top-level declaration. const decl_inst = try gz.makeDeclaration(node); @@ -4819,13 +4879,18 @@ fn testDecl( .is_extern = false, .is_noinline = false, .noalias_bits = 0, + + // Tests don't have a prototype that needs hashing + .proto_hash = .{0} ** 16, }); _ = try decl_block.addBreak(.break_inline, decl_inst, func_inst); + var hash: std.zig.SrcHash = undefined; + astgen.src_hasher.final(&hash); try setDeclaration( decl_inst, - std.zig.hashSrc(tree.getNodeSource(node)), + hash, test_name, decl_block.decl_line, false, @@ -4983,10 +5048,12 @@ fn structDeclInner( } }; - var fields_hasher = std.zig.SrcHasher.init(.{}); - fields_hasher.update(@tagName(layout)); + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); + astgen.src_hasher.update(@tagName(layout)); if (backing_int_node != 0) { - fields_hasher.update(tree.getNodeSource(backing_int_node)); + astgen.src_hasher.update(tree.getNodeSource(backing_int_node)); } var sfba = std.heap.stackFallback(256, astgen.arena); @@ -5009,7 +5076,7 @@ fn structDeclInner( .field => |field| field, }; - fields_hasher.update(tree.getNodeSource(member_node)); + astgen.src_hasher.update(tree.getNodeSource(member_node)); if (!is_tuple) { const field_name = try astgen.identAsString(member.ast.main_token); @@ -5139,7 +5206,7 @@ fn structDeclInner( } var fields_hash: std.zig.SrcHash = undefined; - fields_hasher.final(&fields_hash); + astgen.src_hasher.final(&fields_hash); try gz.setStruct(decl_inst, .{ .src_node = node, @@ -5240,11 +5307,13 @@ fn unionDeclInner( var wip_members = try WipMembers.init(gpa, &astgen.scratch, decl_count, field_count, bits_per_field, max_field_size); defer wip_members.deinit(); - var fields_hasher = std.zig.SrcHasher.init(.{}); - fields_hasher.update(@tagName(layout)); - fields_hasher.update(&.{@intFromBool(auto_enum_tok != null)}); + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); + astgen.src_hasher.update(@tagName(layout)); + astgen.src_hasher.update(&.{@intFromBool(auto_enum_tok != null)}); if (arg_node != 0) { - fields_hasher.update(astgen.tree.getNodeSource(arg_node)); + astgen.src_hasher.update(astgen.tree.getNodeSource(arg_node)); } var sfba = std.heap.stackFallback(256, astgen.arena); @@ -5261,7 +5330,7 @@ fn unionDeclInner( .decl => continue, .field => |field| field, }; - fields_hasher.update(astgen.tree.getNodeSource(member_node)); + astgen.src_hasher.update(astgen.tree.getNodeSource(member_node)); member.convertToNonTupleLike(astgen.tree.nodes); if (member.ast.tuple_like) { return astgen.failTok(member.ast.main_token, "union field missing name", .{}); @@ -5364,7 +5433,7 @@ fn unionDeclInner( } var fields_hash: std.zig.SrcHash = undefined; - fields_hasher.final(&fields_hash); + astgen.src_hasher.final(&fields_hash); if (!block_scope.isEmpty()) { _ = try block_scope.addBreak(.break_inline, decl_inst, .void_value); @@ -5578,11 +5647,13 @@ fn containerDecl( var wip_members = try WipMembers.init(gpa, &astgen.scratch, @intCast(counts.decls), @intCast(counts.total_fields), bits_per_field, max_field_size); defer wip_members.deinit(); - var fields_hasher = std.zig.SrcHasher.init(.{}); + const old_hasher = astgen.src_hasher; + defer astgen.src_hasher = old_hasher; + astgen.src_hasher = std.zig.SrcHasher.init(.{}); if (container_decl.ast.arg != 0) { - fields_hasher.update(tree.getNodeSource(container_decl.ast.arg)); + astgen.src_hasher.update(tree.getNodeSource(container_decl.ast.arg)); } - fields_hasher.update(&.{@intFromBool(nonexhaustive)}); + astgen.src_hasher.update(&.{@intFromBool(nonexhaustive)}); var sfba = std.heap.stackFallback(256, astgen.arena); const sfba_allocator = sfba.get(); @@ -5596,7 +5667,7 @@ fn containerDecl( for (container_decl.ast.members) |member_node| { if (member_node == counts.nonexhaustive_node) continue; - fields_hasher.update(tree.getNodeSource(member_node)); + astgen.src_hasher.update(tree.getNodeSource(member_node)); var member = switch (try containerMember(&block_scope, &namespace.base, &wip_members, member_node)) { .decl => continue, .field => |field| field, @@ -5676,7 +5747,7 @@ fn containerDecl( } var fields_hash: std.zig.SrcHash = undefined; - fields_hasher.final(&fields_hash); + astgen.src_hasher.final(&fields_hash); const body = block_scope.instructionsSlice(); const body_len = astgen.countBodyLenAfterFixups(body); @@ -8478,6 +8549,10 @@ fn tunnelThroughClosure( }); } + // Incorporate the capture index into the source hash, so that changes in + // the order of captures cause suitable re-analysis. + astgen.src_hasher.update(std.mem.asBytes(&cur_capture_index)); + // Add an instruction to get the value from the closure. return gz.addExtendedNodeSmall(.closure_get, inner_ref_node, cur_capture_index); } @@ -9306,6 +9381,13 @@ fn builtinCall( }, .src => { + // Incorporate the source location into the source hash, so that + // changes in the source location of `@src()` result in re-analysis. + astgen.src_hasher.update( + std.mem.asBytes(&astgen.source_line) ++ + std.mem.asBytes(&astgen.source_column), + ); + const token_starts = tree.tokens.items(.start); const node_start = token_starts[tree.firstToken(node)]; astgen.advanceSourceCursor(node_start); @@ -12122,6 +12204,9 @@ const GenZir = struct { is_test: bool, is_extern: bool, is_noinline: bool, + + /// Ignored if `body_gz == null`. + proto_hash: std.zig.SrcHash, }) !Zir.Inst.Ref { assert(args.src_node != 0); const astgen = gz.astgen; @@ -12150,15 +12235,7 @@ const GenZir = struct { const columns = args.lbrace_column | (rbrace_column << 16); - const proto_hash: std.zig.SrcHash = switch (node_tags[fn_decl]) { - .fn_decl => sig_hash: { - const proto_node = node_datas[fn_decl].lhs; - break :sig_hash std.zig.hashSrc(tree.getNodeSource(proto_node)); - }, - .test_decl => std.zig.hashSrc(""), // tests don't have a prototype - else => unreachable, - }; - const proto_hash_arr: [4]u32 = @bitCast(proto_hash); + const proto_hash_arr: [4]u32 = @bitCast(args.proto_hash); src_locs_and_hash_buffer = .{ args.lbrace_line, From ceb76b2ba705f362dbd5437ec5804b670298b420 Mon Sep 17 00:00:00 2001 From: mlugg Date: Mon, 19 Aug 2024 09:06:54 +0100 Subject: [PATCH 4/7] test: add incremental compilation test for moving `@src()` call --- test/incremental/move_src | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 test/incremental/move_src diff --git a/test/incremental/move_src b/test/incremental/move_src new file mode 100644 index 0000000000..8313bdfac2 --- /dev/null +++ b/test/incremental/move_src @@ -0,0 +1,29 @@ +#target=x86_64-linux +#update=initial version +#file=main.zig +const std = @import("std"); +pub fn main() !void { + try std.io.getStdOut().writer().print("{d} {d}\n", .{ foo(), bar() }); +} +fn foo() u32 { + return @src().line; +} +fn bar() u32 { + return 123; +} +#expect_stdout="6 123\n" + +#update=add newline +#file=main.zig +const std = @import("std"); +pub fn main() !void { + try std.io.getStdOut().writer().print("{d} {d}\n", .{ foo(), bar() }); +} + +fn foo() u32 { + return @src().line; +} +fn bar() u32 { + return 123; +} +#expect_stdout="7 123\n" From 018262d537959701566f2dfece66a462c3bbc976 Mon Sep 17 00:00:00 2001 From: mlugg Date: Mon, 19 Aug 2024 10:26:51 +0100 Subject: [PATCH 5/7] std: update eval branch quotas after bdbc485 Also, update `std.math.Log2Int[Ceil]` to more efficient implementations that don't use up so much damn quota! --- lib/std/crypto/aes/soft.zig | 2 ++ lib/std/crypto/blake2.zig | 4 ++-- lib/std/enums.zig | 2 +- lib/std/hash/xxhash.zig | 2 +- lib/std/math.zig | 24 ++++++++---------------- lib/std/math/hypot.zig | 1 + lib/std/math/nextafter.zig | 2 +- lib/std/unicode.zig | 1 + 8 files changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/std/crypto/aes/soft.zig b/lib/std/crypto/aes/soft.zig index 9e7af0606d..fd0dfaf001 100644 --- a/lib/std/crypto/aes/soft.zig +++ b/lib/std/crypto/aes/soft.zig @@ -629,6 +629,8 @@ fn generateSbox(invert: bool) [256]u8 { // Generate lookup tables. fn generateTable(invert: bool) [4][256]u32 { + @setEvalBranchQuota(50000); + var table: [4][256]u32 = undefined; for (generateSbox(invert), 0..) |value, index| { diff --git a/lib/std/crypto/blake2.zig b/lib/std/crypto/blake2.zig index 255011de87..1a285080b5 100644 --- a/lib/std/crypto/blake2.zig +++ b/lib/std/crypto/blake2.zig @@ -786,7 +786,7 @@ test "blake2b384 streaming" { test "comptime blake2b384" { comptime { - @setEvalBranchQuota(10000); + @setEvalBranchQuota(20000); var block = [_]u8{0} ** Blake2b384.block_length; var out: [Blake2b384.digest_length]u8 = undefined; @@ -878,7 +878,7 @@ test "blake2b512 keyed" { test "comptime blake2b512" { comptime { - @setEvalBranchQuota(10000); + @setEvalBranchQuota(12000); var block = [_]u8{0} ** Blake2b512.block_length; var out: [Blake2b512.digest_length]u8 = undefined; diff --git a/lib/std/enums.zig b/lib/std/enums.zig index aea194683d..1cc7bde8d2 100644 --- a/lib/std/enums.zig +++ b/lib/std/enums.zig @@ -6,7 +6,7 @@ const testing = std.testing; const EnumField = std.builtin.Type.EnumField; /// Increment this value when adding APIs that add single backwards branches. -const eval_branch_quota_cushion = 5; +const eval_branch_quota_cushion = 10; /// Returns a struct with a field matching each unique named enum element. /// If the enum is extern and has multiple names for the same value, only diff --git a/lib/std/hash/xxhash.zig b/lib/std/hash/xxhash.zig index 88ec3ba372..1d7c8399fc 100644 --- a/lib/std/hash/xxhash.zig +++ b/lib/std/hash/xxhash.zig @@ -890,7 +890,7 @@ test "xxhash32 smhasher" { } }; try Test.do(); - @setEvalBranchQuota(75000); + @setEvalBranchQuota(85000); comptime try Test.do(); } diff --git a/lib/std/math.zig b/lib/std/math.zig index 0c00818a1e..f18739095c 100644 --- a/lib/std/math.zig +++ b/lib/std/math.zig @@ -749,31 +749,23 @@ test rotl { try testing.expect(rotl(@Vector(1, u32), @Vector(1, u32){1 << 31}, @as(isize, -1))[0] == @as(u32, 1) << 30); } -/// Returns an unsigned int type that can hold the number of bits in T -/// - 1. Suitable for 0-based bit indices of T. +/// Returns an unsigned int type that can hold the number of bits in T - 1. +/// Suitable for 0-based bit indices of T. pub fn Log2Int(comptime T: type) type { // comptime ceil log2 if (T == comptime_int) return comptime_int; - comptime var count = 0; - comptime var s = @typeInfo(T).Int.bits - 1; - inline while (s != 0) : (s >>= 1) { - count += 1; - } - - return std.meta.Int(.unsigned, count); + const bits: u16 = @typeInfo(T).Int.bits; + const log2_bits = 16 - @clz(bits - 1); + return std.meta.Int(.unsigned, log2_bits); } /// Returns an unsigned int type that can hold the number of bits in T. pub fn Log2IntCeil(comptime T: type) type { // comptime ceil log2 if (T == comptime_int) return comptime_int; - comptime var count = 0; - comptime var s = @typeInfo(T).Int.bits; - inline while (s != 0) : (s >>= 1) { - count += 1; - } - - return std.meta.Int(.unsigned, count); + const bits: u16 = @typeInfo(T).Int.bits; + const log2_bits = 16 - @clz(bits); + return std.meta.Int(.unsigned, log2_bits); } /// Returns the smallest integer type that can hold both from and to. diff --git a/lib/std/math/hypot.zig b/lib/std/math/hypot.zig index cc0dc17ab1..ddc9408aba 100644 --- a/lib/std/math/hypot.zig +++ b/lib/std/math/hypot.zig @@ -114,6 +114,7 @@ test "hypot.precise" { } test "hypot.special" { + @setEvalBranchQuota(2000); inline for (.{ f16, f32, f64, f128 }) |T| { try expect(math.isNan(hypot(nan(T), 0.0))); try expect(math.isNan(hypot(0.0, nan(T)))); diff --git a/lib/std/math/nextafter.zig b/lib/std/math/nextafter.zig index 717cbf4700..b88648229b 100644 --- a/lib/std/math/nextafter.zig +++ b/lib/std/math/nextafter.zig @@ -144,7 +144,7 @@ test "int" { } test "float" { - @setEvalBranchQuota(3000); + @setEvalBranchQuota(4000); // normal -> normal try expect(nextAfter(f16, 0x1.234p0, 2.0) == 0x1.238p0); diff --git a/lib/std/unicode.zig b/lib/std/unicode.zig index a8fa1454a5..4c6ec1294b 100644 --- a/lib/std/unicode.zig +++ b/lib/std/unicode.zig @@ -535,6 +535,7 @@ fn testUtf16CountCodepoints() !void { } test "utf16 count codepoints" { + @setEvalBranchQuota(2000); try testUtf16CountCodepoints(); try comptime testUtf16CountCodepoints(); } From 2fb78430dbff206af24ecd5e7be163e624fdbb6f Mon Sep 17 00:00:00 2001 From: mlugg Date: Tue, 20 Aug 2024 15:57:43 +0100 Subject: [PATCH 6/7] test: remove accidental hard tab --- test/incremental/delete_comptime_decls | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/incremental/delete_comptime_decls b/test/incremental/delete_comptime_decls index 424dc37ea8..d0da7110c3 100644 --- a/test/incremental/delete_comptime_decls +++ b/test/incremental/delete_comptime_decls @@ -31,7 +31,7 @@ pub fn main() void {} comptime { const x: [*c]u8 = null; var runtime_len: usize = undefined; - runtime_len = 0; + runtime_len = 0; const y = x[0..runtime_len]; _ = y; } From a99ad52b362d966f772f29ad14ae1714218bc033 Mon Sep 17 00:00:00 2001 From: mlugg Date: Tue, 20 Aug 2024 16:05:04 +0100 Subject: [PATCH 7/7] Sema: register correct dependencies for inline calls And add a corresponding test case. --- src/Sema.zig | 7 +++++++ test/incremental/modify_inline_fn | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 test/incremental/modify_inline_fn diff --git a/src/Sema.zig b/src/Sema.zig index b0f2623652..3752cefe3f 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -7598,6 +7598,9 @@ fn analyzeCall( const module_fn = zcu.funcInfo(module_fn_index); + // The call site definitely depends on the function's signature. + try sema.declareDependency(.{ .src_hash = module_fn.zir_body_inst }); + // This is not a function instance, so the function's `Nav` has a // `Cau` -- we don't need to check `generic_owner`. const fn_nav = ip.getNav(module_fn.owner_nav); @@ -7755,6 +7758,10 @@ fn analyzeCall( break :res Air.internedToRef(memoized_call.result); } + // Since we're doing an inline call, we depend on the source code of the whole + // function declaration. + try sema.declareDependency(.{ .src_hash = fn_cau.zir_index }); + new_fn_info.return_type = sema.fn_ret_ty.toIntern(); if (!is_comptime_call and !block.is_typeof) { const zir_tags = sema.code.instructions.items(.tag); diff --git a/test/incremental/modify_inline_fn b/test/incremental/modify_inline_fn new file mode 100644 index 0000000000..633e7a0728 --- /dev/null +++ b/test/incremental/modify_inline_fn @@ -0,0 +1,23 @@ +#target=x86_64-linux +#update=initial version +#file=main.zig +const std = @import("std"); +pub fn main() !void { + const str = getStr(); + try std.io.getStdOut().writeAll(str); +} +inline fn getStr() []const u8 { + return "foo\n"; +} +#expect_stdout="foo\n" +#update=change the string +#file=main.zig +const std = @import("std"); +pub fn main() !void { + const str = getStr(); + try std.io.getStdOut().writeAll(str); +} +inline fn getStr() []const u8 { + return "bar\n"; +} +#expect_stdout="bar\n"