Initialize uninitialized locals in GLSL output
Guarantee that local variables are initialized before they are used
in GLSL output. In HLSL output all variables were already being
initialized.
Locals are initialized using an AST transform. The local variable init
can only be run after some simplification of the AST, so that it is
able to handle complex cases like:
for (int i[2], j = i[0]; i[0] < 3; ++i[0]) {
}
If we're dealing with ESSL 1.00 which lacks array constructors, in
this kind of case the uninitialized array initialization code needs to
be hoisted out of the loop init statement, and the code also needs to
make sure that j's initializer is run after i is initialized.
Another complex case involves nameless structs. This can be an issue
also in ESSL 3.00 and above:
for (struct { float f; } s; s.f < 1.0; ++s.f) {
}
Since the struct doesn't have a name, its constructor can not be used.
We solve this by initializing the struct members individually,
similarly to how arrays are initialized in ESSL 1.00.
Initializing local variables is disabled on Mac and Android for now.
On Mac, invalid behavior was exposed in the WebGL 2.0 tests when
enabling it. On Android, the dEQP test runs failed for an unknown
reason. Bugs have been opened to resolve these issues later.
BUG=angleproject:1966
TEST=angle_end2end_tests, WebGL conformance tests
Change-Id: Ic06927f5b6cc9619bc82c647ee966605cd80bab2
Reviewed-on: https://chromium-review.googlesource.com/504728
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/InitializeVariables.cpp b/src/compiler/translator/InitializeVariables.cpp
index 0c9a959..11ba6fb 100644
--- a/src/compiler/translator/InitializeVariables.cpp
+++ b/src/compiler/translator/InitializeVariables.cpp
@@ -19,7 +19,70 @@
namespace
{
-void InsertInitCode(TIntermSequence *sequence,
+bool IsNamelessStruct(const TIntermTyped *node)
+{
+ return (node->getBasicType() == EbtStruct && node->getType().getStruct()->name() == "");
+}
+
+void AddArrayZeroInitSequence(const TIntermTyped *initializedNode,
+ TIntermSequence *initSequenceOut);
+
+TIntermBinary *CreateZeroInitAssignment(const TIntermTyped *initializedNode)
+{
+ TIntermTyped *zero = TIntermTyped::CreateZero(initializedNode->getType());
+ return new TIntermBinary(EOpAssign, initializedNode->deepCopy(), zero);
+}
+
+void AddStructZeroInitSequence(const TIntermTyped *initializedNode,
+ TIntermSequence *initSequenceOut)
+{
+ ASSERT(initializedNode->getBasicType() == EbtStruct);
+ TStructure *structType = initializedNode->getType().getStruct();
+ for (int i = 0; i < static_cast<int>(structType->fields().size()); ++i)
+ {
+ TIntermBinary *element = new TIntermBinary(
+ EOpIndexDirectStruct, initializedNode->deepCopy(), TIntermTyped::CreateIndexNode(i));
+ if (element->isArray())
+ {
+ AddArrayZeroInitSequence(element, initSequenceOut);
+ }
+ else if (element->getType().isStructureContainingArrays())
+ {
+ AddStructZeroInitSequence(element, initSequenceOut);
+ }
+ else
+ {
+ // Structs can't be defined inside structs, so the type of a struct field can't be a
+ // nameless struct.
+ ASSERT(!IsNamelessStruct(element));
+ initSequenceOut->push_back(CreateZeroInitAssignment(element));
+ }
+ }
+}
+
+void AddArrayZeroInitSequence(const TIntermTyped *initializedNode, TIntermSequence *initSequenceOut)
+{
+ ASSERT(initializedNode->isArray());
+ // Assign the array elements one by one to keep the AST compatible with ESSL 1.00 which
+ // doesn't have array assignment.
+ // Note that it is important to have the array init in the right order to workaround
+ // http://crbug.com/709317
+ for (unsigned int i = 0; i < initializedNode->getArraySize(); ++i)
+ {
+ TIntermBinary *element = new TIntermBinary(EOpIndexDirect, initializedNode->deepCopy(),
+ TIntermTyped::CreateIndexNode(i));
+ if (element->getType().isStructureContainingArrays())
+ {
+ AddStructZeroInitSequence(element, initSequenceOut);
+ }
+ else
+ {
+ initSequenceOut->push_back(CreateZeroInitAssignment(element));
+ }
+ }
+}
+
+void InsertInitCode(TIntermSequence *mainBody,
const InitVariableList &variables,
const TSymbolTable &symbolTable)
{
@@ -27,62 +90,118 @@
{
TString name = TString(var.name.c_str());
+ TIntermSymbol *initializedSymbol = nullptr;
if (var.isArray())
{
- // Assign the array elements one by one to keep the AST compatible with ESSL 1.00 which
- // doesn't have array assignment.
size_t pos = name.find_last_of('[');
if (pos != TString::npos)
{
name = name.substr(0, pos);
}
- TType elementType = sh::GetShaderVariableBasicType(var);
- TType arrayType = elementType;
+ TType arrayType = sh::GetShaderVariableBasicType(var);
arrayType.setArraySize(var.elementCount());
-
- // Workaround for http://crbug.com/709317
- // This loop is reversed to initialize elements in increasing
- // order [0 1 2 ...]. Otherwise, they're initialized in
- // decreasing order [... 2 1 0], due to
- // `sequence->insert(sequence->begin(), ...)` below.
- for (unsigned int i = var.arraySize; i > 0; --i)
- {
- unsigned int index = i - 1;
- TIntermSymbol *arraySymbol = new TIntermSymbol(0, name, arrayType);
- TIntermBinary *element = new TIntermBinary(EOpIndexDirect, arraySymbol,
- TIntermTyped::CreateIndexNode(index));
-
- TIntermTyped *zero = TIntermTyped::CreateZero(elementType);
- TIntermBinary *assignment = new TIntermBinary(EOpAssign, element, zero);
-
- sequence->insert(sequence->begin(), assignment);
- }
+ initializedSymbol = new TIntermSymbol(0, name, arrayType);
}
else if (var.isStruct())
{
TVariable *structInfo = reinterpret_cast<TVariable *>(symbolTable.findGlobal(name));
ASSERT(structInfo);
- TIntermSymbol *symbol = new TIntermSymbol(0, name, structInfo->getType());
- TIntermTyped *zero = TIntermTyped::CreateZero(structInfo->getType());
-
- TIntermBinary *assign = new TIntermBinary(EOpAssign, symbol, zero);
- sequence->insert(sequence->begin(), assign);
+ initializedSymbol = new TIntermSymbol(0, name, structInfo->getType());
}
else
{
- TType type = sh::GetShaderVariableBasicType(var);
- TIntermSymbol *symbol = new TIntermSymbol(0, name, type);
- TIntermTyped *zero = TIntermTyped::CreateZero(type);
-
- TIntermBinary *assign = new TIntermBinary(EOpAssign, symbol, zero);
- sequence->insert(sequence->begin(), assign);
+ TType type = sh::GetShaderVariableBasicType(var);
+ initializedSymbol = new TIntermSymbol(0, name, type);
}
+ TIntermSequence *initCode = CreateInitCode(initializedSymbol);
+ mainBody->insert(mainBody->begin(), initCode->begin(), initCode->end());
}
}
+class InitializeLocalsTraverser : public TIntermTraverser
+{
+ public:
+ InitializeLocalsTraverser(int shaderVersion)
+ : TIntermTraverser(true, false, false), mShaderVersion(shaderVersion)
+ {
+ }
+
+ protected:
+ bool visitDeclaration(Visit visit, TIntermDeclaration *node) override
+ {
+ for (TIntermNode *declarator : *node->getSequence())
+ {
+ if (!mInGlobalScope && !declarator->getAsBinaryNode())
+ {
+ TIntermSymbol *symbol = declarator->getAsSymbolNode();
+ ASSERT(symbol);
+ if (symbol->getSymbol() == "")
+ {
+ continue;
+ }
+
+ // Arrays may need to be initialized one element at a time, since ESSL 1.00 does not
+ // support array constructors or assigning arrays.
+ bool arrayConstructorUnavailable =
+ (symbol->isArray() || symbol->getType().isStructureContainingArrays()) &&
+ mShaderVersion == 100;
+ // Nameless struct constructors can't be referred to, so they also need to be
+ // initialized one element at a time.
+ if (arrayConstructorUnavailable || IsNamelessStruct(symbol))
+ {
+ // SimplifyLoopConditions should have been run so the parent node of this node
+ // should not be a loop.
+ ASSERT(getParentNode()->getAsLoopNode() == nullptr);
+ // SeparateDeclarations should have already been run, so we don't need to worry
+ // about further declarators in this declaration depending on the effects of
+ // this declarator.
+ ASSERT(node->getSequence()->size() == 1);
+ insertStatementsInParentBlock(TIntermSequence(), *CreateInitCode(symbol));
+ }
+ else
+ {
+ TIntermBinary *init = new TIntermBinary(
+ EOpInitialize, symbol, TIntermTyped::CreateZero(symbol->getType()));
+ queueReplacementWithParent(node, symbol, init, OriginalNode::BECOMES_CHILD);
+ }
+ }
+ }
+ return false;
+ }
+
+ private:
+ int mShaderVersion;
+};
+
} // namespace anonymous
+TIntermSequence *CreateInitCode(const TIntermSymbol *initializedSymbol)
+{
+ TIntermSequence *initCode = new TIntermSequence();
+ if (initializedSymbol->isArray())
+ {
+ AddArrayZeroInitSequence(initializedSymbol, initCode);
+ }
+ else if (initializedSymbol->getType().isStructureContainingArrays() ||
+ IsNamelessStruct(initializedSymbol))
+ {
+ AddStructZeroInitSequence(initializedSymbol, initCode);
+ }
+ else
+ {
+ initCode->push_back(CreateZeroInitAssignment(initializedSymbol));
+ }
+ return initCode;
+}
+
+void InitializeUninitializedLocals(TIntermBlock *root, int shaderVersion)
+{
+ InitializeLocalsTraverser traverser(shaderVersion);
+ root->traverse(&traverser);
+ traverser.updateTree();
+}
+
void InitializeVariables(TIntermBlock *root,
const InitVariableList &vars,
const TSymbolTable &symbolTable)