From 1e4f53ab2de355296de690583bd26818264b25ff Mon Sep 17 00:00:00 2001 From: alan-baker Date: Mon, 15 Apr 2024 11:39:23 -0400 Subject: [PATCH] Prevent duplicate SPIR-V decorations (#3570) * Update SPIRV-Tools * https://github.com/KhronosGroup/SPIRV-Tools/pull/5641 added validation that caught errors * Modified glslang to prevent duplicate Restrict and Coherent decorations * Modify createConstructor to avoid adding duplicate RelaxedPrecision decorations when generating a scalar --- SPIRV/GlslangToSpv.cpp | 10 ++++++---- SPIRV/SpvBuilder.cpp | 9 ++++++--- Test/baseResults/spv.bufferhandle13.frag.out | 1 - .../spv.ext.ClosestHitShader_Subgroup.rchit.out | 2 -- Test/baseResults/spv.memoryQualifier.frag.out | 1 - Test/baseResults/spv.nvAtomicFp16Vec.frag.out | 14 -------------- Test/baseResults/vk.relaxed.frag.out | 3 --- Test/baseResults/vk.relaxed.link1.frag.out | 3 --- Test/baseResults/vk.relaxed.stagelink.vert.out | 6 ------ known_good.json | 4 ++-- 10 files changed, 14 insertions(+), 39 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 5be9f37e..1f56c234 100755 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -398,11 +398,11 @@ void TranslateMemoryDecoration(const glslang::TQualifier& qualifier, std::vector bool useVulkanMemoryModel) { if (!useVulkanMemoryModel) { - if (qualifier.isCoherent()) - memory.push_back(spv::DecorationCoherent); if (qualifier.isVolatile()) { memory.push_back(spv::DecorationVolatile); memory.push_back(spv::DecorationCoherent); + } else if (qualifier.isCoherent()) { + memory.push_back(spv::DecorationCoherent); } } if (qualifier.isRestrict()) @@ -5480,8 +5480,10 @@ void TGlslangToSpvTraverser::makeFunctions(const glslang::TIntermSequence& glslF // memory and use RestrictPointer/AliasedPointer. if (originalParam(type.getQualifier().storage, type, false) || !writableParam(type.getQualifier().storage)) { - decorations.push_back(type.getQualifier().isRestrict() ? spv::DecorationRestrict : - spv::DecorationAliased); + // TranslateMemoryDecoration added Restrict decoration already. + if (!type.getQualifier().isRestrict()) { + decorations.push_back(spv::DecorationAliased); + } } else { decorations.push_back(type.getQualifier().isRestrict() ? spv::DecorationRestrictPointerEXT : spv::DecorationAliasedPointerEXT); diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp index 84058a73..6613655a 100644 --- a/SPIRV/SpvBuilder.cpp +++ b/SPIRV/SpvBuilder.cpp @@ -3409,10 +3409,13 @@ Id Builder::createConstructor(Decoration precision, const std::vector& sourc } // If the result is a vector, make it from the gathered constituents. - if (constituents.size() > 0) + if (constituents.size() > 0) { result = createCompositeConstruct(resultTypeId, constituents); - - return setPrecision(result, precision); + return setPrecision(result, precision); + } else { + // Precision was set when generating this component. + return result; + } } // Comments in header diff --git a/Test/baseResults/spv.bufferhandle13.frag.out b/Test/baseResults/spv.bufferhandle13.frag.out index dd430896..84eb884c 100644 --- a/Test/baseResults/spv.bufferhandle13.frag.out +++ b/Test/baseResults/spv.bufferhandle13.frag.out @@ -38,7 +38,6 @@ spv.bufferhandle13.frag Decorate 10(y) Aliased Decorate 15(y) DecorationAliasedPointerEXT Decorate 18(y) Restrict - Decorate 18(y) Restrict Decorate 21(y) Restrict Decorate 21(y) DecorationRestrictPointerEXT Decorate 34(a) DecorationAliasedPointerEXT diff --git a/Test/baseResults/spv.ext.ClosestHitShader_Subgroup.rchit.out b/Test/baseResults/spv.ext.ClosestHitShader_Subgroup.rchit.out index e5b62d7c..d586ffb4 100644 --- a/Test/baseResults/spv.ext.ClosestHitShader_Subgroup.rchit.out +++ b/Test/baseResults/spv.ext.ClosestHitShader_Subgroup.rchit.out @@ -43,11 +43,9 @@ spv.ext.ClosestHitShader_Subgroup.rchit Decorate 42 RelaxedPrecision Decorate 43(gl_SubgroupGtMask) BuiltIn SubgroupGtMaskKHR Decorate 46 RelaxedPrecision - Decorate 46 RelaxedPrecision Decorate 47 RelaxedPrecision Decorate 48(gl_SubgroupLeMask) BuiltIn SubgroupLeMaskKHR Decorate 51 RelaxedPrecision - Decorate 51 RelaxedPrecision Decorate 52 RelaxedPrecision Decorate 53(gl_SubGroupLtMaskARB) BuiltIn SubgroupLtMaskKHR Decorate 59 RelaxedPrecision diff --git a/Test/baseResults/spv.memoryQualifier.frag.out b/Test/baseResults/spv.memoryQualifier.frag.out index e0a5207f..47d63cfd 100644 --- a/Test/baseResults/spv.memoryQualifier.frag.out +++ b/Test/baseResults/spv.memoryQualifier.frag.out @@ -48,7 +48,6 @@ Validation failed Decorate 44(iCube) NonReadable MemberDecorate 49(Data) 0 Offset 0 MemberDecorate 49(Data) 1 Offset 8 - MemberDecorate 50(Buffer) 0 Coherent MemberDecorate 50(Buffer) 0 Volatile MemberDecorate 50(Buffer) 0 Coherent MemberDecorate 50(Buffer) 0 Offset 0 diff --git a/Test/baseResults/spv.nvAtomicFp16Vec.frag.out b/Test/baseResults/spv.nvAtomicFp16Vec.frag.out index 6d5f6da1..3486cf3d 100644 --- a/Test/baseResults/spv.nvAtomicFp16Vec.frag.out +++ b/Test/baseResults/spv.nvAtomicFp16Vec.frag.out @@ -51,72 +51,58 @@ spv.nvAtomicFp16Vec.frag Decorate 11(buf) Binding 0 Decorate 74(fimage1D) DescriptorSet 0 Decorate 74(fimage1D) Binding 0 - Decorate 74(fimage1D) Coherent Decorate 74(fimage1D) Volatile Decorate 74(fimage1D) Coherent Decorate 85(fimage1DArray) DescriptorSet 0 Decorate 85(fimage1DArray) Binding 1 - Decorate 85(fimage1DArray) Coherent Decorate 85(fimage1DArray) Volatile Decorate 85(fimage1DArray) Coherent Decorate 97(fimage2D) DescriptorSet 0 Decorate 97(fimage2D) Binding 2 - Decorate 97(fimage2D) Coherent Decorate 97(fimage2D) Volatile Decorate 97(fimage2D) Coherent Decorate 107(fimage2DArray) DescriptorSet 0 Decorate 107(fimage2DArray) Binding 3 - Decorate 107(fimage2DArray) Coherent Decorate 107(fimage2DArray) Volatile Decorate 107(fimage2DArray) Coherent Decorate 119(fimageCube) DescriptorSet 0 Decorate 119(fimageCube) Binding 5 - Decorate 119(fimageCube) Coherent Decorate 119(fimageCube) Volatile Decorate 119(fimageCube) Coherent Decorate 129(fimageCubeArray) DescriptorSet 0 Decorate 129(fimageCubeArray) Binding 6 - Decorate 129(fimageCubeArray) Coherent Decorate 129(fimageCubeArray) Volatile Decorate 129(fimageCubeArray) Coherent Decorate 139(fimage3D) DescriptorSet 0 Decorate 139(fimage3D) Binding 9 - Decorate 139(fimage3D) Coherent Decorate 139(fimage3D) Volatile Decorate 139(fimage3D) Coherent Decorate 299(fimage1Dv4) DescriptorSet 0 Decorate 299(fimage1Dv4) Binding 10 - Decorate 299(fimage1Dv4) Coherent Decorate 299(fimage1Dv4) Volatile Decorate 299(fimage1Dv4) Coherent Decorate 310(fimage1DArrayv4) DescriptorSet 0 Decorate 310(fimage1DArrayv4) Binding 11 - Decorate 310(fimage1DArrayv4) Coherent Decorate 310(fimage1DArrayv4) Volatile Decorate 310(fimage1DArrayv4) Coherent Decorate 320(fimage2Dv4) DescriptorSet 0 Decorate 320(fimage2Dv4) Binding 12 - Decorate 320(fimage2Dv4) Coherent Decorate 320(fimage2Dv4) Volatile Decorate 320(fimage2Dv4) Coherent Decorate 330(fimage2DArrayv4) DescriptorSet 0 Decorate 330(fimage2DArrayv4) Binding 13 - Decorate 330(fimage2DArrayv4) Coherent Decorate 330(fimage2DArrayv4) Volatile Decorate 330(fimage2DArrayv4) Coherent Decorate 340(fimageCubev4) DescriptorSet 0 Decorate 340(fimageCubev4) Binding 15 - Decorate 340(fimageCubev4) Coherent Decorate 340(fimageCubev4) Volatile Decorate 340(fimageCubev4) Coherent Decorate 350(fimageCubeArrayv4) DescriptorSet 0 Decorate 350(fimageCubeArrayv4) Binding 16 - Decorate 350(fimageCubeArrayv4) Coherent Decorate 350(fimageCubeArrayv4) Volatile Decorate 350(fimageCubeArrayv4) Coherent Decorate 360(fimage3Dv4) DescriptorSet 0 Decorate 360(fimage3Dv4) Binding 19 - Decorate 360(fimage3Dv4) Coherent Decorate 360(fimage3Dv4) Volatile Decorate 360(fimage3Dv4) Coherent 2: TypeVoid diff --git a/Test/baseResults/vk.relaxed.frag.out b/Test/baseResults/vk.relaxed.frag.out index 4d5f64ee..7aed8138 100644 --- a/Test/baseResults/vk.relaxed.frag.out +++ b/Test/baseResults/vk.relaxed.frag.out @@ -728,18 +728,15 @@ gl_FragCoord origin is upper left Name 196 "structUniform.samplers.tn[2]" Name 197 "structUniform.samplers.tn[3]" Name 198 "param" - MemberDecorate 34(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 34(gl_AtomicCounterBlock_0) 0 Volatile MemberDecorate 34(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 34(gl_AtomicCounterBlock_0) 0 Offset 0 - MemberDecorate 34(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 34(gl_AtomicCounterBlock_0) 1 Volatile MemberDecorate 34(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 34(gl_AtomicCounterBlock_0) 1 Offset 4 Decorate 34(gl_AtomicCounterBlock_0) BufferBlock Decorate 36 DescriptorSet 0 Decorate 36 Binding 9 - MemberDecorate 78(gl_AtomicCounterBlock_1) 0 Coherent MemberDecorate 78(gl_AtomicCounterBlock_1) 0 Volatile MemberDecorate 78(gl_AtomicCounterBlock_1) 0 Coherent MemberDecorate 78(gl_AtomicCounterBlock_1) 0 Offset 0 diff --git a/Test/baseResults/vk.relaxed.link1.frag.out b/Test/baseResults/vk.relaxed.link1.frag.out index 1e67646c..161d929b 100644 --- a/Test/baseResults/vk.relaxed.link1.frag.out +++ b/Test/baseResults/vk.relaxed.link1.frag.out @@ -378,15 +378,12 @@ gl_FragCoord origin is upper left Name 68 "o" Name 72 "j" Name 79 "v" - MemberDecorate 16(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 16(gl_AtomicCounterBlock_0) 0 Volatile MemberDecorate 16(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 16(gl_AtomicCounterBlock_0) 0 Offset 0 - MemberDecorate 16(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 16(gl_AtomicCounterBlock_0) 1 Volatile MemberDecorate 16(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 16(gl_AtomicCounterBlock_0) 1 Offset 4 - MemberDecorate 16(gl_AtomicCounterBlock_0) 2 Coherent MemberDecorate 16(gl_AtomicCounterBlock_0) 2 Volatile MemberDecorate 16(gl_AtomicCounterBlock_0) 2 Coherent MemberDecorate 16(gl_AtomicCounterBlock_0) 2 Offset 8 diff --git a/Test/baseResults/vk.relaxed.stagelink.vert.out b/Test/baseResults/vk.relaxed.stagelink.vert.out index 47e1b4fa..a9b96831 100644 --- a/Test/baseResults/vk.relaxed.stagelink.vert.out +++ b/Test/baseResults/vk.relaxed.stagelink.vert.out @@ -465,15 +465,12 @@ gl_FragCoord origin is upper left Name 72 "gl_VertexIndex" Name 82 "gl_InstanceIndex" Name 90 "io" - MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Volatile MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Offset 0 - MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Volatile MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Offset 4 - MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Volatile MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Offset 8 @@ -622,15 +619,12 @@ gl_FragCoord origin is upper left Name 37 "" Name 68 "o" Name 70 "io" - MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Volatile MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 0 Offset 0 - MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Volatile MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 1 Offset 4 - MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Volatile MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Coherent MemberDecorate 14(gl_AtomicCounterBlock_0) 2 Offset 8 diff --git a/known_good.json b/known_good.json index 9faf3946..9324b95f 100644 --- a/known_good.json +++ b/known_good.json @@ -5,14 +5,14 @@ "site" : "github", "subrepo" : "KhronosGroup/SPIRV-Tools", "subdir" : "External/spirv-tools", - "commit": "04896c462d9f3f504c99a4698605b6524af813c1" + "commit": "02470f606fe1571de808cb773d8c521ab201aaff" }, { "name" : "spirv-tools/external/spirv-headers", "site" : "github", "subrepo" : "KhronosGroup/SPIRV-Headers", "subdir" : "External/spirv-tools/external/spirv-headers", - "commit" : "8b246ff75c6615ba4532fe4fde20f1be090c3764" + "commit" : "4f7b471f1a66b6d06462cd4ba57628cc0cd087d7" }, { "name": "googletest",