From 3cf68d5def02a6a057f8960eecd496cfcc119ffb Mon Sep 17 00:00:00 2001 From: Panagiotis Christopoulos Charitos Date: Tue, 23 May 2023 17:22:37 +0200 Subject: [PATCH] Correct the validation of vk::image_format. Fixes #5189 (#5190) * Correct the validation of vk::image_format. Fixes #5189 * - Fix a bug where the image format doesn't propagate from OpTypeArray to OpTypeImage - Add a unit test that tests the vk::image_format on arrays --- tools/clang/include/clang/Basic/Attr.td | 30 ++++++++++++++++++- tools/clang/lib/SPIRV/LowerTypeVisitor.cpp | 10 +++++++ .../vk.attribute.image-format.arrays.hlsl | 17 +++++++++++ .../unittests/SPIRV/CodeGenSpirvTest.cpp | 4 +++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tools/clang/test/CodeGenSPIRV/vk.attribute.image-format.arrays.hlsl diff --git a/tools/clang/include/clang/Basic/Attr.td b/tools/clang/include/clang/Basic/Attr.td index 612f8cff2..5612256e8 100644 --- a/tools/clang/include/clang/Basic/Attr.td +++ b/tools/clang/include/clang/Basic/Attr.td @@ -977,6 +977,23 @@ def RWTexture S->getType()->getAs()->getDecl()->getName() == "RWTexture3D")}]>; +// Global variable of array of "RWTexture" type +def ArrayOfRWTexture + : SubsetSubject< + Var, [{S->hasGlobalStorage() && S->getType()->getAsArrayTypeUnsafe() && + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs() && + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl() && + (S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl()->getName() == + "RWTexture1D" || + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl()->getName() == + "RWTexture1DArray" || + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl()->getName() == + "RWTexture2D" || + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl()->getName() == + "RWTexture2DArray" || + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl()->getName() == + "RWTexture3D")}]>; + // Global variable with "[RW]Buffer" type def Buffer : SubsetSubject< @@ -987,6 +1004,17 @@ def Buffer S->getType()->getAs()->getDecl()->getName() == "RWBuffer")}]>; +// Global variable or array of "[RW]Buffer" type +def ArrayOfBuffer + : SubsetSubject< + Var, [{S->hasGlobalStorage() && S->getType()->getAsArrayTypeUnsafe() && + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs() && + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl() && + (S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl()->getName() == + "Buffer" || + S->getType()->getAsArrayTypeUnsafe()->getElementType()->getAs()->getDecl()->getName() == + "RWBuffer")}]>; + // Global variable with "Texture" or "SamplerState" type def TextureOrSampler : SubsetSubject< @@ -1115,7 +1143,7 @@ def VKCombinedImageSampler : InheritableAttr { def VKImageFormat : InheritableAttr { let Spellings = [CXX11<"vk", "image_format">]; - let Subjects = SubjectList<[RWTexture, Buffer], + let Subjects = SubjectList<[RWTexture, ArrayOfRWTexture, Buffer, ArrayOfBuffer], ErrorDiag, "ExpectedRWTextureOrBuffer">; let Args = [EnumArgument<"ImageFormat", "ImageFormatType", ["unknown", "rgba32f", "rgba16f", "r32f", "rgba8", "rgba8snorm", diff --git a/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp b/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp index 417ff394c..e19d7f4a7 100644 --- a/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp +++ b/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp @@ -199,6 +199,16 @@ bool LowerTypeVisitor::visitInstruction(SpirvInstruction *instr) { if (const auto *imageType = dyn_cast(resultType)) { resultType = spvContext.getImageType(imageType, vkImgFeatures.format); instr->setResultType(resultType); + } else if (const auto *arrayType = dyn_cast(resultType)) { + if (const auto *imageType = + dyn_cast(arrayType->getElementType())) { + auto newImgType = + spvContext.getImageType(imageType, vkImgFeatures.format); + resultType = spvContext.getArrayType(newImgType, + arrayType->getElementCount(), + arrayType->getStride()); + instr->setResultType(resultType); + } } } } diff --git a/tools/clang/test/CodeGenSPIRV/vk.attribute.image-format.arrays.hlsl b/tools/clang/test/CodeGenSPIRV/vk.attribute.image-format.arrays.hlsl new file mode 100644 index 000000000..625ecc2da --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.attribute.image-format.arrays.hlsl @@ -0,0 +1,17 @@ +// RUN: %dxc -T cs_6_0 -E main + +// CHECK: OpTypeImage %float Buffer 2 0 0 2 Rgba16f +[[vk::image_format("rgba16f")]] +RWBuffer RWBuf[2]; + +// CHECK: OpTypeImage %float Buffer 2 0 0 1 Rgba16ui +[[vk::image_format("rgba16ui")]] +Buffer Buf[2]; + +//CHECK: OpTypeImage %float 2D 2 0 0 2 Rgba16f +[[vk::image_format("rgba16f")]] +RWTexture2D Tex[2]; + +[numthreads(1, 1, 1)] +void main() { +} diff --git a/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp index 57310ee03..ceacc19cc 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp @@ -1935,6 +1935,10 @@ TEST_F(FileTest, VulkanAttributeImageFormatO3) { TEST_F(FileTest, VulkanAttributeImageFormatSimple) { runFileTest("vk.attribute.image-format.simple.hlsl", Expect::Success); } +TEST_F(FileTest, VulkanAttributeImageFormatArray) { + runFileTest("vk.attribute.image-format.arrays.hlsl", Expect::Success, + /*runValidation*/ false); +} TEST_F(FileTest, VulkanCLOptionInvertYVS) { runFileTest("vk.cloption.invert-y.vs.hlsl");