From ebb505355d59f61dfc9547f81e14b2d9cd1eb2f0 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Mon, 16 May 2016 19:22:05 -0600 Subject: [PATCH] SPV: Don't add clip/cull distance capabilities unless used. These capabalities were added on declaration of the members, but that is considered too aggressive, as those members are automatically declared in some shaders that don't use them. Now, actual access is needed to make the capabalities be declared. --- SPIRV/GlslangToSpv.cpp | 33 +++++++++++++++++++++++----- Test/baseResults/spv.150.geom.out | 1 - Test/baseResults/spv.430.vert.out | 1 - Test/baseResults/spv.bool.vert.out | 2 -- Test/baseResults/spv.matFun.vert.out | 1 - 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 2b76c049..92d20f84 100755 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -109,7 +109,7 @@ public: protected: spv::Decoration TranslateInterpolationDecoration(const glslang::TQualifier& qualifier); - spv::BuiltIn TranslateBuiltInDecoration(glslang::TBuiltInVariable); + spv::BuiltIn TranslateBuiltInDecoration(glslang::TBuiltInVariable, bool member); spv::ImageFormat TranslateImageFormat(const glslang::TType& type); spv::Id createSpvVariable(const glslang::TIntermSymbol*); spv::Id getSampledType(const glslang::TSampler&); @@ -122,6 +122,7 @@ protected: int getArrayStride(const glslang::TType& arrayType, glslang::TLayoutPacking, glslang::TLayoutMatrix); int getMatrixStride(const glslang::TType& matrixType, glslang::TLayoutPacking, glslang::TLayoutMatrix); void updateMemberOffset(const glslang::TType& structType, const glslang::TType& memberType, int& currentOffset, int& nextOffset, glslang::TLayoutPacking, glslang::TLayoutMatrix); + void declareClipCullCapability(const glslang::TTypeList& members, int member); bool isShaderEntrypoint(const glslang::TIntermAggregate* node); void makeFunctions(const glslang::TIntermSequence&); @@ -393,7 +394,7 @@ spv::Decoration TranslateNoContractionDecoration(const glslang::TQualifier& qual } // Translate glslang built-in variable to SPIR-V built in decoration. -spv::BuiltIn TGlslangToSpvTraverser::TranslateBuiltInDecoration(glslang::TBuiltInVariable builtIn) +spv::BuiltIn TGlslangToSpvTraverser::TranslateBuiltInDecoration(glslang::TBuiltInVariable builtIn, bool member) { switch (builtIn) { case glslang::EbvPointSize: @@ -410,12 +411,20 @@ spv::BuiltIn TGlslangToSpvTraverser::TranslateBuiltInDecoration(glslang::TBuiltI } return spv::BuiltInPointSize; + // These *Distance capabilities logically belong here, but if the member is declared and + // then never used, consumers of SPIR-V prefer the capability not be declared. + // They are now generated when used, rather than here when declared. + // Potentially, the specification should be more clear what the minimum + // use needed is to trigger the capability. + // case glslang::EbvClipDistance: - builder.addCapability(spv::CapabilityClipDistance); + if (! member) + builder.addCapability(spv::CapabilityClipDistance); return spv::BuiltInClipDistance; case glslang::EbvCullDistance: - builder.addCapability(spv::CapabilityCullDistance); + if (! member) + builder.addCapability(spv::CapabilityCullDistance); return spv::BuiltInCullDistance; case glslang::EbvViewportIndex: @@ -926,6 +935,10 @@ bool TGlslangToSpvTraverser::visitBinary(glslang::TVisit /* visit */, glslang::T } else { // normal case for indexing array or structure or block builder.accessChainPush(builder.makeIntConstant(index)); + + // Add capabilities here for accessing clip/cull distance + if (node->getLeft()->getType().isStruct() && ! node->getLeft()->getType().isArray()) + declareClipCullCapability(*node->getLeft()->getType().getStruct(), index); } } return false; @@ -1919,7 +1932,7 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty builder.addMemberDecoration(spvType, member, spv::DecorationMatrixStride, getMatrixStride(glslangType, explicitLayout, subQualifier.layoutMatrix)); // built-in variable decorations - spv::BuiltIn builtIn = TranslateBuiltInDecoration(glslangType.getQualifier().builtIn); + spv::BuiltIn builtIn = TranslateBuiltInDecoration(glslangType.getQualifier().builtIn, true); if (builtIn != spv::BadValue) addMemberDecoration(spvType, member, spv::DecorationBuiltIn, (int)builtIn); } @@ -2172,6 +2185,14 @@ void TGlslangToSpvTraverser::updateMemberOffset(const glslang::TType& /*structTy nextOffset = currentOffset + memberSize; } +void TGlslangToSpvTraverser::declareClipCullCapability(const glslang::TTypeList& members, int member) +{ + if (members[member].type->getQualifier().builtIn == glslang::EbvClipDistance) + builder.addCapability(spv::CapabilityClipDistance); + if (members[member].type->getQualifier().builtIn == glslang::EbvCullDistance) + builder.addCapability(spv::CapabilityCullDistance); +} + bool TGlslangToSpvTraverser::isShaderEntrypoint(const glslang::TIntermAggregate* node) { // have to ignore mangling and just look at the base name @@ -3936,7 +3957,7 @@ spv::Id TGlslangToSpvTraverser::getSymbolId(const glslang::TIntermSymbol* symbol } // built-in variable decorations - spv::BuiltIn builtIn = TranslateBuiltInDecoration(symbol->getQualifier().builtIn); + spv::BuiltIn builtIn = TranslateBuiltInDecoration(symbol->getQualifier().builtIn, false); if (builtIn != spv::BadValue) addDecoration(id, spv::DecorationBuiltIn, (int)builtIn); diff --git a/Test/baseResults/spv.150.geom.out b/Test/baseResults/spv.150.geom.out index b09f0c04..8b021ff9 100755 --- a/Test/baseResults/spv.150.geom.out +++ b/Test/baseResults/spv.150.geom.out @@ -9,7 +9,6 @@ Linked geometry stage: Capability Geometry Capability GeometryPointSize - Capability ClipDistance Capability GeometryStreams 1: ExtInstImport "GLSL.std.450" MemoryModel Logical GLSL450 diff --git a/Test/baseResults/spv.430.vert.out b/Test/baseResults/spv.430.vert.out index a1487d53..12a6c33f 100755 --- a/Test/baseResults/spv.430.vert.out +++ b/Test/baseResults/spv.430.vert.out @@ -10,7 +10,6 @@ Linked vertex stage: // Id's are bound by 66 Capability Shader - Capability ClipDistance 1: ExtInstImport "GLSL.std.450" MemoryModel Logical GLSL450 EntryPoint Vertex 4 "main" 12 23 34 38 41 42 62 65 diff --git a/Test/baseResults/spv.bool.vert.out b/Test/baseResults/spv.bool.vert.out index b6960f94..49a69a0e 100644 --- a/Test/baseResults/spv.bool.vert.out +++ b/Test/baseResults/spv.bool.vert.out @@ -10,8 +10,6 @@ Linked vertex stage: // Id's are bound by 49 Capability Shader - Capability ClipDistance - Capability CullDistance 1: ExtInstImport "GLSL.std.450" MemoryModel Logical GLSL450 EntryPoint Vertex 4 "main" 24 diff --git a/Test/baseResults/spv.matFun.vert.out b/Test/baseResults/spv.matFun.vert.out index ab487763..0aa0a52d 100755 --- a/Test/baseResults/spv.matFun.vert.out +++ b/Test/baseResults/spv.matFun.vert.out @@ -10,7 +10,6 @@ Linked vertex stage: // Id's are bound by 103 Capability Shader - Capability ClipDistance 1: ExtInstImport "GLSL.std.450" MemoryModel Logical GLSL450 EntryPoint Vertex 4 "main" 76 81