From 07a1c4ebfabb8a747beff62463f592b9a11bc8e8 Mon Sep 17 00:00:00 2001 From: Natalie Chouinard Date: Tue, 6 Feb 2024 10:22:27 -0500 Subject: [PATCH] [SPIR-V] Allow SubpassInput without InputAttachmentIndex (#6240) Vulkan allows SubpassInputs without an input attachment index specified, so the error when it is missing has been removed and tests have been updated. As far as I can tell, there aren't earlier Vulkan or SPIR-V versions where the presence of the the input attachment index was explicitly required, so allowing it to be omitted in all cases. Also added a test to verify that this fixes #2808 at the same time. Fixes #6238, #2808 --- docs/SPIR-V.rst | 6 +++-- tools/clang/lib/SPIRV/SpirvEmitter.cpp | 10 ------- .../vk.subpass-input.binding.hlsl | 7 +++++ .../test/CodeGenSPIRV/vk.subpass-input.hlsl | 26 ++++++++++++------- .../vk.subpass-input.missing-attr.error.hlsl | 15 ----------- 5 files changed, 28 insertions(+), 36 deletions(-) delete mode 100644 tools/clang/test/CodeGenSPIRV/vk.subpass-input.missing-attr.error.hlsl diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index 0484a91d5..0f0e29e86 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -213,8 +213,10 @@ For example: [[vk::input_attachment_index(i)]] SubpassInput input; -An ``vk::input_attachment_index`` of ``i`` selects the ith entry in the input -pass list. (See Vulkan API spec for more information.) +A ``vk::input_attachment_index`` of ``i`` selects the ith entry in the input +pass list. A subpass input without a ``vk::input_attachment_index`` will be +associated with the depth/stencil attachment. (See Vulkan API spec for more +information.) Push constants ~~~~~~~~~~~~~~ diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 1b970a3a6..8ce1d0fda 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -1560,16 +1560,6 @@ void SpirvEmitter::doFunctionDecl(const FunctionDecl *decl) { bool SpirvEmitter::validateVKAttributes(const NamedDecl *decl) { bool success = true; - if (const auto *varDecl = dyn_cast(decl)) { - const auto varType = varDecl->getType(); - if ((isSubpassInput(varType) || isSubpassInputMS(varType)) && - !varDecl->hasAttr()) { - emitError("missing vk::input_attachment_index attribute", - varDecl->getLocation()); - success = false; - } - } - if (decl->getAttr()) { if (!decl->isExternallyVisible()) { emitError("SubpassInput(MS) must be externally visible", diff --git a/tools/clang/test/CodeGenSPIRV/vk.subpass-input.binding.hlsl b/tools/clang/test/CodeGenSPIRV/vk.subpass-input.binding.hlsl index 3c9d6044c..210bcd530 100644 --- a/tools/clang/test/CodeGenSPIRV/vk.subpass-input.binding.hlsl +++ b/tools/clang/test/CodeGenSPIRV/vk.subpass-input.binding.hlsl @@ -4,6 +4,8 @@ // CHECK: OpDecorate %SI1 InputAttachmentIndex 1 // CHECK: OpDecorate %SI2 InputAttachmentIndex 2 +// CHECK-NOT: OpDecorate %SI3 InputAttachmentIndex + // CHECK: OpDecorate %SI1 DescriptorSet 0 // CHECK: OpDecorate %SI1 Binding 5 @@ -13,6 +15,9 @@ // CHECK: OpDecorate %SI0 DescriptorSet 0 // CHECK: OpDecorate %SI0 Binding 0 +// CHECK: OpDecorate %SI3 DescriptorSet 0 +// CHECK: OpDecorate %SI3 Binding 1 + [[vk::input_attachment_index(0)]] SubpassInput SI0; @@ -22,6 +27,8 @@ SubpassInput SI1; [[vk::input_attachment_index(2), vk::binding(5, 3)]] SubpassInput SI2; +SubpassInput SI3; + void main() { } diff --git a/tools/clang/test/CodeGenSPIRV/vk.subpass-input.hlsl b/tools/clang/test/CodeGenSPIRV/vk.subpass-input.hlsl index 42b48c671..354c7bc52 100644 --- a/tools/clang/test/CodeGenSPIRV/vk.subpass-input.hlsl +++ b/tools/clang/test/CodeGenSPIRV/vk.subpass-input.hlsl @@ -22,24 +22,28 @@ // CHECK: %type_subpass_image_4 = OpTypeImage %int SubpassData 2 0 1 2 Unknown // CHECK: %_ptr_UniformConstant_type_subpass_image_4 = OpTypePointer UniformConstant %type_subpass_image_4 -// CHCK: %SI_f4 = OpVariable %_ptr_UniformConstant_type_subpass_image UniformConstant +// CHECK: %SI_f4 = OpVariable %_ptr_UniformConstant_type_subpass_image UniformConstant [[vk::input_attachment_index(0)]] SubpassInput SI_f4; -// CHCK: %SI_i3 = OpVariable %_ptr_UniformConstant_type_subpass_image_0 UniformConstant +// CHECK: %SI_i3 = OpVariable %_ptr_UniformConstant_type_subpass_image_0 UniformConstant [[vk::input_attachment_index(1)]] SubpassInput SI_i3; -// CHCK: %SI_u2 = OpVariable %_ptr_UniformConstant_type_subpass_image_1 UniformConstant +// CHECK: %SI_u2 = OpVariable %_ptr_UniformConstant_type_subpass_image_1 UniformConstant [[vk::input_attachment_index(2)]] SubpassInput SI_u2; -// CHCK: %SI_f1 = OpVariable %_ptr_UniformConstant_type_subpass_image UniformConstant +// CHECK: %SI_f1 = OpVariable %_ptr_UniformConstant_type_subpass_image UniformConstant [[vk::input_attachment_index(3)]] SubpassInput SI_f1; -// CHCK: %SIMS_u4 = OpVariable %_ptr_UniformConstant_type_subpass_image_2 UniformConstant +// CHECK: %SIMS_u4 = OpVariable %_ptr_UniformConstant_type_subpass_image_2 UniformConstant [[vk::input_attachment_index(10)]] SubpassInputMS SIMS_u4; -// CHCK: %SIMS_f3 = OpVariable %_ptr_UniformConstant_type_subpass_image_3 UniformConstant +// CHECK: %SIMS_f3 = OpVariable %_ptr_UniformConstant_type_subpass_image_3 UniformConstant [[vk::input_attachment_index(11)]] SubpassInputMS SIMS_f3; -// CHCK: %SIMS_i2 = OpVariable %_ptr_UniformConstant_type_subpass_image_4 UniformConstant +// CHECK: %SIMS_i2 = OpVariable %_ptr_UniformConstant_type_subpass_image_4 UniformConstant [[vk::input_attachment_index(12)]] SubpassInputMS SIMS_i2; -// CHCK: %SIMS_u1 = OpVariable %_ptr_UniformConstant_type_subpass_image_2 UniformConstant +// CHECK: %SIMS_u1 = OpVariable %_ptr_UniformConstant_type_subpass_image_2 UniformConstant [[vk::input_attachment_index(13)]] SubpassInputMS SIMS_u1; +float4 ReadSourceFromTile(SubpassInput spi) { + return spi.SubpassLoad() ; +} + float4 main() : SV_Target { // CHECK: [[img:%[0-9]+]] = OpLoad %type_subpass_image %SI_f4 // CHECK-NEXT: [[texel:%[0-9]+]] = OpImageRead %v4float [[img]] [[v2i00]] None @@ -60,6 +64,10 @@ float4 main() : SV_Target { // CHECK-NEXT: [[val_1:%[0-9]+]] = OpCompositeExtract %float [[texel_2]] 0 // CHECK-NEXT: OpStore %v3 [[val_1]] float v3 = SI_f1.SubpassLoad(); +// CHECK: [[val_2:%[0-9]+]] = OpFunctionCall %v4float %ReadSourceFromTile %param_var_spi +// CHECK-NEXT: OpStore %v4 [[val_2]] + SubpassInput si = SI_f4; + float4 v4 = ReadSourceFromTile(si); // CHECK: [[img_3:%[0-9]+]] = OpLoad %type_subpass_image_2 %SIMS_u4 // CHECK-NEXT: [[texel_3:%[0-9]+]] = OpImageRead %v4uint [[img_3]] [[v2i00]] Sample %int_1 @@ -81,5 +89,5 @@ float4 main() : SV_Target { // CHECK-NEXT: OpStore %v13 [[val_4]] uint v13 = SIMS_u1.SubpassLoad(4); - return v0.x + v1.y + v2.x + v3 + v10.x + v11.y + v12.x + v13; + return v0.x + v1.y + v2.x + v3 + v4.x + v10.x + v11.y + v12.x + v13; } diff --git a/tools/clang/test/CodeGenSPIRV/vk.subpass-input.missing-attr.error.hlsl b/tools/clang/test/CodeGenSPIRV/vk.subpass-input.missing-attr.error.hlsl deleted file mode 100644 index 28a51efa7..000000000 --- a/tools/clang/test/CodeGenSPIRV/vk.subpass-input.missing-attr.error.hlsl +++ /dev/null @@ -1,15 +0,0 @@ -// RUN: not %dxc -T ps_6_0 -E main -fcgl %s -spirv 2>&1 | FileCheck %s - -struct S { - float a; - float2 b; - float c; -}; - -SubpassInput SI0; // error - -void main() { - -} - -// CHECK: :9:14: error: missing vk::input_attachment_index attribute