Skip to content

Commit e5fcc81

Browse files
jonascloudalexrp
authored andcommitted
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 #25638
1 parent 93d54cb commit e5fcc81

File tree

2 files changed

+18
-14
lines changed

2 files changed

+18
-14
lines changed

src/Sema.zig

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9156,7 +9156,7 @@ fn checkMergeAllowed(sema: *Sema, block: *Block, src: LazySrcLoc, peer_ty: Type)
91569156
}
91579157

91589158
const as = peer_ty.ptrAddressSpace(zcu);
9159-
if (!target_util.arePointersLogical(target, as)) {
9159+
if (!target_util.shouldBlockPointerOps(target, as)) {
91609160
return;
91619161
}
91629162

@@ -22669,7 +22669,7 @@ fn ptrCastFull(
2266922669

2267022670
try sema.validateRuntimeValue(block, operand_src, operand);
2267122671

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));
2267322673
const need_null_check = can_cast_to_int and block.wantSafety() and operand_ty.ptrAllowsZero(zcu) and !dest_ty.ptrAllowsZero(zcu);
2267422674
const need_align_check = can_cast_to_int and block.wantSafety() and dest_align.compare(.gt, src_align);
2267522675

@@ -23247,7 +23247,7 @@ fn checkLogicalPtrOperation(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Typ
2324723247
if (zcu.intern_pool.indexToKey(ty.toIntern()) == .ptr_type) {
2324823248
const target = zcu.getTarget();
2324923249
const as = ty.ptrAddressSpace(zcu);
23250-
if (target_util.arePointersLogical(target, as)) {
23250+
if (target_util.shouldBlockPointerOps(target, as)) {
2325123251
return sema.failWithOwnedErrorMsg(block, msg: {
2325223252
const msg = try sema.errMsg(src, "illegal operation on logical pointer of type '{f}'", .{ty.fmt(pt)});
2325323253
errdefer msg.destroy(sema.gpa);
@@ -28100,7 +28100,7 @@ fn validateRuntimeElemAccess(
2810028100
if (zcu.intern_pool.indexToKey(parent_ty.toIntern()) == .ptr_type) {
2810128101
const target = zcu.getTarget();
2810228102
const as = parent_ty.ptrAddressSpace(zcu);
28103-
if (target_util.arePointersLogical(target, as)) {
28103+
if (target_util.shouldBlockPointerOps(target, as)) {
2810428104
return sema.fail(block, elem_index_src, "cannot access element of logical pointer '{f}'", .{parent_ty.fmt(pt)});
2810528105
}
2810628106
}

src/target.zig

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -549,31 +549,35 @@ pub fn addrSpaceCastIsValid(
549549
}
550550
}
551551

552-
/// Under SPIR-V with Vulkan, pointers are not 'real' (physical), but rather 'logical'. Effectively,
553-
/// this means that all such pointers have to be resolvable to a location at compile time, and places
554-
/// a number of restrictions on usage of such pointers. For example, a logical pointer may not be
555-
/// part of a merge (result of a branch) and may not be stored in memory at all. This function returns
556-
/// for a particular architecture and address space wether such pointers are logical.
557-
pub fn arePointersLogical(target: *const std.Target, as: AddressSpace) bool {
552+
/// Returns whether pointer operations (arithmetic, indexing, etc.) should be blocked
553+
/// for the given address space on the target architecture.
554+
///
555+
/// Under SPIR-V with Vulkan
556+
/// (a) all physical pointers (.physical_storage_buffer, .global) always support pointer operations,
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 {
558561
if (target.os.tag != .vulkan) return false;
559562

560563
return switch (as) {
561564
// TODO: Vulkan doesn't support pointers in the generic address space, we
562565
// should remove this case but this requires a change in defaultAddressSpace().
563-
// For now, at least disable them from being regarded as physical.
564566
.generic => true,
565567
// For now, all global pointers are represented using StorageBuffer or CrossWorkgroup,
566568
// so these are real pointers.
567-
.global => false,
568-
.physical_storage_buffer => false,
569+
// Physical pointers always support operations
570+
.global, .physical_storage_buffer => false,
571+
// Logical pointers that support operations with VariablePointers capability
569572
.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
570575
.constant,
571576
.local,
572577
.input,
573578
.output,
574579
.uniform,
575580
.push_constant,
576-
.storage_buffer,
577581
=> true,
578582
else => unreachable,
579583
};

0 commit comments

Comments
 (0)