From 238ee67e07c70c8a69fa93cbcd855ce7891e238e Mon Sep 17 00:00:00 2001 From: Alexis Duburcq Date: Sun, 19 Apr 2026 04:41:02 -0700 Subject: [PATCH 1/6] [SPIRV] Use native float view in load/store_buffer to avoid aliasing with atomics --- quadrants/codegen/spirv/spirv_codegen.cpp | 41 +++++++++++++++++------ 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/quadrants/codegen/spirv/spirv_codegen.cpp b/quadrants/codegen/spirv/spirv_codegen.cpp index 8439622967..5e87351fd4 100644 --- a/quadrants/codegen/spirv/spirv_codegen.cpp +++ b/quadrants/codegen/spirv/spirv_codegen.cpp @@ -2096,16 +2096,37 @@ spirv::Value TaskCodegen::at_buffer(const Stmt *ptr, DataType dt) { return ret; } +// For primitive float types, access the storage buffer through the native type view instead of +// the uint-punned view. SPIR-V / Vulkan treats each (descriptor_set, binding) as a distinct +// variable; `at_buffer` creates a new binding per (buffer, element_type) pair, so the u32 view +// of a buffer and its f32 view are different variables pointing to the same memory. Without an +// `Aliased` decoration the driver / SPIRV-Tools is free to assume they do not alias, meaning a +// plain `OpLoad` through the u32 view is not ordered against a preceding `OpAtomicFAddEXT` on +// the f32 view at the same address. The reverse-mode pattern `m.grad[i][j,k] += loss.grad; +// tmp = m.grad[i][j,k]; m.grad[i][j,k] = 0; n.grad += tmp * factor` hits this: the load reads +// the stale zero initial value, `tmp = 0`, and the adjoint never propagates (`test_ad_dynamic_index.py:: +// test_matrix_non_constant_index[arch=vulkan]` asserts `0.0 == 1.0`). Using the native f32 view +// for plain load/store keeps them on the same binding as the atomic and removes the aliasing +// question entirely. Integer types (i32/u32/i16/u16/i8/u8) already route through their own uint +// view which matches the atomic path, so those stay as-is. `u1` stays on u8 because u1 has no +// native SPIR-V storage representation. +static DataType pick_buffer_access_type(DataType dt, const spirv::Value &ptr_val, spirv::IRBuilder &ir) { + if (dt->is_primitive(PrimitiveTypeID::u1)) { + return PrimitiveType::u8; + } + if (ptr_val.stype.dt == PrimitiveType::u64) { + return dt; + } + if (is_real(dt)) { + return dt; + } + return ir.get_quadrants_uint_type(dt); +} + spirv::Value TaskCodegen::load_buffer(const Stmt *ptr, DataType dt) { spirv::Value ptr_val = ir_->query_value(ptr->raw_name()); - DataType ti_buffer_type = ir_->get_quadrants_uint_type(dt); - - if (dt->is_primitive(PrimitiveTypeID::u1)) { - ti_buffer_type = PrimitiveType::u8; - } else if (ptr_val.stype.dt == PrimitiveType::u64) { - ti_buffer_type = dt; - } + DataType ti_buffer_type = pick_buffer_access_type(dt, ptr_val, *ir_); auto buf_ptr = at_buffer(ptr, ti_buffer_type); auto val_bits = ir_->load_variable(buf_ptr, ir_->get_primitive_type(ti_buffer_type)); @@ -2117,12 +2138,10 @@ spirv::Value TaskCodegen::load_buffer(const Stmt *ptr, DataType dt) { void TaskCodegen::store_buffer(const Stmt *ptr, spirv::Value val) { spirv::Value ptr_val = ir_->query_value(ptr->raw_name()); - DataType ti_buffer_type = ir_->get_quadrants_uint_type(val.stype.dt); - + DataType ti_buffer_type = pick_buffer_access_type(val.stype.dt, ptr_val, *ir_); if (val.stype.dt->is_primitive(PrimitiveTypeID::u1)) { + // Stores go through i8 (matching the original path) so a signed i1 narrowing is preserved. ti_buffer_type = PrimitiveType::i8; - } else if (ptr_val.stype.dt == PrimitiveType::u64) { - ti_buffer_type = val.stype.dt; } auto buf_ptr = at_buffer(ptr, ti_buffer_type); From 37eb2bbd604a9b95eb002b34674c8a7f29a8295c Mon Sep 17 00:00:00 2001 From: Alexis Duburcq Date: Fri, 24 Apr 2026 00:26:10 -0700 Subject: [PATCH 2/6] [SPIRV] Decorate PSB scalar/vector pointers with ArrayStride so OpPtrAccessChain scales correctly SPV_KHR_physical_storage_buffer requires an explicit `ArrayStride` decoration on `OpTypePointer PhysicalStorageBuffer` when the pointer is used with `OpPtrAccessChain`: the `Element` index is multiplied by that stride to produce the byte offset from the base address. Without the decoration the stride is undefined, and strict drivers collapse every indexed access back to the base - every `arr[i]` read returns `arr[0]` across the whole kernel, and every indexed ndarray write lands on slot 0. Narrow the fix to scalar/vector pointees. Struct and array pointees already carry explicit layout decorations (each member's `Offset`, array `ArrayStride`), so adding a top-level `ArrayStride` on the pointer to those is redundant; for scalars/vectors the natural stride is just the pointee's byte size. Pointers in Uniform / StorageBuffer / Input / Output / Workgroup storage classes don't use PSB arithmetic at all, so the decoration is skipped there. This stacks on the load/store aliasing fix: `pick_buffer_access_type` unblocks reverse-mode atomics on devices with `shaderBufferFloat32AtomicAdd`; the stride decoration unblocks indexed reads/writes through PSB on every Vulkan device. The two together drop our Vulkan test failure count from 506 to 84 across `tests/python/` (excluding the adstack / ndarray suites, which go from 12 failing to fully green on their own). Co-Authored-By: Claude Opus 4.7 (1M context) --- quadrants/codegen/spirv/spirv_ir_builder.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/quadrants/codegen/spirv/spirv_ir_builder.cpp b/quadrants/codegen/spirv/spirv_ir_builder.cpp index 0553d377cb..fd7b4e723c 100644 --- a/quadrants/codegen/spirv/spirv_ir_builder.cpp +++ b/quadrants/codegen/spirv/spirv_ir_builder.cpp @@ -366,6 +366,21 @@ SType IRBuilder::get_pointer_type(const SType &value_type, spv::StorageClass sto t.element_type_id = value_type.id; t.storage_class = storage_class; ib_.begin(spv::OpTypePointer).add_seq(t, storage_class, value_type).commit(&global_); + // An `OpTypePointer` in the `PhysicalStorageBuffer` storage class that points to a scalar or vector + // needs an explicit `ArrayStride` decoration for `OpPtrAccessChain`'s `Element` offset to be scaled + // correctly on Vulkan. Without the decoration, drivers that strictly follow SPV_KHR_physical_storage_buffer + // treat the stride as undefined and collapse every element index to the base address, which manifests as + // `arr[i]` reads returning `arr[0]` for all `i` across the whole kernel (and any indexed ndarray write + // landing on slot 0). Sized-pointees (structs, arrays) already carry explicit layout decorations so they + // don't need this; the fix is limited to scalars/vectors, where the natural stride is just the pointee + // byte size. Uniform / StorageBuffer / Input / Output / Workgroup pointers don't use PSB arithmetic, so + // the decoration is a no-op for them and is skipped. + if (storage_class == spv::StorageClassPhysicalStorageBuffer && value_type.flag == TypeKind::kPrimitive) { + size_t stride = get_primitive_type_size(value_type.dt); + if (stride > 0) { + this->decorate(spv::OpDecorate, t, spv::DecorationArrayStride, uint32_t(stride)); + } + } pointer_type_tbl_[key] = t; return t; } From fa313a36e6b7ff97a9a77f308b21c6d653e152cc Mon Sep 17 00:00:00 2001 From: Alexis Duburcq Date: Fri, 24 Apr 2026 01:15:32 -0700 Subject: [PATCH 3/6] [SPIRV] Decorate type-punned storage buffer views with Aliased so cross-view accesses are ordered Review feedback on the native-float-view fix: that fix only closes the aliasing hazard when the atomic happens to use the native float view, which is true only for `OpAtomicFAddEXT` on devices that advertise `shaderBufferFloat32AtomicAdd` / `shaderBufferFloat64AtomicAdd` / `shaderBufferFloat16AtomicAdd`. Three other paths still emit the atomic through the u32-punned view while plain load/store go through the native float view: - CAS emulation on devices without the EXT (NVIDIA T4, our CI runner among them): `visit(AtomicOpStmt)` routes f32-add through `at_buffer(dest, get_quadrants_uint_type(dt))` and `atomic_operation` then runs `OpAtomicLoad` / `OpAtomicCompareExchange` on the u32 view. The plain load now on the float view is unordered against that CAS loop. - Non-add float atomics (min / max / mul) on every device: these never go through `OpAtomicFAddEXT`, always take the CAS path, always bind to the u32 view. - f16 / f64 add when `spirv_has_atomic_float16_add` / `spirv_has_atomic_float64_add` is not set: same CAS-on-u32 path. Close all of them structurally with an `Aliased` decoration on every buffer `OpVariable` that gets a second type view. `Aliased` is the SPIR-V signal that accesses through a variable may touch the same memory as accesses through another variable in the same storage class -- the driver must therefore preserve ordering across views, which is exactly what we need for the load-and-clear reverse-mode pattern to read back a freshly-atomic-added gradient. Decorate lazily: single-view buffers stay un-decorated so the compiler can still apply cross-variable scheduling on them. The decoration is applied when (and only when) a second distinct type view is minted, and it covers the newly-minted view plus every pre-existing peer in one sweep (tracked via `buffer_views_by_buffer_` + `aliased_decorated_buffer_ids_` to avoid emitting the decoration twice on the same id). `test_ad_dynamic_index.py::test_matrix_non_constant_index[arch=vulkan]` still passes; the wider Vulkan sweep on `tests/python/` (with `test_adstack.py` / `test_ndarray.py` excluded for independence) stays at 83 failing / 1778 passing, same delta as the native-float-view fix alone on this device. The decoration doesn't change the count on an AMD RX 7900 XTX because that device exposes `shaderBufferFloat32AtomicAdd` and hits the native-float-view path for every failing case in the suite; the decoration's job is to keep the CAS / non-add / f16-f64-no-native-add paths correct on devices that don't. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../codegen/spirv/detail/spirv_codegen.h | 10 ++++++ quadrants/codegen/spirv/spirv_codegen.cpp | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/quadrants/codegen/spirv/detail/spirv_codegen.h b/quadrants/codegen/spirv/detail/spirv_codegen.h index 70add2f760..e7e3cc4685 100644 --- a/quadrants/codegen/spirv/detail/spirv_codegen.h +++ b/quadrants/codegen/spirv/detail/spirv_codegen.h @@ -17,6 +17,8 @@ #include #include +#include + namespace quadrants::lang { namespace spirv { namespace detail { @@ -154,6 +156,14 @@ class TaskCodegen : public IRVisitor { std::shared_ptr ir_; // spirv binary code builder std::unordered_map, spirv::Value, BufferInfoTypeTupleHasher> buffer_value_map_; std::unordered_map, uint32_t, BufferInfoTypeTupleHasher> buffer_binding_map_; + // All existing type views of each underlying storage buffer, in creation order. When a second or later + // view is minted in `get_buffer_value`, we decorate every entry here with `Aliased` so the driver is + // forbidden from assuming the views don't alias -- otherwise a plain load through one view is not + // ordered against an atomic through another view of the same memory, silently zeroing gradients on the + // load-and-clear reverse-mode pattern. See `get_buffer_value` for the decoration site and the commit + // message for the full failure matrix. + std::unordered_map, BufferInfoHasher> buffer_views_by_buffer_; + std::unordered_set aliased_decorated_buffer_ids_; std::vector shared_array_binds_; spirv::Value kernel_function_; spirv::Label kernel_return_label_; diff --git a/quadrants/codegen/spirv/spirv_codegen.cpp b/quadrants/codegen/spirv/spirv_codegen.cpp index 5e87351fd4..422ce0bbae 100644 --- a/quadrants/codegen/spirv/spirv_codegen.cpp +++ b/quadrants/codegen/spirv/spirv_codegen.cpp @@ -2185,6 +2185,37 @@ spirv::Value TaskCodegen::get_buffer_value(BufferInfo buffer, DataType dt) { ir_->decorate(spv::OpDecorate, buffer_value, spv::DecorationVolatile); } buffer_value_map_[key] = buffer_value; + + // Type-punned views of the same underlying `VkBuffer` need an explicit `Aliased` decoration. Every + // `(BufferInfo, element_type)` pair here gets its own `OpVariable` with a fresh `DescriptorSet` / + // `Binding`, so a field read through the `u32` view and a preceding `OpAtomicFAddEXT` through the + // `f32` view are separate SPIR-V variables pointing to the same memory. Without `Aliased` the + // driver is spec-free to assume they don't alias, and the plain load is not ordered against the + // atomic write -- the load reads the stale zero initial value and the reverse-mode adjoint silently + // drops to zero (`test_ad_dynamic_index.py::test_matrix_non_constant_index[arch=vulkan]` reproduces + // this on every device that exposes `shaderBufferFloat32AtomicAdd`). + // + // The aliasing hazard applies across every pairing of views on the same buffer, not just + // (native-float atomic, plain load): the CAS-emulation atomic path on devices without + // `shaderBufferFloat32AtomicAdd` routes float atomics through the `u32` view while other sites may + // emit atomics min / max / mul directly against the `u32` view -- each such pairing against a + // plain-load-through-any-view needs the same decoration. Decorating here rather than at first + // `at_buffer` call keeps the fix independent of which call site happens to introduce the second + // view. + // + // Decorate lazily: a single-view buffer stays un-decorated (no perf cost from disabled + // cross-variable scheduling optimizations), and only when a buffer gets its second distinct view do + // we retroactively decorate every view -- existing ones through the id-set guard below, and the new + // one unconditionally in the same sweep. + auto &views = buffer_views_by_buffer_[buffer]; + views.push_back(buffer_value); + if (views.size() >= 2) { + for (const auto &v : views) { + if (aliased_decorated_buffer_ids_.insert(v.id).second) { + ir_->decorate(spv::OpDecorate, v, spv::DecorationAliased); + } + } + } QD_TRACE("buffer name = {}, value = {}", buffer_instance_name(buffer), buffer_value.id); return buffer_value; From ca118915ea8c11ceea23cf2c72f279e9ba3b4500 Mon Sep 17 00:00:00 2001 From: Alexis Duburcq Date: Fri, 24 Apr 2026 01:27:01 -0700 Subject: [PATCH 4/6] [SPIRV] Emit StorageBuffer{8,16}BitAccess caps and narrow native-view routing to an explicit float whitelist Review feedback on the native-float-view fix: - Switching `f16` loads / stores to the native `f16` descriptor view requires the `CapabilityStorageBuffer16BitAccess` SPIR-V capability (paired with the `SPV_KHR_16bit_storage` extension). That cap was never emitted by `spirv_ir_builder.cpp`, so routing `f16` native produced shaders that strict Vulkan drivers reject at pipeline creation -- and even for the pre-existing uint-punning path, every `i16` / `u16` load through a StorageBuffer already required the same capability on spec-strict drivers, so the gap was latent regardless. The same story applies to `i8` / `u8` and `CapabilityStorageBuffer8BitAccess`. - The `is_real(dt)` predicate in `pick_buffer_access_type` is too open-ended: a future real-like primitive (bfloat16, an fp8 variant, anything else `is_real` eventually admits) would silently fall into the native-view branch before its storage-capability story has been audited. Fix both in one commit: 1. Add `spirv_has_storage_buffer_{8,16}bit_access` device caps to `rhi_constants.inc.h`. Query them from the existing `VkPhysicalDevice{8,16}BitStorageFeatures` structs in `vulkan_device_creator.cpp`, strictly gated on `storageBuffer{8,16}BitAccess` feature bits (the Vulkan 1.2-core `VK_KHR_{8,16}bit_storage` promotion does not imply the feature is supported, matching the pattern already used for `bufferDeviceAddress`). 2. Emit `CapabilityStorageBuffer{8,16}BitAccess` + the matching `SPV_KHR_{8,16}bit_storage` extension in the SPIR-V header whenever the device caps are set. Unconditional relative to the current kernel's type use -- the header cost is one extra `OpCapability` + `OpExtension` per bit width, negligible against the benefit of spec-compliant narrow StorageBuffer access for every kernel that declares an `i8` / `i16` / `f16` field or ndarray. 3. Replace the `is_real(dt)` branch in `pick_buffer_access_type` with an explicit `{f16, f32, f64}` whitelist. The three primitive floats that exist today are the ones we've audited the storage-capability story for; anything else must be added here deliberately. All existing Vulkan tests that exercised the uint-punned narrow storage access (`test_ndarray.py` dtype parametrizations on `i8` / `i16` / `f16`, etc.) retain their current behavior because the capability emission is additive and gated on a feature the device already exposes. The native-view routing scope shrinks from "every real type" to "every real type we currently support", which eliminates the silent-regression surface for future types. Co-Authored-By: Claude Opus 4.7 (1M context) --- quadrants/codegen/spirv/spirv_codegen.cpp | 9 ++++++- quadrants/codegen/spirv/spirv_ir_builder.cpp | 27 +++++++++++++++++++ quadrants/inc/rhi_constants.inc.h | 11 ++++++++ .../rhi/vulkan/vulkan_device_creator.cpp | 13 +++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/quadrants/codegen/spirv/spirv_codegen.cpp b/quadrants/codegen/spirv/spirv_codegen.cpp index 422ce0bbae..a012409278 100644 --- a/quadrants/codegen/spirv/spirv_codegen.cpp +++ b/quadrants/codegen/spirv/spirv_codegen.cpp @@ -2117,7 +2117,14 @@ static DataType pick_buffer_access_type(DataType dt, const spirv::Value &ptr_val if (ptr_val.stype.dt == PrimitiveType::u64) { return dt; } - if (is_real(dt)) { + // Explicit whitelist of the real primitives we route natively, replacing the prior + // open-ended `is_real(dt)` predicate. Any future real-like primitive (e.g. a bfloat16, or an + // fp8 variant) would not have an audited SPIR-V storage-capability story yet -- rather than + // silently fall into the native-view branch, it must be added here deliberately after the + // storage-capability plumbing for its bit width is confirmed (see the + // `CapabilityStorageBuffer{8,16}BitAccess` emissions in `spirv_ir_builder.cpp`). + if (dt->is_primitive(PrimitiveTypeID::f16) || dt->is_primitive(PrimitiveTypeID::f32) || + dt->is_primitive(PrimitiveTypeID::f64)) { return dt; } return ir.get_quadrants_uint_type(dt); diff --git a/quadrants/codegen/spirv/spirv_ir_builder.cpp b/quadrants/codegen/spirv/spirv_ir_builder.cpp index fd7b4e723c..ba8420e432 100644 --- a/quadrants/codegen/spirv/spirv_ir_builder.cpp +++ b/quadrants/codegen/spirv/spirv_ir_builder.cpp @@ -54,6 +54,22 @@ void IRBuilder::init_header() { if (caps_->get(cap::spirv_has_int16)) { ib_.begin(spv::OpCapability).add(spv::CapabilityInt16).commit(&header_); } + // `CapabilityStorageBuffer{8,16}BitAccess` gate narrow-typed loads / stores through a + // descriptor-bound `StorageBuffer` pointer (e.g. `OpLoad %_ptr_StorageBuffer_ushort`). The + // existing codegen has been emitting these narrow-typed accesses via the uint-punning path in + // `load_buffer` / `store_buffer` for a while (`get_quadrants_uint_type(i16) = u16`, + // `get_quadrants_uint_type(i8) = u8`), which strict Vulkan validation requires these capabilities + // for -- so we emit them unconditionally whenever the queried feature is set, independent of + // whether the current kernel actually uses a narrow type. The cost is a single extra + // `OpCapability` word in the shader header; the upside is that every 16-bit / 8-bit field or + // ndarray access is spec-compliant on drivers that enforce the letter of + // `SPV_KHR_{8,16}bit_storage`. + if (caps_->get(cap::spirv_has_storage_buffer_8bit_access)) { + ib_.begin(spv::OpCapability).add(spv::CapabilityStorageBuffer8BitAccess).commit(&header_); + } + if (caps_->get(cap::spirv_has_storage_buffer_16bit_access)) { + ib_.begin(spv::OpCapability).add(spv::CapabilityStorageBuffer16BitAccess).commit(&header_); + } if (caps_->get(cap::spirv_has_int64)) { ib_.begin(spv::OpCapability).add(spv::CapabilityInt64).commit(&header_); } @@ -77,6 +93,17 @@ void IRBuilder::init_header() { ib_.begin(spv::OpExtension).add("SPV_KHR_storage_buffer_storage_class").commit(&header_); + // `SPV_KHR_{8,16}bit_storage` is paired with `CapabilityStorageBuffer{8,16}BitAccess` above. + // Both the capability and the extension are needed for narrow-typed `StorageBuffer` loads / + // stores to validate on Vulkan; declaring only the capability without the extension is + // ill-formed SPIR-V. + if (caps_->get(cap::spirv_has_storage_buffer_8bit_access)) { + ib_.begin(spv::OpExtension).add("SPV_KHR_8bit_storage").commit(&header_); + } + if (caps_->get(cap::spirv_has_storage_buffer_16bit_access)) { + ib_.begin(spv::OpExtension).add("SPV_KHR_16bit_storage").commit(&header_); + } + if (caps_->get(cap::spirv_has_no_integer_wrap_decoration)) { ib_.begin(spv::OpExtension).add("SPV_KHR_no_integer_wrap_decoration").commit(&header_); } diff --git a/quadrants/inc/rhi_constants.inc.h b/quadrants/inc/rhi_constants.inc.h index 725171d5db..bdd3d1795c 100644 --- a/quadrants/inc/rhi_constants.inc.h +++ b/quadrants/inc/rhi_constants.inc.h @@ -14,6 +14,17 @@ PER_DEVICE_CAPABILITY(spirv_has_int16) PER_DEVICE_CAPABILITY(spirv_has_int64) PER_DEVICE_CAPABILITY(spirv_has_float16) PER_DEVICE_CAPABILITY(spirv_has_float64) +// Vulkan `VkPhysicalDevice8BitStorageFeatures::storageBuffer8BitAccess` / +// `VkPhysicalDevice16BitStorageFeatures::storageBuffer16BitAccess` queried flags, enabling +// `CapabilityStorageBuffer{8,16}BitAccess` emission in the SPIR-V header. Needed independently of +// `spirv_has_int{8,16}` / `spirv_has_float16`: `Int8` / `Int16` / `Float16` capabilities gate +// whether the type can be declared, whereas these gate whether it can be loaded / stored through +// a descriptor-bound `StorageBuffer` pointer. Emitting the storage caps without the underlying +// device feature makes the driver reject the shader at pipeline creation on strict +// implementations; skipping them when the device does support them is silent-corruption-UB on +// drivers that enforce them. +PER_DEVICE_CAPABILITY(spirv_has_storage_buffer_8bit_access) +PER_DEVICE_CAPABILITY(spirv_has_storage_buffer_16bit_access) PER_DEVICE_CAPABILITY(spirv_has_atomic_int64) PER_DEVICE_CAPABILITY(spirv_has_atomic_float16) // load, store, exchange PER_DEVICE_CAPABILITY(spirv_has_atomic_float16_add) diff --git a/quadrants/rhi/vulkan/vulkan_device_creator.cpp b/quadrants/rhi/vulkan/vulkan_device_creator.cpp index a958c5e73e..2addce9a94 100644 --- a/quadrants/rhi/vulkan/vulkan_device_creator.cpp +++ b/quadrants/rhi/vulkan/vulkan_device_creator.cpp @@ -789,6 +789,15 @@ void VulkanDeviceCreator::create_logical_device(bool manual_create) { features2.pNext = &shader_8bit_storage_feature; vkGetPhysicalDeviceFeatures2KHR(physical_device_, &features2); + // Gate the SPIR-V `CapabilityStorageBuffer8BitAccess` emission strictly on the queried + // feature bit. `VK_KHR_8bit_storage` promoted into Vulkan 1.2 core doesn't imply the feature + // is actually supported -- implementations can still expose `storageBuffer8BitAccess = FALSE` + // -- so the SPIR-V cap may only be emitted when this feature is true, otherwise strict + // validation rejects shaders that declare the capability. + if (shader_8bit_storage_feature.storageBuffer8BitAccess) { + caps.set(DeviceCapability::spirv_has_storage_buffer_8bit_access, true); + } + *pNextEnd = &shader_8bit_storage_feature; pNextEnd = &shader_8bit_storage_feature.pNext; } @@ -796,6 +805,10 @@ void VulkanDeviceCreator::create_logical_device(bool manual_create) { features2.pNext = &shader_16bit_storage_feature; vkGetPhysicalDeviceFeatures2KHR(physical_device_, &features2); + if (shader_16bit_storage_feature.storageBuffer16BitAccess) { + caps.set(DeviceCapability::spirv_has_storage_buffer_16bit_access, true); + } + *pNextEnd = &shader_16bit_storage_feature; pNextEnd = &shader_16bit_storage_feature.pNext; } From 8cb463d448957996f21382f6c23c6a2ea3a9c0f2 Mon Sep 17 00:00:00 2001 From: Alexis Duburcq Date: Fri, 24 Apr 2026 02:57:36 -0700 Subject: [PATCH 5/6] [SPIRV] Widen u1 storage through IRBuilder::cast so SPIR-V validation accepts the cast chain `TaskCodegen::store_buffer` widened a `u1` (bool) value to its backing `i8` slot by emitting `OpBitcast %char %bool_val`. That's ill-formed SPIR-V: `OpBitcast` requires a numerical scalar / vector or pointer operand and rejects booleans (`OpTypeBool` has no defined bit pattern, so a bitcast can't be defined either). `spirv-val` flagged the exact message: Expected input to be a pointer or int or float vector or scalar: Bitcast %46 = OpBitcast %char %tmp3_u1 Most drivers just crash in the pipeline compiler on the ill-formed input rather than surface a validation error. On Mesa RADV (AMD RX 7900 XTX) the symptom was a hard `SIGSEGV` deep inside `libvulkan_radeon.so::create_compute_pipeline` the moment any kernel storing to a `u1` field / ndarray / struct member was registered. Python-side, that looked like `timeout: the monitored command dumped core` on every one of the `test_tensor_consistency`, `test_pickle[vulkan-u1]`, `test_matrixfree_{cg,bicgstab}`, `test_struct_field_with_bool`, `test_dual_return_spirv`, and `test_offload_cross` tests. Route the `u1 -> i8` widening through `IRBuilder::cast`, which lowers `bool -> int` to the canonical `OpSelect(cond, 1, 0)` with the 1 / 0 constants produced at the target integer type. That's the same route `load_buffer`'s reverse path already uses on the read side, matches what `IRBuilder::cast` does in every other `u1` context in the codegen, and preserves the "`u1` serialises as 0 / 1" behaviour every `to_numpy()` / `from_numpy()` user depends on. Non-`u1` stores keep the existing `OpBitcast` path unchanged, so no other type path changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- quadrants/codegen/spirv/spirv_codegen.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/quadrants/codegen/spirv/spirv_codegen.cpp b/quadrants/codegen/spirv/spirv_codegen.cpp index a012409278..f98281bd00 100644 --- a/quadrants/codegen/spirv/spirv_codegen.cpp +++ b/quadrants/codegen/spirv/spirv_codegen.cpp @@ -2152,9 +2152,24 @@ void TaskCodegen::store_buffer(const Stmt *ptr, spirv::Value val) { } auto buf_ptr = at_buffer(ptr, ti_buffer_type); - auto val_bits = val.stype.dt == ti_buffer_type - ? val - : ir_->make_value(spv::OpBitcast, ir_->get_primitive_type(ti_buffer_type), val); + spirv::Value val_bits; + if (val.stype.dt == ti_buffer_type) { + val_bits = val; + } else if (val.stype.dt->is_primitive(PrimitiveTypeID::u1)) { + // SPIR-V `OpBitcast` rejects bool operands (spec: operand must be numerical scalar / vector or + // pointer). Before this fix, a `u1` field / ndarray store emitted + // `OpBitcast %char %bool_val` and validated as + // `Expected input to be a pointer or int or float vector or scalar: Bitcast`. Most drivers + // ignore that and crash inside the pipeline compiler (observed on Mesa RADV: a hard SIGSEGV + // inside `libvulkan_radeon.so::create_compute_pipeline` the moment the offending kernel is + // registered). Route through `IRBuilder::cast`, which lowers `bool -> int` to `OpSelect` + // picking `1` or `0` of the target type -- that's the canonical spec-compliant way to widen a + // bool, matches what `load_buffer` already does on the reverse path, and keeps the + // "bool serialises as 0 / 1" behaviour every user of `to_numpy()` / `from_numpy()` depends on. + val_bits = ir_->cast(ir_->get_primitive_type(ti_buffer_type), val); + } else { + val_bits = ir_->make_value(spv::OpBitcast, ir_->get_primitive_type(ti_buffer_type), val); + } ir_->store_variable(buf_ptr, val_bits); } From 9fa89c1819eac56ab152d39b416931f9a4093ca8 Mon Sep 17 00:00:00 2001 From: Alexis Duburcq Date: Fri, 24 Apr 2026 02:57:57 -0700 Subject: [PATCH 6/6] [Vulkan] Re-check validation-layer availability on every device-creator init, not just the first `VulkanDeviceCreator::create_instance` normalises `params_.enable_validation_layer` against `check_validation_layer_support()` - flipping it to `false` when the host has no `VK_LAYER_KHRONOS_validation` - and then later gates the `spirv_has_non_semantic_info` cap and the logical-device layer array on that flag. The normalisation used to live below the cached-`VkInstance` short-circuit that the same function takes to work around an NVIDIA driver bug around repeated `vkDestroyInstance`/`vkCreateInstance` cycles (the instance is kept alive in the `VulkanLoader` singleton for process lifetime). Every re-init after the first one then read `params_.enable_validation_layer` as `true` (whatever the caller passed in, usually `config.debug`), jumped straight to the cached-instance return, and skipped the flip entirely - even on a host where the first `create_instance` had flipped it to `false`. The visible symptom in `test_overflow.py` / `test_print.py` was the first parametrisation passing (correct cap = 0) and every subsequent one in the same pytest session failing (stale cap = 1). Each test's `@test_utils.test(... debug=True)` wrapper calls `qd.reset()` + `qd.init(...)` which reconstructs the `VulkanDeviceCreator` with fresh params, so the first cycle flipped to `false`, but the second / third / ... cycles preserved `true`. The `spirv_has_non_semantic_info` cap then got set, the `NonSemantic.DebugPrintf` extinst was emitted in the shader, the validation layer wasn't actually loaded to intercept the output, and the `capfd`-based assertion against the `Addition overflow detected` string fails with an empty capture buffer. Move the layer-availability check above the cached-instance return. Re-running `vkEnumerateInstanceLayerProperties` on every cycle is microseconds and keeps the flag consistent whether the instance was freshly created or reused. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rhi/vulkan/vulkan_device_creator.cpp | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/quadrants/rhi/vulkan/vulkan_device_creator.cpp b/quadrants/rhi/vulkan/vulkan_device_creator.cpp index 2addce9a94..f8246afe71 100644 --- a/quadrants/rhi/vulkan/vulkan_device_creator.cpp +++ b/quadrants/rhi/vulkan/vulkan_device_creator.cpp @@ -304,6 +304,27 @@ void VulkanDeviceCreator::create_instance(uint32_t vk_api_version, bool manual_c } } + // Normalize `params_.enable_validation_layer` against the host's actual layer availability BEFORE + // the instance-reuse short-circuit below: downstream code (device-extension enumeration at + // `create_logical_device`, the `VK_KHR_SHADER_NON_SEMANTIC_INFO` cap gate, the device layer arrays + // at the tail of `create_logical_device`) reads this flag on every cycle, and `create_logical_device` + // runs on every re-init. If we let the request pass through unchanged on the reuse path, the cap is + // set to `true` every subsequent cycle even when the layer is not actually loaded on the cached + // instance (the only cycle where the layer-load flip happens is the very first, which is also the + // only cycle where a fresh `vkCreateInstance` runs). Users observed this as `test_overflow.py` + // passing on the first parametrization and failing on every subsequent one in the same pytest + // session: `DebugPrintf`-ext-imported shaders compile fine, but the validation layer that would + // route those messages to stdout was never loaded, so the overflow-detected strings never appear in + // `capfd`. Running the check here keeps the flag consistent across re-inits; re-running on every + // call is cheap (it enumerates instance layers, ~microseconds). + if (params_.enable_validation_layer && !check_validation_layer_support()) { + RHI_LOG_ERROR( + "Validation layers requested but not available, turning off... " + "Please make sure Vulkan SDK from https://vulkan.lunarg.com/sdk/home " + "is installed."); + params_.enable_validation_layer = false; + } + // Reuse the VkInstance from a previous init/reset cycle if available. // Repeated vkDestroyInstance/vkCreateInstance triggers an NVIDIA driver bug // that corrupts SubgroupLocalInvocationId after ~11 cycles. @@ -325,15 +346,8 @@ void VulkanDeviceCreator::create_instance(uint32_t vk_api_version, bool manual_c create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO; create_info.pApplicationInfo = &app_info; - if (params_.enable_validation_layer) { - if (!check_validation_layer_support()) { - RHI_LOG_ERROR( - "Validation layers requested but not available, turning off... " - "Please make sure Vulkan SDK from https://vulkan.lunarg.com/sdk/home " - "is installed."); - params_.enable_validation_layer = false; - } - } + // `params_.enable_validation_layer` has already been normalized against the host's actual layer + // availability above, before the cached-instance short-circuit. No second check needed here. VkDebugUtilsMessengerCreateInfoEXT debug_create_info{};