From e5fcc8192da248b890b5f3c8eb65ddec010ba0ef Mon Sep 17 00:00:00 2001
From: jonascloud <bahnes.jonas@icloud.com>
Date: Mon, 20 Oct 2025 07:18:44 +0200
Subject: [PATCH] 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
---
 src/Sema.zig   |  8 ++++----
 src/target.zig | 24 ++++++++++++++----------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/src/Sema.zig b/src/Sema.zig
index 3361d1a59c..69e35853cf 100644
--- a/src/Sema.zig
+++ b/src/Sema.zig
@@ -9156,7 +9156,7 @@ fn checkMergeAllowed(sema: *Sema, block: *Block, src: LazySrcLoc, peer_ty: Type)
     }
 
     const as = peer_ty.ptrAddressSpace(zcu);
-    if (!target_util.arePointersLogical(target, as)) {
+    if (!target_util.shouldBlockPointerOps(target, as)) {
         return;
     }
 
@@ -22669,7 +22669,7 @@ fn ptrCastFull(
 
     try sema.validateRuntimeValue(block, operand_src, operand);
 
-    const can_cast_to_int = !target_util.arePointersLogical(zcu.getTarget(), operand_ty.ptrAddressSpace(zcu));
+    const can_cast_to_int = !target_util.shouldBlockPointerOps(zcu.getTarget(), operand_ty.ptrAddressSpace(zcu));
     const need_null_check = can_cast_to_int and block.wantSafety() and operand_ty.ptrAllowsZero(zcu) and !dest_ty.ptrAllowsZero(zcu);
     const need_align_check = can_cast_to_int and block.wantSafety() and dest_align.compare(.gt, src_align);
 
@@ -23247,7 +23247,7 @@ fn checkLogicalPtrOperation(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Typ
     if (zcu.intern_pool.indexToKey(ty.toIntern()) == .ptr_type) {
         const target = zcu.getTarget();
         const as = ty.ptrAddressSpace(zcu);
-        if (target_util.arePointersLogical(target, as)) {
+        if (target_util.shouldBlockPointerOps(target, as)) {
             return sema.failWithOwnedErrorMsg(block, msg: {
                 const msg = try sema.errMsg(src, "illegal operation on logical pointer of type '{f}'", .{ty.fmt(pt)});
                 errdefer msg.destroy(sema.gpa);
@@ -28100,7 +28100,7 @@ fn validateRuntimeElemAccess(
     if (zcu.intern_pool.indexToKey(parent_ty.toIntern()) == .ptr_type) {
         const target = zcu.getTarget();
         const as = parent_ty.ptrAddressSpace(zcu);
-        if (target_util.arePointersLogical(target, as)) {
+        if (target_util.shouldBlockPointerOps(target, as)) {
             return sema.fail(block, elem_index_src, "cannot access element of logical pointer '{f}'", .{parent_ty.fmt(pt)});
         }
     }
diff --git a/src/target.zig b/src/target.zig
index 3a1b9f93af..134aa36c55 100644
--- a/src/target.zig
+++ b/src/target.zig
@@ -549,31 +549,35 @@ pub fn addrSpaceCastIsValid(
     }
 }
 
-/// Under SPIR-V with Vulkan, pointers are not 'real' (physical), but rather 'logical'. Effectively,
-/// this means that all such pointers have to be resolvable to a location at compile time, and places
-/// a number of restrictions on usage of such pointers. For example, a logical pointer may not be
-/// 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: *const std.Target, as: AddressSpace) bool {
+/// Returns whether pointer operations (arithmetic, indexing, etc.) should be blocked
+/// for the given address space on the target architecture.
+///
+/// Under SPIR-V with Vulkan
+/// (a) all physical pointers (.physical_storage_buffer, .global) always support pointer operations,
+/// (b) by default logical pointers (.constant, .input, .output, etc.) never support operations
+/// (c) some logical pointers (.storage_buffer, .shared) do support operations when
+///     the VariablePointers capability is enabled (which enables OpPtrAccessChain).
+pub fn shouldBlockPointerOps(target: *const std.Target, as: AddressSpace) bool {
     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 StorageBuffer or CrossWorkgroup,
         // so these are real pointers.
-        .global => false,
-        .physical_storage_buffer => false,
+        // Physical pointers always support operations
+        .global, .physical_storage_buffer => false,
+        // Logical pointers that support operations with VariablePointers capability
         .shared => !target.cpu.features.isEnabled(@intFromEnum(std.Target.spirv.Feature.variable_pointers)),
+        .storage_buffer => !target.cpu.features.isEnabled(@intFromEnum(std.Target.spirv.Feature.variable_pointers)),
+        // Logical pointers that never support operations
         .constant,
         .local,
         .input,
         .output,
         .uniform,
         .push_constant,
-        .storage_buffer,
         => true,
         else => unreachable,
     };