From 4ca87963b4cf03cabae324ce369612a4f6895a9e Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Wed, 21 Oct 2026 13:21:06 -0700 Subject: [PATCH 01/13] [SPIR-V] Support RaytracingAccelerationStructure from ResourceDescriptorHeap --- docs/ReleaseNotes.md | 3 + docs/SPIR-V.rst | 89 ++++++++-- .../clang/include/clang/SPIRV/AstTypeProbe.h | 4 + .../include/clang/SPIRV/FeatureManager.h | 10 ++ .../clang/include/clang/SPIRV/SpirvBuilder.h | 31 +++- tools/clang/include/clang/SPIRV/SpirvModule.h | 5 + tools/clang/lib/SPIRV/AstTypeProbe.cpp | 7 + tools/clang/lib/SPIRV/CapabilityVisitor.cpp | 37 +++++ tools/clang/lib/SPIRV/CapabilityVisitor.h | 6 + tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 13 ++ tools/clang/lib/SPIRV/DeclResultIdMapper.h | 8 + tools/clang/lib/SPIRV/EmitVisitor.cpp | 2 +- tools/clang/lib/SPIRV/FeatureManager.cpp | 14 ++ tools/clang/lib/SPIRV/SpirvBuilder.cpp | 50 ++++-- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 144 +++++++++++++--- tools/clang/lib/SPIRV/SpirvEmitter.h | 18 +- tools/clang/lib/SPIRV/SpirvModule.cpp | 24 ++- ...xt.acceleration-structure.alias.error.hlsl | 51 ++++++ ...riptorheap.ext.acceleration-structure.hlsl | 42 +++++ ...t.acceleration-structure.stride.error.hlsl | 46 ++++++ ...stride.accelstruct-nv-ray-tracing-ext.hlsl | 26 +++ ...ay-stride.accelstruct-ray-tracing-ext.hlsl | 25 +++ ...ptorheap.ext.array-stride.accelstruct.hlsl | 154 ++++++++++++++++++ ...array-stride.unreachable-first-access.hlsl | 28 ++++ ...ap.ext.mixed-alias-assign-value.error.hlsl | 19 +++ 25 files changed, 796 insertions(+), 60 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 02b1755bf8d..b3ef691493d 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -25,6 +25,9 @@ The included licenses apply to the following files: buffer-like resources (`StructuredBuffer`, `ByteAddressBuffer`, `ConstantBuffer`, `TextureBuffer`, and RW variants) via `SPV_KHR_untyped_pointers`. Requires `-fspv-target-env=vulkan1.3`. +- Extended `-fspv-use-descriptor-heap` to support `RaytracingAccelerationStructure` + loaded from `ResourceDescriptorHeap`. Outside ray-tracing stages this requires + `-fspv-extension=SPV_KHR_ray_tracing` or `-fspv-extension=SPV_KHR_ray_query`. Place release notes for the upcoming release below this line and remove this line upon naming the release. Refer to previous for appropriate section names. diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index ef0dd3a3a2d..a4739350036 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -2143,13 +2143,27 @@ handle into a function-scope image variable: %texel_ptr = OpUntypedImageTexelPointerEXT %uptr_image %image_type %descriptor %coord %sample %old = OpAtomicIAdd %uint %texel_ptr %scope %semantics %value +``RaytracingAccelerationStructure`` resources loaded from +``ResourceDescriptorHeap`` follow the same access-chain-then-load shape over a +runtime array of ``OpTypeAccelerationStructureKHR``: + +.. code:: spirv + + %accel_type = OpTypeAccelerationStructureKHR + %accel_array = OpTypeRuntimeArray %accel_type + OpDecorateId %accel_array ArrayStrideIdEXT %resource_stride + %descriptor = OpUntypedAccessChainKHR %uptr_uc %accel_array %resource_heap %index + %accel = OpLoad %accel_type %descriptor + This path supports texture, RWTexture, sampler, Buffer/RWBuffer, StructuredBuffer/RWStructuredBuffer without associated counter operations, -ByteAddressBuffer/RWByteAddressBuffer, ConstantBuffer, and TextureBuffer heap -loads, including direct field and array-element accesses for -``ConstantBuffer`` and ``TextureBuffer``. ``NonUniformResourceIndex`` is -accepted, but no ``NonUniform`` decoration is emitted on the -``OpUntypedAccessChainKHR`` result or on the loaded descriptor; +ByteAddressBuffer/RWByteAddressBuffer, ConstantBuffer, TextureBuffer, and +``RaytracingAccelerationStructure`` heap loads, including direct field and +array-element accesses for ``ConstantBuffer`` and ``TextureBuffer``. +Acceleration structure loads are additionally subject to the stride requirement +described in `Descriptor heap array stride`_ below. +``NonUniformResourceIndex`` is accepted, but no ``NonUniform`` decoration is +emitted on the ``OpUntypedAccessChainKHR`` result or on the loaded descriptor; ``SPV_EXT_descriptor_heap`` deprecates the decoration for heap accesses and drivers handle divergent heap indices natively. The index operand itself may still carry ``NonUniform`` from the surrounding expression. @@ -2167,15 +2181,24 @@ Buffer-like local variables initialized from a heap access are tracked by their heap index, and later uses are re-lowered as fresh access chains. Image variables retain their loaded descriptor handle for ordinary reads and writes; the tracked heap index is used when an atomic operation requires -``OpUntypedImageTexelPointerEXT``. Acceleration-structure aliases retain the -initially loaded handle. Because index-tracked aliases do not model -control-flow merges, DXC rejects a variable that holds both a bound resource -and a heap descriptor, whether through a conditional assignment, a -reassignment back to a bound resource, or an assignment inside a loop:: +``OpUntypedImageTexelPointerEXT``. A ``RaytracingAccelerationStructure`` +variable initialized from a heap access is a name for the handle loaded by its +initializer. + +These aliases are compile-time state and do not model control-flow merges, so +DXC rejects a variable that holds both a bound resource and a heap descriptor, +whether through a conditional assignment, a reassignment back to a bound +resource, or an assignment inside a loop:: error: mixing bound and descriptor heap resources in the same variable is not supported with SPV_EXT_descriptor_heap +A heap-initialized ``RaytracingAccelerationStructure`` variable cannot be +reassigned at all, even from another heap access:: + + error: reassigning a RaytracingAccelerationStructure initialized from + ResourceDescriptorHeap is not supported with SPV_EXT_descriptor_heap + Supporting these forms requires modelling the alias as a value with real control-flow merges instead of as compile-time state. @@ -2183,13 +2206,16 @@ control-flow merges instead of as compile-time state. cannot yet be passed to or returned from user functions; access the resource directly at each call or return site. -Two further restrictions on the heap access expression itself produce -diagnostics. The object being subscripted must be a direct reference to the -builtin ``ResourceDescriptorHeap`` or ``SamplerDescriptorHeap`` variable, and -the subscript result must be immediately converted to a concrete resource type -so that DXC can select a descriptor type for the access. A subscript whose -result is discarded, or used in a context that supplies no target resource -type, is rejected. +Three further restrictions on heap accesses produce diagnostics: + +- The object being subscripted must be a direct reference to the builtin + ``ResourceDescriptorHeap`` or ``SamplerDescriptorHeap`` variable. +- The subscript result must be immediately converted to a concrete resource + type so that DXC can select a descriptor type for the access. A subscript + whose result is discarded, or used in a context that supplies no target + resource type, is rejected. +- A local ``RaytracingAccelerationStructure`` must be initialized from a + loadable heap access, since the alias has no backing descriptor otherwise. Descriptor heap array stride ++++++++++++++++++++++++++++ @@ -2199,7 +2225,8 @@ rather than a literal ``ArrayStride``, because descriptor sizes are not known until pipeline creation. The shared value is built from ``OpConstantSizeOfEXT`` and ``OpSpecConstantOp`` and evaluates to ``max(sizeof(image_descriptor), sizeof(buffer_descriptor))``. The sampler heap -carries its own ``ArrayStrideIdEXT`` equal to ``sizeof(sampler_descriptor)``. +carries its own ``ArrayStrideIdEXT`` equal to ``sizeof(sampler_descriptor)``, +regardless of resource heap contents. The ``OpConstantSizeOfEXT`` operands are placeholder types chosen only for their descriptor class. All image types report the same descriptor size, so the @@ -2208,6 +2235,32 @@ types the shader actually uses; a module will normally contain both the placeholder type and the distinct image types its heap accesses lower to. The stride value is built once and cached on first use. +When acceleration structure descriptors may appear on the resource heap, the +formula expands to a three-way max +``max(max(sizeof(image_descriptor), sizeof(buffer_descriptor)), sizeof(acceleration_structure))``. + +Because the stride is cached on first use, this decision is committed before +code generation and is **not** based on whether the shader actually performs an +acceleration structure heap load: a ray-tracing shader that only heap-loads a +texture still gets the three-way max. The widening happens when either + +- any entry point is a ray-tracing stage, or +- the user explicitly passed ``-fspv-extension=SPV_KHR_ray_query``, + ``-fspv-extension=SPV_KHR_ray_tracing`` or + ``-fspv-extension=SPV_NV_ray_tracing``. + +The second condition requires an explicit ``-fspv-extension`` flag. In the +default extension mode DXC allows the ray-tracing and ray-query extensions +implicitly, but that does not widen the stride. The acceleration structure type +added by widening is declared with the capability of the listed extension +(``RayQueryKHR``, ``RayTracingKHR`` or ``RayTracingNV``); if several are listed, +``SPV_KHR_ray_query`` is preferred, then ``SPV_KHR_ray_tracing``, then +``SPV_NV_ray_tracing``. Because the stride cannot be widened once it has been +built, a shader that is not a ray-tracing stage and loads a +``RaytracingAccelerationStructure`` from ``ResourceDescriptorHeap`` without an +explicit ray extension flag is rejected rather than given a stride that may be +too narrow. + HLSL Expressions ================ diff --git a/tools/clang/include/clang/SPIRV/AstTypeProbe.h b/tools/clang/include/clang/SPIRV/AstTypeProbe.h index 214df82d455..207d02a3b20 100644 --- a/tools/clang/include/clang/SPIRV/AstTypeProbe.h +++ b/tools/clang/include/clang/SPIRV/AstTypeProbe.h @@ -250,6 +250,10 @@ bool isBuffer(QualType type); /// \brief Returns true if the given type is the HLSL RWBuffer type. bool isRWBuffer(QualType type); +/// \brief Returns true if the given type is the HLSL +/// RaytracingAccelerationStructure type. +bool isRaytracingAccelerationStructure(QualType type); + /// \brief Returns true if the given type is an HLSL Texture type. bool isTexture(QualType); diff --git a/tools/clang/include/clang/SPIRV/FeatureManager.h b/tools/clang/include/clang/SPIRV/FeatureManager.h index 8f32990d92c..4d0081299f8 100644 --- a/tools/clang/include/clang/SPIRV/FeatureManager.h +++ b/tools/clang/include/clang/SPIRV/FeatureManager.h @@ -117,6 +117,15 @@ class FeatureManager { /// Returns true if the given extension is set in allowedExtensions bool isExtensionEnabled(Extension) const; + /// \brief Returns the allowed extension that provides the acceleration + /// structure type, or Extension::Unknown if there is none. + Extension getAccelerationStructureProvider() const; + + /// \brief Returns true if the user listed, with -fspv-extension, an extension + /// that provides the acceleration structure type. + /// When no list is given, returns false. + bool hasExplicitAccelerationStructureProvider() const; + /// Returns true if the target environment is Vulkan 1.1 or above. /// Returns false otherwise. bool isTargetEnvVulkan1p1OrAbove() const; @@ -181,6 +190,7 @@ class FeatureManager { DiagnosticsEngine &diags; llvm::SmallBitVector allowedExtensions; + bool explicitExtensionList = false; spv_target_env targetEnv; std::string targetEnvStr; }; diff --git a/tools/clang/include/clang/SPIRV/SpirvBuilder.h b/tools/clang/include/clang/SPIRV/SpirvBuilder.h index 9370a489fe4..16198bc7117 100644 --- a/tools/clang/include/clang/SPIRV/SpirvBuilder.h +++ b/tools/clang/include/clang/SPIRV/SpirvBuilder.h @@ -843,8 +843,30 @@ class SpirvBuilder { SpirvInstruction *op1, SpirvInstruction *op2, SpirvInstruction *op3, SourceLocation loc); - /// \brief Shared ArrayStrideIdEXT operand for resource-heap runtime arrays: - /// max(sizeof(image), sizeof(buffer)). + /// \brief Records that acceleration structures may be loaded from the + /// resource heap, so the resource-heap stride must cover their descriptor. + /// + /// The stride is built once, on the first getResourceHeapArrayStride() call, + /// and cannot change afterwards. A call after that point has no effect, so + /// resourceHeapStrideIncludesAccelStruct() stays false and code generation + /// must reject acceleration structure heap loads. + void noteResourceHeapHasAccelStruct() { + if (!resourceHeapArrayStride) + resourceHeapHasAccelStruct = true; + } + + /// \brief Returns true if the resource-heap stride covers the acceleration + /// structure descriptor. Acceleration structure heap loads are only valid + /// when this is true. + bool resourceHeapStrideIncludesAccelStruct() const { + return resourceHeapHasAccelStruct; + } + + /// \brief Returns the ArrayStrideIdEXT operand shared by every resource-heap + /// runtime array: the largest descriptor size among images and buffers, and + /// among acceleration structures if noteResourceHeapHasAccelStruct() was + /// called first. The value is built from specialization constants because + /// descriptor sizes are only known at pipeline creation. SpirvInstruction *getResourceHeapArrayStride(); /// \brief Shared ArrayStrideIdEXT operand for sampler-heap runtime arrays: @@ -975,6 +997,11 @@ class SpirvBuilder { SpirvInstruction *resourceHeapArrayStride = nullptr; SpirvInstruction *samplerHeapArrayStride = nullptr; + /// Set by noteResourceHeapHasAccelStruct(). When true, + /// getResourceHeapArrayStride() includes the acceleration structure + /// descriptor size. + bool resourceHeapHasAccelStruct = false; + SpirvDebugInfoNone *debugNone; /// DebugExpression that does not reference any DebugOperation diff --git a/tools/clang/include/clang/SPIRV/SpirvModule.h b/tools/clang/include/clang/SPIRV/SpirvModule.h index cf35b2d0e4e..db82efcf0c9 100644 --- a/tools/clang/include/clang/SPIRV/SpirvModule.h +++ b/tools/clang/include/clang/SPIRV/SpirvModule.h @@ -149,6 +149,10 @@ class SpirvModule { // Adds a constant to the module. void addConstant(SpirvConstant *); + // Adds an OpSpecConstantOp to the module. Operands must be added before + // their users. + void addSpecConstantOp(SpirvInstruction *); + // Adds an Undef to the module. void addUndef(SpirvUndef *); @@ -216,6 +220,7 @@ class SpirvModule { decorations; std::vector constants; + std::vector specConstantOps; std::vector undefs; std::vector variables; // A vector of functions in the module in the order that they should be diff --git a/tools/clang/lib/SPIRV/AstTypeProbe.cpp b/tools/clang/lib/SPIRV/AstTypeProbe.cpp index d112d1863d7..0b821fe5007 100644 --- a/tools/clang/lib/SPIRV/AstTypeProbe.cpp +++ b/tools/clang/lib/SPIRV/AstTypeProbe.cpp @@ -895,6 +895,13 @@ bool isBuffer(QualType type) { hlsl::GetHLSLResourceClass(type) == hlsl::DXIL::ResourceClass::SRV; } +bool isRaytracingAccelerationStructure(QualType type) { + if (const auto *rt = type->getAs()) { + return rt->getDecl()->getName() == "RaytracingAccelerationStructure"; + } + return false; +} + bool isRWTexture(QualType type) { return hlsl::DXIL::IsAnyTexture(hlsl::GetHLSLResourceKind(type)) && hlsl::GetHLSLResourceClass(type) == hlsl::DXIL::ResourceClass::UAV; diff --git a/tools/clang/lib/SPIRV/CapabilityVisitor.cpp b/tools/clang/lib/SPIRV/CapabilityVisitor.cpp index 46aea23186b..1491826e8f9 100644 --- a/tools/clang/lib/SPIRV/CapabilityVisitor.cpp +++ b/tools/clang/lib/SPIRV/CapabilityVisitor.cpp @@ -208,6 +208,10 @@ void CapabilityVisitor::addCapabilityForType(const SpirvType *type, else if (const auto *sampledImageType = dyn_cast(type)) { addCapabilityForType(sampledImageType->getImageType(), loc, sc); } + // Acceleration structure type + else if (isa(type)) { + addCapabilityForAccelerationStructure(loc); + } // Pointer type else if (const auto *ptrType = dyn_cast(type)) { addCapabilityForType(ptrType->getPointeeType(), loc, @@ -239,6 +243,39 @@ void CapabilityVisitor::addCapabilityForType(const SpirvType *type, } } +void CapabilityVisitor::addCapabilityForAccelerationStructure( + SourceLocation loc) { + const Extension provider = featureManager.getAccelerationStructureProvider(); + switch (provider) { + case Extension::KHR_ray_query: + addExtension(provider, "acceleration structure type", loc); + addCapability(spv::Capability::RayQueryKHR, loc); + break; + case Extension::KHR_ray_tracing: + addExtension(provider, "acceleration structure type", loc); + addCapability(spv::Capability::RayTracingKHR, loc); + break; + case Extension::NV_ray_tracing: + addExtension(provider, "acceleration structure type", loc); + addCapability(spv::Capability::RayTracingNV, loc); + break; + default: + // No allowed extension provides the type. Report the standard + // "extension not permitted" error instead of emitting an invalid module. + featureManager.requestExtension(Extension::KHR_ray_tracing, + "acceleration structure type", loc); + break; + } +} + +bool CapabilityVisitor::visit(SpirvConstantSizeOfEXT *inst) { + // The operand type is not the result type, so the generic handling of + // instructions does not see it. + addCapabilityForType(inst->getOperandType(), inst->getSourceLocation(), + spv::StorageClass::Max); + return visitInstruction(inst); +} + bool CapabilityVisitor::visit(SpirvDecoration *decor) { const auto loc = decor->getSourceLocation(); switch (decor->getDecoration()) { diff --git a/tools/clang/lib/SPIRV/CapabilityVisitor.h b/tools/clang/lib/SPIRV/CapabilityVisitor.h index 35d4b5a18b9..24165f4c91a 100644 --- a/tools/clang/lib/SPIRV/CapabilityVisitor.h +++ b/tools/clang/lib/SPIRV/CapabilityVisitor.h @@ -40,6 +40,7 @@ class CapabilityVisitor : public Visitor { bool visit(SpirvDemoteToHelperInvocation *) override; bool visit(SpirvIsHelperInvocationEXT *) override; bool visit(SpirvReadClock *) override; + bool visit(SpirvConstantSizeOfEXT *) override; using Visitor::visit; @@ -58,6 +59,11 @@ class CapabilityVisitor : public Visitor { void addCapabilityForType(const SpirvType *, SourceLocation loc, spv::StorageClass sc); + /// Adds the extension and capability that make the acceleration structure + /// type valid, chosen with FeatureManager::getAccelerationStructureProvider. + /// Reports an error if no allowed extension provides the type. + void addCapabilityForAccelerationStructure(SourceLocation loc); + /// Checks that the given extension is a valid extension for the target /// environment (e.g. Vulkan 1.0). And if so, utilizes the SpirvBuilder to add /// the given extension to the SPIR-V module in memory. diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 9aaa36c97ba..46967298c70 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -1144,6 +1144,19 @@ DeclResultIdMapper::createFnVar(const VarDecl *var, return varInstr; } +void DeclResultIdMapper::registerFnVarAlias(const VarDecl *var, + SpirvInstruction *varInstr) { + assert(varInstr); + registerVariableForDecl(var, createDeclSpirvInfo(varInstr)); +} + +bool DeclResultIdMapper::hasFnVarAlias(const VarDecl *var) const { + const DeclSpirvInfo *info = getDeclSpirvInfo(var); + if (!info || !info->instr) + return false; + return !isa(info->instr); +} + SpirvDebugGlobalVariable *DeclResultIdMapper::createDebugGlobalVariable( SpirvVariable *var, const QualType &type, const SourceLocation &loc, const StringRef &name) { diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.h b/tools/clang/lib/SPIRV/DeclResultIdMapper.h index 6a96ae8f45d..d39f7fb8579 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.h +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.h @@ -285,6 +285,14 @@ class DeclResultIdMapper { SpirvVariable *createFnVar(const VarDecl *var, llvm::Optional init); + /// \brief Maps a function-scope variable to an existing instruction instead + /// of creating a function variable for it. + void registerFnVarAlias(const VarDecl *var, SpirvInstruction *varInstr); + + /// \brief Returns true if the decl is mapped to an instruction that is not a + /// variable, as done by registerFnVarAlias. + bool hasFnVarAlias(const VarDecl *var) const; + /// \brief Creates a file-scope variable and returns its instruction. SpirvVariable *createFileVar(const VarDecl *var, llvm::Optional init); diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index 74b92939eb3..7b6a2c53bcd 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -1463,7 +1463,7 @@ bool EmitVisitor::visit(SpirvSpecConstantUnaryOp *inst) { curInst.push_back(getOrAssignResultId(inst)); curInst.push_back(static_cast(inst->getSpecConstantopcode())); curInst.push_back(getOrAssignResultId(inst->getOperand())); - finalizeInstruction(&mainBinary); + finalizeInstruction(&typeConstantBinary); emitDebugNameForInstruction(getOrAssignResultId(inst), inst->getDebugName()); return true; diff --git a/tools/clang/lib/SPIRV/FeatureManager.cpp b/tools/clang/lib/SPIRV/FeatureManager.cpp index 5e6fa6e4caf..5bfac224f1a 100644 --- a/tools/clang/lib/SPIRV/FeatureManager.cpp +++ b/tools/clang/lib/SPIRV/FeatureManager.cpp @@ -99,6 +99,7 @@ FeatureManager::FeatureManager(DiagnosticsEngine &de, } targetEnv = *targetEnvOpt; + explicitExtensionList = !opts.allowedExtensions.empty(); if (opts.allowedExtensions.empty()) { // If no explicit extension control from command line, use the default mode: // allowing all extensions that are enabled by default. @@ -381,6 +382,19 @@ bool FeatureManager::isExtensionEnabled(Extension ext) const { return allowed; } +Extension FeatureManager::getAccelerationStructureProvider() const { + for (Extension ext : {Extension::KHR_ray_query, Extension::KHR_ray_tracing, + Extension::NV_ray_tracing}) + if (isExtensionEnabled(ext)) + return ext; + return Extension::Unknown; +} + +bool FeatureManager::hasExplicitAccelerationStructureProvider() const { + return explicitExtensionList && + getAccelerationStructureProvider() != Extension::Unknown; +} + bool FeatureManager::enabledByDefault(Extension ext) { switch (ext) { // KHR_ray_tracing and NV_ray_tracing are mutually exclusive so enable only diff --git a/tools/clang/lib/SPIRV/SpirvBuilder.cpp b/tools/clang/lib/SPIRV/SpirvBuilder.cpp index 2610c269401..6a841a99c89 100644 --- a/tools/clang/lib/SPIRV/SpirvBuilder.cpp +++ b/tools/clang/lib/SPIRV/SpirvBuilder.cpp @@ -468,20 +468,18 @@ SpirvBinaryOp *SpirvBuilder::createBinaryOp(spv::Op op, QualType resultType, SpirvSpecConstantBinaryOp *SpirvBuilder::createSpecConstantBinaryOp( spv::Op op, QualType resultType, SpirvInstruction *lhs, SpirvInstruction *rhs, SourceLocation loc) { - assert(insertPoint && "null insert point"); auto *instruction = new (context) SpirvSpecConstantBinaryOp(op, resultType, loc, lhs, rhs); - insertPoint->addInstruction(instruction); + mod->addSpecConstantOp(instruction); return instruction; } SpirvSpecConstantTernaryOp *SpirvBuilder::createSpecConstantTernaryOp( spv::Op op, QualType resultType, SpirvInstruction *op1, SpirvInstruction *op2, SpirvInstruction *op3, SourceLocation loc) { - assert(insertPoint && "null insert point"); auto *instruction = new (context) SpirvSpecConstantTernaryOp(op, resultType, loc, op1, op2, op3); - insertPoint->addInstruction(instruction); + mod->addSpecConstantOp(instruction); return instruction; } @@ -2060,16 +2058,21 @@ SpirvInstruction *SpirvBuilder::getResourceHeapArrayStride() { if (resourceHeapArrayStride) return resourceHeapArrayStride; - // ResourceDescriptorHeap is a flat array: any descriptor may sit at any slot. - // DX12 semantics require all resource arrays share stride = - // max(sizeof(image), sizeof(buffer)). - // VkPhysicalDeviceDescriptorHeapPropertiesEXT reports one size per category - // (imageDescriptorSize / bufferDescriptorSize); textures lower to - // OpTypeImage, so image/buffer covers all relevant HLSL resource kinds. Sizes - // are driver-defined (known only at pipeline creation), so the max is - // computed via OpSpecConstantOp over two OpConstantSizeOfEXT placeholders. - // A canonical sampled 2D float image and a Uniform buffer serve as - // representatives; subtype and storage class do not affect the category size. + // ResourceDescriptorHeap is a flat array in which any descriptor may sit at + // any slot, so every resource runtime array must share one stride: + // max(sizeof(image), sizeof(buffer) [, sizeof(acceleration structure)]) + // + // Descriptor sizes are only known at pipeline creation, so the maximum is + // computed with OpSpecConstantOp over OpConstantSizeOfEXT. Each descriptor + // category is measured through one placeholder type, because all types in a + // category have the same descriptor size: + // image: a sampled 2D float image. Textures lower to OpTypeImage, so this + // covers all texture resources. + // buffer: a Uniform buffer. + // accel: the acceleration structure type, included only when + // noteResourceHeapHasAccelStruct() was called. That type needs a + // ray-tracing or ray-query capability, which is always declared + // in the cases that make that call. const SpirvType *placeholderImage = context.getImageType( context.getFloatType(32), spv::Dim::Dim2D, ImageType::WithDepth::No, /*arrayed*/ false, /*ms*/ false, ImageType::WithSampler::Yes, @@ -2077,13 +2080,30 @@ SpirvInstruction *SpirvBuilder::getResourceHeapArrayStride() { const SpirvType *placeholderBuffer = context.getBufferEXTType(spv::StorageClass::Uniform); + // The stride is cached and shared by every heap access in the module, so it + // is built from module-level instructions. Instructions owned by the block of + // the first access would be lost if that block were unreachable. SpirvInstruction *imageSize = getConstantSizeOfEXT(placeholderImage); SpirvInstruction *bufferSize = getConstantSizeOfEXT(placeholderBuffer); SpirvInstruction *imageIsBigger = createSpecConstantBinaryOp( spv::Op::OpUGreaterThan, astContext.BoolTy, imageSize, bufferSize, {}); - resourceHeapArrayStride = + SpirvInstruction *stride = createSpecConstantTernaryOp(spv::Op::OpSelect, astContext.UnsignedIntTy, imageIsBigger, imageSize, bufferSize, {}); + + if (resourceHeapHasAccelStruct) { + // The "NV" in the type's name is historical; it lowers to + // OpTypeAccelerationStructureKHR. + const SpirvType *placeholderAS = context.getAccelerationStructureTypeNV(); + SpirvInstruction *asSize = getConstantSizeOfEXT(placeholderAS); + SpirvInstruction *strideIsBigger = createSpecConstantBinaryOp( + spv::Op::OpUGreaterThan, astContext.BoolTy, stride, asSize, {}); + stride = + createSpecConstantTernaryOp(spv::Op::OpSelect, astContext.UnsignedIntTy, + strideIsBigger, stride, asSize, {}); + } + + resourceHeapArrayStride = stride; return resourceHeapArrayStride; } diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 11242e7415e..c035662cc75 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -67,6 +67,21 @@ bool isImplicitVarDeclInVkNamespace(const Decl *decl) { return false; } +// Returns true if the given shader kind is a ray-tracing stage. +bool shaderModelKindIsRayTracing(hlsl::ShaderModel::Kind kind) { + switch (kind) { + case hlsl::ShaderModel::Kind::RayGeneration: + case hlsl::ShaderModel::Kind::Intersection: + case hlsl::ShaderModel::Kind::AnyHit: + case hlsl::ShaderModel::Kind::ClosestHit: + case hlsl::ShaderModel::Kind::Miss: + case hlsl::ShaderModel::Kind::Callable: + return true; + default: + return false; + } +} + // Returns true if the given decl has the given semantic. bool hasSemantic(const DeclaratorDecl *decl, hlsl::DXIL::SemanticKind semanticKind) { @@ -803,6 +818,45 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) { return; } + // Decide whether the resource-heap stride must cover acceleration structures. + // The stride is built on the first descriptor heap access and cannot change + // afterwards, so this has to be decided before any function is translated. + // It is decided up front rather than per access, so a shader that merely + // might load an acceleration structure from the heap is covered. + // + // The stride covers acceleration structures if any of these holds: + // - An entry point is a ray-tracing stage. + // - The user listed an extension that provides the acceleration structure + // type (SPV_KHR_ray_query, SPV_KHR_ray_tracing or SPV_NV_ray_tracing) + // with -fspv-extension. Without that option the feature manager reports + // the KHR extensions as enabled for every shader, which says nothing + // about this shader, so the default mode is not a trigger. + // + // The capability that makes the acceleration structure type valid is + // declared from the type itself, using the same provider (see + // CapabilityVisitor), so it is always consistent with this decision. + // + // Heap accesses in global initializers are translated after this point, so + // they cannot build the stride early. If one ever did, the call below would + // have no effect and acceleration structure heap loads would be rejected + // rather than given a stride that may be too narrow. + if (spirvOptions.useDescriptorHeap) { + bool needsAccelStruct = false; + + for (const FunctionInfo *fi : workQueue) + if (shaderModelKindIsRayTracing(fi->shaderModelKind)) { + needsAccelStruct = true; + break; + } + + if (!needsAccelStruct) + needsAccelStruct = + featureManager.hasExplicitAccelerationStructureProvider(); + + if (needsAccelStruct) + spvBuilder.noteResourceHeapHasAccelStruct(); + } + // Translate all functions reachable from the entry function. // The queue can grow in the meanwhile; so need to keep evaluating // workQueue.size(). @@ -2033,6 +2087,21 @@ bool SpirvEmitter::tryToCreateDescriptorHeapAlias(const VarDecl *decl, return tryToAssignDescriptorHeapBufferAlias(decl, init); } + if (isRaytracingAccelerationStructure(decl->getType())) { + if (SpirvInstruction *initVal = loadIfGLValue(init)) { + // The variable becomes a name for the loaded handle: acceleration + // structure handles are not stored in function variables. Record the + // heap state so that any later assignment to it is diagnosed. + declIdMapper.registerFnVarAlias(decl, initVal); + descriptorHeapVarState[decl] = DescriptorHeapVarState::Heap; + } else { + emitError("cannot create descriptor heap acceleration structure alias " + "from initializer", + init->getExprLoc()); + } + return true; + } + return false; } @@ -3220,11 +3289,13 @@ SpirvEmitter::tryToAssignToDescriptorHeapAlias( assignExpr->getExprLoc())) return tryToAssignToDescriptorHeapBuffer(assignExpr); - // The assignment was rejected. A heap buffer alias is represented only by its - // index variable, so the normal path has no destination to store into and - // would null-deref; consume the assignment instead. Image aliases do have a + // The assignment was rejected. Buffer aliases (tracked only by an index + // variable) and acceleration structure aliases (a name for a loaded handle) + // have no variable for the normal assignment path to store into, so it would + // dereference null; consume the assignment instead. Image aliases do have a // function variable, so they can fall through to the plain handle store. - if (descriptorHeapBufferAliasVars.count(dstVar)) + if (descriptorHeapBufferAliasVars.count(dstVar) || + declIdMapper.hasFnVarAlias(dstVar)) return static_cast(nullptr); return llvm::None; } @@ -3252,7 +3323,11 @@ bool SpirvEmitter::tryHandleDescriptorHeapAssignment(const BinaryOperator *expr, if (llvm::Optional aliasResult = tryToAssignToDescriptorHeapAlias(expr)) { - *result = aliasResult.getValue(); + // A consumed assignment has no value of its own, but it can still be used + // as one, e.g. `(b = bound).Load(0)`; a null here would crash the code + // that consumes it. + *result = aliasResult.getValue() ? aliasResult.getValue() + : spvBuilder.getUndef(expr->getType()); return true; } @@ -5300,24 +5375,42 @@ bool SpirvEmitter::diagnoseDescriptorHeapAliasMixing(const VarDecl *dstVar, // Only the resource kinds that use the compile-time alias mechanism can be // miscompiled by a mixed assignment. Other resources are stored into a real // function variable, which merges correctly across control flow. + // RaytracingAccelerationStructure is covered too: a heap-initialized one is + // a compile-time alias (see tryToCreateDescriptorHeapAlias). const QualType dstType = dstVar->getType(); - if (!isRWTexture(dstType) && !isRWBuffer(dstType) && + const bool isASType = isRaytracingAccelerationStructure(dstType); + if (!isASType && !isRWTexture(dstType) && !isRWBuffer(dstType) && !isConstantTextureBuffer(dstType) && !isAKindOfStructuredOrByteBuffer(dstType)) return false; const bool srcIsHeap = isHeapSourcedValue(srcExpr->IgnoreParenCasts()); const bool wasHeap = descriptorHeapImageAliasVars.count(dstVar) || - descriptorHeapBufferAliasVars.count(dstVar); + descriptorHeapBufferAliasVars.count(dstVar) || + (stateIt != descriptorHeapVarState.end() && + stateIt->second == DescriptorHeapVarState::Heap); const bool wasBound = stateIt != descriptorHeapVarState.end() && stateIt->second == DescriptorHeapVarState::Bound; - const bool mixingDetected = - (srcIsHeap && wasBound) || (!srcIsHeap && wasHeap); + // A heap-initialized acceleration structure is a compile-time name for the + // handle loaded by its initializer, with no variable to store a new handle + // into, so any reassignment is rejected, including from another heap access. + // Image and buffer aliases can be retargeted heap-to-heap; for them only + // crossing between bound and heap resources is rejected. + const bool isASReassignment = isASType && wasHeap && srcIsHeap; + const bool mixingDetected = (isASType && wasHeap) || + (srcIsHeap && wasBound) || + (!srcIsHeap && wasHeap); if (mixingDetected) { - emitError("mixing bound and descriptor heap resources in the same variable " - "is not supported with SPV_EXT_descriptor_heap", - loc); + if (isASReassignment) + emitError("reassigning a RaytracingAccelerationStructure initialized " + "from ResourceDescriptorHeap is not supported with " + "SPV_EXT_descriptor_heap", + loc); + else + emitError("mixing bound and descriptor heap resources in the same " + "variable is not supported with SPV_EXT_descriptor_heap", + loc); // Leave the alias in place: heap-initialized buffer vars have doVarDecl // return early (before createFnVar), making the alias their only handle; // erasing it crashes downstream. The emitted error fails the compilation. @@ -7231,6 +7324,20 @@ SpirvEmitter::doCXXOperatorCallExpr(const CXXOperatorCallExpr *expr, return nullptr; } + // The stride is fixed before any function is translated and cannot be + // widened here. Reject the load rather than use a stride that may be + // too narrow for an acceleration structure descriptor. + if (isRaytracingAccelerationStructure(resourceType) && + !spvBuilder.resourceHeapStrideIncludesAccelStruct()) { + emitError("acceleration structure loaded from ResourceDescriptorHeap " + "requires the resource heap stride to account for " + "acceleration structure descriptors; compile with " + "-fspv-extension=SPV_KHR_ray_tracing or " + "-fspv-extension=SPV_KHR_ray_query", + expr->getExprLoc()); + return nullptr; + } + if (isAKindOfStructuredOrByteBuffer(resourceType) || isConstantTextureBuffer(resourceType)) return emitDescriptorHeapBufferAccess(resourceType, var, index, expr, @@ -9693,13 +9800,12 @@ void SpirvEmitter::createSpecConstant(const VarDecl *varDecl) { const SpirvType * SpirvEmitter::getDescriptorHeapRuntimeArrayType(const SpirvType *elemType) { - // SPV_EXT_descriptor_heap: apply a client-API-defined byte stride via an - // ArrayStrideIdEXT decoration. The sampler heap holds a single descriptor - // type, so its stride is the sampler descriptor size. The resource heap is a - // shared flat array in which any resource descriptor may sit at any slot, so - // every resource runtime array must use one common stride: max(sizeof(image), - // sizeof(buffer)). Using the accessed element size would be wrong for the - // resource heap. + // Descriptor sizes are defined by the client API, so the stride is an + // ArrayStrideIdEXT operand rather than a literal. The sampler heap holds one + // descriptor type. The resource heap is a flat array in which any resource + // descriptor may sit at any slot, so all its runtime arrays share one stride + // (see SpirvBuilder::getResourceHeapArrayStride) instead of the stride of the + // accessed type. SpirvInstruction *strideId = isa(elemType) ? spvBuilder.getSamplerHeapArrayStride() : spvBuilder.getResourceHeapArrayStride(); diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.h b/tools/clang/lib/SPIRV/SpirvEmitter.h index f7c023c4f92..56effc43308 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.h +++ b/tools/clang/lib/SPIRV/SpirvEmitter.h @@ -1279,9 +1279,18 @@ class SpirvEmitter : public ASTConsumer { const Expr *srcExpr); /// \brief Diagnoses a local resource variable assigned from both a bound - /// resource and ResourceDescriptorHeap. + /// resource and ResourceDescriptorHeap, and any reassignment of a + /// heap-initialized RaytracingAccelerationStructure. /// - /// Issues a diagnostic and returns true if rejected. + /// Heap-initialized variables are recorded at compile time (the alias maps + /// for images and buffers, registerFnVarAlias for acceleration structures) + /// and every later use is lowered from that record, ignoring control flow. + /// That is only correct if the variable holds the same heap descriptor on + /// every path, so other assignments are rejected instead of miscompiled. + /// + /// Returns true if the assignment is rejected, either now or because the + /// variable was already diagnosed. Callers need no further handling: + /// alias-recording helpers skip rejected variables. bool diagnoseDescriptorHeapAliasMixing(const VarDecl *dstVar, const Expr *srcExpr, SourceLocation loc); @@ -1746,11 +1755,14 @@ class SpirvEmitter : public ASTConsumer { bool counterUnsupported = false; }; /// Tracks per-variable assignment history for the mixed-alias diagnostic. + /// Heap: an acceleration structure variable was initialized from + /// ResourceDescriptorHeap. Image and buffer variables are tracked by + /// their alias maps instead. /// Bound: the variable has been assigned from a bound resource; a later heap /// assignment to it would be a diagnosable mix. /// Mixed: mixing was already diagnosed; alias recording stays suppressed for /// the remainder of the function so the error fires only once. - enum class DescriptorHeapVarState : uint8_t { Bound, Mixed }; + enum class DescriptorHeapVarState : uint8_t { Heap, Bound, Mixed }; llvm::DenseMap descriptorHeapVarState; diff --git a/tools/clang/lib/SPIRV/SpirvModule.cpp b/tools/clang/lib/SPIRV/SpirvModule.cpp index 4ba707548f2..f8e16e7422e 100644 --- a/tools/clang/lib/SPIRV/SpirvModule.cpp +++ b/tools/clang/lib/SPIRV/SpirvModule.cpp @@ -17,8 +17,8 @@ namespace spirv { SpirvModule::SpirvModule() : capabilities({}), extensions({}), extInstSets({}), memoryModel(nullptr), entryPoints({}), executionModes({}), moduleProcesses({}), decorations({}), - constants({}), undefs({}), variables({}), functions({}), - debugInstructions({}), perVertexInterp(false) {} + constants({}), specConstantOps({}), undefs({}), variables({}), + functions({}), debugInstructions({}), perVertexInterp(false) {} SpirvModule::~SpirvModule() { for (auto *cap : capabilities) @@ -43,6 +43,8 @@ SpirvModule::~SpirvModule() { decoration->releaseMemory(); for (auto *constant : constants) constant->releaseMemory(); + for (auto *op : specConstantOps) + op->releaseMemory(); for (auto *undef : undefs) undef->releaseMemory(); for (auto *var : variables) @@ -87,6 +89,13 @@ bool SpirvModule::invokeVisitor(Visitor *visitor, bool reverseOrder) { return false; } + for (auto iter = specConstantOps.rbegin(); iter != specConstantOps.rend(); + ++iter) { + auto *op = *iter; + if (!op->invokeVisitor(visitor)) + return false; + } + for (auto iter = constants.rbegin(); iter != constants.rend(); ++iter) { auto *constant = *iter; if (!constant->invokeVisitor(visitor)) @@ -211,6 +220,10 @@ bool SpirvModule::invokeVisitor(Visitor *visitor, bool reverseOrder) { if (!constant->invokeVisitor(visitor)) return false; + for (auto *op : specConstantOps) + if (!op->invokeVisitor(visitor)) + return false; + for (auto undef : undefs) if (!undef->invokeVisitor(visitor)) return false; @@ -357,6 +370,13 @@ void SpirvModule::addConstant(SpirvConstant *constant) { constants.push_back(constant); } +void SpirvModule::addSpecConstantOp(SpirvInstruction *op) { + assert(op && (isa(op) || + isa(op) || + isa(op))); + specConstantOps.push_back(op); +} + void SpirvModule::addUndef(SpirvUndef *undef) { assert(undef); undefs.push_back(undef); diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl new file mode 100644 index 00000000000..e6c926cab2f --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl @@ -0,0 +1,51 @@ +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=0 %s 2>&1 | FileCheck %s --check-prefix=MIX +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=1 %s 2>&1 | FileCheck %s --check-prefix=MIX +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=2 %s 2>&1 | FileCheck %s --check-prefix=REASSIGN +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=3 %s 2>&1 | FileCheck %s --check-prefix=MIX + +// Verifies: a RaytracingAccelerationStructure initialized from +// ResourceDescriptorHeap is a compile-time name for the loaded handle, so an +// assignment that would change it is diagnosed, without crashing the compiler. +// +// CASE=0 and CASE=1 assign a bound acceleration structure to a heap-initialized +// variable, once as a statement and once as a value. CASE=2 reassigns it from +// another heap access, which is also rejected, with a message that does not +// mention bound resources. CASE=3 assigns a heap acceleration structure to a +// variable that was assigned a bound one before. + +// MIX: error: mixing bound and descriptor heap resources in the same variable is not supported +// MIX-NOT: Internal compiler error +// REASSIGN: error: reassigning a RaytracingAccelerationStructure initialized from ResourceDescriptorHeap is not supported +// REASSIGN-NOT: Internal compiler error + +RaytracingAccelerationStructure bound : register(t0); +RWBuffer output : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) { + RayDesc ray; + ray.Origin = float3(0.0, 0.0, 0.0); + ray.Direction = float3(0.0, 0.0, 1.0); + ray.TMin = 0.0; + ray.TMax = 1000.0; + RayQuery q; +#if CASE == 0 + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + scene = bound; + q.TraceRayInline(scene, RAY_FLAG_NONE, 0xff, ray); +#elif CASE == 1 + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + RaytracingAccelerationStructure other = (scene = bound); + q.TraceRayInline(other, RAY_FLAG_NONE, 0xff, ray); +#elif CASE == 2 + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + scene = ResourceDescriptorHeap[2]; + q.TraceRayInline(scene, RAY_FLAG_NONE, 0xff, ray); +#elif CASE == 3 + RaytracingAccelerationStructure scene; + scene = bound; + scene = ResourceDescriptorHeap[1]; + q.TraceRayInline(scene, RAY_FLAG_NONE, 0xff, ray); +#endif + output[tid.x] = q.Proceed() ? 1.0 : 0.0; +} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.hlsl new file mode 100644 index 00000000000..d0f9e7675fb --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.hlsl @@ -0,0 +1,42 @@ +// RUN: %dxc -T lib_6_6 -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_KHR_ray_tracing -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -spirv %s | FileCheck %s + +// Verifies: a RaytracingAccelerationStructure loaded from ResourceDescriptorHeap +// is read from a runtime array of OpTypeAccelerationStructureKHR with +// OpUntypedAccessChainKHR followed by OpLoad, and the loaded handle is used by +// OpTraceRayKHR. + +// CHECK: OpCapability RayTracingKHR +// CHECK: OpExtension "SPV_KHR_ray_tracing" + +// CHECK-DAG: %[[UntypedUniformConstant:[a-zA-Z0-9_]+]] = OpTypeUntypedPointerKHR UniformConstant +// CHECK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR +// CHECK-DAG: %[[ASArray:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[Accel]] +// CHECK: %[[ResourceHeap:[a-zA-Z0-9_]+]] = OpUntypedVariableKHR %[[UntypedUniformConstant]] UniformConstant + +struct Payload { + float4 color; +}; + +struct Attribute { + float2 bary; +}; + +[shader("closesthit")] +void main(inout Payload payload, in Attribute attr) { + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[3]; + + RayDesc ray; + ray.Origin = float3(0.0f, 0.0f, 0.0f); + ray.Direction = float3(0.0f, 0.0f, -1.0f); + ray.TMin = 0.0f; + ray.TMax = 1000.0f; + + Payload childPayload = { float4(attr.bary, 0.0f, 1.0f) }; + + // CHECK: %[[Desc:[a-zA-Z0-9_]+]] = OpUntypedAccessChainKHR %[[UntypedUniformConstant]] %[[ASArray]] %[[ResourceHeap]] %uint_3 + // CHECK: %[[Scene:[a-zA-Z0-9_]+]] = OpLoad %[[Accel]] %[[Desc]] + // CHECK: OpTraceRayKHR %[[Scene]] + TraceRay(scene, 0x0, 0xff, 0, 1, 0, ray, childPayload); + + payload.color = childPayload.color; +} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl new file mode 100644 index 00000000000..5c94f614b9f --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl @@ -0,0 +1,46 @@ +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 -spirv %s 2>&1 | FileCheck %s + +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 \ +// RUN: -fspv-extension=SPV_EXT_descriptor_heap \ +// RUN: -fspv-extension=SPV_KHR_untyped_pointers \ +// RUN: -fspv-extension=SPV_KHR_ray_query \ +// RUN: -spirv %s | FileCheck %s --check-prefix=OK + +// Verifies: an acceleration structure loaded from ResourceDescriptorHeap is +// rejected when the resource-heap stride was not widened to include the +// acceleration structure descriptor size. +// +// The stride is decided before the code-gen loop (see HandleTranslationUnit) +// and frozen on its first use, so it cannot be widened once an acceleration +// structure heap access is reached. Without the widening this shader would +// silently get max(sizeof(image), sizeof(buffer)), which may be too narrow. +// +// A compute shader is not a ray-tracing stage, so the widening only happens +// when the user lists a ray-tracing/ray-query extension explicitly. The first +// run does not, and must fail; the second one does, and must compile. + +// CHECK: error: acceleration structure loaded from ResourceDescriptorHeap requires the resource heap stride to account for acceleration structure descriptors; compile with -fspv-extension=SPV_KHR_ray_tracing or -fspv-extension=SPV_KHR_ray_query + +// OK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR +// OK-DAG: %[[AccelSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Accel]] + +RWBuffer output : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) { + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + + RayDesc ray; + ray.Origin = float3(0.0, 0.0, 0.0); + ray.Direction = float3(0.0, 0.0, 1.0); + ray.TMin = 0.0; + ray.TMax = 1000.0; + + RayQuery q; + q.TraceRayInline(scene, RAY_FLAG_NONE, 0xff, ray); + bool hit = q.Proceed(); + + output[tid.x] = float4(hit ? 1.0 : 0.0, 0.0, 0.0, 0.0); +} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl new file mode 100644 index 00000000000..f380b4cf95c --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl @@ -0,0 +1,26 @@ +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_NV_ray_tracing -spirv %s | FileCheck %s + +// Verifies: listing SPV_NV_ray_tracing with -fspv-extension widens the +// resource-heap stride to cover acceleration structures, even for a stage that +// is not a ray-tracing stage. The acceleration structure type is declared with +// the NV capability and extension, matching the extension the user listed. + +// CHECK-DAG: OpCapability RayTracingNV +// CHECK-DAG: OpExtension "SPV_NV_ray_tracing" + +// CHECK-DAG: %[[ImgSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %{{[a-zA-Z0-9_]+}} +// CHECK-DAG: %[[BufSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %type_buffer_ext +// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %accelerationStructureNV +// CHECK-DAG: %[[IB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[ImgSz]] %[[BufSz]] +// CHECK-DAG: %[[MaxIB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IB]] %[[ImgSz]] %[[BufSz]] +// CHECK-DAG: %[[IBA:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[MaxIB]] %[[ASSz]] +// CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IBA]] %[[MaxIB]] %[[ASSz]] +// CHECK-DAG: OpDecorateId %{{[a-zA-Z0-9_]+}} ArrayStrideIdEXT %[[Stride]] + +RWBuffer o : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 t : SV_DispatchThreadID) { + Texture2D tx = ResourceDescriptorHeap[1]; + o[t.x] = tx.Load(int3(0, 0, 0)); +} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl new file mode 100644 index 00000000000..e2960ef978b --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl @@ -0,0 +1,25 @@ +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_tracing -spirv %s | FileCheck %s + +// Verifies: listing SPV_KHR_ray_tracing with -fspv-extension widens the +// resource-heap stride to cover acceleration structures, even for a stage that +// is not a ray-tracing stage. + +// CHECK: OpCapability RayTracingKHR + +// CHECK-DAG: %[[ImgSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %{{[a-zA-Z0-9_]+}} +// CHECK-DAG: %[[BufSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %type_buffer_ext +// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %accelerationStructureNV +// CHECK-DAG: %[[IB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[ImgSz]] %[[BufSz]] +// CHECK-DAG: %[[MaxIB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IB]] %[[ImgSz]] %[[BufSz]] +// CHECK-DAG: %[[IBA:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[MaxIB]] %[[ASSz]] +// CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IBA]] %[[MaxIB]] %[[ASSz]] +// CHECK-DAG: OpDecorateId %{{[a-zA-Z0-9_]+}} ArrayStrideIdEXT %[[Stride]] + +RWBuffer output : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) { + Texture2D tex = ResourceDescriptorHeap[0]; + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + output[tid.x] = tex.Load(int3(tid.x, 0, 0)); +} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl new file mode 100644 index 00000000000..be1f37e79e5 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl @@ -0,0 +1,154 @@ +// Verifies that the shared resource-heap array stride covers acceleration +// structure descriptors, i.e. it is +// max(max(sizeof(image), sizeof(buffer)), sizeof(acceleration structure)) +// and that every resource runtime array uses it. +// +// The stride is widened before any code is generated, for either of these +// causes. Each configuration below triggers exactly one of them: +// - an entry point is a ray-tracing stage (lib_6_6 with RT_STAGE, using the +// default extensions); +// - the user listed SPV_KHR_ray_query with -fspv-extension (cs_6_6). +// +// The texture is accessed before the acceleration structure. The stride must +// already cover the acceleration structure when the texture's runtime array is +// created, otherwise that array would use a narrower stride. +// +// Without either cause the stride has only the image and buffer terms; that is +// covered by sm6_6.descriptorheap.ext.array-stride.hlsl. + +// RUN: %dxc -T lib_6_6 -D RT_STAGE -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 -spirv %s \ +// RUN: | FileCheck %s --check-prefixes=CHECK,RT + +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 \ +// RUN: -fspv-extension=SPV_EXT_descriptor_heap \ +// RUN: -fspv-extension=SPV_KHR_untyped_pointers \ +// RUN: -fspv-extension=SPV_KHR_ray_query \ +// RUN: -spirv %s | FileCheck %s + +// RUN: %dxc -T lib_6_6 -D RT_STAGE -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 -spirv %s \ +// RUN: | FileCheck %s --check-prefix=SZRT + +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 \ +// RUN: -fspv-extension=SPV_EXT_descriptor_heap \ +// RUN: -fspv-extension=SPV_KHR_untyped_pointers \ +// RUN: -fspv-extension=SPV_KHR_ray_query \ +// RUN: -spirv %s | FileCheck %s --check-prefixes=SZRQ,NOSAMP + +// Element (descriptor) types. +// CHECK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR + +// Two distinct OpTypeImage types appear in the output: +// +// 1) ImgPlaceholder: a canonical sampled 2D float image, used only as the +// operand of OpConstantSizeOfEXT. All image types have the same descriptor +// size, so one placeholder stands for every image. +// +// 2) TexDesc: the type Texture2D lowers to. It is the element type of +// the texture heap runtime array. +// +// CHECK-DAG: %[[ImgPlaceholder:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 0 0 0 1 Unknown +// CHECK-DAG: %[[TexDesc:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 2 0 0 1 Unknown + +// UBuf is the placeholder for the buffer descriptor size; the shader does not +// declare it. +// CHECK-DAG: %[[UBuf:[a-zA-Z0-9_]+]] = OpTypeBufferEXT Uniform + +// The sampler is only used by the ray-tracing stage shader. +// RT-DAG: %[[Samp:[a-zA-Z0-9_]+]] = OpTypeSampler + +// Heap runtime arrays of the accessed element types. +// CHECK-DAG: %[[AccelArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[Accel]] +// CHECK-DAG: %[[ImgArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[TexDesc]] +// RT-DAG: %[[SampArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[Samp]] + +// Resource stride = max(max(image_size, buffer_size), accel_size). +// CHECK-DAG: %[[ImgSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[ImgPlaceholder]] +// CHECK-DAG: %[[BufSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[UBuf]] +// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Accel]] +// RT-DAG: %[[SampSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Samp]] + +// Three-way max: max(max(img, buf), accel). +// CHECK-DAG: %[[IB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[ImgSz]] %[[BufSz]] +// CHECK-DAG: %[[MaxIB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IB]] %[[ImgSz]] %[[BufSz]] +// CHECK-DAG: %[[IBA:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[MaxIB]] %[[ASSz]] +// CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IBA]] %[[MaxIB]] %[[ASSz]] + +// All resource arrays share the three-way max stride. The texture array is +// created first; if the stride had not been widened by then, its decoration +// would reference the two-way max and the first check below would fail. +// CHECK-DAG: OpDecorateId %[[ImgArr]] ArrayStrideIdEXT %[[Stride]] +// CHECK-DAG: OpDecorateId %[[AccelArr]] ArrayStrideIdEXT %[[Stride]] + +// The sampler heap has its own stride. +// RT-DAG: OpDecorateId %[[SampArr]] ArrayStrideIdEXT %[[SampSz]] + +// The ray-tracing stage run measures image, buffer, acceleration structure and +// sampler descriptors; the ray query run measures the first three. The checks +// above assert each of those queries, so what remains is to rule out extra +// queries, such as one per accessed element type. +// +// The lowered Texture2D type must never be measured: the image placeholder +// stands for every image descriptor. +// SZRT-DAG: %[[SZRT_Tex:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 2 0 0 1 Unknown +// SZRT-NOT: OpConstantSizeOfEXT %uint %[[SZRT_Tex]] +// SZRQ-DAG: %[[SZRQ_Tex:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 2 0 0 1 Unknown +// SZRQ-NOT: OpConstantSizeOfEXT %uint %[[SZRQ_Tex]] + +// The ray query shader uses no sampler, so no sampler size query can be +// emitted for it. +// NOSAMP-NOT: OpTypeSampler + +#ifdef RT_STAGE + +struct Payload { float4 color; }; +struct Attribute { float2 bary; }; + +[shader("closesthit")] +void main(inout Payload payload, in Attribute attr) { + // Ordering stress test: access texture first. + Texture2D tex = ResourceDescriptorHeap[0]; + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + SamplerState samp = SamplerDescriptorHeap[0]; + + float4 color = tex.SampleLevel(samp, float2(0.0, 0.0), 0.0); + payload.color = color; + + RayDesc ray; + ray.Origin = float3(0.0, 0.0, 0.0); + ray.Direction = float3(0.0, 0.0, -1.0); + ray.TMin = 0.0; + ray.TMax = 1000.0; + + Payload child = { color }; + TraceRay(scene, 0x0, 0xff, 0, 1, 0, ray, child); +} + +#else // !RT_STAGE: compute shader using RayQuery + +RWBuffer output : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) { + // Ordering stress test: access texture first. + Texture2D tex = ResourceDescriptorHeap[0]; + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + + RayDesc ray; + ray.Origin = float3(0.0, 0.0, 0.0); + ray.Direction = float3(0.0, 0.0, 1.0); + ray.TMin = 0.0; + ray.TMax = 1000.0; + + RayQuery q; + q.TraceRayInline(scene, RAY_FLAG_NONE, 0xff, ray); + bool hit = q.Proceed(); + + int3 coord = int3(tid.x, 0, 0); + output[tid.x] = tex.Load(coord) + float4(hit ? 1.0 : 0.0, 0.0, 0.0, 0.0); +} + +#endif // RT_STAGE diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl new file mode 100644 index 00000000000..76a79221c3e --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl @@ -0,0 +1,28 @@ +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -spirv %s | FileCheck %s + +// Verifies: the shared resource-heap stride is defined even when the first +// heap access is in an unreachable block. +// +// The stride is built on the first heap access and shared by every resource +// runtime array, so its instructions must not depend on the block of that +// access. Here the first access is dead code after `return`; the second access +// is reachable and reuses the cached stride. + +// CHECK-DAG: %[[ImgBigger:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan +// CHECK-DAG: %[[ResSize:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[ImgBigger]] +// CHECK-DAG: OpDecorateId %{{[a-zA-Z0-9_]+}} ArrayStrideIdEXT %[[ResSize]] + +RWBuffer o : register(u0); +cbuffer C : register(b0) { uint c; }; + +[numthreads(1, 1, 1)] +void main(uint3 t : SV_DispatchThreadID) { + if (c == 1) { + o[0] = 1; + return; + Texture2D dead = ResourceDescriptorHeap[0]; + o[1] = dead.Load(int3(0, 0, 0)); + } + Texture2D tx = ResourceDescriptorHeap[1]; + o[t.x] = tx.Load(int3(0, 0, 0)); +} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl new file mode 100644 index 00000000000..c86608e8d68 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl @@ -0,0 +1,19 @@ +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -spirv %s 2>&1 | FileCheck %s + +// Verifies: a rejected bound/heap assignment that is used as a value is +// diagnosed without crashing the compiler. +// +// The rejected assignment has no SPIR-V value of its own, but the enclosing +// expression, here a method call on the assignment result, still needs one. + +// CHECK: mixing bound and descriptor heap resources in the same variable is not supported +// CHECK-NOT: Internal compiler error + +StructuredBuffer bound : register(t0); +RWBuffer o : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 t : SV_DispatchThreadID) { + StructuredBuffer b = ResourceDescriptorHeap[1]; + o[t.x] = (b = bound).Load(0); +} From 37ed94495b020342fee746aa0799ead1ad4cc760 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Mon, 26 Oct 2026 14:49:05 -0700 Subject: [PATCH 02/13] review: minor clean up --- tools/clang/include/clang/SPIRV/FeatureManager.h | 2 +- tools/clang/include/clang/SPIRV/SpirvBuilder.h | 6 +++--- tools/clang/lib/SPIRV/AstTypeProbe.cpp | 6 ++---- tools/clang/lib/SPIRV/FeatureManager.cpp | 6 +++--- tools/clang/lib/SPIRV/SpirvBuilder.cpp | 2 +- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 11 +++++------ ...orheap.ext.acceleration-structure.alias.error.hlsl | 8 ++++---- 7 files changed, 19 insertions(+), 22 deletions(-) diff --git a/tools/clang/include/clang/SPIRV/FeatureManager.h b/tools/clang/include/clang/SPIRV/FeatureManager.h index 4d0081299f8..9a84b12dfe4 100644 --- a/tools/clang/include/clang/SPIRV/FeatureManager.h +++ b/tools/clang/include/clang/SPIRV/FeatureManager.h @@ -190,7 +190,7 @@ class FeatureManager { DiagnosticsEngine &diags; llvm::SmallBitVector allowedExtensions; - bool explicitExtensionList = false; + bool hasExplicitExtensions = false; spv_target_env targetEnv; std::string targetEnvStr; }; diff --git a/tools/clang/include/clang/SPIRV/SpirvBuilder.h b/tools/clang/include/clang/SPIRV/SpirvBuilder.h index 16198bc7117..285c1deb163 100644 --- a/tools/clang/include/clang/SPIRV/SpirvBuilder.h +++ b/tools/clang/include/clang/SPIRV/SpirvBuilder.h @@ -850,7 +850,7 @@ class SpirvBuilder { /// and cannot change afterwards. A call after that point has no effect, so /// resourceHeapStrideIncludesAccelStruct() stays false and code generation /// must reject acceleration structure heap loads. - void noteResourceHeapHasAccelStruct() { + void registerResourceHeapAccelStruct() { if (!resourceHeapArrayStride) resourceHeapHasAccelStruct = true; } @@ -864,7 +864,7 @@ class SpirvBuilder { /// \brief Returns the ArrayStrideIdEXT operand shared by every resource-heap /// runtime array: the largest descriptor size among images and buffers, and - /// among acceleration structures if noteResourceHeapHasAccelStruct() was + /// among acceleration structures if registerResourceHeapAccelStruct() was /// called first. The value is built from specialization constants because /// descriptor sizes are only known at pipeline creation. SpirvInstruction *getResourceHeapArrayStride(); @@ -997,7 +997,7 @@ class SpirvBuilder { SpirvInstruction *resourceHeapArrayStride = nullptr; SpirvInstruction *samplerHeapArrayStride = nullptr; - /// Set by noteResourceHeapHasAccelStruct(). When true, + /// Set by registerResourceHeapAccelStruct(). When true, /// getResourceHeapArrayStride() includes the acceleration structure /// descriptor size. bool resourceHeapHasAccelStruct = false; diff --git a/tools/clang/lib/SPIRV/AstTypeProbe.cpp b/tools/clang/lib/SPIRV/AstTypeProbe.cpp index 0b821fe5007..2b01ab8071e 100644 --- a/tools/clang/lib/SPIRV/AstTypeProbe.cpp +++ b/tools/clang/lib/SPIRV/AstTypeProbe.cpp @@ -896,10 +896,8 @@ bool isBuffer(QualType type) { } bool isRaytracingAccelerationStructure(QualType type) { - if (const auto *rt = type->getAs()) { - return rt->getDecl()->getName() == "RaytracingAccelerationStructure"; - } - return false; + return hlsl::GetHLSLResourceKind(type) == + hlsl::DXIL::ResourceKind::RTAccelerationStructure; } bool isRWTexture(QualType type) { diff --git a/tools/clang/lib/SPIRV/FeatureManager.cpp b/tools/clang/lib/SPIRV/FeatureManager.cpp index 5bfac224f1a..61331cfcfa0 100644 --- a/tools/clang/lib/SPIRV/FeatureManager.cpp +++ b/tools/clang/lib/SPIRV/FeatureManager.cpp @@ -99,8 +99,8 @@ FeatureManager::FeatureManager(DiagnosticsEngine &de, } targetEnv = *targetEnvOpt; - explicitExtensionList = !opts.allowedExtensions.empty(); - if (opts.allowedExtensions.empty()) { + hasExplicitExtensions = !opts.allowedExtensions.empty(); + if (!hasExplicitExtensions) { // If no explicit extension control from command line, use the default mode: // allowing all extensions that are enabled by default. allowAllKnownExtensions(); @@ -391,7 +391,7 @@ Extension FeatureManager::getAccelerationStructureProvider() const { } bool FeatureManager::hasExplicitAccelerationStructureProvider() const { - return explicitExtensionList && + return hasExplicitExtensions && getAccelerationStructureProvider() != Extension::Unknown; } diff --git a/tools/clang/lib/SPIRV/SpirvBuilder.cpp b/tools/clang/lib/SPIRV/SpirvBuilder.cpp index 6a841a99c89..3217a616eaa 100644 --- a/tools/clang/lib/SPIRV/SpirvBuilder.cpp +++ b/tools/clang/lib/SPIRV/SpirvBuilder.cpp @@ -2070,7 +2070,7 @@ SpirvInstruction *SpirvBuilder::getResourceHeapArrayStride() { // covers all texture resources. // buffer: a Uniform buffer. // accel: the acceleration structure type, included only when - // noteResourceHeapHasAccelStruct() was called. That type needs a + // registerResourceHeapAccelStruct() was called. That type needs a // ray-tracing or ray-query capability, which is always declared // in the cases that make that call. const SpirvType *placeholderImage = context.getImageType( diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index c035662cc75..51b95c12eda 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -854,7 +854,7 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) { featureManager.hasExplicitAccelerationStructureProvider(); if (needsAccelStruct) - spvBuilder.noteResourceHeapHasAccelStruct(); + spvBuilder.registerResourceHeapAccelStruct(); } // Translate all functions reachable from the entry function. @@ -5396,12 +5396,11 @@ bool SpirvEmitter::diagnoseDescriptorHeapAliasMixing(const VarDecl *dstVar, // into, so any reassignment is rejected, including from another heap access. // Image and buffer aliases can be retargeted heap-to-heap; for them only // crossing between bound and heap resources is rejected. - const bool isASReassignment = isASType && wasHeap && srcIsHeap; - const bool mixingDetected = (isASType && wasHeap) || - (srcIsHeap && wasBound) || - (!srcIsHeap && wasHeap); + const bool isASReassignment = isASType && wasHeap; + const bool mixingDetected = + (srcIsHeap && wasBound) || (!srcIsHeap && wasHeap); - if (mixingDetected) { + if (isASReassignment || mixingDetected) { if (isASReassignment) emitError("reassigning a RaytracingAccelerationStructure initialized " "from ResourceDescriptorHeap is not supported with " diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl index e6c926cab2f..164475c942d 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.alias.error.hlsl @@ -1,5 +1,5 @@ -// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=0 %s 2>&1 | FileCheck %s --check-prefix=MIX -// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=1 %s 2>&1 | FileCheck %s --check-prefix=MIX +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=0 %s 2>&1 | FileCheck %s --check-prefix=REASSIGN +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=1 %s 2>&1 | FileCheck %s --check-prefix=REASSIGN // RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=2 %s 2>&1 | FileCheck %s --check-prefix=REASSIGN // RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv -DCASE=3 %s 2>&1 | FileCheck %s --check-prefix=MIX @@ -9,9 +9,9 @@ // // CASE=0 and CASE=1 assign a bound acceleration structure to a heap-initialized // variable, once as a statement and once as a value. CASE=2 reassigns it from -// another heap access, which is also rejected, with a message that does not +// another heap access. All three get the reassignment error, which does not // mention bound resources. CASE=3 assigns a heap acceleration structure to a -// variable that was assigned a bound one before. +// variable that was assigned a bound one before, which is the mixing error. // MIX: error: mixing bound and descriptor heap resources in the same variable is not supported // MIX-NOT: Internal compiler error From 5b519d78f4d5898771ed687e3b5c3485cf505a91 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Mon, 26 Oct 2026 15:42:06 -0700 Subject: [PATCH 03/13] review: refactor capabilities --- docs/SPIR-V.rst | 2 +- tools/clang/lib/SPIRV/CapabilityVisitor.cpp | 34 +++++---------------- tools/clang/lib/SPIRV/CapabilityVisitor.h | 5 --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 7 +++-- 4 files changed, 13 insertions(+), 35 deletions(-) diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index a4739350036..7b9311a2893 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -2194,7 +2194,7 @@ resource, or an assignment inside a loop:: supported with SPV_EXT_descriptor_heap A heap-initialized ``RaytracingAccelerationStructure`` variable cannot be -reassigned at all, even from another heap access:: +reassigned at all, whether from a bound resource or from another heap access:: error: reassigning a RaytracingAccelerationStructure initialized from ResourceDescriptorHeap is not supported with SPV_EXT_descriptor_heap diff --git a/tools/clang/lib/SPIRV/CapabilityVisitor.cpp b/tools/clang/lib/SPIRV/CapabilityVisitor.cpp index 1491826e8f9..7d9860d5b7f 100644 --- a/tools/clang/lib/SPIRV/CapabilityVisitor.cpp +++ b/tools/clang/lib/SPIRV/CapabilityVisitor.cpp @@ -210,7 +210,14 @@ void CapabilityVisitor::addCapabilityForType(const SpirvType *type, } // Acceleration structure type else if (isa(type)) { - addCapabilityForAccelerationStructure(loc); + // SPV_KHR_ray_query and SPV_KHR_ray_tracing are declared for every module + // in visit(SpirvModule *). NV is not, because declaring it there would add + // it to modules that never use the type. Skip it when a KHR extension + // already provides the type, so only one extension is declared. + if (!featureManager.isExtensionEnabled(Extension::KHR_ray_query) && + !featureManager.isExtensionEnabled(Extension::KHR_ray_tracing)) + addExtensionAndCapabilitiesIfEnabled(Extension::NV_ray_tracing, + {spv::Capability::RayTracingNV}); } // Pointer type else if (const auto *ptrType = dyn_cast(type)) { @@ -243,31 +250,6 @@ void CapabilityVisitor::addCapabilityForType(const SpirvType *type, } } -void CapabilityVisitor::addCapabilityForAccelerationStructure( - SourceLocation loc) { - const Extension provider = featureManager.getAccelerationStructureProvider(); - switch (provider) { - case Extension::KHR_ray_query: - addExtension(provider, "acceleration structure type", loc); - addCapability(spv::Capability::RayQueryKHR, loc); - break; - case Extension::KHR_ray_tracing: - addExtension(provider, "acceleration structure type", loc); - addCapability(spv::Capability::RayTracingKHR, loc); - break; - case Extension::NV_ray_tracing: - addExtension(provider, "acceleration structure type", loc); - addCapability(spv::Capability::RayTracingNV, loc); - break; - default: - // No allowed extension provides the type. Report the standard - // "extension not permitted" error instead of emitting an invalid module. - featureManager.requestExtension(Extension::KHR_ray_tracing, - "acceleration structure type", loc); - break; - } -} - bool CapabilityVisitor::visit(SpirvConstantSizeOfEXT *inst) { // The operand type is not the result type, so the generic handling of // instructions does not see it. diff --git a/tools/clang/lib/SPIRV/CapabilityVisitor.h b/tools/clang/lib/SPIRV/CapabilityVisitor.h index 24165f4c91a..2ae648a07e0 100644 --- a/tools/clang/lib/SPIRV/CapabilityVisitor.h +++ b/tools/clang/lib/SPIRV/CapabilityVisitor.h @@ -59,11 +59,6 @@ class CapabilityVisitor : public Visitor { void addCapabilityForType(const SpirvType *, SourceLocation loc, spv::StorageClass sc); - /// Adds the extension and capability that make the acceleration structure - /// type valid, chosen with FeatureManager::getAccelerationStructureProvider. - /// Reports an error if no allowed extension provides the type. - void addCapabilityForAccelerationStructure(SourceLocation loc); - /// Checks that the given extension is a valid extension for the target /// environment (e.g. Vulkan 1.0). And if so, utilizes the SpirvBuilder to add /// the given extension to the SPIR-V module in memory. diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 51b95c12eda..2f3913ac135 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -5393,9 +5393,10 @@ bool SpirvEmitter::diagnoseDescriptorHeapAliasMixing(const VarDecl *dstVar, stateIt->second == DescriptorHeapVarState::Bound; // A heap-initialized acceleration structure is a compile-time name for the // handle loaded by its initializer, with no variable to store a new handle - // into, so any reassignment is rejected, including from another heap access. - // Image and buffer aliases can be retargeted heap-to-heap; for them only - // crossing between bound and heap resources is rejected. + // into, so any reassignment is rejected, whether the new value is a bound + // resource or another heap access. Image and buffer aliases can be + // retargeted heap-to-heap; for them only crossing between bound and heap + // resources is rejected. const bool isASReassignment = isASType && wasHeap; const bool mixingDetected = (srcIsHeap && wasBound) || (!srcIsHeap && wasHeap); From e6c187376bde5e19a36dcff2450489f9375359a1 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Mon, 26 Oct 2026 15:54:21 -0700 Subject: [PATCH 04/13] review: test refactor --- ...stride.accelstruct-nv-ray-tracing-ext.hlsl | 26 ------------------- ...ay-stride.accelstruct-ray-tracing-ext.hlsl | 16 ++++++++---- 2 files changed, 11 insertions(+), 31 deletions(-) delete mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl deleted file mode 100644 index f380b4cf95c..00000000000 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-nv-ray-tracing-ext.hlsl +++ /dev/null @@ -1,26 +0,0 @@ -// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_NV_ray_tracing -spirv %s | FileCheck %s - -// Verifies: listing SPV_NV_ray_tracing with -fspv-extension widens the -// resource-heap stride to cover acceleration structures, even for a stage that -// is not a ray-tracing stage. The acceleration structure type is declared with -// the NV capability and extension, matching the extension the user listed. - -// CHECK-DAG: OpCapability RayTracingNV -// CHECK-DAG: OpExtension "SPV_NV_ray_tracing" - -// CHECK-DAG: %[[ImgSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %{{[a-zA-Z0-9_]+}} -// CHECK-DAG: %[[BufSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %type_buffer_ext -// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %accelerationStructureNV -// CHECK-DAG: %[[IB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[ImgSz]] %[[BufSz]] -// CHECK-DAG: %[[MaxIB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IB]] %[[ImgSz]] %[[BufSz]] -// CHECK-DAG: %[[IBA:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[MaxIB]] %[[ASSz]] -// CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IBA]] %[[MaxIB]] %[[ASSz]] -// CHECK-DAG: OpDecorateId %{{[a-zA-Z0-9_]+}} ArrayStrideIdEXT %[[Stride]] - -RWBuffer o : register(u0); - -[numthreads(1, 1, 1)] -void main(uint3 t : SV_DispatchThreadID) { - Texture2D tx = ResourceDescriptorHeap[1]; - o[t.x] = tx.Load(int3(0, 0, 0)); -} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl index e2960ef978b..962f7311981 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl @@ -1,10 +1,16 @@ -// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_tracing -spirv %s | FileCheck %s +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_tracing -spirv %s | FileCheck %s --check-prefixes=CHECK,KHR +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_NV_ray_tracing -spirv %s | FileCheck %s --check-prefixes=CHECK,NV -// Verifies: listing SPV_KHR_ray_tracing with -fspv-extension widens the -// resource-heap stride to cover acceleration structures, even for a stage that -// is not a ray-tracing stage. +// Verifies: listing SPV_KHR_ray_tracing or SPV_NV_ray_tracing with +// -fspv-extension widens the resource-heap stride to cover acceleration +// structures, even for a stage that is not a ray-tracing stage. The +// acceleration structure type is declared with the capability and extension of +// the listed extension. -// CHECK: OpCapability RayTracingKHR +// KHR-DAG: OpCapability RayTracingKHR +// KHR-DAG: OpExtension "SPV_KHR_ray_tracing" +// NV-DAG: OpCapability RayTracingNV +// NV-DAG: OpExtension "SPV_NV_ray_tracing" // CHECK-DAG: %[[ImgSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %{{[a-zA-Z0-9_]+}} // CHECK-DAG: %[[BufSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %type_buffer_ext From 1bcd19f9369889ed2ae9f9e249ecdb421b24f69a Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Mon, 26 Oct 2026 16:17:59 -0700 Subject: [PATCH 05/13] review: merge stride extension tests --- ...heap.ext.array-stride.accelstruct-ext.hlsl | 36 ++++++ ...ay-stride.accelstruct-ray-tracing-ext.hlsl | 31 ----- ...ptorheap.ext.array-stride.accelstruct.hlsl | 109 ++++-------------- 3 files changed, 59 insertions(+), 117 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl delete mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl new file mode 100644 index 00000000000..3a8e890963c --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl @@ -0,0 +1,36 @@ +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_query -spirv %s | FileCheck %s --check-prefixes=CHECK,RQ +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_tracing -spirv %s | FileCheck %s --check-prefixes=CHECK,KHR +// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_NV_ray_tracing -spirv %s | FileCheck %s --check-prefixes=CHECK,NV + +// Verifies: listing an extension that provides the acceleration structure type +// (SPV_KHR_ray_query, SPV_KHR_ray_tracing or SPV_NV_ray_tracing) with +// -fspv-extension widens the resource-heap stride to cover acceleration +// structures, even for a stage that is not a ray-tracing stage. The type is +// declared with the capability and extension of the listed extension. +// +// The full stride formula is checked in +// sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl; here it is enough that +// the acceleration structure size is the last term of the stride and that the +// texture array, which is accessed first, already uses the widened stride. + +// RQ-DAG: OpCapability RayQueryKHR +// RQ-DAG: OpExtension "SPV_KHR_ray_query" +// KHR-DAG: OpCapability RayTracingKHR +// KHR-DAG: OpExtension "SPV_KHR_ray_tracing" +// NV-DAG: OpCapability RayTracingNV +// NV-DAG: OpExtension "SPV_NV_ray_tracing" + +// CHECK-DAG: %[[TexDesc:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 2 0 0 1 Unknown +// CHECK-DAG: %[[ImgArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[TexDesc]] +// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %accelerationStructureNV +// CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %{{[a-zA-Z0-9_]+}} %{{[a-zA-Z0-9_]+}} %[[ASSz]] +// CHECK-DAG: OpDecorateId %[[ImgArr]] ArrayStrideIdEXT %[[Stride]] + +RWBuffer output : register(u0); + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) { + Texture2D tex = ResourceDescriptorHeap[0]; + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + output[tid.x] = tex.Load(int3(tid.x, 0, 0)); +} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl deleted file mode 100644 index 962f7311981..00000000000 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ray-tracing-ext.hlsl +++ /dev/null @@ -1,31 +0,0 @@ -// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_KHR_ray_tracing -spirv %s | FileCheck %s --check-prefixes=CHECK,KHR -// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -fspv-extension=SPV_EXT_descriptor_heap -fspv-extension=SPV_KHR_untyped_pointers -fspv-extension=SPV_NV_ray_tracing -spirv %s | FileCheck %s --check-prefixes=CHECK,NV - -// Verifies: listing SPV_KHR_ray_tracing or SPV_NV_ray_tracing with -// -fspv-extension widens the resource-heap stride to cover acceleration -// structures, even for a stage that is not a ray-tracing stage. The -// acceleration structure type is declared with the capability and extension of -// the listed extension. - -// KHR-DAG: OpCapability RayTracingKHR -// KHR-DAG: OpExtension "SPV_KHR_ray_tracing" -// NV-DAG: OpCapability RayTracingNV -// NV-DAG: OpExtension "SPV_NV_ray_tracing" - -// CHECK-DAG: %[[ImgSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %{{[a-zA-Z0-9_]+}} -// CHECK-DAG: %[[BufSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %type_buffer_ext -// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %accelerationStructureNV -// CHECK-DAG: %[[IB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[ImgSz]] %[[BufSz]] -// CHECK-DAG: %[[MaxIB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IB]] %[[ImgSz]] %[[BufSz]] -// CHECK-DAG: %[[IBA:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[MaxIB]] %[[ASSz]] -// CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IBA]] %[[MaxIB]] %[[ASSz]] -// CHECK-DAG: OpDecorateId %{{[a-zA-Z0-9_]+}} ArrayStrideIdEXT %[[Stride]] - -RWBuffer output : register(u0); - -[numthreads(1, 1, 1)] -void main(uint3 tid : SV_DispatchThreadID) { - Texture2D tex = ResourceDescriptorHeap[0]; - RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; - output[tid.x] = tex.Load(int3(tid.x, 0, 0)); -} diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl index be1f37e79e5..d5297fe1ceb 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl @@ -1,42 +1,23 @@ +// RUN: %dxc -T lib_6_6 -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -spirv %s | FileCheck %s + // Verifies that the shared resource-heap array stride covers acceleration // structure descriptors, i.e. it is // max(max(sizeof(image), sizeof(buffer)), sizeof(acceleration structure)) // and that every resource runtime array uses it. // -// The stride is widened before any code is generated, for either of these -// causes. Each configuration below triggers exactly one of them: -// - an entry point is a ray-tracing stage (lib_6_6 with RT_STAGE, using the -// default extensions); -// - the user listed SPV_KHR_ray_query with -fspv-extension (cs_6_6). -// -// The texture is accessed before the acceleration structure. The stride must -// already cover the acceleration structure when the texture's runtime array is -// created, otherwise that array would use a narrower stride. +// A ray-tracing stage widens the stride with the default extensions. The stride +// is a heap layout contract decided before any code is generated, so it does not +// depend on what the shader loads. Widening from a listed extension, and the +// rejection of an acceleration structure load when the stride was not widened, +// are covered by sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl and +// sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl. Without +// any cause the stride has only the image and buffer terms; that is covered by +// sm6_6.descriptorheap.ext.array-stride.hlsl. // -// Without either cause the stride has only the image and buffer terms; that is -// covered by sm6_6.descriptorheap.ext.array-stride.hlsl. - -// RUN: %dxc -T lib_6_6 -D RT_STAGE -fspv-use-descriptor-heap \ -// RUN: -fspv-target-env=vulkan1.3 -spirv %s \ -// RUN: | FileCheck %s --check-prefixes=CHECK,RT - -// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ -// RUN: -fspv-target-env=vulkan1.3 \ -// RUN: -fspv-extension=SPV_EXT_descriptor_heap \ -// RUN: -fspv-extension=SPV_KHR_untyped_pointers \ -// RUN: -fspv-extension=SPV_KHR_ray_query \ -// RUN: -spirv %s | FileCheck %s - -// RUN: %dxc -T lib_6_6 -D RT_STAGE -fspv-use-descriptor-heap \ -// RUN: -fspv-target-env=vulkan1.3 -spirv %s \ -// RUN: | FileCheck %s --check-prefix=SZRT - -// RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ -// RUN: -fspv-target-env=vulkan1.3 \ -// RUN: -fspv-extension=SPV_EXT_descriptor_heap \ -// RUN: -fspv-extension=SPV_KHR_untyped_pointers \ -// RUN: -fspv-extension=SPV_KHR_ray_query \ -// RUN: -spirv %s | FileCheck %s --check-prefixes=SZRQ,NOSAMP +// The shader accesses a texture, then an acceleration structure, then a +// sampler. The stride must already cover the acceleration structure when the +// texture's runtime array is created, otherwise that array would use a narrower +// stride. // Element (descriptor) types. // CHECK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR @@ -56,20 +37,18 @@ // UBuf is the placeholder for the buffer descriptor size; the shader does not // declare it. // CHECK-DAG: %[[UBuf:[a-zA-Z0-9_]+]] = OpTypeBufferEXT Uniform - -// The sampler is only used by the ray-tracing stage shader. -// RT-DAG: %[[Samp:[a-zA-Z0-9_]+]] = OpTypeSampler +// CHECK-DAG: %[[Samp:[a-zA-Z0-9_]+]] = OpTypeSampler // Heap runtime arrays of the accessed element types. // CHECK-DAG: %[[AccelArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[Accel]] // CHECK-DAG: %[[ImgArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[TexDesc]] -// RT-DAG: %[[SampArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[Samp]] +// CHECK-DAG: %[[SampArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[Samp]] // Resource stride = max(max(image_size, buffer_size), accel_size). // CHECK-DAG: %[[ImgSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[ImgPlaceholder]] // CHECK-DAG: %[[BufSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[UBuf]] // CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Accel]] -// RT-DAG: %[[SampSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Samp]] +// CHECK-DAG: %[[SampSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Samp]] // Three-way max: max(max(img, buf), accel). // CHECK-DAG: %[[IB:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[ImgSz]] %[[BufSz]] @@ -79,37 +58,21 @@ // All resource arrays share the three-way max stride. The texture array is // created first; if the stride had not been widened by then, its decoration -// would reference the two-way max and the first check below would fail. +// would reference the two-way max and the first check below would fail. The +// sampler heap has its own stride. // CHECK-DAG: OpDecorateId %[[ImgArr]] ArrayStrideIdEXT %[[Stride]] // CHECK-DAG: OpDecorateId %[[AccelArr]] ArrayStrideIdEXT %[[Stride]] +// CHECK-DAG: OpDecorateId %[[SampArr]] ArrayStrideIdEXT %[[SampSz]] -// The sampler heap has its own stride. -// RT-DAG: OpDecorateId %[[SampArr]] ArrayStrideIdEXT %[[SampSz]] - -// The ray-tracing stage run measures image, buffer, acceleration structure and -// sampler descriptors; the ray query run measures the first three. The checks -// above assert each of those queries, so what remains is to rule out extra -// queries, such as one per accessed element type. -// -// The lowered Texture2D type must never be measured: the image placeholder -// stands for every image descriptor. -// SZRT-DAG: %[[SZRT_Tex:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 2 0 0 1 Unknown -// SZRT-NOT: OpConstantSizeOfEXT %uint %[[SZRT_Tex]] -// SZRQ-DAG: %[[SZRQ_Tex:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 2 0 0 1 Unknown -// SZRQ-NOT: OpConstantSizeOfEXT %uint %[[SZRQ_Tex]] - -// The ray query shader uses no sampler, so no sampler size query can be -// emitted for it. -// NOSAMP-NOT: OpTypeSampler - -#ifdef RT_STAGE +// That no other size query is emitted per accessed type is checked once, in +// sm6_6.descriptorheap.ext.array-stride.hlsl, because the stride is built by +// the same cached code whatever it contains. struct Payload { float4 color; }; struct Attribute { float2 bary; }; [shader("closesthit")] void main(inout Payload payload, in Attribute attr) { - // Ordering stress test: access texture first. Texture2D tex = ResourceDescriptorHeap[0]; RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; SamplerState samp = SamplerDescriptorHeap[0]; @@ -126,29 +89,3 @@ void main(inout Payload payload, in Attribute attr) { Payload child = { color }; TraceRay(scene, 0x0, 0xff, 0, 1, 0, ray, child); } - -#else // !RT_STAGE: compute shader using RayQuery - -RWBuffer output : register(u0); - -[numthreads(1, 1, 1)] -void main(uint3 tid : SV_DispatchThreadID) { - // Ordering stress test: access texture first. - Texture2D tex = ResourceDescriptorHeap[0]; - RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; - - RayDesc ray; - ray.Origin = float3(0.0, 0.0, 0.0); - ray.Direction = float3(0.0, 0.0, 1.0); - ray.TMin = 0.0; - ray.TMax = 1000.0; - - RayQuery q; - q.TraceRayInline(scene, RAY_FLAG_NONE, 0xff, ray); - bool hit = q.Proceed(); - - int3 coord = int3(tid.x, 0, 0); - output[tid.x] = tex.Load(coord) + float4(hit ? 1.0 : 0.0, 0.0, 0.0, 0.0); -} - -#endif // RT_STAGE From e31344fa7630a93bb8876512df783a88ed88debd Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Tue, 27 Oct 2026 11:46:16 -0700 Subject: [PATCH 06/13] review: very minor documentation formatting fix --- docs/SPIR-V.rst | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index 7b9311a2893..4eef55d2dba 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -2188,13 +2188,17 @@ initializer. These aliases are compile-time state and do not model control-flow merges, so DXC rejects a variable that holds both a bound resource and a heap descriptor, whether through a conditional assignment, a reassignment back to a bound -resource, or an assignment inside a loop:: +resource, or an assignment inside a loop: + +.. code:: text error: mixing bound and descriptor heap resources in the same variable is not supported with SPV_EXT_descriptor_heap A heap-initialized ``RaytracingAccelerationStructure`` variable cannot be -reassigned at all, whether from a bound resource or from another heap access:: +reassigned at all, whether from a bound resource or from another heap access: + +.. code:: text error: reassigning a RaytracingAccelerationStructure initialized from ResourceDescriptorHeap is not supported with SPV_EXT_descriptor_heap From 416689916127fcc4b2ac1dd1bc19c43aa3dcc24d Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Tue, 27 Oct 2026 14:55:24 -0700 Subject: [PATCH 07/13] review: clarify documentation on first heap access --- docs/ReleaseNotes.md | 6 ++- docs/SPIR-V.rst | 40 ++++++++----------- .../clang/include/clang/SPIRV/SpirvBuilder.h | 4 +- tools/clang/lib/SPIRV/CapabilityVisitor.cpp | 8 ++-- tools/clang/lib/SPIRV/SpirvBuilder.cpp | 10 ++--- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 21 ++++------ ...t.acceleration-structure.stride.error.hlsl | 19 ++++----- ...heap.ext.array-stride.accelstruct-ext.hlsl | 5 ++- ...ptorheap.ext.array-stride.accelstruct.hlsl | 24 +++++------ ...sm6_6.descriptorheap.ext.array-stride.hlsl | 9 +++-- ...array-stride.unreachable-first-access.hlsl | 12 +++--- 11 files changed, 74 insertions(+), 84 deletions(-) diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index b3ef691493d..77d1193c573 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -26,8 +26,10 @@ The included licenses apply to the following files: `ConstantBuffer`, `TextureBuffer`, and RW variants) via `SPV_KHR_untyped_pointers`. Requires `-fspv-target-env=vulkan1.3`. - Extended `-fspv-use-descriptor-heap` to support `RaytracingAccelerationStructure` - loaded from `ResourceDescriptorHeap`. Outside ray-tracing stages this requires - `-fspv-extension=SPV_KHR_ray_tracing` or `-fspv-extension=SPV_KHR_ray_query`. + loaded from `ResourceDescriptorHeap`. Outside ray-tracing stages, list + `-fspv-extension=SPV_KHR_ray_query`, `SPV_KHR_ray_tracing` or + `SPV_NV_ray_tracing` so the resource heap stride covers acceleration structure + descriptors; otherwise compilation fails. Place release notes for the upcoming release below this line and remove this line upon naming the release. Refer to previous for appropriate section names. diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index 4eef55d2dba..45021716027 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -2229,41 +2229,35 @@ rather than a literal ``ArrayStride``, because descriptor sizes are not known until pipeline creation. The shared value is built from ``OpConstantSizeOfEXT`` and ``OpSpecConstantOp`` and evaluates to ``max(sizeof(image_descriptor), sizeof(buffer_descriptor))``. The sampler heap -carries its own ``ArrayStrideIdEXT`` equal to ``sizeof(sampler_descriptor)``, -regardless of resource heap contents. +carries its own ``ArrayStrideIdEXT`` equal to ``sizeof(sampler_descriptor)``. The ``OpConstantSizeOfEXT`` operands are placeholder types chosen only for their descriptor class. All image types report the same descriptor size, so the placeholder is a plain sampled 2D float image and bears no relation to the image types the shader actually uses; a module will normally contain both the -placeholder type and the distinct image types its heap accesses lower to. The -stride value is built once and cached on first use. +placeholder type and the distinct image types its heap accesses lower to. -When acceleration structure descriptors may appear on the resource heap, the -formula expands to a three-way max +The stride is a property of the heap layout, not of the shader's loads. DXC +decides it before translating any function, from the shader stage and the +``-fspv-extension`` list. When acceleration structure descriptors may be on the +resource heap, the formula expands to a three-way max ``max(max(sizeof(image_descriptor), sizeof(buffer_descriptor)), sizeof(acceleration_structure))``. - -Because the stride is cached on first use, this decision is committed before -code generation and is **not** based on whether the shader actually performs an -acceleration structure heap load: a ray-tracing shader that only heap-loads a -texture still gets the three-way max. The widening happens when either +A ray-tracing shader that only heap-loads a texture still gets the three-way +max. DXC adds the acceleration structure term when either - any entry point is a ray-tracing stage, or -- the user explicitly passed ``-fspv-extension=SPV_KHR_ray_query``, +- the user lists ``-fspv-extension=SPV_KHR_ray_query``, ``-fspv-extension=SPV_KHR_ray_tracing`` or ``-fspv-extension=SPV_NV_ray_tracing``. -The second condition requires an explicit ``-fspv-extension`` flag. In the -default extension mode DXC allows the ray-tracing and ray-query extensions -implicitly, but that does not widen the stride. The acceleration structure type -added by widening is declared with the capability of the listed extension -(``RayQueryKHR``, ``RayTracingKHR`` or ``RayTracingNV``); if several are listed, -``SPV_KHR_ray_query`` is preferred, then ``SPV_KHR_ray_tracing``, then -``SPV_NV_ray_tracing``. Because the stride cannot be widened once it has been -built, a shader that is not a ray-tracing stage and loads a -``RaytracingAccelerationStructure`` from ``ResourceDescriptorHeap`` without an -explicit ray extension flag is rejected rather than given a stride that may be -too narrow. +Extensions allowed by default do not satisfy the second condition. The +acceleration structure type added by widening is declared with the capability +of the listed extension (``RayQueryKHR``, ``RayTracingKHR`` or +``RayTracingNV``); if several are listed, ``SPV_KHR_ray_query`` is preferred, +then ``SPV_KHR_ray_tracing``, then ``SPV_NV_ray_tracing``. A shader that is not +a ray-tracing stage and heap-loads a ``RaytracingAccelerationStructure`` +without such an extension is rejected, because its stride would not cover the +acceleration structure descriptor. HLSL Expressions ================ diff --git a/tools/clang/include/clang/SPIRV/SpirvBuilder.h b/tools/clang/include/clang/SPIRV/SpirvBuilder.h index 285c1deb163..fa25f77a9c6 100644 --- a/tools/clang/include/clang/SPIRV/SpirvBuilder.h +++ b/tools/clang/include/clang/SPIRV/SpirvBuilder.h @@ -846,8 +846,8 @@ class SpirvBuilder { /// \brief Records that acceleration structures may be loaded from the /// resource heap, so the resource-heap stride must cover their descriptor. /// - /// The stride is built once, on the first getResourceHeapArrayStride() call, - /// and cannot change afterwards. A call after that point has no effect, so + /// Must be called before getResourceHeapArrayStride(), which builds and + /// caches the stride. A later call is ignored, so /// resourceHeapStrideIncludesAccelStruct() stays false and code generation /// must reject acceleration structure heap loads. void registerResourceHeapAccelStruct() { diff --git a/tools/clang/lib/SPIRV/CapabilityVisitor.cpp b/tools/clang/lib/SPIRV/CapabilityVisitor.cpp index 7d9860d5b7f..db297d18693 100644 --- a/tools/clang/lib/SPIRV/CapabilityVisitor.cpp +++ b/tools/clang/lib/SPIRV/CapabilityVisitor.cpp @@ -210,10 +210,10 @@ void CapabilityVisitor::addCapabilityForType(const SpirvType *type, } // Acceleration structure type else if (isa(type)) { - // SPV_KHR_ray_query and SPV_KHR_ray_tracing are declared for every module - // in visit(SpirvModule *). NV is not, because declaring it there would add - // it to modules that never use the type. Skip it when a KHR extension - // already provides the type, so only one extension is declared. + // visit(SpirvModule *) declares SPV_KHR_ray_query when enabled, otherwise + // SPV_KHR_ray_tracing. NV is not declared there because that would add it + // to modules that never use the type, so declare it here, and only when no + // KHR extension provides the type. if (!featureManager.isExtensionEnabled(Extension::KHR_ray_query) && !featureManager.isExtensionEnabled(Extension::KHR_ray_tracing)) addExtensionAndCapabilitiesIfEnabled(Extension::NV_ray_tracing, diff --git a/tools/clang/lib/SPIRV/SpirvBuilder.cpp b/tools/clang/lib/SPIRV/SpirvBuilder.cpp index 3217a616eaa..1f26f742e35 100644 --- a/tools/clang/lib/SPIRV/SpirvBuilder.cpp +++ b/tools/clang/lib/SPIRV/SpirvBuilder.cpp @@ -2070,9 +2070,8 @@ SpirvInstruction *SpirvBuilder::getResourceHeapArrayStride() { // covers all texture resources. // buffer: a Uniform buffer. // accel: the acceleration structure type, included only when - // registerResourceHeapAccelStruct() was called. That type needs a - // ray-tracing or ray-query capability, which is always declared - // in the cases that make that call. + // registerResourceHeapAccelStruct() was called. CapabilityVisitor + // declares its capability from the type. const SpirvType *placeholderImage = context.getImageType( context.getFloatType(32), spv::Dim::Dim2D, ImageType::WithDepth::No, /*arrayed*/ false, /*ms*/ false, ImageType::WithSampler::Yes, @@ -2080,9 +2079,8 @@ SpirvInstruction *SpirvBuilder::getResourceHeapArrayStride() { const SpirvType *placeholderBuffer = context.getBufferEXTType(spv::StorageClass::Uniform); - // The stride is cached and shared by every heap access in the module, so it - // is built from module-level instructions. Instructions owned by the block of - // the first access would be lost if that block were unreachable. + // The stride is shared by the whole module, so it is built from module-level + // instructions rather than in the current block, which may be unreachable. SpirvInstruction *imageSize = getConstantSizeOfEXT(placeholderImage); SpirvInstruction *bufferSize = getConstantSizeOfEXT(placeholderBuffer); SpirvInstruction *imageIsBigger = createSpecConstantBinaryOp( diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 2f3913ac135..6995c602185 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -818,11 +818,11 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) { return; } - // Decide whether the resource-heap stride must cover acceleration structures. - // The stride is built on the first descriptor heap access and cannot change - // afterwards, so this has to be decided before any function is translated. - // It is decided up front rather than per access, so a shader that merely - // might load an acceleration structure from the heap is covered. + // Decide whether the resource-heap stride covers acceleration structure + // descriptors. The stride is a heap-layout contract shared by every resource + // runtime array, so it is decided here from the stage and the extension list, + // not from what the shader loads. SpirvBuilder builds it lazily and then + // fixes it, so this must run before any function is translated. // // The stride covers acceleration structures if any of these holds: // - An entry point is a ray-tracing stage. @@ -835,11 +835,6 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) { // The capability that makes the acceleration structure type valid is // declared from the type itself, using the same provider (see // CapabilityVisitor), so it is always consistent with this decision. - // - // Heap accesses in global initializers are translated after this point, so - // they cannot build the stride early. If one ever did, the call below would - // have no effect and acceleration structure heap loads would be rejected - // rather than given a stride that may be too narrow. if (spirvOptions.useDescriptorHeap) { bool needsAccelStruct = false; @@ -7324,9 +7319,9 @@ SpirvEmitter::doCXXOperatorCallExpr(const CXXOperatorCallExpr *expr, return nullptr; } - // The stride is fixed before any function is translated and cannot be - // widened here. Reject the load rather than use a stride that may be - // too narrow for an acceleration structure descriptor. + // The stride was decided without an acceleration structure term (not a + // ray-tracing stage, no ray extension listed). Reject the load: the + // heap would be indexed with a stride that may be too narrow. if (isRaytracingAccelerationStructure(resourceType) && !spvBuilder.resourceHeapStrideIncludesAccelStruct()) { emitError("acceleration structure loaded from ResourceDescriptorHeap " diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl index 5c94f614b9f..faa97fb968f 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl @@ -12,19 +12,20 @@ // rejected when the resource-heap stride was not widened to include the // acceleration structure descriptor size. // -// The stride is decided before the code-gen loop (see HandleTranslationUnit) -// and frozen on its first use, so it cannot be widened once an acceleration -// structure heap access is reached. Without the widening this shader would -// silently get max(sizeof(image), sizeof(buffer)), which may be too narrow. +// The stride is decided in HandleTranslationUnit from the stage and the +// -fspv-extension list. Without a ray-tracing stage or a listed provider +// extension it has no acceleration structure term, so the load is rejected: +// max(sizeof(image), sizeof(buffer)) may be too narrow. // -// A compute shader is not a ray-tracing stage, so the widening only happens -// when the user lists a ray-tracing/ray-query extension explicitly. The first -// run does not, and must fail; the second one does, and must compile. +// A compute shader is not a ray-tracing stage. The first run lists no provider +// extension and must fail. The second lists SPV_KHR_ray_query, which this +// shader also needs for RayQuery (SPV_KHR_ray_tracing alone does not provide +// the RayQueryKHR capability), and must compile. // CHECK: error: acceleration structure loaded from ResourceDescriptorHeap requires the resource heap stride to account for acceleration structure descriptors; compile with -fspv-extension=SPV_KHR_ray_tracing or -fspv-extension=SPV_KHR_ray_query -// OK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR -// OK-DAG: %[[AccelSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Accel]] +// OK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR +// OK-DAG: OpConstantSizeOfEXT %uint %[[Accel]] RWBuffer output : register(u0); diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl index 3a8e890963c..82078dd29ce 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl @@ -11,7 +11,7 @@ // The full stride formula is checked in // sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl; here it is enough that // the acceleration structure size is the last term of the stride and that the -// texture array, which is accessed first, already uses the widened stride. +// texture runtime array uses the widened stride. // RQ-DAG: OpCapability RayQueryKHR // RQ-DAG: OpExtension "SPV_KHR_ray_query" @@ -22,7 +22,8 @@ // CHECK-DAG: %[[TexDesc:[a-zA-Z0-9_]+]] = OpTypeImage %float 2D 2 0 0 1 Unknown // CHECK-DAG: %[[ImgArr:[a-zA-Z0-9_]+]] = OpTypeRuntimeArray %[[TexDesc]] -// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %accelerationStructureNV +// CHECK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR +// CHECK-DAG: %[[ASSz:[a-zA-Z0-9_]+]] = OpConstantSizeOfEXT %uint %[[Accel]] // CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %{{[a-zA-Z0-9_]+}} %{{[a-zA-Z0-9_]+}} %[[ASSz]] // CHECK-DAG: OpDecorateId %[[ImgArr]] ArrayStrideIdEXT %[[Stride]] diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl index d5297fe1ceb..bb9366bfb78 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct.hlsl @@ -6,17 +6,17 @@ // and that every resource runtime array uses it. // // A ray-tracing stage widens the stride with the default extensions. The stride -// is a heap layout contract decided before any code is generated, so it does not -// depend on what the shader loads. Widening from a listed extension, and the -// rejection of an acceleration structure load when the stride was not widened, -// are covered by sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl and +// is a heap layout contract decided from the stage and the extension list, so +// it does not depend on what the shader loads. Widening from a listed +// extension, and the rejection of an acceleration structure load when the +// stride was not widened, are covered by +// sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl and // sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl. Without -// any cause the stride has only the image and buffer terms; that is covered by -// sm6_6.descriptorheap.ext.array-stride.hlsl. +// either cause the stride has only the image and buffer terms; that is covered +// by sm6_6.descriptorheap.ext.array-stride.hlsl. // -// The shader accesses a texture, then an acceleration structure, then a -// sampler. The stride must already cover the acceleration structure when the -// texture's runtime array is created, otherwise that array would use a narrower +// The shader loads a texture, an acceleration structure and a sampler. Every +// resource runtime array, including the texture array, must use the widened // stride. // Element (descriptor) types. @@ -56,10 +56,8 @@ // CHECK-DAG: %[[IBA:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan %[[MaxIB]] %[[ASSz]] // CHECK-DAG: %[[Stride:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[IBA]] %[[MaxIB]] %[[ASSz]] -// All resource arrays share the three-way max stride. The texture array is -// created first; if the stride had not been widened by then, its decoration -// would reference the two-way max and the first check below would fail. The -// sampler heap has its own stride. +// All resource arrays share the three-way max stride, the texture array +// included. The sampler heap has its own stride. // CHECK-DAG: OpDecorateId %[[ImgArr]] ArrayStrideIdEXT %[[Stride]] // CHECK-DAG: OpDecorateId %[[AccelArr]] ArrayStrideIdEXT %[[Stride]] // CHECK-DAG: OpDecorateId %[[SampArr]] ArrayStrideIdEXT %[[SampSz]] diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.hlsl index d3722a3a451..3cca4afff02 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.hlsl @@ -12,8 +12,9 @@ // bufferDescriptorSize); textures lower to OpTypeImage, so image/buffer // covers all relevant HLSL resource kinds. // -// NOTE: This covers the case with no RaytracingAccelerationStructures. -// When present, the stride formula expands to a three-way max: +// NOTE: This covers the default case: no ray-tracing stage and no ray extension +// listed. Otherwise the stride gains a third term, as covered by +// sm6_6.descriptorheap.ext.array-stride.accelstruct*.hlsl: // max(max(sizeof(image), sizeof(buffer)), sizeof(accelerationStructure)). // // Both sizes are driver defined and known only at pipeline creation time, so @@ -69,8 +70,8 @@ // buffer, and sampler, each asserted by a CHECK-DAG above. Regression test: no // per-element or per-access behavior; otherwise the StorageBuffer descriptor // would be measured too, adding a fourth query. -// COUNT-DAG: %[[SBBufDesc:[a-zA-Z0-9_]+]] = OpTypeBufferEXT StorageBuffer{{$}} -// COUNT-NOT: OpConstantSizeOfEXT %uint %[[SBBufDesc]] +// COUNT-DAG: %[[SBBufDesc:[a-zA-Z0-9_]+]] = OpTypeBufferEXT StorageBuffer{{$}} +// COUNT-NOT: OpConstantSizeOfEXT %uint %[[SBBufDesc]] struct Constants { uint value; diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl index 76a79221c3e..ee686e656bc 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.unreachable-first-access.hlsl @@ -1,12 +1,12 @@ // RUN: %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap -fspv-target-env=vulkan1.3 -spirv %s | FileCheck %s -// Verifies: the shared resource-heap stride is defined even when the first -// heap access is in an unreachable block. +// Verifies: the resource-heap stride is valid SPIR-V even when the first heap +// access in the source is in an unreachable block. // -// The stride is built on the first heap access and shared by every resource -// runtime array, so its instructions must not depend on the block of that -// access. Here the first access is dead code after `return`; the second access -// is reachable and reuses the cached stride. +// The stride is shared by the whole module, so its instructions are emitted at +// module level rather than in the block of any access. Here the first access +// is dead code after `return`; the second is reachable and uses the same +// stride. // CHECK-DAG: %[[ImgBigger:[a-zA-Z0-9_]+]] = OpSpecConstantOp %bool UGreaterThan // CHECK-DAG: %[[ResSize:[a-zA-Z0-9_]+]] = OpSpecConstantOp %uint Select %[[ImgBigger]] From 219f302c6ca8f84e5a9822fa4acb9cf57e67e9d1 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Tue, 27 Oct 2026 16:09:17 -0700 Subject: [PATCH 08/13] review: fix OpSpecConstantOp to go to constants --- tools/clang/include/clang/SPIRV/SpirvBuilder.h | 4 ++++ tools/clang/include/clang/SPIRV/SpirvModule.h | 4 ++-- tools/clang/lib/SPIRV/EmitVisitor.cpp | 2 ++ tools/clang/lib/SPIRV/EmitVisitor.h | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/clang/include/clang/SPIRV/SpirvBuilder.h b/tools/clang/include/clang/SPIRV/SpirvBuilder.h index fa25f77a9c6..11fa34d97be 100644 --- a/tools/clang/include/clang/SPIRV/SpirvBuilder.h +++ b/tools/clang/include/clang/SPIRV/SpirvBuilder.h @@ -231,6 +231,8 @@ class SpirvBuilder { SpirvInstruction *lhs, SpirvInstruction *rhs, SourceLocation loc, SourceRange range = {}); + /// \brief Creates a binary OpSpecConstantOp at module level, not in the + /// current basic block. SpirvSpecConstantBinaryOp *createSpecConstantBinaryOp(spv::Op op, QualType resultType, SpirvInstruction *lhs, @@ -838,6 +840,8 @@ class SpirvBuilder { /// integer. SpirvConstant *getConstantSizeOfEXT(const SpirvType *operandType); + /// \brief Creates a ternary OpSpecConstantOp at module level, not in the + /// current basic block. SpirvSpecConstantTernaryOp * createSpecConstantTernaryOp(spv::Op op, QualType resultType, SpirvInstruction *op1, SpirvInstruction *op2, diff --git a/tools/clang/include/clang/SPIRV/SpirvModule.h b/tools/clang/include/clang/SPIRV/SpirvModule.h index db82efcf0c9..31fb5ce96ec 100644 --- a/tools/clang/include/clang/SPIRV/SpirvModule.h +++ b/tools/clang/include/clang/SPIRV/SpirvModule.h @@ -149,8 +149,8 @@ class SpirvModule { // Adds a constant to the module. void addConstant(SpirvConstant *); - // Adds an OpSpecConstantOp to the module. Operands must be added before - // their users. + // Adds an OpSpecConstantOp to the module. These are emitted after all + // constants, in insertion order. void addSpecConstantOp(SpirvInstruction *); // Adds an Undef to the module. diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index 7b6a2c53bcd..5e4b5c44b95 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -1430,6 +1430,8 @@ bool EmitVisitor::visit(SpirvSelect *inst) { return true; } +// OpSpecConstantOp is a module-scope instruction. All three forms below are +// emitted with the types and constants, never into mainBinary (function bodies). bool EmitVisitor::visit(SpirvSpecConstantTernaryOp *inst) { initInstruction(inst); curInst.push_back(inst->getResultTypeId()); diff --git a/tools/clang/lib/SPIRV/EmitVisitor.h b/tools/clang/lib/SPIRV/EmitVisitor.h index d3c97c6281f..72e587dca40 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.h +++ b/tools/clang/lib/SPIRV/EmitVisitor.h @@ -463,7 +463,7 @@ class EmitVisitor : public Visitor { std::vector annotationsBinary; // All forward pointer type declaration instructions std::vector fwdDeclBinary; - // All other type and constant instructions + // All other type and constant instructions, including OpSpecConstantOp std::vector typeConstantBinary; // All global variable declarations (all OpVariable instructions whose Storage // Class is not Function) From 4f078ea34cdf32b8b2a393242efb3015bfdd6d2e Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Tue, 27 Oct 2026 16:22:51 -0700 Subject: [PATCH 09/13] clang-format --- tools/clang/lib/SPIRV/EmitVisitor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index 5e4b5c44b95..1c22005eace 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -1431,7 +1431,7 @@ bool EmitVisitor::visit(SpirvSelect *inst) { } // OpSpecConstantOp is a module-scope instruction. All three forms below are -// emitted with the types and constants, never into mainBinary (function bodies). +// emitted with the types and constants. bool EmitVisitor::visit(SpirvSpecConstantTernaryOp *inst) { initInstruction(inst); curInst.push_back(inst->getResultTypeId()); From 6843fc5e1ced0167a6f05b51b9ae114a8d27756a Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Tue, 27 Oct 2026 20:34:34 -0700 Subject: [PATCH 10/13] skip debug line for module level OpSpecConstantOp --- tools/clang/lib/SPIRV/EmitVisitor.cpp | 13 ++++++++++--- .../CodeGenSPIRV/vk.spec-constant.debug.hlsl | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/vk.spec-constant.debug.hlsl diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index 1c22005eace..f6cd3240c51 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -460,9 +460,16 @@ void EmitVisitor::initInstruction(SpirvInstruction *inst) { if (auto *var = dyn_cast(inst)) isGlobalVar = var->getStorageClass() != spv::StorageClass::Function; const auto op = inst->getopcode(); - emitDebugLine(op, inst->getSourceLocation(), inst->getSourceRange(), - isGlobalVar ? &globalVarsBinary : &mainBinary, - isa(inst)); + + // OpSpecConstantOp is module-scope: it is emitted with the types and + // constants, before the debug sources and any function. + const bool isSpecConstantOp = isa(inst) || + isa(inst) || + isa(inst); + if (!isSpecConstantOp) + emitDebugLine(op, inst->getSourceLocation(), inst->getSourceRange(), + isGlobalVar ? &globalVarsBinary : &mainBinary, + isa(inst)); // Initialize the current instruction for emitting. curInst.clear(); diff --git a/tools/clang/test/CodeGenSPIRV/vk.spec-constant.debug.hlsl b/tools/clang/test/CodeGenSPIRV/vk.spec-constant.debug.hlsl new file mode 100644 index 00000000000..d3accd1bfa0 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.spec-constant.debug.hlsl @@ -0,0 +1,19 @@ +// RUN: %dxc -T ps_6_0 -E main -fspv-debug=line -spirv %s | FileCheck %s +// RUN: %dxc -T ps_6_0 -E main -fspv-debug=vulkan-with-source -spirv %s | FileCheck %s + +// Verifies: arithmetic on a vk::constant_id value compiles with debug info. +// +// OpSpecConstantOp is a module-scope instruction. It is visited before the +// debug sources and the functions, so no line information may be emitted for +// it. With Vulkan debug info a DebugLine would reference a source id that has +// not been assigned yet, and compilation fails. + +// CHECK: [[mul:%[0-9]+]] = OpSpecConstantOp %int IMul %n %int_2 +// CHECK: OpIAdd %int [[mul]] %int_1 + +[[vk::constant_id(1)]] const int n = 4; + +float4 main() : SV_Target { + int m = n * 2 + 1; + return float4(m, 0, 0, 1); +} From 77b49287dbc73611472b67f69cb4105c0a7e6105 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Wed, 28 Oct 2026 10:47:06 -0700 Subject: [PATCH 11/13] register placeholder after failed heap AS alias lowering --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 9 +++++++++ ...ptorheap.ext.acceleration-structure.stride.error.hlsl | 5 +++++ ....descriptorheap.ext.array-stride.accelstruct-ext.hlsl | 1 - 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 6995c602185..97cf8020cec 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -2090,9 +2090,18 @@ bool SpirvEmitter::tryToCreateDescriptorHeapAlias(const VarDecl *decl, declIdMapper.registerFnVarAlias(decl, initVal); descriptorHeapVarState[decl] = DescriptorHeapVarState::Heap; } else { + // The initializer could not be lowered and has been diagnosed (here or + // while evaluating it). Register a placeholder so that later uses of the + // variable do not hit the "unregistered decl" fatal error, which would + // bury the real diagnostic. Mixed keeps later assignments to it from + // being diagnosed again. The error fails the compilation, so the + // placeholder is never emitted. emitError("cannot create descriptor heap acceleration structure alias " "from initializer", init->getExprLoc()); + declIdMapper.registerFnVarAlias(decl, + spvBuilder.getUndef(decl->getType())); + descriptorHeapVarState[decl] = DescriptorHeapVarState::Mixed; } return true; } diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl index faa97fb968f..84c2cd40dbc 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.stride.error.hlsl @@ -24,6 +24,11 @@ // CHECK: error: acceleration structure loaded from ResourceDescriptorHeap requires the resource heap stride to account for acceleration structure descriptors; compile with -fspv-extension=SPV_KHR_ray_tracing or -fspv-extension=SPV_KHR_ray_query +// The variable initialized from the rejected load must not cascade into a +// fatal error that hides the diagnostic above. +// CHECK-NOT: unregistered decl +// CHECK-NOT: fatal error + // OK-DAG: %[[Accel:[a-zA-Z0-9_]+]] = OpTypeAccelerationStructureKHR // OK-DAG: OpConstantSizeOfEXT %uint %[[Accel]] diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl index 82078dd29ce..2e870d6065d 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.array-stride.accelstruct-ext.hlsl @@ -32,6 +32,5 @@ RWBuffer output : register(u0); [numthreads(1, 1, 1)] void main(uint3 tid : SV_DispatchThreadID) { Texture2D tex = ResourceDescriptorHeap[0]; - RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; output[tid.x] = tex.Load(int3(tid.x, 0, 0)); } From 52d13ec6a42bda2c6757e1555f7eea6b4c397315 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Fri, 30 Oct 2026 11:30:30 -0700 Subject: [PATCH 12/13] Avoid Debug-only assert in mixed-alias assign-value test --- ...descriptorheap.ext.mixed-alias-assign-value.error.hlsl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl index c86608e8d68..6b3f0ecc65c 100644 --- a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.mixed-alias-assign-value.error.hlsl @@ -4,7 +4,10 @@ // diagnosed without crashing the compiler. // // The rejected assignment has no SPIR-V value of its own, but the enclosing -// expression, here a method call on the assignment result, still needs one. +// expression, here the initializer of another variable, still needs one. +// The assignment is used as an initializer rather than as the object of a +// method call: a method call on a parenthesized assignment trips the +// assertion getValueKind() == VK_LValue in Debug builds. // CHECK: mixing bound and descriptor heap resources in the same variable is not supported // CHECK-NOT: Internal compiler error @@ -15,5 +18,6 @@ RWBuffer o : register(u0); [numthreads(1, 1, 1)] void main(uint3 t : SV_DispatchThreadID) { StructuredBuffer b = ResourceDescriptorHeap[1]; - o[t.x] = (b = bound).Load(0); + StructuredBuffer c = (b = bound); + o[t.x] = c.Load(0); } From d582350ff69d462914a49eeb05f4376d23b75dd5 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov Date: Mon, 2 Nov 2026 13:09:48 -0800 Subject: [PATCH 13/13] Reject out/inout parameters bound to heap-initialized acceleration structures --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 26 +++++++++++ ...cceleration-structure.out-param.error.hlsl | 43 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.out-param.error.hlsl diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 97cf8020cec..983735439ef 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -3601,6 +3601,32 @@ SpirvInstruction *SpirvEmitter::processCall(const CallExpr *callExpr) { return spvBuilder.getUndef(retTy); } + // A heap-initialized acceleration structure is a compile-time alias for + // the loaded handle, with no variable to write to. Resource out/inout + // arguments are never copied back, so a callee's assignment would be lost. + if (spirvOptions.useDescriptorHeap && + (param->hasAttr() || param->hasAttr()) && + isRaytracingAccelerationStructure(paramType.getNonReferenceType())) { + const auto *argRef = dyn_cast(arg->IgnoreParens()); + const auto *argVar = + argRef ? dyn_cast(argRef->getDecl()) : nullptr; + const auto stateIt = argVar ? descriptorHeapVarState.find(argVar) + : descriptorHeapVarState.end(); + if (stateIt != descriptorHeapVarState.end() && + stateIt->second == DescriptorHeapVarState::Heap) { + emitError("passing a RaytracingAccelerationStructure initialized from " + "ResourceDescriptorHeap to an out/inout parameter is not " + "supported with SPV_EXT_descriptor_heap", + arg->getLocStart()); + + // Return a typed undef so downstream consumers remain valid + QualType retTy = callExpr->getCallReturnType(astContext); + if (retTy->isVoidType()) + return nullptr; + return spvBuilder.getUndef(retTy); + } + } + // Get the evaluation info if this argument is referencing some variable // *as a whole*, in which case we can avoid creating the temporary variable // for it if it can act as out parameter. diff --git a/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.out-param.error.hlsl b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.out-param.error.hlsl new file mode 100644 index 00000000000..c98577d8ce1 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/sm6_6.descriptorheap.ext.acceleration-structure.out-param.error.hlsl @@ -0,0 +1,43 @@ +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 \ +// RUN: -fspv-extension=SPV_EXT_descriptor_heap \ +// RUN: -fspv-extension=SPV_KHR_untyped_pointers \ +// RUN: -fspv-extension=SPV_KHR_ray_query \ +// RUN: -DOUT -spirv %s 2>&1 | FileCheck %s + +// RUN: not %dxc -T cs_6_6 -E main -fspv-use-descriptor-heap \ +// RUN: -fspv-target-env=vulkan1.3 \ +// RUN: -fspv-extension=SPV_EXT_descriptor_heap \ +// RUN: -fspv-extension=SPV_KHR_untyped_pointers \ +// RUN: -fspv-extension=SPV_KHR_ray_query \ +// RUN: -DINOUT -spirv %s 2>&1 | FileCheck %s + +// Verifies: a RaytracingAccelerationStructure initialized from +// ResourceDescriptorHeap is only a compile-time name for the loaded handle. +// Passing it to an out/inout parameter would drop any assignment the callee +// makes (resource parameters are not copied back), so it is rejected instead +// of silently tracing against the stale heap handle. + +// CHECK: error: passing a RaytracingAccelerationStructure initialized from ResourceDescriptorHeap to an out/inout parameter is not supported with SPV_EXT_descriptor_heap +// CHECK-NOT: fatal error + +RaytracingAccelerationStructure bound : register(t0); +RWBuffer output : register(u0); + +#if defined(OUT) +#define QUALIFIER out +#else +#define QUALIFIER inout +#endif + +void setIt(QUALIFIER RaytracingAccelerationStructure o, + RaytracingAccelerationStructure b) { + o = b; +} + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) { + RaytracingAccelerationStructure scene = ResourceDescriptorHeap[1]; + setIt(scene, bound); + output[tid.x] = float4(1.0, 0.0, 0.0, 0.0); +}