From 97527a42c5a09564fa94d491a0c1d48c0fe66e28 Mon Sep 17 00:00:00 2001 From: Kleshzz Date: Wed, 22 Jul 2026 17:43:11 +0300 Subject: [PATCH] spirv: improve Assembler source locations, error formatting --- src/codegen/spirv/Assembler.zig | 35 ++++++++++++++++----------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/codegen/spirv/Assembler.zig b/src/codegen/spirv/Assembler.zig index aa80b4257ecc0756a463b58a550155ba56f22d04..5da442650eb089ecb89e68e9f9ffc21551adcf21 100644 --- a/src/codegen/spirv/Assembler.zig +++ b/src/codegen/spirv/Assembler.zig @@ -24,6 +24,7 @@ inst: struct { opcode: Opcode = undefined, operands: std.ArrayList(Operand) = .empty, string_bytes: std.ArrayList(u8) = .empty, + inst_offset: u32 = 0, fn result(ass: @This()) ?AsmValue.Ref { for (ass.operands.items[0..@min(ass.operands.items.len, 2)]) |op| { @@ -35,7 +36,7 @@ inst: struct { return null; } } = .{}, -value_map: std.array_hash_map.String(AsmValue) = .{}, +value_map: std.array_hash_map.String(AsmValue) = .empty, inst_map: std.array_hash_map.String(void) = .empty, const Operand = union(enum) { @@ -82,7 +83,7 @@ pub fn assemble(ass: *Assembler, src: []const u8) Error!void { if (ass.inst_map.count() == 0) { const instructions = spec.InstructionSet.core.instructions(); try ass.inst_map.ensureUnusedCapacity(gpa, @intCast(instructions.len)); - for (spec.InstructionSet.core.instructions(), 0..) |inst, i| { + for (instructions, 0..) |inst, i| { const entry = try ass.inst_map.getOrPut(gpa, inst.name); assert(entry.index == i); } @@ -114,12 +115,13 @@ fn addError(ass: *Assembler, offset: u32, comptime fmt: []const u8, args: anytyp } fn fail(ass: *Assembler, offset: u32, comptime fmt: []const u8, args: anytype) Error { + @branchHint(.cold); try ass.addError(offset, fmt, args); return error.AssembleFail; } fn todo(ass: *Assembler, comptime fmt: []const u8, args: anytype) Error { - return ass.fail(0, "todo: " ++ fmt, args); + return ass.fail(ass.inst.inst_offset, "todo: " ++ fmt, args); } const AsmValue = union(enum) { @@ -209,9 +211,8 @@ fn processInstruction(ass: *Assembler) !void { switch (ass.value_map.values()[result_ref]) { .just_declared => ass.value_map.values()[result_ref] = result, else => { - // TODO: Improve source location. const name = ass.value_map.keys()[result_ref]; - return ass.fail(0, "duplicate definition of %{s}", .{name}); + return ass.fail(ass.inst.inst_offset, "duplicate definition of %{s}", .{name}); }, } } @@ -229,12 +230,11 @@ fn processTypeInstruction(ass: *Assembler) !AsmValue { 0 => .unsigned, 1 => .signed, else => { - // TODO: Improve source location. - return ass.fail(0, "{} is not a valid signedness (expected 0 or 1)", .{operands[2].literal32}); + return ass.fail(ass.inst.inst_offset, "{} is not a valid signedness (expected 0 or 1)", .{operands[2].literal32}); }, }; const width = std.math.cast(u16, operands[1].literal32) orelse { - return ass.fail(0, "int type of {} bits is too large", .{operands[1].literal32}); + return ass.fail(ass.inst.inst_offset, "int type of {} bits is too large", .{operands[1].literal32}); }; break :blk try cg.intType(signedness, width); }, @@ -243,7 +243,7 @@ fn processTypeInstruction(ass: *Assembler) !AsmValue { switch (bits) { 16, 32, 64 => {}, else => { - return ass.fail(0, "{} is not a valid bit count for floats (expected 16, 32 or 64)", .{bits}); + return ass.fail(ass.inst.inst_offset, "{} is not a valid bit count for floats (expected 16, 32 or 64)", .{bits}); }, } break :blk try cg.floatType(@intCast(bits)); @@ -445,11 +445,11 @@ fn processSpecConstVector(ass: *Assembler) !?AsmValue { const gpa = cg.gpa; const ty_ref = switch (ass.inst.operands.items[0]) { .ref_id => |i| i, - else => return ass.fail(0, "missing result type", .{}), + else => return ass.fail(ass.inst.inst_offset, "missing result type", .{}), }; const composite_ty_id = switch (try ass.resolveRef(ty_ref)) { .ty => |id| id, - else => return ass.fail(0, "%ty must be a type", .{}), + else => return ass.fail(ass.inst.inst_offset, "%ty must be a type", .{}), }; const globals = &cg.sections.globals; @@ -483,7 +483,7 @@ fn processSpecConstVector(ass: *Assembler) !?AsmValue { } const spec_id_word = std.math.cast(u32, spec_id_base + i) orelse { - return ass.fail(0, "SpecId {} does not fit in 32 bits", .{spec_id_base + i}); + return ass.fail(ass.inst.inst_offset, "SpecId {} does not fit in 32 bits", .{spec_id_base + i}); }; try annotations.emitRaw(gpa, .OpDecorate, 3); annotations.writeOperand(Id, elem_id); @@ -505,8 +505,7 @@ fn resolveMaybeForwardRef(ass: *Assembler, ref: AsmValue.Ref) !AsmValue { switch (value) { .just_declared => { const name = ass.value_map.keys()[ref]; - // TODO: Improve source location. - return ass.fail(0, "ass-referential parameter %{s}", .{name}); + return ass.fail(ass.inst.inst_offset, "self-referential parameter %{s}", .{name}); }, else => return value, } @@ -518,8 +517,7 @@ fn resolveRef(ass: *Assembler, ref: AsmValue.Ref) !AsmValue { .just_declared => unreachable, .unresolved_forward_reference => { const name = ass.value_map.keys()[ref]; - // TODO: Improve source location. - return ass.fail(0, "reference to undeclared result-id %{s}", .{name}); + return ass.fail(ass.inst.inst_offset, "reference to undeclared result-id %{s}", .{name}); }, else => return value, } @@ -536,6 +534,7 @@ fn parseInstruction(ass: *Assembler) !void { ass.inst.opcode = undefined; ass.inst.operands.clearRetainingCapacity(); ass.inst.string_bytes.clearRetainingCapacity(); + ass.inst.inst_offset = ass.currentToken().start; const lhs_result_tok = ass.currentToken(); const maybe_lhs_result: ?AsmValue.Ref = if (ass.eatToken(.result_id_assign)) blk: { @@ -589,8 +588,8 @@ fn parseInstruction(ass: *Assembler) !void { .required => if (ass.isAtInstructionBoundary()) { return ass.fail( ass.currentToken().start, - "missing required operand", // TODO: Operand name? - .{}, + "missing required operand '{s}'", + .{@tagName(operand.kind)}, ); } else { try ass.parseOperand(operand.kind); -- 2.54.0