Commit 453926f5 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Remove inactive shader inputs in the translator

Inactive vertex attributes are harmless to remove. Between two consecutive stages, the input varyings must be a subset of the previous stage's output varyings. This means removing inactive input varyings is also harmless. Removing inactive output varyings is not possible though. GLSL allows a varying to not be written by the previous stage even if it's used in the current stage (values will be undefined, but it's not an error). This means that an inactive output varying may still need to exist as part of the shader interface in case the following stage has that varying as input (and is active). Bug: angleproject:3394 Change-Id: I7302973d2b8356d9f54a66f8259c32f245a99904 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2009986Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent 1d480fcc
...@@ -733,7 +733,8 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root, ...@@ -733,7 +733,8 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
// inactive samplers is not yet supported. Note also that currently, CollectVariables marks // inactive samplers is not yet supported. Note also that currently, CollectVariables marks
// every field of an active uniform that's of struct type as active, i.e. no extracted sampler // every field of an active uniform that's of struct type as active, i.e. no extracted sampler
// is inactive. // is inactive.
if (!RemoveInactiveInterfaceVariables(this, root, getUniforms(), getInterfaceBlocks())) if (!RemoveInactiveInterfaceVariables(this, root, getAttributes(), getInputVaryings(),
getUniforms(), getInterfaceBlocks()))
{ {
return false; return false;
} }
......
...@@ -24,20 +24,30 @@ class RemoveInactiveInterfaceVariablesTraverser : public TIntermTraverser ...@@ -24,20 +24,30 @@ class RemoveInactiveInterfaceVariablesTraverser : public TIntermTraverser
{ {
public: public:
RemoveInactiveInterfaceVariablesTraverser( RemoveInactiveInterfaceVariablesTraverser(
const std::vector<sh::ShaderVariable> &attributes,
const std::vector<sh::ShaderVariable> &inputVaryings,
const std::vector<sh::ShaderVariable> &uniforms, const std::vector<sh::ShaderVariable> &uniforms,
const std::vector<sh::InterfaceBlock> &interfaceBlocks); const std::vector<sh::InterfaceBlock> &interfaceBlocks);
bool visitDeclaration(Visit visit, TIntermDeclaration *node) override; bool visitDeclaration(Visit visit, TIntermDeclaration *node) override;
private: private:
const std::vector<sh::ShaderVariable> &mAttributes;
const std::vector<sh::ShaderVariable> &mInputVaryings;
const std::vector<sh::ShaderVariable> &mUniforms; const std::vector<sh::ShaderVariable> &mUniforms;
const std::vector<sh::InterfaceBlock> &mInterfaceBlocks; const std::vector<sh::InterfaceBlock> &mInterfaceBlocks;
}; };
RemoveInactiveInterfaceVariablesTraverser::RemoveInactiveInterfaceVariablesTraverser( RemoveInactiveInterfaceVariablesTraverser::RemoveInactiveInterfaceVariablesTraverser(
const std::vector<sh::ShaderVariable> &attributes,
const std::vector<sh::ShaderVariable> &inputVaryings,
const std::vector<sh::ShaderVariable> &uniforms, const std::vector<sh::ShaderVariable> &uniforms,
const std::vector<sh::InterfaceBlock> &interfaceBlocks) const std::vector<sh::InterfaceBlock> &interfaceBlocks)
: TIntermTraverser(true, false, false), mUniforms(uniforms), mInterfaceBlocks(interfaceBlocks) : TIntermTraverser(true, false, false),
mAttributes(attributes),
mInputVaryings(inputVaryings),
mUniforms(uniforms),
mInterfaceBlocks(interfaceBlocks)
{} {}
template <typename Variable> template <typename Variable>
...@@ -71,22 +81,32 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit, ...@@ -71,22 +81,32 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit,
const TType &type = declarator->getType(); const TType &type = declarator->getType();
// Only remove uniform and interface block declarations. // Remove all shader interface variables except outputs, i.e. uniforms, interface blocks and
// inputs.
// //
// Note: Don't remove varyings. Imagine a situation where the VS doesn't write to a varying // Imagine a situation where the VS doesn't write to a varying but the FS reads from it. This
// but the FS reads from it. This is allowed, though the value of the varying is undefined. // is allowed, though the value of the varying is undefined. If the varying is removed here,
// If the varying is removed here, the situation is changed to VS not declaring the varying, // the situation is changed to VS not declaring the varying, but the FS reading from it, which
// but the FS reading from it, which is not allowed. // is not allowed. That's why inactive shader outputs are not removed.
bool removeDeclaration = false; bool removeDeclaration = false;
const TQualifier qualifier = type.getQualifier();
if (type.isInterfaceBlock()) if (type.isInterfaceBlock())
{ {
removeDeclaration = !IsVariableActive(mInterfaceBlocks, type.getInterfaceBlock()->name()); removeDeclaration = !IsVariableActive(mInterfaceBlocks, type.getInterfaceBlock()->name());
} }
else if (type.getQualifier() == EvqUniform) else if (qualifier == EvqUniform)
{ {
removeDeclaration = !IsVariableActive(mUniforms, asSymbol->getName()); removeDeclaration = !IsVariableActive(mUniforms, asSymbol->getName());
} }
else if (qualifier == EvqAttribute || qualifier == EvqVertexIn)
{
removeDeclaration = !IsVariableActive(mAttributes, asSymbol->getName());
}
else if (IsShaderIn(qualifier))
{
removeDeclaration = !IsVariableActive(mInputVaryings, asSymbol->getName());
}
if (removeDeclaration) if (removeDeclaration)
{ {
...@@ -101,10 +121,13 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit, ...@@ -101,10 +121,13 @@ bool RemoveInactiveInterfaceVariablesTraverser::visitDeclaration(Visit visit,
bool RemoveInactiveInterfaceVariables(TCompiler *compiler, bool RemoveInactiveInterfaceVariables(TCompiler *compiler,
TIntermBlock *root, TIntermBlock *root,
const std::vector<sh::ShaderVariable> &attributes,
const std::vector<sh::ShaderVariable> &inputVaryings,
const std::vector<sh::ShaderVariable> &uniforms, const std::vector<sh::ShaderVariable> &uniforms,
const std::vector<sh::InterfaceBlock> &interfaceBlocks) const std::vector<sh::InterfaceBlock> &interfaceBlocks)
{ {
RemoveInactiveInterfaceVariablesTraverser traverser(uniforms, interfaceBlocks); RemoveInactiveInterfaceVariablesTraverser traverser(attributes, inputVaryings, uniforms,
interfaceBlocks);
root->traverse(&traverser); root->traverse(&traverser);
return traverser.updateTree(compiler, root); return traverser.updateTree(compiler, root);
} }
......
...@@ -29,6 +29,8 @@ class TSymbolTable; ...@@ -29,6 +29,8 @@ class TSymbolTable;
ANGLE_NO_DISCARD bool RemoveInactiveInterfaceVariables( ANGLE_NO_DISCARD bool RemoveInactiveInterfaceVariables(
TCompiler *compiler, TCompiler *compiler,
TIntermBlock *root, TIntermBlock *root,
const std::vector<sh::ShaderVariable> &attributes,
const std::vector<sh::ShaderVariable> &inputVaryings,
const std::vector<sh::ShaderVariable> &uniforms, const std::vector<sh::ShaderVariable> &uniforms,
const std::vector<sh::InterfaceBlock> &interfaceBlocks); const std::vector<sh::InterfaceBlock> &interfaceBlocks);
......
...@@ -53,7 +53,6 @@ constexpr char kXfbOutMarkerBegin[] = "@@ XFB-OUT"; ...@@ -53,7 +53,6 @@ constexpr char kXfbOutMarkerBegin[] = "@@ XFB-OUT";
constexpr char kMarkerEnd[] = " @@"; constexpr char kMarkerEnd[] = " @@";
constexpr char kParamsBegin = '('; constexpr char kParamsBegin = '(';
constexpr char kParamsEnd = ')'; constexpr char kParamsEnd = ')';
constexpr char kInactiveVariableSubstitution[] = "// ";
constexpr uint32_t kANGLEPositionLocationOffset = 1; constexpr uint32_t kANGLEPositionLocationOffset = 1;
constexpr uint32_t kXfbANGLEPositionLocationOffset = 2; constexpr uint32_t kXfbANGLEPositionLocationOffset = 2;
...@@ -998,27 +997,6 @@ void CleanupUnusedEntities(bool useOldRewriteStructSamplers, ...@@ -998,27 +997,6 @@ void CleanupUnusedEntities(bool useOldRewriteStructSamplers,
const gl::ProgramLinkedResources &resources, const gl::ProgramLinkedResources &resources,
gl::ShaderMap<IntermediateShaderSource> *shaderSources) gl::ShaderMap<IntermediateShaderSource> *shaderSources)
{ {
IntermediateShaderSource &vertexSource = (*shaderSources)[gl::ShaderType::Vertex];
if (!vertexSource.empty())
{
gl::Shader *glVertexShader = programState.getAttachedShader(gl::ShaderType::Vertex);
ASSERT(glVertexShader != nullptr);
// The attributes in the programState could have been filled with active attributes only
// depending on the shader version. If there is inactive attributes left, we have to remove
// their @@ QUALIFIER and @@ LAYOUT markers.
for (const sh::ShaderVariable &attribute : glVertexShader->getAllAttributes())
{
if (attribute.active)
{
continue;
}
vertexSource.eraseLayoutAndQualifierSpecifiers(attribute.name,
kInactiveVariableSubstitution);
}
}
// Remove all the markers for unused varyings. // Remove all the markers for unused varyings.
for (const std::string &varyingName : resources.varyingPacking.getInactiveVaryingNames()) for (const std::string &varyingName : resources.varyingPacking.getInactiveVaryingNames())
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment