Skip to content

A
angle
Commit cc56e1aa by Shahbaz Youssefi Committed by Commit Bot
Options

Vulkan: Remove special output of structs and keep them in AST

The code that separated out declaration of nameless structs from uniform variable declarations is generalized to separate out declaration of all structs from uniform variable declarations. As a result, a pass that outputs the struct types at the top of the shader is no longer necessary. The struct declarations are kept in the AST to be output to GLSL as they normally would when not used in conjunction with uniform variables. After this change, the Vulkan and Metal translators no longer intermix transformations and code generation; transformations are done first entirely in AST, and code generation is done at the end. Bug: angleproject:2461 Bug: angleproject:4889 Change-Id: Ieb4d3f7091845e50c3dc399603ab01182692521d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2818153 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com>
parent 18c6d628
Showing with 48 additions and 98 deletions
src/compiler.gni
...@@ -318,8 +318,6 @@ angle_translator_lib_vulkan_sources = [ ...@@ -318,8 +318,6 @@ angle_translator_lib_vulkan_sources = [
"src/compiler/translator/tree_ops/vulkan/FlagSamplersWithTexelFetch.h", "src/compiler/translator/tree_ops/vulkan/FlagSamplersWithTexelFetch.h",
"src/compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.cpp", "src/compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.cpp",
"src/compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.h", "src/compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.h",
"src/compiler/translator/tree_ops/vulkan/NameEmbeddedUniformStructs.cpp",
"src/compiler/translator/tree_ops/vulkan/NameEmbeddedUniformStructs.h",
"src/compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.cpp", "src/compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.cpp",
"src/compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.h", "src/compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.h",
"src/compiler/translator/tree_ops/vulkan/RemoveInactiveInterfaceVariables.cpp", "src/compiler/translator/tree_ops/vulkan/RemoveInactiveInterfaceVariables.cpp",
...@@ -340,6 +338,8 @@ angle_translator_lib_vulkan_sources = [ ...@@ -340,6 +338,8 @@ angle_translator_lib_vulkan_sources = [
"src/compiler/translator/tree_ops/vulkan/RewriteR32fImages.h", "src/compiler/translator/tree_ops/vulkan/RewriteR32fImages.h",
"src/compiler/translator/tree_ops/vulkan/RewriteStructSamplers.cpp", "src/compiler/translator/tree_ops/vulkan/RewriteStructSamplers.cpp",
"src/compiler/translator/tree_ops/vulkan/RewriteStructSamplers.h", "src/compiler/translator/tree_ops/vulkan/RewriteStructSamplers.h",
"src/compiler/translator/tree_ops/vulkan/SeparateStructFromUniformDeclarations.cpp",
"src/compiler/translator/tree_ops/vulkan/SeparateStructFromUniformDeclarations.h",
] ]
if (is_android) { if (is_android) {
angle_translator_sources += [ angle_translator_sources += [
......
src/compiler/translator/TranslatorMetal.cpp
...@@ -181,14 +181,10 @@ bool TranslatorMetal::translate(TIntermBlock *root, ...@@ -181,14 +181,10 @@ bool TranslatorMetal::translate(TIntermBlock *root,
{ {
TInfoSinkBase sink; TInfoSinkBase sink;
TOutputVulkanGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(),
getNameMap(), &getSymbolTable(), getShaderType(),
getShaderVersion(), getOutputType(), false, true, compileOptions);
SpecConstMetal specConst(&getSymbolTable(), compileOptions, getShaderType()); SpecConstMetal specConst(&getSymbolTable(), compileOptions, getShaderType());
DriverUniformMetal driverUniforms; DriverUniformMetal driverUniforms;
if (!TranslatorVulkan::translateImpl(sink, root, compileOptions, perfDiagnostics, &specConst, if (!TranslatorVulkan::translateImpl(sink, root, compileOptions, perfDiagnostics, &specConst,
&driverUniforms, &outputGLSL)) &driverUniforms))
{ {
return false; return false;
} }
...@@ -244,6 +240,9 @@ bool TranslatorMetal::translate(TIntermBlock *root, ...@@ -244,6 +240,9 @@ bool TranslatorMetal::translate(TIntermBlock *root,
} }
// Write translated shader. // Write translated shader.
TOutputVulkanGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(),
getNameMap(), &getSymbolTable(), getShaderType(),
getShaderVersion(), getOutputType(), false, true, compileOptions);
root->traverse(&outputGLSL); root->traverse(&outputGLSL);
return compileToSpirv(sink); return compileToSpirv(sink);
......
src/compiler/translator/TranslatorVulkan.cpp
...@@ -22,7 +22,6 @@ ...@@ -22,7 +22,6 @@
#include "compiler/translator/glslang_wrapper.h" #include "compiler/translator/glslang_wrapper.h"
#include "compiler/translator/tree_ops/vulkan/FlagSamplersWithTexelFetch.h" #include "compiler/translator/tree_ops/vulkan/FlagSamplersWithTexelFetch.h"
#include "compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.h" #include "compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.h"
#include "compiler/translator/tree_ops/vulkan/NameEmbeddedUniformStructs.h"
#include "compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.h" #include "compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.h"
#include "compiler/translator/tree_ops/vulkan/RemoveInactiveInterfaceVariables.h" #include "compiler/translator/tree_ops/vulkan/RemoveInactiveInterfaceVariables.h"
#include "compiler/translator/tree_ops/vulkan/ReplaceForShaderFramebufferFetch.h" #include "compiler/translator/tree_ops/vulkan/ReplaceForShaderFramebufferFetch.h"
...@@ -33,6 +32,7 @@ ...@@ -33,6 +32,7 @@
#include "compiler/translator/tree_ops/vulkan/RewriteInterpolateAtOffset.h" #include "compiler/translator/tree_ops/vulkan/RewriteInterpolateAtOffset.h"
#include "compiler/translator/tree_ops/vulkan/RewriteR32fImages.h" #include "compiler/translator/tree_ops/vulkan/RewriteR32fImages.h"
#include "compiler/translator/tree_ops/vulkan/RewriteStructSamplers.h" #include "compiler/translator/tree_ops/vulkan/RewriteStructSamplers.h"
#include "compiler/translator/tree_ops/vulkan/SeparateStructFromUniformDeclarations.h"
#include "compiler/translator/tree_util/BuiltIn.h" #include "compiler/translator/tree_util/BuiltIn.h"
#include "compiler/translator/tree_util/DriverUniform.h" #include "compiler/translator/tree_util/DriverUniform.h"
#include "compiler/translator/tree_util/FindFunction.h" #include "compiler/translator/tree_util/FindFunction.h"
...@@ -62,53 +62,6 @@ constexpr gl::ShaderMap<const char *> kDefaultUniformNames = { ...@@ -62,53 +62,6 @@ constexpr gl::ShaderMap<const char *> kDefaultUniformNames = {
{gl::ShaderType::Compute, vk::kDefaultUniformsNameCS}, {gl::ShaderType::Compute, vk::kDefaultUniformsNameCS},
}; };
// This traverses nodes, find the struct ones and add their declarations to the sink. It also
// removes the nodes from the tree as it processes them.
class DeclareStructTypesTraverser : public TIntermTraverser
{
public:
explicit DeclareStructTypesTraverser(TOutputVulkanGLSL *outputVulkanGLSL)
: TIntermTraverser(true, false, false), mOutputVulkanGLSL(outputVulkanGLSL)
{}
bool visitDeclaration(Visit visit, TIntermDeclaration *node) override
{
ASSERT(visit == PreVisit);
if (!mInGlobalScope)
{
return false;
}
const TIntermSequence &sequence = *(node->getSequence());
TIntermTyped *declarator = sequence.front()->getAsTyped();
const TType &type = declarator->getType();
if (type.isStructSpecifier())
{
const TStructure *structure = type.getStruct();
// Embedded structs should be parsed away by now.
ASSERT(structure->symbolType() != SymbolType::Empty);
mOutputVulkanGLSL->writeStructType(structure);
TIntermSymbol *symbolNode = declarator->getAsSymbolNode();
if (symbolNode && symbolNode->variable().symbolType() == SymbolType::Empty)
{
// Remove the struct specifier declaration from the tree so it isn't parsed again.
TIntermSequence emptyReplacement;
mMultiReplacements.emplace_back(getParentNode()->getAsBlock(), node,
std::move(emptyReplacement));
}
}
return false;
}
private:
TOutputVulkanGLSL *mOutputVulkanGLSL;
};
bool IsDefaultUniform(const TType &type) bool IsDefaultUniform(const TType &type)
{ {
return type.getQualifier() == EvqUniform && type.getInterfaceBlock() == nullptr && return type.getQualifier() == EvqUniform && type.getInterfaceBlock() == nullptr &&
...@@ -816,8 +769,7 @@ bool TranslatorVulkan::translateImpl(TInfoSinkBase &sink, ...@@ -816,8 +769,7 @@ bool TranslatorVulkan::translateImpl(TInfoSinkBase &sink,
ShCompileOptions compileOptions, ShCompileOptions compileOptions,
PerformanceDiagnostics * /*perfDiagnostics*/, PerformanceDiagnostics * /*perfDiagnostics*/,
SpecConst *specConst, SpecConst *specConst,
DriverUniform *driverUniforms, DriverUniform *driverUniforms)
TOutputVulkanGLSL *outputGLSL)
{ {
if (getShaderType() == GL_VERTEX_SHADER) if (getShaderType() == GL_VERTEX_SHADER)
{ {
...@@ -887,7 +839,7 @@ bool TranslatorVulkan::translateImpl(TInfoSinkBase &sink, ...@@ -887,7 +839,7 @@ bool TranslatorVulkan::translateImpl(TInfoSinkBase &sink,
if (aggregateTypesUsedForUniforms > 0) if (aggregateTypesUsedForUniforms > 0)
{ {
if (!NameEmbeddedStructUniforms(this, root, &getSymbolTable())) if (!SeparateStructFromUniformDeclarations(this, root, &getSymbolTable()))
{ {
return false; return false;
} }
...@@ -899,14 +851,6 @@ bool TranslatorVulkan::translateImpl(TInfoSinkBase &sink, ...@@ -899,14 +851,6 @@ bool TranslatorVulkan::translateImpl(TInfoSinkBase &sink,
return false; return false;
} }
defaultUniformCount -= removedUniformsCount; defaultUniformCount -= removedUniformsCount;
// We must declare the struct types before using them.
DeclareStructTypesTraverser structTypesTraverser(outputGLSL);
root->traverse(&structTypesTraverser);
if (!structTypesTraverser.updateTree(this, root))
{
return false;
}
} }
// Replace array of array of opaque uniforms with a flattened array. This is run after // Replace array of array of opaque uniforms with a flattened array. This is run after
...@@ -1373,18 +1317,13 @@ bool TranslatorVulkan::translate(TIntermBlock *root, ...@@ -1373,18 +1317,13 @@ bool TranslatorVulkan::translate(TIntermBlock *root,
bool enablePrecision = (compileOptions & SH_IGNORE_PRECISION_QUALIFIERS) == 0; bool enablePrecision = (compileOptions & SH_IGNORE_PRECISION_QUALIFIERS) == 0;
TOutputVulkanGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(),
getNameMap(), &getSymbolTable(), getShaderType(),
getShaderVersion(), getOutputType(), precisionEmulation,
enablePrecision, compileOptions);
SpecConst specConst(&getSymbolTable(), compileOptions, getShaderType()); SpecConst specConst(&getSymbolTable(), compileOptions, getShaderType());
if ((compileOptions & SH_USE_SPECIALIZATION_CONSTANT) != 0) if ((compileOptions & SH_USE_SPECIALIZATION_CONSTANT) != 0)
{ {
DriverUniform driverUniforms; DriverUniform driverUniforms;
if (!translateImpl(sink, root, compileOptions, perfDiagnostics, &specConst, &driverUniforms, if (!translateImpl(sink, root, compileOptions, perfDiagnostics, &specConst,
&outputGLSL)) &driverUniforms))
{ {
return false; return false;
} }
...@@ -1393,13 +1332,17 @@ bool TranslatorVulkan::translate(TIntermBlock *root, ...@@ -1393,13 +1332,17 @@ bool TranslatorVulkan::translate(TIntermBlock *root,
{ {
DriverUniformExtended driverUniformsExt; DriverUniformExtended driverUniformsExt;
if (!translateImpl(sink, root, compileOptions, perfDiagnostics, &specConst, if (!translateImpl(sink, root, compileOptions, perfDiagnostics, &specConst,
&driverUniformsExt, &outputGLSL)) &driverUniformsExt))
{ {
return false; return false;
} }
} }
// Write translated shader. // Write translated shader.
TOutputVulkanGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(),
getNameMap(), &getSymbolTable(), getShaderType(),
getShaderVersion(), getOutputType(), precisionEmulation,
enablePrecision, compileOptions);
root->traverse(&outputGLSL); root->traverse(&outputGLSL);
return compileToSpirv(sink); return compileToSpirv(sink);
......
src/compiler/translator/TranslatorVulkan.h
...@@ -39,8 +39,7 @@ class TranslatorVulkan : public TCompiler ...@@ -39,8 +39,7 @@ class TranslatorVulkan : public TCompiler
ShCompileOptions compileOptions, ShCompileOptions compileOptions,
PerformanceDiagnostics *perfDiagnostics, PerformanceDiagnostics *perfDiagnostics,
SpecConst *specConst, SpecConst *specConst,
DriverUniform *driverUniforms, DriverUniform *driverUniforms);
TOutputVulkanGLSL *outputGLSL);
// Give subclass such as TranslatorMetal a chance to do depth transform before // Give subclass such as TranslatorMetal a chance to do depth transform before
// TranslatorVulkan apply its own transform. // TranslatorVulkan apply its own transform.
......
src/compiler/translator/tree_ops/vulkan/NameEmbeddedUniformStructs.cpp → src/compiler/translator/tree_ops/vulkan/SeparateStructFromUniformDeclarations.cpp
...@@ -3,10 +3,10 @@ ...@@ -3,10 +3,10 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// //
// NameEmbeddedUniformStructs: Gives nameless uniform struct internal names. // SeparateStructFromUniformDeclarations: Separate struct declarations from uniform declarations.
// //
#include "compiler/translator/tree_ops/vulkan/NameEmbeddedUniformStructs.h" #include "compiler/translator/tree_ops/vulkan/SeparateStructFromUniformDeclarations.h"
#include "compiler/translator/SymbolTable.h" #include "compiler/translator/SymbolTable.h"
#include "compiler/translator/tree_util/IntermTraverse.h" #include "compiler/translator/tree_util/IntermTraverse.h"
...@@ -41,12 +41,7 @@ class Traverser : public TIntermTraverser ...@@ -41,12 +41,7 @@ class Traverser : public TIntermTraverser
if (type.isStructSpecifier() && type.getQualifier() == EvqUniform) if (type.isStructSpecifier() && type.getQualifier() == EvqUniform)
{ {
const TStructure *structure = type.getStruct(); doReplacement(decl, declarator, type.getStruct());
if (structure->symbolType() == SymbolType::Empty)
{
doReplacement(decl, declarator, structure);
}
} }
return false; return false;
...@@ -67,9 +62,13 @@ class Traverser : public TIntermTraverser ...@@ -67,9 +62,13 @@ class Traverser : public TIntermTraverser
const TStructure *oldStructure) const TStructure *oldStructure)
{ {
// struct <structName> { ... }; // struct <structName> { ... };
TStructure *structure = new TStructure(mSymbolTable, kEmptyImmutableString, const TStructure *structure = oldStructure;
&oldStructure->fields(), SymbolType::AngleInternal); if (oldStructure->symbolType() == SymbolType::Empty)
TType *namedType = new TType(structure, true); {
structure = new TStructure(mSymbolTable, kEmptyImmutableString, &oldStructure->fields(),
SymbolType::AngleInternal);
}
TType *namedType = new TType(structure, true);
namedType->setQualifier(EvqGlobal); namedType->setQualifier(EvqGlobal);
TVariable *structVariable = TVariable *structVariable =
...@@ -107,10 +106,12 @@ class Traverser : public TIntermTraverser ...@@ -107,10 +106,12 @@ class Traverser : public TIntermTraverser
}; };
} // anonymous namespace } // anonymous namespace
bool NameEmbeddedStructUniforms(TCompiler *compiler, TIntermBlock *root, TSymbolTable *symbolTable) bool SeparateStructFromUniformDeclarations(TCompiler *compiler,
TIntermBlock *root,
TSymbolTable *symbolTable)
{ {
Traverser nameStructs(symbolTable); Traverser separateStructDecls(symbolTable);
root->traverse(&nameStructs); root->traverse(&separateStructDecls);
return nameStructs.updateTree(compiler, root); return separateStructDecls.updateTree(compiler, root);
} }
} // namespace sh } // namespace sh
src/compiler/translator/tree_ops/vulkan/NameEmbeddedUniformStructs.h → src/compiler/translator/tree_ops/vulkan/SeparateStructFromUniformDeclarations.h
...@@ -3,7 +3,8 @@ ...@@ -3,7 +3,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// //
// NameEmbeddedUniformStructs: Gives nameless uniform struct internal names. // SeparateStructFromUniformDeclarations: Separate struct declarations from uniform declarations.
// It necessarily gives nameless uniform structs internal names.
// //
// For example: // For example:
// uniform struct { int a; } uni; // uniform struct { int a; } uni;
...@@ -11,9 +12,16 @@ ...@@ -11,9 +12,16 @@
// struct s1 { int a; }; // struct s1 { int a; };
// uniform s1 uni; // uniform s1 uni;
// //
// And:
// uniform struct S { int a; } uni;
// becomes:
// struct S { int a; };
// uniform S uni;
//
//
#ifndef COMPILER_TRANSLATOR_TREEOPS_VULKAN_NAMEEMBEDDEDUNIFORMSTRUCTS_H_ #ifndef COMPILER_TRANSLATOR_TREEOPS_VULKAN_SEPARATESTRUCTFROMUNIFORMDECLARATIONS_H_
#define COMPILER_TRANSLATOR_TREEOPS_VULKAN_NAMEEMBEDDEDUNIFORMSTRUCTS_H_ #define COMPILER_TRANSLATOR_TREEOPS_VULKAN_SEPARATESTRUCTFROMUNIFORMDECLARATIONS_H_
#include "common/angleutils.h" #include "common/angleutils.h"
...@@ -23,9 +31,9 @@ class TCompiler; ...@@ -23,9 +31,9 @@ class TCompiler;
class TIntermBlock; class TIntermBlock;
class TSymbolTable; class TSymbolTable;
ANGLE_NO_DISCARD bool NameEmbeddedStructUniforms(TCompiler *compiler, ANGLE_NO_DISCARD bool SeparateStructFromUniformDeclarations(TCompiler *compiler,
TIntermBlock *root, TIntermBlock *root,
TSymbolTable *symbolTable); TSymbolTable *symbolTable);
} // namespace sh } // namespace sh
#endif // COMPILER_TRANSLATOR_TREEOPS_VULKAN_NAMEEMBEDDEDUNIFORMSTRUCTS_H_ #endif // COMPILER_TRANSLATOR_TREEOPS_VULKAN_SEPARATESTRUCTFROMUNIFORMDECLARATIONS_H_
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