| author |  jonascloud <bahnes.jonas@icloud.com> 2025-10-20 07:18:44+02:00 |
| committer |  Alex Rønne Petersen <alex@alexrp.com> 2025-10-23 03:21:15+02:00 |
| log | e5fcc8192da248b890b5f3c8eb65ddec010ba0ef |
| tree | 31a390aa0bb59bab314a07ec789c5c288f093643 |
| parent | 93d54cb8664b38224f983c588f0a203b3bf7d84b |
spir-v: Fix .storage_buffer pointer indexing
Renames arePointersLogical to shouldBlockPointerOps for clarity
adds capability check to allow pointer ops on .storage_buffer when
variable_pointers capability is enabled.
Fixes #256382 files changed, 18 insertions(+), 14 deletions(-)
src/Sema.zig+4-4
| ... | @@ -9156,7 +9156,7 @@ fn checkMergeAllowed(sema: *Sema, block: *Block, src: LazySrcLoc, peer_ty: Type) | ... | @@ -9156,7 +9156,7 @@ fn checkMergeAllowed(sema: *Sema, block: *Block, src: LazySrcLoc, peer_ty: Type) |
| 9156 | } | 9156 | } |
| 9157 | 9157 | ||
| 9158 | const as = peer_ty.ptrAddressSpace(zcu); | 9158 | const as = peer_ty.ptrAddressSpace(zcu); |
| 9159 | if (!target_util.arePointersLogical(target, as)) { | 9159 | if (!target_util.shouldBlockPointerOps(target, as)) { |
| 9160 | return; | 9160 | return; |
| 9161 | } | 9161 | } |
| 9162 | 9162 | ||
| ... | @@ -22669,7 +22669,7 @@ fn ptrCastFull( | ... | @@ -22669,7 +22669,7 @@ fn ptrCastFull( |
| 22669 | 22669 | ||
| 22670 | try sema.validateRuntimeValue(block, operand_src, operand); | 22670 | try sema.validateRuntimeValue(block, operand_src, operand); |
| 22671 | 22671 | ||
| 22672 | const can_cast_to_int = !target_util.arePointersLogical(zcu.getTarget(), operand_ty.ptrAddressSpace(zcu)); | 22672 | const can_cast_to_int = !target_util.shouldBlockPointerOps(zcu.getTarget(), operand_ty.ptrAddressSpace(zcu)); |
| 22673 | const need_null_check = can_cast_to_int and block.wantSafety() and operand_ty.ptrAllowsZero(zcu) and !dest_ty.ptrAllowsZero(zcu); | 22673 | const need_null_check = can_cast_to_int and block.wantSafety() and operand_ty.ptrAllowsZero(zcu) and !dest_ty.ptrAllowsZero(zcu); |
| 22674 | const need_align_check = can_cast_to_int and block.wantSafety() and dest_align.compare(.gt, src_align); | 22674 | const need_align_check = can_cast_to_int and block.wantSafety() and dest_align.compare(.gt, src_align); |
| 22675 | 22675 | ||
| ... | @@ -23247,7 +23247,7 @@ fn checkLogicalPtrOperation(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Typ | ... | @@ -23247,7 +23247,7 @@ fn checkLogicalPtrOperation(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Typ |
| 23247 | if (zcu.intern_pool.indexToKey(ty.toIntern()) == .ptr_type) { | 23247 | if (zcu.intern_pool.indexToKey(ty.toIntern()) == .ptr_type) { |
| 23248 | const target = zcu.getTarget(); | 23248 | const target = zcu.getTarget(); |
| 23249 | const as = ty.ptrAddressSpace(zcu); | 23249 | const as = ty.ptrAddressSpace(zcu); |
| 23250 | if (target_util.arePointersLogical(target, as)) { | 23250 | if (target_util.shouldBlockPointerOps(target, as)) { |
| 23251 | return sema.failWithOwnedErrorMsg(block, msg: { | 23251 | return sema.failWithOwnedErrorMsg(block, msg: { |
| 23252 | const msg = try sema.errMsg(src, "illegal operation on logical pointer of type '{f}'", .{ty.fmt(pt)}); | 23252 | const msg = try sema.errMsg(src, "illegal operation on logical pointer of type '{f}'", .{ty.fmt(pt)}); |
| 23253 | errdefer msg.destroy(sema.gpa); | 23253 | errdefer msg.destroy(sema.gpa); |
| ... | @@ -28100,7 +28100,7 @@ fn validateRuntimeElemAccess( | ... | @@ -28100,7 +28100,7 @@ fn validateRuntimeElemAccess( |
| 28100 | if (zcu.intern_pool.indexToKey(parent_ty.toIntern()) == .ptr_type) { | 28100 | if (zcu.intern_pool.indexToKey(parent_ty.toIntern()) == .ptr_type) { |
| 28101 | const target = zcu.getTarget(); | 28101 | const target = zcu.getTarget(); |
| 28102 | const as = parent_ty.ptrAddressSpace(zcu); | 28102 | const as = parent_ty.ptrAddressSpace(zcu); |
| 28103 | if (target_util.arePointersLogical(target, as)) { | 28103 | if (target_util.shouldBlockPointerOps(target, as)) { |
| 28104 | return sema.fail(block, elem_index_src, "cannot access element of logical pointer '{f}'", .{parent_ty.fmt(pt)}); | 28104 | return sema.fail(block, elem_index_src, "cannot access element of logical pointer '{f}'", .{parent_ty.fmt(pt)}); |
| 28105 | } | 28105 | } |
| 28106 | } | 28106 | } |
src/target.zig+14-10
| ... | @@ -549,31 +549,35 @@ pub fn addrSpaceCastIsValid( | ... | @@ -549,31 +549,35 @@ pub fn addrSpaceCastIsValid( |
| 549 | } | 549 | } |
| 550 | } | 550 | } |
| 551 | 551 | ||
| 552 | /// Under SPIR-V with Vulkan, pointers are not 'real' (physical), but rather 'logical'. Effectively, | 552 | /// Returns whether pointer operations (arithmetic, indexing, etc.) should be blocked |
| 553 | /// this means that all such pointers have to be resolvable to a location at compile time, and places | 553 | /// for the given address space on the target architecture. |
| 554 | /// a number of restrictions on usage of such pointers. For example, a logical pointer may not be | 554 | /// |
| 555 | /// part of a merge (result of a branch) and may not be stored in memory at all. This function returns | 555 | /// Under SPIR-V with Vulkan |
| 556 | /// for a particular architecture and address space wether such pointers are logical. | 556 | /// (a) all physical pointers (.physical_storage_buffer, .global) always support pointer operations, |
| 557 | pub fn arePointersLogical(target: *const std.Target, as: AddressSpace) bool { | 557 | /// (b) by default logical pointers (.constant, .input, .output, etc.) never support operations |
| 558 | /// (c) some logical pointers (.storage_buffer, .shared) do support operations when | ||
| 559 | /// the VariablePointers capability is enabled (which enables OpPtrAccessChain). | ||
| 560 | pub fn shouldBlockPointerOps(target: *const std.Target, as: AddressSpace) bool { | ||
| 558 | if (target.os.tag != .vulkan) return false; | 561 | if (target.os.tag != .vulkan) return false; |
| 559 | 562 | ||
| 560 | return switch (as) { | 563 | return switch (as) { |
| 561 | // TODO: Vulkan doesn't support pointers in the generic address space, we | 564 | // TODO: Vulkan doesn't support pointers in the generic address space, we |
| 562 | // should remove this case but this requires a change in defaultAddressSpace(). | 565 | // should remove this case but this requires a change in defaultAddressSpace(). |
| 563 | // For now, at least disable them from being regarded as physical. | ||
| 564 | .generic => true, | 566 | .generic => true, |
| 565 | // For now, all global pointers are represented using StorageBuffer or CrossWorkgroup, | 567 | // For now, all global pointers are represented using StorageBuffer or CrossWorkgroup, |
| 566 | // so these are real pointers. | 568 | // so these are real pointers. |
| 567 | .global => false, | 569 | // Physical pointers always support operations |
| 568 | .physical_storage_buffer => false, | 570 | .global, .physical_storage_buffer => false, |
| 571 | // Logical pointers that support operations with VariablePointers capability | ||
| 569 | .shared => !target.cpu.features.isEnabled(@intFromEnum(std.Target.spirv.Feature.variable_pointers)), | 572 | .shared => !target.cpu.features.isEnabled(@intFromEnum(std.Target.spirv.Feature.variable_pointers)), |
| 573 | .storage_buffer => !target.cpu.features.isEnabled(@intFromEnum(std.Target.spirv.Feature.variable_pointers)), | ||
| 574 | // Logical pointers that never support operations | ||
| 570 | .constant, | 575 | .constant, |
| 571 | .local, | 576 | .local, |
| 572 | .input, | 577 | .input, |
| 573 | .output, | 578 | .output, |
| 574 | .uniform, | 579 | .uniform, |
| 575 | .push_constant, | 580 | .push_constant, |
| 576 | .storage_buffer, | ||
| 577 | => true, | 581 | => true, |
| 578 | else => unreachable, | 582 | else => unreachable, |
| 579 | }; | 583 | }; |