From 135f8fcba837098a5ee0ed699d59445716908825 Mon Sep 17 00:00:00 2001 From: Shahbaz Youssefi Date: Mon, 13 Jan 2020 16:18:41 -0500 Subject: [PATCH] Vulkan: Remove inactive uniforms in the translator By removing inactive uniforms in the translator, glslang wrapper doesn't need to comment them out. Additionally, inactive uniforms don't find their way in the default uniform block, reducing its size if there's a mix of active and inactive uniforms. As collateral, it also fixes a bug where inactive uniforms of struct type were not correctly removed by glslang wrapper. Bug: angleproject:3394 Bug: angleproject:4211 Bug: angleproject:4248 Change-Id: I874747070e875fe24bf59d39d1322e319e280a16 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1999278 Commit-Queue: Shahbaz Youssefi Reviewed-by: Jamie Madill --- src/compiler/translator/TranslatorVulkan.cpp | 2 +- src/compiler/translator/blocklayout.cpp | 24 +++++++++++++------ src/compiler/translator/blocklayout.h | 22 +++++++++++++---- .../RemoveInactiveInterfaceVariables.cpp | 10 ++++---- .../renderer/glslang_wrapper_utils.cpp | 14 +++++------ src/libANGLE/renderer/metal/ProgramMtl.mm | 2 +- src/libANGLE/renderer/vulkan/ProgramVk.cpp | 2 +- 7 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/compiler/translator/TranslatorVulkan.cpp b/src/compiler/translator/TranslatorVulkan.cpp index 1505c1970f9ba..e353e5789073d 100644 --- a/src/compiler/translator/TranslatorVulkan.cpp +++ b/src/compiler/translator/TranslatorVulkan.cpp @@ -712,7 +712,7 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root, int atomicCounterCount = 0; for (const auto &uniform : getUniforms()) { - if (!uniform.isBuiltIn() && uniform.staticUse && !gl::IsOpaqueType(uniform.type)) + if (!uniform.isBuiltIn() && uniform.active && !gl::IsOpaqueType(uniform.type)) { ++defaultUniformCount; } diff --git a/src/compiler/translator/blocklayout.cpp b/src/compiler/translator/blocklayout.cpp index 96566302f7552..9cbc96b3650ba 100644 --- a/src/compiler/translator/blocklayout.cpp +++ b/src/compiler/translator/blocklayout.cpp @@ -48,10 +48,18 @@ void GetInterfaceBlockInfo(const std::vector &fields, const std::string &prefix, BlockLayoutEncoder *encoder, bool inRowMajorLayout, + bool onlyActiveVariables, BlockLayoutMap *blockInfoOut) { BlockLayoutMapVisitor visitor(blockInfoOut, prefix, encoder); - TraverseShaderVariables(fields, inRowMajorLayout, &visitor); + if (onlyActiveVariables) + { + TraverseActiveShaderVariables(fields, inRowMajorLayout, &visitor); + } + else + { + TraverseShaderVariables(fields, inRowMajorLayout, &visitor); + } } void TraverseStructVariable(const ShaderVariable &variable, @@ -345,17 +353,19 @@ void GetInterfaceBlockInfo(const std::vector &fields, { // Matrix packing is always recorded in individual fields, so they'll set the row major layout // flag to true if needed. - GetInterfaceBlockInfo(fields, prefix, encoder, false, blockInfoOut); + // Iterates over all variables. + GetInterfaceBlockInfo(fields, prefix, encoder, false, false, blockInfoOut); } -void GetUniformBlockInfo(const std::vector &uniforms, - const std::string &prefix, - BlockLayoutEncoder *encoder, - BlockLayoutMap *blockInfoOut) +void GetActiveUniformBlockInfo(const std::vector &uniforms, + const std::string &prefix, + BlockLayoutEncoder *encoder, + BlockLayoutMap *blockInfoOut) { // Matrix packing is always recorded in individual fields, so they'll set the row major layout // flag to true if needed. - GetInterfaceBlockInfo(uniforms, prefix, encoder, false, blockInfoOut); + // Iterates only over the active variables. + GetInterfaceBlockInfo(uniforms, prefix, encoder, false, true, blockInfoOut); } // VariableNameVisitor implementation. diff --git a/src/compiler/translator/blocklayout.h b/src/compiler/translator/blocklayout.h index 1ccce716e1ed6..22867956b434e 100644 --- a/src/compiler/translator/blocklayout.h +++ b/src/compiler/translator/blocklayout.h @@ -179,10 +179,10 @@ void GetInterfaceBlockInfo(const std::vector &fields, BlockLayoutMap *blockInfoOut); // Used for laying out the default uniform block on the Vulkan backend. -void GetUniformBlockInfo(const std::vector &uniforms, - const std::string &prefix, - BlockLayoutEncoder *encoder, - BlockLayoutMap *blockInfoOut); +void GetActiveUniformBlockInfo(const std::vector &uniforms, + const std::string &prefix, + BlockLayoutEncoder *encoder, + BlockLayoutMap *blockInfoOut); class ShaderVariableVisitor { @@ -298,6 +298,20 @@ void TraverseShaderVariables(const std::vector &vars, TraverseShaderVariable(var, isRowMajorLayout, visitor); } } + +template +void TraverseActiveShaderVariables(const std::vector &vars, + bool isRowMajorLayout, + ShaderVariableVisitor *visitor) +{ + for (const T &var : vars) + { + if (var.active) + { + TraverseShaderVariable(var, isRowMajorLayout, visitor); + } + } +} } // namespace sh #endif // COMMON_BLOCKLAYOUT_H_ diff --git a/src/compiler/translator/tree_ops/RemoveInactiveInterfaceVariables.cpp b/src/compiler/translator/tree_ops/RemoveInactiveInterfaceVariables.cpp index 251ad00b6048c..af501d3d41d48 100644 --- a/src/compiler/translator/tree_ops/RemoveInactiveInterfaceVariables.cpp +++ b/src/compiler/translator/tree_ops/RemoveInactiveInterfaceVariables.cpp @@ -41,7 +41,7 @@ RemoveInactiveInterfaceVariablesTraverser::RemoveInactiveInterfaceVariablesTrave {} template -bool isVariableActive(const std::vector &mVars, const ImmutableString &name) +bool IsVariableActive(const std::vector &mVars, const ImmutableString &name) { for (const Variable &var : mVars) { @@ -71,7 +71,7 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit, const TType &type = declarator->getType(); - // Only remove opaque uniform and interface block declarations. + // Only remove uniform and interface block declarations. // // Note: Don't remove varyings. Imagine a situation where the VS doesn't write to a varying // but the FS reads from it. This is allowed, though the value of the varying is undefined. @@ -81,11 +81,11 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit, if (type.isInterfaceBlock()) { - removeDeclaration = !isVariableActive(mInterfaceBlocks, type.getInterfaceBlock()->name()); + removeDeclaration = !IsVariableActive(mInterfaceBlocks, type.getInterfaceBlock()->name()); } - else if (type.getQualifier() == EvqUniform && IsOpaqueType(type.getBasicType())) + else if (type.getQualifier() == EvqUniform) { - removeDeclaration = !isVariableActive(mUniforms, asSymbol->getName()); + removeDeclaration = !IsVariableActive(mUniforms, asSymbol->getName()); } if (removeDeclaration) diff --git a/src/libANGLE/renderer/glslang_wrapper_utils.cpp b/src/libANGLE/renderer/glslang_wrapper_utils.cpp index 48edc358ce8b3..2d6484c9c46ff 100644 --- a/src/libANGLE/renderer/glslang_wrapper_utils.cpp +++ b/src/libANGLE/renderer/glslang_wrapper_utils.cpp @@ -1045,20 +1045,18 @@ void CleanupUnusedEntities(bool useOldRewriteStructSamplers, } } - // Comment out unused default uniforms. This relies on the fact that the shader compiler - // outputs uniforms to a single line. + // Comment out inactive samplers. This relies on the fact that the shader compiler outputs + // uniforms to a single line. for (const gl::UnusedUniform &unusedUniform : resources.unusedUniforms) { - if (unusedUniform.isImage || unusedUniform.isAtomicCounter) + if (!unusedUniform.isSampler) { continue; } - std::string uniformName = unusedUniform.isSampler - ? useOldRewriteStructSamplers - ? GetMappedSamplerNameOld(unusedUniform.name) - : GlslangGetMappedSamplerName(unusedUniform.name) - : unusedUniform.name; + std::string uniformName = useOldRewriteStructSamplers + ? GetMappedSamplerNameOld(unusedUniform.name) + : GlslangGetMappedSamplerName(unusedUniform.name); for (IntermediateShaderSource &shaderSource : *shaderSources) { diff --git a/src/libANGLE/renderer/metal/ProgramMtl.mm b/src/libANGLE/renderer/metal/ProgramMtl.mm index 8a16589dd6821..07dc5cab5441c 100644 --- a/src/libANGLE/renderer/metal/ProgramMtl.mm +++ b/src/libANGLE/renderer/metal/ProgramMtl.mm @@ -124,7 +124,7 @@ void InitDefaultUniformBlock(const std::vector &uniforms, } sh::Std140BlockEncoder blockEncoder; - sh::GetUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut); + sh::GetActiveUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut); size_t blockSize = blockEncoder.getCurrentOffset(); diff --git a/src/libANGLE/renderer/vulkan/ProgramVk.cpp b/src/libANGLE/renderer/vulkan/ProgramVk.cpp index aa37535635ec3..5b83aa176389e 100644 --- a/src/libANGLE/renderer/vulkan/ProgramVk.cpp +++ b/src/libANGLE/renderer/vulkan/ProgramVk.cpp @@ -58,7 +58,7 @@ void InitDefaultUniformBlock(const std::vector &uniforms, } VulkanDefaultBlockEncoder blockEncoder; - sh::GetUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut); + sh::GetActiveUniformBlockInfo(uniforms, "", &blockEncoder, blockLayoutMapOut); size_t blockSize = blockEncoder.getCurrentOffset();