Accept const array initialization in shader parsing
Array constructors are not folded, unlike all other constant expressions.
Change initializer parsing path so that it accepts constant initializers
whether they are folded or not.
Some parts need to be adapted to work with expressions that are qualified
as constant but that are not necessarily folded:
1. Identifier parsing
2. Indexing parsing
3. Field selection parsing
4. HLSL output for variable declarations
5. Determining unary operator result type
6. Determining binary operator result type
7. Determining built-in function call result type
8. Determining ternary operator result type
Corner cases that are not supported yet:
1. Using array constructors inside case labels
2. Using array constructors inside array size expressions
3. Detecting when a negative constant expression containing an array
constructor is used to index an array
In these cases being able to constant fold the expression is essential to
validating that the code is correct, so they require a more sophisticated
solution. For now we keep the old code that rejects the shader if ANGLE
hasn't been able to constant fold the case label or array size. In case of
indexing an array with a negative constant expression containing an array
constructor, ANGLE will simply treat it as a non-constant expression.
BUG=541551
BUG=angleproject:1094
TEST=dEQP-GLES3.functional.shaders.constant_expressions.* (all pass),
angle_unittests
Change-Id: I0cbc47afd1651a4dece3d68acf7ec72a01fdf047
Reviewed-on: https://chromium-review.googlesource.com/310231
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 0a089c6..d35e88e 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -320,6 +320,19 @@
return true;
}
+bool TIntermAggregate::areChildrenConstQualified()
+{
+ for (TIntermNode *&child : mSequence)
+ {
+ TIntermTyped *typed = child->getAsTyped();
+ if (typed && typed->getQualifier() != EvqConst)
+ {
+ return false;
+ }
+ }
+ return true;
+}
+
void TIntermAggregate::setPrecisionFromChildren()
{
mGotPrecisionFromChildren = true;
@@ -587,7 +600,10 @@
}
}
- mType.setQualifier(EvqTemporary);
+ if (mOperand->getQualifier() == EvqConst)
+ mType.setQualifier(EvqConst);
+ else
+ mType.setQualifier(EvqTemporary);
}
//
@@ -612,10 +628,12 @@
mLeft->getPrecision(), mRight->getPrecision());
getTypePointer()->setPrecision(higherPrecision);
+ TQualifier resultQualifier = EvqConst;
// Binary operations results in temporary variables unless both
// operands are const.
if (mLeft->getQualifier() != EvqConst || mRight->getQualifier() != EvqConst)
{
+ resultQualifier = EvqTemporary;
getTypePointer()->setQualifier(EvqTemporary);
}
@@ -670,14 +688,15 @@
if (mLeft->isVector())
{
mOp = EOpVectorTimesMatrix;
- setType(TType(basicType, higherPrecision, EvqTemporary,
+ setType(TType(basicType, higherPrecision, resultQualifier,
static_cast<unsigned char>(mRight->getCols()), 1));
}
else
{
mOp = EOpMatrixTimesScalar;
- setType(TType(basicType, higherPrecision, EvqTemporary,
- static_cast<unsigned char>(mRight->getCols()), static_cast<unsigned char>(mRight->getRows())));
+ setType(TType(basicType, higherPrecision, resultQualifier,
+ static_cast<unsigned char>(mRight->getCols()),
+ static_cast<unsigned char>(mRight->getRows())));
}
}
else if (mLeft->isMatrix() && !mRight->isMatrix())
@@ -685,7 +704,7 @@
if (mRight->isVector())
{
mOp = EOpMatrixTimesVector;
- setType(TType(basicType, higherPrecision, EvqTemporary,
+ setType(TType(basicType, higherPrecision, resultQualifier,
static_cast<unsigned char>(mLeft->getRows()), 1));
}
else
@@ -696,8 +715,9 @@
else if (mLeft->isMatrix() && mRight->isMatrix())
{
mOp = EOpMatrixTimesMatrix;
- setType(TType(basicType, higherPrecision, EvqTemporary,
- static_cast<unsigned char>(mRight->getCols()), static_cast<unsigned char>(mLeft->getRows())));
+ setType(TType(basicType, higherPrecision, resultQualifier,
+ static_cast<unsigned char>(mRight->getCols()),
+ static_cast<unsigned char>(mLeft->getRows())));
}
else if (!mLeft->isMatrix() && !mRight->isMatrix())
{
@@ -708,7 +728,7 @@
else if (mLeft->isVector() || mRight->isVector())
{
mOp = EOpVectorTimesScalar;
- setType(TType(basicType, higherPrecision, EvqTemporary,
+ setType(TType(basicType, higherPrecision, resultQualifier,
static_cast<unsigned char>(nominalSize), 1));
}
}
@@ -751,8 +771,9 @@
else if (mLeft->isMatrix() && mRight->isMatrix())
{
mOp = EOpMatrixTimesMatrixAssign;
- setType(TType(basicType, higherPrecision, EvqTemporary,
- static_cast<unsigned char>(mRight->getCols()), static_cast<unsigned char>(mLeft->getRows())));
+ setType(TType(basicType, higherPrecision, resultQualifier,
+ static_cast<unsigned char>(mRight->getCols()),
+ static_cast<unsigned char>(mLeft->getRows())));
}
else if (!mLeft->isMatrix() && !mRight->isMatrix())
{
@@ -765,7 +786,7 @@
if (!mLeft->isVector())
return false;
mOp = EOpVectorTimesScalarAssign;
- setType(TType(basicType, higherPrecision, EvqTemporary,
+ setType(TType(basicType, higherPrecision, resultQualifier,
static_cast<unsigned char>(mLeft->getNominalSize()), 1));
}
}
@@ -835,8 +856,9 @@
{
const int secondarySize = std::max(
mLeft->getSecondarySize(), mRight->getSecondarySize());
- setType(TType(basicType, higherPrecision, EvqTemporary,
- static_cast<unsigned char>(nominalSize), static_cast<unsigned char>(secondarySize)));
+ setType(TType(basicType, higherPrecision, resultQualifier,
+ static_cast<unsigned char>(nominalSize),
+ static_cast<unsigned char>(secondarySize)));
if (mLeft->isArray())
{
ASSERT(mLeft->getArraySize() == mRight->getArraySize());
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 6ea7eb6..a28799f 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -516,6 +516,7 @@
void setUseEmulatedFunction() { mUseEmulatedFunction = true; }
bool getUseEmulatedFunction() { return mUseEmulatedFunction; }
+ bool areChildrenConstQualified();
void setPrecisionFromChildren();
void setBuiltInFunctionPrecision();
diff --git a/src/compiler/translator/Intermediate.cpp b/src/compiler/translator/Intermediate.cpp
index 87bf2eb..62c6f13 100644
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -309,15 +309,20 @@
TIntermTyped *TIntermediate::addSelection(TIntermTyped *cond, TIntermTyped *trueBlock, TIntermTyped *falseBlock,
const TSourceLoc &line)
{
+ TQualifier resultQualifier = EvqTemporary;
+ if (cond->getQualifier() == EvqConst && trueBlock->getQualifier() == EvqConst &&
+ falseBlock->getQualifier() == EvqConst)
+ {
+ resultQualifier = EvqConst;
+ }
// Right now it's safe to fold ternary operators only when all operands
// are constant. If only the condition is constant, it's theoretically
// possible to fold the ternary operator, but that requires making sure
// that the node returned from here won't be treated as a constant
// expression in case the node that gets eliminated was not a constant
// expression.
- if (cond->getAsConstantUnion() &&
- trueBlock->getAsConstantUnion() &&
- falseBlock->getAsConstantUnion())
+ if (resultQualifier == EvqConst && cond->getAsConstantUnion() &&
+ trueBlock->getAsConstantUnion() && falseBlock->getAsConstantUnion())
{
if (cond->getAsConstantUnion()->getBConst(0))
return trueBlock;
@@ -329,7 +334,7 @@
// Make a selection node.
//
TIntermSelection *node = new TIntermSelection(cond, trueBlock, falseBlock, trueBlock->getType());
- node->getTypePointer()->setQualifier(EvqTemporary);
+ node->getTypePointer()->setQualifier(resultQualifier);
node->setLine(line);
return node;
diff --git a/src/compiler/translator/OutputHLSL.cpp b/src/compiler/translator/OutputHLSL.cpp
index 117c34a..1c7b0c4 100644
--- a/src/compiler/translator/OutputHLSL.cpp
+++ b/src/compiler/translator/OutputHLSL.cpp
@@ -1869,7 +1869,9 @@
TIntermTyped *variable = (*sequence)[0]->getAsTyped();
ASSERT(sequence->size() == 1);
- if (variable && (variable->getQualifier() == EvqTemporary || variable->getQualifier() == EvqGlobal))
+ if (variable &&
+ (variable->getQualifier() == EvqTemporary ||
+ variable->getQualifier() == EvqGlobal || variable->getQualifier() == EvqConst))
{
ensureStructDefined(variable->getType());
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 84ba5e4..03a4fd3 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -1188,7 +1188,7 @@
{
const TVariable *variable = getNamedVariable(location, name, symbol);
- if (variable->getType().getQualifier() == EvqConst)
+ if (variable->getType().getQualifier() == EvqConst && variable->getConstPointer())
{
TConstantUnion *constArray = variable->getConstPointer();
TType t(variable->getType());
@@ -1310,9 +1310,17 @@
variable->getType().setQualifier(EvqTemporary);
return true;
}
+
+ // Save the constant folded value to the variable if possible. For example array
+ // initializers are not folded, since that way copying the array literal to multiple places
+ // in the shader is avoided.
+ // TODO(oetuaho@nvidia.com): Consider constant folding array initialization in cases where
+ // it would be beneficial.
if (initializer->getAsConstantUnion())
{
variable->shareConstPointer(initializer->getAsConstantUnion()->getUnionArrayPointer());
+ *intermNode = nullptr;
+ return false;
}
else if (initializer->getAsSymbolNode())
{
@@ -1321,34 +1329,22 @@
const TVariable *tVar = static_cast<const TVariable *>(symbol);
TConstantUnion *constArray = tVar->getConstPointer();
- variable->shareConstPointer(constArray);
- }
- else
- {
- std::stringstream extraInfoStream;
- extraInfoStream << "'" << variable->getType().getCompleteString() << "'";
- std::string extraInfo = extraInfoStream.str();
- error(line, " cannot assign to", "=", extraInfo.c_str());
- variable->getType().setQualifier(EvqTemporary);
- return true;
+ if (constArray)
+ {
+ variable->shareConstPointer(constArray);
+ *intermNode = nullptr;
+ return false;
+ }
}
}
- if (qualifier != EvqConst)
+ TIntermSymbol *intermSymbol = intermediate.addSymbol(
+ variable->getUniqueId(), variable->getName(), variable->getType(), line);
+ *intermNode = createAssign(EOpInitialize, intermSymbol, initializer, line);
+ if (*intermNode == nullptr)
{
- TIntermSymbol *intermSymbol = intermediate.addSymbol(
- variable->getUniqueId(), variable->getName(), variable->getType(), line);
- *intermNode = createAssign(EOpInitialize, intermSymbol, initializer, line);
- if (*intermNode == nullptr)
- {
- assignError(line, "=", intermSymbol->getCompleteString(),
- initializer->getCompleteString());
- return true;
- }
- }
- else
- {
- *intermNode = nullptr;
+ assignError(line, "=", intermSymbol->getCompleteString(), initializer->getCompleteString());
+ return true;
}
return false;
@@ -2791,16 +2787,17 @@
recover();
index = 0;
}
- if (baseExpression->getType().getQualifier() == EvqConst)
+ if (baseExpression->getType().getQualifier() == EvqConst &&
+ baseExpression->getAsConstantUnion())
{
if (baseExpression->isArray())
{
- // constant folding for arrays
+ // constant folding for array indexing
indexedExpression = addConstArrayNode(index, baseExpression, location);
}
else if (baseExpression->isVector())
{
- // constant folding for vectors
+ // constant folding for vector indexing
TVectorFields fields;
fields.num = 1;
fields.offsets[0] =
@@ -2809,7 +2806,7 @@
}
else if (baseExpression->isMatrix())
{
- // constant folding for matrices
+ // constant folding for matrix indexing
indexedExpression = addConstMatrixNode(index, baseExpression, location);
}
}
@@ -2948,7 +2945,8 @@
recover();
}
- if (baseExpression->getType().getQualifier() == EvqConst)
+ if (baseExpression->getType().getQualifier() == EvqConst &&
+ baseExpression->getAsConstantUnion())
{
// constant folding for vector fields
indexedExpression = addConstVectorNode(fields, baseExpression, fieldLocation);
@@ -2998,7 +2996,8 @@
}
if (fieldFound)
{
- if (baseExpression->getType().getQualifier() == EvqConst)
+ if (baseExpression->getType().getQualifier() == EvqConst &&
+ baseExpression->getAsConstantUnion())
{
indexedExpression = addConstStruct(fieldString, baseExpression, dotLocation);
if (indexedExpression == 0)
@@ -3090,6 +3089,11 @@
indexedExpression = baseExpression;
}
+ if (baseExpression->getQualifier() == EvqConst)
+ {
+ indexedExpression->getTypePointer()->setQualifier(EvqConst);
+ }
+
return indexedExpression;
}
@@ -3919,6 +3923,10 @@
intermediate.setAggregateOperator(paramNode, op, loc);
aggregate->setType(fnCandidate->getReturnType());
aggregate->setPrecisionFromChildren();
+ if (aggregate->areChildrenConstQualified())
+ {
+ aggregate->getTypePointer()->setQualifier(EvqConst);
+ }
// Some built-in functions have out parameters too.
functionCallLValueErrorCheck(fnCandidate, aggregate);
diff --git a/src/compiler/translator/SeparateArrayInitialization.cpp b/src/compiler/translator/SeparateArrayInitialization.cpp
index f8d1466..78aeb8d 100644
--- a/src/compiler/translator/SeparateArrayInitialization.cpp
+++ b/src/compiler/translator/SeparateArrayInitialization.cpp
@@ -9,6 +9,10 @@
// will effectively become
// type[n] a;
// a = initializer;
+//
+// Note that if the array is declared as const, the initialization is still split, making the AST
+// technically invalid. Because of that this transformation should only be used when subsequent
+// stages don't care about const qualifiers.
#include "compiler/translator/SeparateArrayInitialization.h"
diff --git a/src/compiler/translator/SeparateArrayInitialization.h b/src/compiler/translator/SeparateArrayInitialization.h
index d7c3ae6..18c275f 100644
--- a/src/compiler/translator/SeparateArrayInitialization.h
+++ b/src/compiler/translator/SeparateArrayInitialization.h
@@ -9,6 +9,10 @@
// will effectively become
// type[n] a;
// a = initializer;
+//
+// Note that if the array is declared as const, the initialization is still split, making the AST
+// technically invalid. Because of that this transformation should only be used when subsequent
+// stages don't care about const qualifiers.
#ifndef COMPILER_TRANSLATOR_SEPARATEARRAYINITIALIZATION_H_
#define COMPILER_TRANSLATOR_SEPARATEARRAYINITIALIZATION_H_