Commit 7682ced08e

Robin Voetter <robin@voetter.nl>
2024-11-02 15:22:48
spirv: track global OpVariables properly in assembler
Also cleans up the assembler a bit in general.
1 parent 4fbc100
Changed files (2)
src
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);
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