From 979b4102588fbb0d066060a5a6b6e10f584158ac Mon Sep 17 00:00:00 2001 From: Robin Voetter Date: Sat, 8 Apr 2023 19:05:48 +0200 Subject: [PATCH] spirv: Do not generate the Alignment attribute on pointers for now It seems that some implementations may have problems with these right now, like Intel and Rusticl. In theory, these attributes should be superficial on the pointer type, as alignment guarantees are also added via the alignment option of the OpLoad and OpStore instructions. Therefore, get rid of them for now. --- src/codegen/spirv.zig | 41 +++++++++++---------------------- src/codegen/spirv/Assembler.zig | 5 +--- src/codegen/spirv/Module.zig | 6 ++--- src/codegen/spirv/type.zig | 4 ++-- 4 files changed, 20 insertions(+), 36 deletions(-) diff --git a/src/codegen/spirv.zig b/src/codegen/spirv.zig index 63a5942ca9858ee97a12c687dd9a36dfe1d00c3b..58da5539ac9d46a3491a0adbdb2896a772e3f75d 100644 --- a/src/codegen/spirv.zig +++ b/src/codegen/spirv.zig @@ -825,6 +825,11 @@ pub const DeclGen = struct { // - Underaligned pointers. These need to be packed into the word array by using a mixture of // OpSpecConstantOp instructions such as OpConvertPtrToU, OpBitcast, OpShift, etc. + // TODO: Implement alignment here. + // This is hoing to require some hacks because there is no real way to + // set an OpVariable's alignment. + _ = alignment; + assert(storage_class != .Generic and storage_class != .Function); log.debug("lowerIndirectConstant: ty = {}, val = {}", .{ ty.fmt(self.module), val.fmtDebug() }); @@ -832,7 +837,7 @@ pub const DeclGen = struct { const section = &self.spv.globals.section; const ty_ref = try self.resolveType(ty, .indirect); - const ptr_ty_ref = try self.spv.ptrType(ty_ref, storage_class, alignment); + const ptr_ty_ref = try self.spv.ptrType(ty_ref, storage_class, 0); // const target = self.getTarget(); @@ -874,7 +879,7 @@ pub const DeclGen = struct { try icl.flush(); const constant_struct_ty_ref = try self.spv.simpleStructType(icl.members.items); - const ptr_constant_struct_ty_ref = try self.spv.ptrType(constant_struct_ty_ref, storage_class, alignment); + const ptr_constant_struct_ty_ref = try self.spv.ptrType(constant_struct_ty_ref, storage_class, 0); const constant_struct_id = self.spv.allocId(); try section.emit(self.spv.gpa, .OpSpecConstantComposite, .{ @@ -909,7 +914,7 @@ pub const DeclGen = struct { }); if (cast_to_generic) { - const generic_ptr_ty_ref = try self.spv.ptrType(ty_ref, .Generic, alignment); + const generic_ptr_ty_ref = try self.spv.ptrType(ty_ref, .Generic, 0); try section.emitSpecConstantOp(self.spv.gpa, .OpPtrCastToGeneric, .{ .id_result_type = self.typeId(generic_ptr_ty_ref), .id_result = result_id, @@ -1165,21 +1170,16 @@ pub const DeclGen = struct { .Pointer => { const ptr_info = ty.ptrInfo().data; - const ptr_payload = try self.spv.arena.create(SpvType.Payload.Pointer); - ptr_payload.* = .{ - .storage_class = spvStorageClass(ptr_info.@"addrspace"), - .child_type = try self.resolveType(ptr_info.pointee_type, .indirect), - // Note: only available in Kernels! - .alignment = ty.ptrAlignment(target) * 8, - }; - const ptr_ty_id = try self.spv.resolveType(SpvType.initPayload(&ptr_payload.base)); + const storage_class = spvStorageClass(ptr_info.@"addrspace"); + const child_ty_ref = try self.resolveType(ptr_info.pointee_type, .indirect); + const ptr_ty_ref = try self.spv.ptrType(child_ty_ref, storage_class, 0); if (ptr_info.size != .Slice) { - return ptr_ty_id; + return ptr_ty_ref; } return try self.spv.simpleStructType(&.{ - .{ .ty = ptr_ty_id, .name = "ptr" }, + .{ .ty = ptr_ty_ref, .name = "ptr" }, .{ .ty = try self.sizeType(), .name = "len" }, }); }, @@ -1356,7 +1356,7 @@ pub const DeclGen = struct { /// the name of an error in the text executor. fn generateTestEntryPoint(self: *DeclGen, name: []const u8, spv_test_decl_index: SpvModule.Decl.Index) !void { const anyerror_ty_ref = try self.resolveType(Type.anyerror, .direct); - const ptr_anyerror_ty_ref = try self.spv.ptrType(anyerror_ty_ref, .CrossWorkgroup, null); + const ptr_anyerror_ty_ref = try self.spv.ptrType(anyerror_ty_ref, .CrossWorkgroup, 0); const void_ty_ref = try self.resolveType(Type.void, .direct); const kernel_proto_ty_ref = blk: { @@ -1497,19 +1497,6 @@ pub const DeclGen = struct { final_storage_class == .Generic, decl.@"align", ); - - // if (storage_class == .Generic) { - // const section = &self.spv.globals.section; - // const ty_ref = try self.resolveType(decl.ty, .indirect); - // const ptr_ty_ref = try self.spv.ptrType(ty_ref, storage_class, decl.@"align"); - // // TODO: Can we eliminate this cast? - // // TODO: Const-wash pointer? - // try section.emitSpecConstantOp(self.spv.gpa, .OpPtrCastToGeneric, .{ - // .id_result_type = self.typeId(ptr_ty_ref), - // .id_result = global_result_id, - // .pointer = casted_result_id, - // }); - // } } } diff --git a/src/codegen/spirv/Assembler.zig b/src/codegen/spirv/Assembler.zig index fcfdbf56ee454a8906cde29b45740b20b2bdc79a..eebf43866dd41e8e34ae15d5d688d5acfd541919 100644 --- a/src/codegen/spirv/Assembler.zig +++ b/src/codegen/spirv/Assembler.zig @@ -388,10 +388,7 @@ fn processTypeInstruction(self: *Assembler) !AsmValue { payload.* = .{ .storage_class = @intToEnum(spec.StorageClass, operands[1].value), .child_type = try self.resolveTypeRef(operands[2].ref_id), - // TODO: Fetch these values from decorations. - .array_stride = 0, - .alignment = null, - .max_byte_offset = null, + // TODO: Fetch decorations }; break :blk SpvType.initPayload(&payload.base); }, diff --git a/src/codegen/spirv/Module.zig b/src/codegen/spirv/Module.zig index f7d392e9e3b8ea98c1375ff66110a86c98174fc6..7ae6cb0c6ab77a79bd1383b4895339140b993de5 100644 --- a/src/codegen/spirv/Module.zig +++ b/src/codegen/spirv/Module.zig @@ -573,8 +573,8 @@ pub fn emitType(self: *Module, ty: Type) error{OutOfMemory}!IdResultType { if (info.array_stride != 0) { try self.decorate(ref_id, .{ .ArrayStride = .{ .array_stride = info.array_stride } }); } - if (info.alignment) |alignment| { - try self.decorate(ref_id, .{ .Alignment = .{ .alignment = alignment } }); + if (info.alignment != 0) { + try self.decorate(ref_id, .{ .Alignment = .{ .alignment = info.alignment } }); } if (info.max_byte_offset) |max_byte_offset| { try self.decorate(ref_id, .{ .MaxByteOffset = .{ .max_byte_offset = max_byte_offset } }); @@ -753,7 +753,7 @@ pub fn ptrType( self: *Module, child: Type.Ref, storage_class: spec.StorageClass, - alignment: ?u32, + alignment: u32, ) !Type.Ref { const ptr_payload = try self.arena.create(Type.Payload.Pointer); ptr_payload.* = .{ diff --git a/src/codegen/spirv/type.zig b/src/codegen/spirv/type.zig index 118563c6fa3cff23d4b416ac7f0abc47e9fc084e..2e1661c14e1f6d53f9eb11c94500d174d1bbb631 100644 --- a/src/codegen/spirv/type.zig +++ b/src/codegen/spirv/type.zig @@ -547,8 +547,8 @@ pub const Type = extern union { /// This is valid for pointers to elements of an array. /// If zero, no stride is present. array_stride: u32 = 0, - /// Type has the 'Alignment' decoration. - alignment: ?u32, + /// If nonzero, type has the 'Alignment' decoration. + alignment: u32 = 0, /// Type has the 'MaxByteOffset' decoration. max_byte_offset: ?u32 = null, }; -- 2.54.0