From 0901328f12e7ea3d05dc1d5b4a588e595c4bc0bc Mon Sep 17 00:00:00 2001 From: Ali Cheraghi Date: Wed, 7 May 2025 15:03:42 +0330 Subject: [PATCH] spirv: write error value in an storage buffer --- lib/std/Target.zig | 2 +- lib/std/Target/spirv.zig | 8 ++- lib/std/builtin.zig | 1 + src/codegen/spirv.zig | 113 +++++++++++++++++++---------------- src/codegen/spirv/Module.zig | 26 ++++---- src/target.zig | 21 ++++--- 6 files changed, 96 insertions(+), 75 deletions(-) diff --git a/lib/std/Target.zig b/lib/std/Target.zig index 9148fd5fdc..bf5a6369b5 100644 --- a/lib/std/Target.zig +++ b/lib/std/Target.zig @@ -2014,7 +2014,7 @@ pub const Cpu = struct { .global, .local, .shared => is_gpu, .constant => is_gpu and (context == null or context == .constant), .param => is_nvptx, - .input, .output, .uniform, .push_constant, .storage_buffer => is_spirv, + .input, .output, .uniform, .push_constant, .storage_buffer, .physical_storage_buffer => is_spirv, }; } }; diff --git a/lib/std/Target/spirv.zig b/lib/std/Target/spirv.zig index a2575b2fe8..90abacdd08 100644 --- a/lib/std/Target/spirv.zig +++ b/lib/std/Target/spirv.zig @@ -21,6 +21,7 @@ pub const Feature = enum { generic_pointer, vector16, shader, + variable_pointers, physical_storage_buffer, }; @@ -129,6 +130,11 @@ pub const all_features = blk: { .description = "Enable SPV_KHR_physical_storage_buffer extension and the PhysicalStorageBufferAddresses capability", .dependencies = featureSet(&[_]Feature{.v1_0}), }; + result[@intFromEnum(Feature.variable_pointers)] = .{ + .llvm_name = null, + .description = "Enable SPV_KHR_variable_pointers extension and the (VariablePointers, VariablePointersStorageBuffer) capabilities", + .dependencies = featureSet(&[_]Feature{.v1_0}), + }; const ti = @typeInfo(Feature); for (&result, 0..) |*elem, i| { elem.index = i; @@ -147,7 +153,7 @@ pub const cpu = struct { pub const vulkan_v1_2: CpuModel = .{ .name = "vulkan_v1_2", .llvm_name = null, - .features = featureSet(&[_]Feature{ .v1_5, .shader, .physical_storage_buffer }), + .features = featureSet(&[_]Feature{ .v1_5, .shader }), }; pub const opencl_v2: CpuModel = .{ diff --git a/lib/std/builtin.zig b/lib/std/builtin.zig index 852b94c324..1683cc500b 100644 --- a/lib/std/builtin.zig +++ b/lib/std/builtin.zig @@ -531,6 +531,7 @@ pub const AddressSpace = enum(u5) { uniform, push_constant, storage_buffer, + physical_storage_buffer, // AVR address spaces. flash, diff --git a/src/codegen/spirv.zig b/src/codegen/spirv.zig index 66fd40d8de..b5bba61016 100644 --- a/src/codegen/spirv.zig +++ b/src/codegen/spirv.zig @@ -169,12 +169,10 @@ pub const Object = struct { /// via the usual `intern_map` mechanism. ptr_types: PtrTypeMap = .{}, - /// For test declarations for Vulkan, we have to add a push constant with a pointer to a - /// buffer that we can use. We only need to generate this once, this holds the link information + /// For test declarations for Vulkan, we have to add a buffer. + /// We only need to generate this once, this holds the link information /// related to that. - error_push_constant: ?struct { - push_constant_ptr: SpvModule.Decl.Index, - } = null, + error_buffer: ?SpvModule.Decl.Index = null, pub fn init(gpa: Allocator, target: std.Target) Object { return .{ @@ -1739,15 +1737,34 @@ const NavGen = struct { fn spvStorageClass(self: *NavGen, as: std.builtin.AddressSpace) StorageClass { return switch (as) { .generic => if (self.spv.hasFeature(.generic_pointer)) .Generic else .Function, + .global => { + if (self.spv.hasFeature(.kernel)) return .CrossWorkgroup; + return .StorageBuffer; + }, + .push_constant => { + assert(self.spv.hasFeature(.shader)); + return .PushConstant; + }, + .output => { + assert(self.spv.hasFeature(.shader)); + return .Output; + }, + .uniform => { + assert(self.spv.hasFeature(.shader)); + return .Uniform; + }, + .storage_buffer => { + assert(self.spv.hasFeature(.shader)); + return .StorageBuffer; + }, + .physical_storage_buffer => { + assert(self.spv.hasFeature(.physical_storage_buffer)); + return .PhysicalStorageBuffer; + }, + .constant => .UniformConstant, .shared => .Workgroup, .local => .Function, - .global => if (self.spv.hasFeature(.shader)) .PhysicalStorageBuffer else .CrossWorkgroup, - .constant => .UniformConstant, - .push_constant => .PushConstant, .input => .Input, - .output => .Output, - .uniform => .Uniform, - .storage_buffer => .StorageBuffer, .gs, .fs, .ss, @@ -2713,38 +2730,32 @@ const NavGen = struct { }); }, .vulkan, .opengl => { - const ptr_ptr_anyerror_ty_id = self.spv.allocId(); - try self.spv.sections.types_globals_constants.emit(self.spv.gpa, .OpTypePointer, .{ - .id_result = ptr_ptr_anyerror_ty_id, - .storage_class = .PushConstant, - .type = ptr_anyerror_ty_id, - }); - - if (self.object.error_push_constant == null) { + if (self.object.error_buffer == null) { const spv_err_decl_index = try self.spv.allocDecl(.global); try self.spv.declareDeclDeps(spv_err_decl_index, &.{}); - const push_constant_struct_ty_id = self.spv.allocId(); - try self.spv.structType(push_constant_struct_ty_id, &.{ptr_anyerror_ty_id}, &.{"error_out_ptr"}); - try self.spv.decorate(push_constant_struct_ty_id, .Block); - try self.spv.decorateMember(push_constant_struct_ty_id, 0, .{ .Offset = .{ .byte_offset = 0 } }); + const buffer_struct_ty_id = self.spv.allocId(); + try self.spv.structType(buffer_struct_ty_id, &.{anyerror_ty_id}, &.{"error_out"}); + try self.spv.decorate(buffer_struct_ty_id, .Block); + try self.spv.decorateMember(buffer_struct_ty_id, 0, .{ .Offset = .{ .byte_offset = 0 } }); - const ptr_push_constant_struct_ty_id = self.spv.allocId(); + const ptr_buffer_struct_ty_id = self.spv.allocId(); try self.spv.sections.types_globals_constants.emit(self.spv.gpa, .OpTypePointer, .{ - .id_result = ptr_push_constant_struct_ty_id, - .storage_class = .PushConstant, - .type = push_constant_struct_ty_id, + .id_result = ptr_buffer_struct_ty_id, + .storage_class = self.spvStorageClass(.global), + .type = buffer_struct_ty_id, }); + const buffer_struct_id = self.spv.declPtr(spv_err_decl_index).result_id; try self.spv.sections.types_globals_constants.emit(self.spv.gpa, .OpVariable, .{ - .id_result_type = ptr_push_constant_struct_ty_id, - .id_result = self.spv.declPtr(spv_err_decl_index).result_id, - .storage_class = .PushConstant, + .id_result_type = ptr_buffer_struct_ty_id, + .id_result = buffer_struct_id, + .storage_class = self.spvStorageClass(.global), }); + try self.spv.decorate(buffer_struct_id, .{ .DescriptorSet = .{ .descriptor_set = 0 } }); + try self.spv.decorate(buffer_struct_id, .{ .Binding = .{ .binding_point = 0 } }); - self.object.error_push_constant = .{ - .push_constant_ptr = spv_err_decl_index, - }; + self.object.error_buffer = spv_err_decl_index; } try self.spv.sections.execution_modes.emit(self.spv.gpa, .OpExecutionMode, .{ @@ -2767,24 +2778,16 @@ const NavGen = struct { .id_result = self.spv.allocId(), }); - const spv_err_decl_index = self.object.error_push_constant.?.push_constant_ptr; - const push_constant_id = self.spv.declPtr(spv_err_decl_index).result_id; + const spv_err_decl_index = self.object.error_buffer.?; + const buffer_id = self.spv.declPtr(spv_err_decl_index).result_id; try decl_deps.append(spv_err_decl_index); const zero_id = try self.constInt(Type.u32, 0); - // We cannot use OpInBoundsAccessChain to dereference cross-storage class, so we have to use - // a load. - const tmp = self.spv.allocId(); try section.emit(self.spv.gpa, .OpInBoundsAccessChain, .{ - .id_result_type = ptr_ptr_anyerror_ty_id, - .id_result = tmp, - .base = push_constant_id, - .indexes = &.{zero_id}, - }); - try section.emit(self.spv.gpa, .OpLoad, .{ .id_result_type = ptr_anyerror_ty_id, .id_result = p_error_id, - .pointer = tmp, + .base = buffer_id, + .indexes = &.{zero_id}, }); }, else => unreachable, @@ -4562,7 +4565,8 @@ const NavGen = struct { const field_int_id = blk: { if (field_ty.isPtrAtRuntime(zcu)) { assert(self.spv.hasFeature(.addresses) or - (self.spv.hasFeature(.physical_storage_buffer) and field_ty.ptrAddressSpace(zcu) == .storage_buffer)); + (self.spv.hasFeature(.physical_storage_buffer) and + field_ty.ptrAddressSpace(zcu) == .storage_buffer)); break :blk try self.intFromPtr(field_id); } break :blk try self.bitCast(field_int_ty, field_ty, field_id); @@ -4969,13 +4973,16 @@ const NavGen = struct { if (payload_ty.hasRuntimeBitsIgnoreComptime(zcu)) { const pl_ptr_ty_id = try self.ptrType(layout.payload_ty, .Function, .indirect); const pl_ptr_id = try self.accessChain(pl_ptr_ty_id, tmp_id, &.{layout.payload_index}); - const active_pl_ptr_ty_id = try self.ptrType(payload_ty, .Function, .indirect); - const active_pl_ptr_id = self.spv.allocId(); - try self.func.body.emit(self.spv.gpa, .OpBitcast, .{ - .id_result_type = active_pl_ptr_ty_id, - .id_result = active_pl_ptr_id, - .operand = pl_ptr_id, - }); + const active_pl_ptr_id = if (!layout.payload_ty.eql(payload_ty, zcu)) blk: { + const active_pl_ptr_ty_id = try self.ptrType(payload_ty, .Function, .indirect); + const active_pl_ptr_id = self.spv.allocId(); + try self.func.body.emit(self.spv.gpa, .OpBitcast, .{ + .id_result_type = active_pl_ptr_ty_id, + .id_result = active_pl_ptr_id, + .operand = pl_ptr_id, + }); + break :blk active_pl_ptr_id; + } else pl_ptr_id; try self.store(payload_ty, active_pl_ptr_id, payload.?, .{}); } else { diff --git a/src/codegen/spirv/Module.zig b/src/codegen/spirv/Module.zig index 8f69276e1e..16c32c26d5 100644 --- a/src/codegen/spirv/Module.zig +++ b/src/codegen/spirv/Module.zig @@ -350,6 +350,11 @@ pub fn finalize(self: *Module, a: Allocator) ![]Word { .vector16 => try self.addCapability(.Vector16), // Shader .shader => try self.addCapability(.Shader), + .variable_pointers => { + try self.addExtension("SPV_KHR_variable_pointers"); + try self.addCapability(.VariablePointersStorageBuffer); + try self.addCapability(.VariablePointers); + }, .physical_storage_buffer => { try self.addExtension("SPV_KHR_physical_storage_buffer"); try self.addCapability(.PhysicalStorageBufferAddresses); @@ -364,20 +369,17 @@ pub fn finalize(self: *Module, a: Allocator) ![]Word { // Emit memory model const addressing_model: spec.AddressingModel = blk: { if (self.hasFeature(.shader)) { - break :blk switch (self.target.cpu.arch) { - .spirv32 => .Logical, // TODO: I don't think this will ever be implemented. - .spirv64 => .PhysicalStorageBuffer64, - else => unreachable, - }; - } else if (self.hasFeature(.kernel)) { - break :blk switch (self.target.cpu.arch) { - .spirv32 => .Physical32, - .spirv64 => .Physical64, - else => unreachable, - }; + assert(self.target.cpu.arch == .spirv64); + if (self.hasFeature(.physical_storage_buffer)) break :blk .PhysicalStorageBuffer64; + break :blk .Logical; } - unreachable; + assert(self.hasFeature(.kernel)); + break :blk switch (self.target.cpu.arch) { + .spirv32 => .Physical32, + .spirv64 => .Physical64, + else => unreachable, + }; }; try self.sections.memory_model.emit(self.gpa, .OpMemoryModel, .{ .addressing_model = addressing_model, diff --git a/src/target.zig b/src/target.zig index 4931b11eba..c5b2d97efb 100644 --- a/src/target.zig +++ b/src/target.zig @@ -501,21 +501,26 @@ pub fn addrSpaceCastIsValid( /// part of a merge (result of a branch) and may not be stored in memory at all. This function returns /// for a particular architecture and address space wether such pointers are logical. pub fn arePointersLogical(target: std.Target, as: AddressSpace) bool { - if (target.os.tag != .vulkan) { - return false; - } + if (target.os.tag != .vulkan) return false; return switch (as) { // TODO: Vulkan doesn't support pointers in the generic address space, we // should remove this case but this requires a change in defaultAddressSpace(). // For now, at least disable them from being regarded as physical. .generic => true, - // For now, all global pointers are represented using PhysicalStorageBuffer, so these are real - // pointers. + // For now, all global pointers are represented using StorageBuffer or CrossWorkgroup, + // so these are real pointers. .global => false, - // TODO: Allowed with VK_KHR_variable_pointers. - .shared => true, - .constant, .local, .input, .output, .uniform, .push_constant, .storage_buffer => true, + .physical_storage_buffer => false, + .shared => !target.cpu.features.isEnabled(@intFromEnum(std.Target.spirv.Feature.variable_pointers)), + .constant, + .local, + .input, + .output, + .uniform, + .push_constant, + .storage_buffer, + => true, else => unreachable, }; }