From 7682ced08ea44d7535031d2c6075dc7ca28722a6 Mon Sep 17 00:00:00 2001 From: Robin Voetter Date: Sat, 2 Nov 2024 15:22:48 +0100 Subject: [PATCH] spirv: track global OpVariables properly in assembler Also cleans up the assembler a bit in general. --- src/codegen/spirv.zig | 51 ++++++++++++--------------------- src/codegen/spirv/Assembler.zig | 35 +++++++++++++++------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/codegen/spirv.zig b/src/codegen/spirv.zig index 2bb4f1d3e0..02e0d75d18 100644 --- a/src/codegen/spirv.zig +++ b/src/codegen/spirv.zig @@ -6529,6 +6529,13 @@ const NavGen = struct { return self.todo("implement inline asm with more than 1 output", .{}); } + var as = SpvAssembler{ + .gpa = self.gpa, + .spv = self.spv, + .func = &self.func, + }; + defer as.deinit(); + var output_extra_i = extra_i; for (outputs) |output| { if (output != .none) { @@ -6541,7 +6548,6 @@ const NavGen = struct { // TODO: Record output and use it somewhere. } - var input_extra_i = extra_i; for (inputs) |input| { const extra_bytes = std.mem.sliceAsBytes(self.air.extra[extra_i..]); const constraint = std.mem.sliceTo(extra_bytes, 0); @@ -6549,36 +6555,6 @@ const NavGen = struct { // This equation accounts for the fact that even if we have exactly 4 bytes // for the string, we still use the next u32 for the null terminator. extra_i += (constraint.len + name.len + (2 + 3)) / 4; - // TODO: Record input and use it somewhere. - _ = input; - } - - { - var clobber_i: u32 = 0; - while (clobber_i < clobbers_len) : (clobber_i += 1) { - const clobber = std.mem.sliceTo(std.mem.sliceAsBytes(self.air.extra[extra_i..]), 0); - extra_i += clobber.len / 4 + 1; - // TODO: Record clobber and use it somewhere. - } - } - - const asm_source = std.mem.sliceAsBytes(self.air.extra[extra_i..])[0..extra.data.source_len]; - - var as = SpvAssembler{ - .gpa = self.gpa, - .src = asm_source, - .spv = self.spv, - .func = &self.func, - }; - defer as.deinit(); - - for (inputs) |input| { - const extra_bytes = std.mem.sliceAsBytes(self.air.extra[input_extra_i..]); - const constraint = std.mem.sliceTo(extra_bytes, 0); - const name = std.mem.sliceTo(extra_bytes[constraint.len + 1 ..], 0); - // This equation accounts for the fact that even if we have exactly 4 bytes - // for the string, we still use the next u32 for the null terminator. - input_extra_i += (constraint.len + name.len + (2 + 3)) / 4; if (self.typeOf(input).zigTypeTag(zcu) == .type) { // This assembly input is a type instead of a value. @@ -6592,7 +6568,18 @@ const NavGen = struct { } } - as.assemble() catch |err| switch (err) { + { + var clobber_i: u32 = 0; + while (clobber_i < clobbers_len) : (clobber_i += 1) { + const clobber = std.mem.sliceTo(std.mem.sliceAsBytes(self.air.extra[extra_i..]), 0); + extra_i += clobber.len / 4 + 1; + // TODO: Record clobber and use it somewhere. + } + } + + const asm_source = std.mem.sliceAsBytes(self.air.extra[extra_i..])[0..extra.data.source_len]; + + as.assemble(asm_source) catch |err| switch (err) { error.AssembleFail => { // TODO: For now the compiler only supports a single error message per decl, // so to translate the possible multiple errors from the assembler, emit diff --git a/src/codegen/spirv/Assembler.zig b/src/codegen/spirv/Assembler.zig index 9e39f2ed09..8fcbad7189 100644 --- a/src/codegen/spirv/Assembler.zig +++ b/src/codegen/spirv/Assembler.zig @@ -151,7 +151,8 @@ gpa: Allocator, errors: std.ArrayListUnmanaged(ErrorMsg) = .empty, /// The source code that is being assembled. -src: []const u8, +/// This is set when calling `assemble()`. +src: []const u8 = undefined, /// The module that this assembly is associated to. /// Instructions like OpType*, OpDecorate, etc are emitted into this module. @@ -211,7 +212,10 @@ pub fn deinit(self: *Assembler) void { self.instruction_map.deinit(self.gpa); } -pub fn assemble(self: *Assembler) Error!void { +pub fn assemble(self: *Assembler, src: []const u8) Error!void { + self.src = src; + self.errors.clearRetainingCapacity(); + // Populate the opcode map if it isn't already if (self.instruction_map.count() == 0) { const instructions = spec.InstructionSet.core.instructions(); @@ -369,6 +373,7 @@ fn processTypeInstruction(self: *Assembler) !AsmValue { /// - Function-local instructions are emitted in `self.func`. fn processGenericInstruction(self: *Assembler) !?AsmValue { const operands = self.inst.operands.items; + var maybe_spv_decl_index: ?SpvModule.Decl.Index = null; const section = switch (self.inst.opcode.class()) { .ConstantCreation => &self.spv.sections.types_globals_constants, .Annotation => &self.spv.sections.annotations, @@ -378,12 +383,15 @@ fn processGenericInstruction(self: *Assembler) !?AsmValue { .OpExecutionMode, .OpExecutionModeId => &self.spv.sections.execution_modes, .OpVariable => switch (@as(spec.StorageClass, @enumFromInt(operands[2].value))) { .Function => &self.func.prologue, - .UniformConstant => &self.spv.sections.types_globals_constants, - else => { - // This is currently disabled because global variables are required to be - // emitted in the proper order, and this should be honored in inline assembly - // as well. - return self.todo("global variables", .{}); + // These don't need to be marked in the dependency system. + // Probably we should add them anyway, then filter out PushConstant globals. + .PushConstant => &self.spv.sections.types_globals_constants, + else => section: { + maybe_spv_decl_index = try self.spv.allocDecl(.global); + try self.func.decl_deps.put(self.spv.gpa, maybe_spv_decl_index.?, {}); + // TODO: In theory this can be non-empty if there is an initializer which depends on another global... + try self.spv.declareDeclDeps(maybe_spv_decl_index.?, &.{}); + break :section &self.spv.sections.types_globals_constants; }, }, // Default case - to be worked out further. @@ -409,7 +417,10 @@ fn processGenericInstruction(self: *Assembler) !?AsmValue { section.writeDoubleWord(dword); }, .result_id => { - maybe_result_id = self.spv.allocId(); + maybe_result_id = if (maybe_spv_decl_index) |spv_decl_index| + self.spv.declPtr(spv_decl_index).result_id + else + self.spv.allocId(); try section.ensureUnusedCapacity(self.spv.gpa, 1); section.writeOperand(IdResult, maybe_result_id.?); }, @@ -475,8 +486,8 @@ fn resolveRefId(self: *Assembler, ref: AsmValue.Ref) !IdRef { /// error message has been emitted into `self.errors`. fn parseInstruction(self: *Assembler) !void { self.inst.opcode = undefined; - self.inst.operands.shrinkRetainingCapacity(0); - self.inst.string_bytes.shrinkRetainingCapacity(0); + self.inst.operands.clearRetainingCapacity(); + self.inst.string_bytes.clearRetainingCapacity(); const lhs_result_tok = self.currentToken(); const maybe_lhs_result: ?AsmValue.Ref = if (self.eatToken(.result_id_assign)) blk: { @@ -848,6 +859,8 @@ fn tokenText(self: Assembler, tok: Token) []const u8 { /// Tokenize `self.src` and put the tokens in `self.tokens`. /// Any errors encountered are appended to `self.errors`. fn tokenize(self: *Assembler) !void { + self.tokens.clearRetainingCapacity(); + var offset: u32 = 0; while (true) { const tok = try self.nextToken(offset);