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/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index b4fcbb7..81c72d4 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -219,8 +219,7 @@
mGeometryShaderInvocations(0),
mGeometryShaderMaxVertices(-1),
mMaxGeometryShaderInvocations(resources.MaxGeometryShaderInvocations),
- mMaxGeometryShaderMaxVertices(resources.MaxGeometryOutputVertices),
- mGlInVariableWithArraySize(nullptr)
+ mMaxGeometryShaderMaxVertices(resources.MaxGeometryOutputVertices)
{
}
@@ -443,6 +442,36 @@
}
}
+void TParseContext::markStaticReadIfSymbol(TIntermNode *node)
+{
+ TIntermSwizzle *swizzleNode = node->getAsSwizzleNode();
+ if (swizzleNode)
+ {
+ markStaticReadIfSymbol(swizzleNode->getOperand());
+ return;
+ }
+ TIntermBinary *binaryNode = node->getAsBinaryNode();
+ if (binaryNode)
+ {
+ switch (binaryNode->getOp())
+ {
+ case EOpIndexDirect:
+ case EOpIndexIndirect:
+ case EOpIndexDirectStruct:
+ case EOpIndexDirectInterfaceBlock:
+ markStaticReadIfSymbol(binaryNode->getLeft());
+ return;
+ default:
+ return;
+ }
+ }
+ TIntermSymbol *symbolNode = node->getAsSymbolNode();
+ if (symbolNode)
+ {
+ symbolTable.markStaticRead(symbolNode->variable());
+ }
+}
+
// Both test and if necessary, spit out an error, to see if the node is really
// an l-value that can be operated on this way.
bool TParseContext::checkCanBeLValue(const TSourceLoc &line, const char *op, TIntermTyped *node)
@@ -584,6 +613,7 @@
TIntermSymbol *symNode = node->getAsSymbolNode();
if (message.empty() && symNode != nullptr)
{
+ symbolTable.markStaticWrite(symNode->variable());
return true;
}
@@ -689,6 +719,7 @@
for (TIntermNode *arg : arguments)
{
+ markStaticReadIfSymbol(arg);
const TIntermTyped *argTyped = arg->getAsTyped();
ASSERT(argTyped != nullptr);
if (type.getBasicType() != EbtStruct && IsOpaqueType(argTyped->getBasicType()))
@@ -1665,15 +1696,20 @@
{
TQualifier qual = fnCandidate->getParam(i)->getType().getQualifier();
TIntermTyped *argument = (*(fnCall->getSequence()))[i]->getAsTyped();
- if (!IsImage(argument->getBasicType()) && (IsQualifierUnspecified(qual) || qual == EvqIn ||
- qual == EvqInOut || qual == EvqConstReadOnly))
+ bool argumentIsRead = (IsQualifierUnspecified(qual) || qual == EvqIn || qual == EvqInOut ||
+ qual == EvqConstReadOnly);
+ if (argumentIsRead)
{
- if (argument->getMemoryQualifier().writeonly)
+ markStaticReadIfSymbol(argument);
+ if (!IsImage(argument->getBasicType()))
{
- error(argument->getLine(),
- "Writeonly value cannot be passed for 'in' or 'inout' parameters.",
- fnCall->functionName());
- return;
+ if (argument->getMemoryQualifier().writeonly)
+ {
+ error(argument->getLine(),
+ "Writeonly value cannot be passed for 'in' or 'inout' parameters.",
+ fnCall->functionName());
+ return;
+ }
}
}
if (qual == EvqOut || qual == EvqInOut)
@@ -1878,8 +1914,8 @@
else if ((mGeometryShaderInputPrimitiveType != EptUndefined) &&
(variableType.getQualifier() == EvqPerVertexIn))
{
- ASSERT(mGlInVariableWithArraySize != nullptr);
- node = new TIntermSymbol(mGlInVariableWithArraySize);
+ ASSERT(symbolTable.getGlInVariableWithArraySize() != nullptr);
+ node = new TIntermSymbol(symbolTable.getGlInVariableWithArraySize());
}
else
{
@@ -1994,6 +2030,7 @@
}
*initNode = new TIntermBinary(EOpInitialize, intermSymbol, initializer);
+ markStaticReadIfSymbol(initializer);
(*initNode)->setLine(line);
return true;
}
@@ -2036,8 +2073,19 @@
TIntermTyped *typedCond = nullptr;
if (cond)
{
+ markStaticReadIfSymbol(cond);
typedCond = cond->getAsTyped();
}
+ if (expr)
+ {
+ markStaticReadIfSymbol(expr);
+ }
+ // In case the loop body was not parsed as a block and contains a statement that simply refers
+ // to a variable, we need to mark it as statically used.
+ if (body)
+ {
+ markStaticReadIfSymbol(body);
+ }
if (cond == nullptr || typedCond)
{
if (type == ELoopDoWhile)
@@ -2084,6 +2132,16 @@
const TSourceLoc &loc)
{
bool isScalarBool = checkIsScalarBool(loc, cond);
+ // In case the conditional statements were not parsed as blocks and contain a statement that
+ // simply refers to a variable, we need to mark them as statically used.
+ if (code.node1)
+ {
+ markStaticReadIfSymbol(code.node1);
+ }
+ if (code.node2)
+ {
+ markStaticReadIfSymbol(code.node2);
+ }
// For compile time constant conditions, prune the code now.
if (isScalarBool && cond->getAsConstantUnion())
@@ -2099,6 +2157,7 @@
}
TIntermIfElse *node = new TIntermIfElse(cond, EnsureBlock(code.node1), EnsureBlock(code.node2));
+ markStaticReadIfSymbol(cond);
node->setLine(loc);
return node;
@@ -2346,9 +2405,9 @@
// input primitive declaration.
if (mGeometryShaderInputPrimitiveType != EptUndefined)
{
- ASSERT(mGlInVariableWithArraySize != nullptr);
+ ASSERT(symbolTable.getGlInVariableWithArraySize() != nullptr);
type->sizeOutermostUnsizedArray(
- mGlInVariableWithArraySize->getType().getOutermostArraySize());
+ symbolTable.getGlInVariableWithArraySize()->getType().getOutermostArraySize());
}
else
{
@@ -2818,17 +2877,7 @@
void TParseContext::setGeometryShaderInputArraySize(unsigned int inputArraySize,
const TSourceLoc &line)
{
- if (mGlInVariableWithArraySize == nullptr)
- {
- const TSymbol *glPerVertex = symbolTable.findBuiltIn(ImmutableString("gl_PerVertex"), 310);
- const TInterfaceBlock *glPerVertexBlock = static_cast<const TInterfaceBlock *>(glPerVertex);
- TType *glInType = new TType(glPerVertexBlock, EvqPerVertexIn, TLayoutQualifier::Create());
- glInType->makeArray(inputArraySize);
- mGlInVariableWithArraySize =
- new TVariable(&symbolTable, ImmutableString("gl_in"), glInType, SymbolType::BuiltIn,
- TExtension::EXT_geometry_shader);
- }
- else if (mGlInVariableWithArraySize->getType().getOutermostArraySize() != inputArraySize)
+ if (!symbolTable.setGlInArraySize(inputArraySize))
{
error(line,
"Array size or input primitive declaration doesn't match the size of earlier sized "
@@ -4010,6 +4059,7 @@
}
}
+ markStaticReadIfSymbol(indexExpression);
TIntermBinary *node = new TIntermBinary(EOpIndexIndirect, baseExpression, indexExpression);
node->setLine(location);
// Indirect indexing can never be constant folded.
@@ -4793,6 +4843,7 @@
return nullptr;
}
+ markStaticReadIfSymbol(init);
TIntermSwitch *node = new TIntermSwitch(init, statementList);
node->setLine(loc);
return node;
@@ -4889,6 +4940,7 @@
return nullptr;
}
+ markStaticReadIfSymbol(child);
TIntermUnary *node = new TIntermUnary(op, child);
node->setLine(loc);
@@ -5292,6 +5344,9 @@
}
TIntermBinary *node = new TIntermBinary(op, left, right);
+ ASSERT(op != EOpAssign);
+ markStaticReadIfSymbol(left);
+ markStaticReadIfSymbol(right);
node->setLine(loc);
return expressionOrFoldedResult(node);
}
@@ -5354,6 +5409,11 @@
assignError(loc, "assign", left->getCompleteString(), right->getCompleteString());
return left;
}
+ if (op != EOpAssign)
+ {
+ markStaticReadIfSymbol(left);
+ }
+ markStaticReadIfSymbol(right);
node->setLine(loc);
return node;
}
@@ -5375,6 +5435,9 @@
}
TIntermBinary *commaNode = TIntermBinary::CreateComma(left, right, mShaderVersion);
+ markStaticReadIfSymbol(left);
+ markStaticReadIfSymbol(right);
+ commaNode->setLine(loc);
return expressionOrFoldedResult(commaNode);
}
@@ -5420,6 +5483,7 @@
{
if (expression != nullptr)
{
+ markStaticReadIfSymbol(expression);
ASSERT(op == EOpReturn);
mFunctionReturnsValue = true;
if (mCurrentFunctionType->getBasicType() == EbtVoid)
@@ -5436,6 +5500,15 @@
return node;
}
+void TParseContext::appendStatement(TIntermBlock *block, TIntermNode *statement)
+{
+ if (statement != nullptr)
+ {
+ markStaticReadIfSymbol(statement);
+ block->appendStatement(statement);
+ }
+}
+
void TParseContext::checkTextureGather(TIntermAggregate *functionCall)
{
ASSERT(functionCall->getOp() == EOpCallBuiltInFunction);
@@ -5895,6 +5968,9 @@
}
TIntermTernary *node = new TIntermTernary(cond, trueExpression, falseExpression);
+ markStaticReadIfSymbol(cond);
+ markStaticReadIfSymbol(trueExpression);
+ markStaticReadIfSymbol(falseExpression);
node->setLine(loc);
return expressionOrFoldedResult(node);
}