Collect static use information during parsing
We now collect metadata for variables in the symbol table. The
metadata is stored in a map using the variable unique id as a key, so
we can store the variables themselves as constexpr while still having
dynamic metadata.
For now we collect whether a variable is statically read or written.
This can be used to more accurately determine whether a variable is
statically used, but can also enable more optimizations in the future,
such as pruning variables that are never read or folding variables
that are never written after initialization. The collection is done
during parsing, so that nothing is pruned from the AST before the
static use is recorded.
Static writes are flagged in ParseContext::checkCanBeLValue, as that
function is already called for all variables that are written.
Static reads are flagged whenever there's an operation that requires
a variable to be read. This includes:
* Unary and binary math ops
* Comma ops
* Ternary ops
* Assignments
* Returning the variable
* Passing the variable as an in or inout argument to a function
* Using the variable as a constructor argument
* Using the variable as an if statement condition
* Using the variable as a loop condition or expression
* Using the variable as an index
* Using the variable as a switch statement init expression
In case there are statements that simply refer to a variable without
doing operations on it, the variable is being treated as statically
read. Examples of such statements:
my_var;
my_arr[2];
These are a bit of a corner case, but it makes sense to treat them as
static use for validation purposes.
Collecting correct static use information costs us a bit of compiler
performance, but the regression is on the order of just a few percent
in the compiler perf tests.
BUG=angleproject:2262
TEST=angle_unittests, angle_end2end_tests
Change-Id: Ib0d7add7e4a7d11bffeb2a4861eeea982c562234
Reviewed-on: https://chromium-review.googlesource.com/977964
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/CollectVariables.cpp b/src/compiler/translator/CollectVariables.cpp
index 30294dc..05196fd 100644
--- a/src/compiler/translator/CollectVariables.cpp
+++ b/src/compiler/translator/CollectVariables.cpp
@@ -68,21 +68,21 @@
return nullptr;
}
-// Note that this shouldn't be called for interface blocks - static use information is collected for
+// Note that this shouldn't be called for interface blocks - active information is collected for
// individual fields in case of interface blocks.
-void MarkStaticallyUsed(ShaderVariable *variable)
+void MarkActive(ShaderVariable *variable)
{
- if (!variable->staticUse)
+ if (!variable->active)
{
if (variable->isStruct())
{
// Conservatively assume all fields are statically used as well.
for (auto &field : variable->fields)
{
- MarkStaticallyUsed(&field);
+ MarkActive(&field);
}
}
- variable->staticUse = true;
+ ASSERT(variable->staticUse);
variable->active = true;
}
}
@@ -126,9 +126,12 @@
private:
std::string getMappedName(const TSymbol *symbol) const;
- void setFieldOrVariableProperties(const TType &type, ShaderVariable *variableOut) const;
+ void setFieldOrVariableProperties(const TType &type,
+ bool staticUse,
+ ShaderVariable *variableOut) const;
void setFieldProperties(const TType &type,
const ImmutableString &name,
+ bool staticUse,
ShaderVariable *variableOut) const;
void setCommonVariableProperties(const TType &type,
const TVariable &variable,
@@ -322,8 +325,6 @@
ASSERT(glInType.getQualifier() == EvqPerVertexIn);
InterfaceBlock info;
recordInterfaceBlock("gl_in", glInType, &info);
- info.staticUse = true;
- info.active = true;
mPerVertexInAdded = true;
mInBlocks->push_back(info);
@@ -551,15 +552,18 @@
}
if (var)
{
- MarkStaticallyUsed(var);
+ MarkActive(var);
}
}
void CollectVariablesTraverser::setFieldOrVariableProperties(const TType &type,
+ bool staticUse,
ShaderVariable *variableOut) const
{
ASSERT(variableOut);
+ variableOut->staticUse = staticUse;
+
const TStructure *structure = type.getStruct();
if (!structure)
{
@@ -582,7 +586,7 @@
// Regardless of the variable type (uniform, in/out etc.) its fields are always plain
// ShaderVariable objects.
ShaderVariable fieldVariable;
- setFieldProperties(*field->type(), field->name(), &fieldVariable);
+ setFieldProperties(*field->type(), field->name(), staticUse, &fieldVariable);
variableOut->fields.push_back(fieldVariable);
}
}
@@ -594,10 +598,11 @@
void CollectVariablesTraverser::setFieldProperties(const TType &type,
const ImmutableString &name,
+ bool staticUse,
ShaderVariable *variableOut) const
{
ASSERT(variableOut);
- setFieldOrVariableProperties(type, variableOut);
+ setFieldOrVariableProperties(type, staticUse, variableOut);
variableOut->name.assign(name.data(), name.length());
variableOut->mappedName = HashName(name, mHashFunction, nullptr).data();
}
@@ -608,7 +613,8 @@
{
ASSERT(variableOut);
- setFieldOrVariableProperties(type, variableOut);
+ variableOut->staticUse = mSymbolTable->isStaticallyUsed(variable);
+ setFieldOrVariableProperties(type, variableOut->staticUse, variableOut);
ASSERT(variable.symbolType() != SymbolType::Empty);
variableOut->name.assign(variable.name().data(), variable.name().length());
variableOut->mappedName = getMappedName(&variable);
@@ -684,6 +690,18 @@
if (instanceName != nullptr)
{
interfaceBlock->instanceName = instanceName;
+ const TSymbol *blockSymbol = nullptr;
+ if (strncmp(instanceName, "gl_in", 5u) == 0)
+ {
+ blockSymbol = mSymbolTable->getGlInVariableWithArraySize();
+ }
+ else
+ {
+ blockSymbol = mSymbolTable->findGlobal(ImmutableString(instanceName));
+ }
+ ASSERT(blockSymbol && blockSymbol->isVariable());
+ interfaceBlock->staticUse =
+ mSymbolTable->isStaticallyUsed(*static_cast<const TVariable *>(blockSymbol));
}
ASSERT(!interfaceBlockType.isArrayOfArrays()); // Disallowed by GLSL ES 3.10 section 4.3.9
interfaceBlock->arraySize = interfaceBlockType.isArray() ? interfaceBlockType.getOutermostArraySize() : 0;
@@ -699,16 +717,36 @@
}
// Gather field information
+ bool anyFieldStaticallyUsed = false;
for (const TField *field : blockType->fields())
{
const TType &fieldType = *field->type();
+ bool staticUse = false;
+ if (instanceName == nullptr)
+ {
+ // Static use of individual fields has been recorded, since they are present in the
+ // symbol table as variables.
+ const TSymbol *fieldSymbol = mSymbolTable->findGlobal(field->name());
+ ASSERT(fieldSymbol && fieldSymbol->isVariable());
+ staticUse =
+ mSymbolTable->isStaticallyUsed(*static_cast<const TVariable *>(fieldSymbol));
+ if (staticUse)
+ {
+ anyFieldStaticallyUsed = true;
+ }
+ }
+
InterfaceBlockField fieldVariable;
- setFieldProperties(fieldType, field->name(), &fieldVariable);
+ setFieldProperties(fieldType, field->name(), staticUse, &fieldVariable);
fieldVariable.isRowMajorLayout =
(fieldType.getLayoutQualifier().matrixPacking == EmpRowMajor);
interfaceBlock->fields.push_back(fieldVariable);
}
+ if (anyFieldStaticallyUsed)
+ {
+ interfaceBlock->staticUse = true;
+ }
}
Uniform CollectVariablesTraverser::recordUniform(const TIntermSymbol &variable) const
@@ -826,7 +864,7 @@
{
if (binaryNode->getOp() == EOpIndexDirectInterfaceBlock)
{
- // NOTE: we do not determine static use for individual blocks of an array
+ // NOTE: we do not determine static use / activeness for individual blocks of an array.
TIntermTyped *blockNode = binaryNode->getLeft()->getAsTyped();
ASSERT(blockNode);
@@ -860,10 +898,13 @@
namedBlock = findNamedInterfaceBlock(interfaceBlock->name());
}
ASSERT(namedBlock);
- namedBlock->staticUse = true;
+ ASSERT(namedBlock->staticUse);
namedBlock->active = true;
unsigned int fieldIndex = static_cast<unsigned int>(constantUnion->getIConst(0));
ASSERT(fieldIndex < namedBlock->fields.size());
+ // TODO(oetuaho): Would be nicer to record static use of fields of named interface blocks
+ // more accurately at parse time - now we only mark the fields statically used if they are
+ // active. http://anglebug.com/2440
namedBlock->fields[fieldIndex].staticUse = true;
namedBlock->fields[fieldIndex].active = true;