From cb893e4e17fa1769c9c3541a9cfa004ce34d4643 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Tue, 13 May 2025 12:51:05 -0400 Subject: [PATCH 1/2] [SPIRV] Treat vk::Spirv*Type as opaque when reconstructing It is possible to have two struct types in spir-v that are the same except for the decorations. Sometimes we have to reconstruct the value from one type to another. In the case of a vk::SpirvType, we do not know anything about the type, so this should not happen. When trying to reconstuct the value, we should simply return the original value. Fixes #6963 --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 11 +++++++---- .../CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 7337a33b01..c5c1cd5db3 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -7081,14 +7081,17 @@ SpirvInstruction *SpirvEmitter::reconstructValue(SpirvInstruction *srcVal, // Structs if (const auto *recordType = valType->getAs()) { - assert(recordType->isStructureType()); - if (isTypeInVkNamespace(recordType) && - recordType->getDecl()->getName().equals("BufferPointer")) { - // Uniquely among structs, vk::BufferPointer lowers to a pointer type. + (recordType->getDecl()->getName().equals("BufferPointer") || + recordType->getDecl()->getName().equals("SpirvType")) || + recordType->getDecl()->getName().equals("SpirvOpaqueType")) { + // vk::BufferPointer lowers to a pointer type. No need to reconstruct + // the value. The vk::Spirv*Type should be treated an opaque type. All we + // can do is leave it the same. return srcVal; } + assert(recordType->isStructureType()); LowerTypeVisitor lowerTypeVisitor(astContext, spvContext, spirvOptions, spvBuilder); const StructType *spirvStructType = diff --git a/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl b/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl index 7be0713e48..c2892cfc29 100644 --- a/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl +++ b/tools/clang/test/CodeGenSPIRV/intrinsics.vkrawbufferload.hlsl @@ -12,7 +12,16 @@ struct BufferData { float3 v; }; +using MyInt = vk::SpirvType< + /*spv::OpTypeInt*/21, + 1,1, // size and alignment + vk::Literal >, // bits + vk::Literal > // signed +>; + uint64_t Address; + +[[vk::ext_capability(/* Int16 */ 22)]] float4 main() : SV_Target0 { // CHECK: [[addr:%[0-9]+]] = OpLoad %ulong // CHECK-NEXT: [[buf:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_float [[addr]] @@ -50,5 +59,10 @@ float4 main() : SV_Target0 { // CHECK-NEXT: [[load:%[0-9]+]] = OpLoad %BufferData_0 [[buf]] Aligned 4 d = vk::RawBufferLoad(0); + // CHECK: [[buf:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_spirvIntrinsicType %ulong_0 + // CHECK-NEXT: [[load:%[0-9]+]] = OpLoad %spirvIntrinsicType [[buf]] Aligned 4 + // CHECK-NEXT: OpStore %mi [[load]] + MyInt mi = vk::RawBufferLoad(0); + return float4(w.x, x, y, z); } From 2b3ced52c31346c81e77fd507b4df51284f98509 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Wed, 14 May 2025 09:45:15 -0400 Subject: [PATCH 2/2] Fix misplaced ) --- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index c5c1cd5db3..4aeaceeb5a 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -7082,9 +7082,9 @@ SpirvInstruction *SpirvEmitter::reconstructValue(SpirvInstruction *srcVal, // Structs if (const auto *recordType = valType->getAs()) { if (isTypeInVkNamespace(recordType) && - (recordType->getDecl()->getName().equals("BufferPointer") || - recordType->getDecl()->getName().equals("SpirvType")) || - recordType->getDecl()->getName().equals("SpirvOpaqueType")) { + (recordType->getDecl()->getName().equals("BufferPointer") || + recordType->getDecl()->getName().equals("SpirvType") || + recordType->getDecl()->getName().equals("SpirvOpaqueType"))) { // vk::BufferPointer lowers to a pointer type. No need to reconstruct // the value. The vk::Spirv*Type should be treated an opaque type. All we // can do is leave it the same.