Fix deferring global array initialization

src/compiler/translator/DeferGlobalInitializers.cpp

@@ -27,17 +27,20 @@ void SetInternalFunctionName(TIntermAggregate *functionNode, const char *name)
     functionNode->setNameObj(nameObj);
 }
 
-TIntermAggregate *CreateFunctionPrototypeNode(const char *name)
+TIntermAggregate *CreateFunctionPrototypeNode(const char *name, const int functionId)
 {
     TIntermAggregate *functionNode = new TIntermAggregate(EOpPrototype);
 
     SetInternalFunctionName(functionNode, name);
     TType returnType(EbtVoid);
     functionNode->setType(returnType);
+    functionNode->setFunctionId(functionId);
     return functionNode;
 }
 
-TIntermAggregate *CreateFunctionDefinitionNode(const char *name, TIntermAggregate *functionBody)
+TIntermAggregate *CreateFunctionDefinitionNode(const char *name,
+                                               TIntermAggregate *functionBody,
+                                               const int functionId)
 {
     TIntermAggregate *functionNode = new TIntermAggregate(EOpFunction);
     TIntermAggregate *paramsNode = new TIntermAggregate(EOpParameters);
@@ -47,16 +50,19 @@ TIntermAggregate *CreateFunctionDefinitionNode(const char *name, TIntermAggregat
     SetInternalFunctionName(functionNode, name);
     TType returnType(EbtVoid);
     functionNode->setType(returnType);
+    functionNode->setFunctionId(functionId);
     return functionNode;
 }
 
-TIntermAggregate *CreateFunctionCallNode(const char *name)
+TIntermAggregate *CreateFunctionCallNode(const char *name, const int functionId)
 {
     TIntermAggregate *functionNode = new TIntermAggregate(EOpFunctionCall);
 
+    functionNode->setUserDefined();
     SetInternalFunctionName(functionNode, name);
     TType returnType(EbtVoid);
     functionNode->setType(returnType);
+    functionNode->setFunctionId(functionId);
     return functionNode;
 }
 
@@ -86,8 +92,9 @@ bool DeferGlobalInitializersTraverser::visitBinary(Visit visit, TIntermBinary *n
         ASSERT(symbolNode);
         TIntermTyped *expression = node->getRight();
 
-        if (symbolNode->getQualifier() == EvqGlobal &&
-            (expression->getQualifier() != EvqConst || expression->getAsConstantUnion() == nullptr))
+        if (mInGlobalScope && (expression->getQualifier() != EvqConst ||
+                               (expression->getAsConstantUnion() == nullptr &&
+                                !expression->isConstructorWithOnlyConstantUnionParameters())))
         {
             // For variables which are not constant, defer their real initialization until
             // after we initialize uniforms.
@@ -95,13 +102,35 @@ bool DeferGlobalInitializersTraverser::visitBinary(Visit visit, TIntermBinary *n
             // since otherwise there's a chance that HLSL output will generate extra statements
             // from the initializer expression.
             TIntermBinary *deferredInit = new TIntermBinary(EOpAssign);
-            deferredInit->setLeft(node->getLeft()->deepCopy());
+            deferredInit->setLeft(symbolNode->deepCopy());
             deferredInit->setRight(node->getRight());
             deferredInit->setType(node->getType());
             mDeferredInitializers.push_back(deferredInit);
 
+            // Change const global to a regular global if its initialization is deferred.
+            // This can happen if ANGLE has not been able to fold the constant expression used
+            // as an initializer.
+            ASSERT(symbolNode->getQualifier() == EvqConst ||
+                   symbolNode->getQualifier() == EvqGlobal);
+            if (symbolNode->getQualifier() == EvqConst)
+            {
+                // All of the siblings in the same declaration need to have consistent qualifiers.
+                auto *siblings = getParentNode()->getAsAggregate()->getSequence();
+                for (TIntermNode *siblingNode : *siblings)
+                {
+                    TIntermBinary *siblingBinary = siblingNode->getAsBinaryNode();
+                    if (siblingBinary)
+                    {
+                        ASSERT(siblingBinary->getOp() == EOpInitialize);
+                        siblingBinary->getLeft()->getTypePointer()->setQualifier(EvqGlobal);
+                    }
+                    siblingNode->getAsTyped()->getTypePointer()->setQualifier(EvqGlobal);
+                }
+                // This node is one of the siblings.
+                ASSERT(symbolNode->getQualifier() == EvqGlobal);
+            }
             // Remove the initializer from the global scope and just declare the global instead.
-            mReplacements.push_back(NodeUpdateEntry(getParentNode(), node, node->getLeft(), false));
+            mReplacements.push_back(NodeUpdateEntry(getParentNode(), node, symbolNode, false));
         }
     }
     return false;
@@ -113,13 +142,15 @@ void DeferGlobalInitializersTraverser::insertInitFunction(TIntermNode *root)
     {
         return;
     }
+    const int initFunctionId  = TSymbolTable::nextUniqueId();
     TIntermAggregate *rootAgg = root->getAsAggregate();
     ASSERT(rootAgg != nullptr && rootAgg->getOp() == EOpSequence);
 
     const char *functionName = "initializeDeferredGlobals";
 
     // Add function prototype to the beginning of the shader
-    TIntermAggregate *functionPrototypeNode = CreateFunctionPrototypeNode(functionName);
+    TIntermAggregate *functionPrototypeNode =
+        CreateFunctionPrototypeNode(functionName, initFunctionId);
     rootAgg->getSequence()->insert(rootAgg->getSequence()->begin(), functionPrototypeNode);
 
     // Add function definition to the end of the shader
@@ -130,7 +161,7 @@ void DeferGlobalInitializersTraverser::insertInitFunction(TIntermNode *root)
         functionBody->push_back(deferredInit);
     }
     TIntermAggregate *functionDefinition =
-        CreateFunctionDefinitionNode(functionName, functionBodyNode);
+        CreateFunctionDefinitionNode(functionName, functionBodyNode, initFunctionId);
     rootAgg->getSequence()->push_back(functionDefinition);
 
     // Insert call into main function
@@ -140,7 +171,8 @@ void DeferGlobalInitializersTraverser::insertInitFunction(TIntermNode *root)
         if (nodeAgg != nullptr && nodeAgg->getOp() == EOpFunction &&
             TFunction::unmangleName(nodeAgg->getName()) == "main")
         {
-            TIntermAggregate *functionCallNode = CreateFunctionCallNode(functionName);
+            TIntermAggregate *functionCallNode =
+                CreateFunctionCallNode(functionName, initFunctionId);
 
             TIntermNode *mainBody         = nodeAgg->getSequence()->back();
             TIntermAggregate *mainBodyAgg = mainBody->getAsAggregate();

src/compiler/translator/IntermNode.cpp

@@ -351,6 +351,21 @@ TIntermTyped::TIntermTyped(const TIntermTyped &node) : TIntermNode(), mType(node
     mLine = node.mLine;
 }
 
+bool TIntermTyped::isConstructorWithOnlyConstantUnionParameters()
+{
+    TIntermAggregate *constructor = getAsAggregate();
+    if (!constructor || !constructor->isConstructor())
+    {
+        return false;
+    }
+    for (TIntermNode *&node : *constructor->getSequence())
+    {
+        if (!node->getAsConstantUnion())
+            return false;
+    }
+    return true;
+}
+
 TIntermConstantUnion::TIntermConstantUnion(const TIntermConstantUnion &node) : TIntermTyped(node)
 {
     mUnionArrayPointer = node.mUnionArrayPointer;

src/compiler/translator/IntermNode.h

@@ -155,6 +155,8 @@ class TIntermTyped : public TIntermNode
 
     int getArraySize() const { return mType.getArraySize(); }
 
+    bool isConstructorWithOnlyConstantUnionParameters();
+
   protected:
     TType mType;
 
@@ -486,12 +488,15 @@ class TIntermAggregate : public TIntermOperator
     TIntermAggregate()
         : TIntermOperator(EOpNull),
           mUserDefined(false),
+          mFunctionId(0),
           mUseEmulatedFunction(false),
           mGotPrecisionFromChildren(false)
     {
     }
     TIntermAggregate(TOperator op)
         : TIntermOperator(op),
+          mUserDefined(false),
+          mFunctionId(0),
           mUseEmulatedFunction(false),
           mGotPrecisionFromChildren(false)
     {
@@ -673,6 +678,7 @@ class TIntermTraverser : angle::NonCopyable
           postVisit(postVisit),
           mDepth(0),
           mMaxDepth(0),
+          mInGlobalScope(true),
           mTemporaryIndex(nullptr)
     {
     }
@@ -767,6 +773,8 @@ class TIntermTraverser : angle::NonCopyable
     // All the nodes from root to the current node's parent during traversing.
     TVector<TIntermNode *> mPath;
 
+    bool mInGlobalScope;
+
     // To replace a single node with another on the parent node
     struct NodeUpdateEntry
     {

src/compiler/translator/IntermTraverse.cpp

@@ -390,6 +390,8 @@ void TIntermTraverser::traverseAggregate(TIntermAggregate *node)
 
         if (node->getOp() == EOpSequence)
             pushParentBlock(node);
+        else if (node->getOp() == EOpFunction)
+            mInGlobalScope = false;
 
         for (auto *child : *sequence)
         {
@@ -406,6 +408,8 @@ void TIntermTraverser::traverseAggregate(TIntermAggregate *node)
 
         if (node->getOp() == EOpSequence)
             popParentBlock();
+        else if (node->getOp() == EOpFunction)
+            mInGlobalScope = true;
 
         decrementDepth();
     }
@@ -481,6 +485,8 @@ void TLValueTrackingTraverser::traverseAggregate(TIntermAggregate *node)
         {
             if (node->getOp() == EOpSequence)
                 pushParentBlock(node);
+            else if (node->getOp() == EOpFunction)
+                mInGlobalScope = false;
 
             // Find the built-in function corresponding to this op so that we can determine the
             // in/out qualifiers of its parameters.
@@ -533,6 +539,8 @@ void TLValueTrackingTraverser::traverseAggregate(TIntermAggregate *node)
 
             if (node->getOp() == EOpSequence)
                 popParentBlock();
+            else if (node->getOp() == EOpFunction)
+                mInGlobalScope = true;
         }
 
         decrementDepth();

src/compiler/translator/OutputHLSL.cpp

@@ -882,19 +882,17 @@ bool OutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
       case EOpInitialize:
         if (visit == PreVisit)
         {
-            // GLSL allows to write things like "float x = x;" where a new variable x is defined
-            // and the value of an existing variable x is assigned. HLSL uses C semantics (the
-            // new variable is created before the assignment is evaluated), so we need to convert
-            // this to "float t = x, x = t;".
-
             TIntermSymbol *symbolNode = node->getLeft()->getAsSymbolNode();
             ASSERT(symbolNode);
             TIntermTyped *expression = node->getRight();
 
             // Global initializers must be constant at this point.
-            ASSERT(symbolNode->getQualifier() != EvqGlobal ||
-                   (expression->getQualifier() == EvqConst &&
-                    expression->getAsConstantUnion() != nullptr));
+            ASSERT(symbolNode->getQualifier() != EvqGlobal || canWriteAsHLSLLiteral(expression));
+
+            // GLSL allows to write things like "float x = x;" where a new variable x is defined
+            // and the value of an existing variable x is assigned. HLSL uses C semantics (the
+            // new variable is created before the assignment is evaluated), so we need to convert
+            // this to "float t = x, x = t;".
             if (writeSameSymbolInitializer(out, symbolNode, expression))
             {
                 // Skip initializing the rest of the expression
@@ -2654,22 +2652,8 @@ bool OutputHLSL::canWriteAsHLSLLiteral(TIntermTyped *expression)
 {
     // We support writing constant unions and constructors that only take constant unions as
     // parameters as HLSL literals.
-    if (expression->getAsConstantUnion())
-    {
-        return true;
-    }
-    if (expression->getQualifier() != EvqConst || !expression->getAsAggregate() ||
-        !expression->getAsAggregate()->isConstructor())
-    {
-        return false;
-    }
-    TIntermAggregate *constructor = expression->getAsAggregate();
-    for (TIntermNode *&node : *constructor->getSequence())
-    {
-        if (!node->getAsConstantUnion())
-            return false;
-    }
-    return true;
+    return expression->getAsConstantUnion() ||
+           expression->isConstructorWithOnlyConstantUnionParameters();
 }
 
 bool OutputHLSL::writeConstantInitialization(TInfoSinkBase &out,

src/tests/gl_tests/GLSLTest.cpp

@@ -1577,6 +1577,39 @@ TEST_P(GLSLTest_ES3, LargeNumberOfFloat4Parameters)
     EXPECT_NE(0u, program);
 }
 
+// This test was written specifically to stress DeferGlobalInitializers AST transformation.
+// Test a shader where a global constant array is initialized with an expression containing array
+// indexing. This initializer is tricky to constant fold, so if it's not constant folded it needs to
+// be handled in a way that doesn't generate statements in the global scope in HLSL output.
+// Also includes multiple array initializers in one declaration, where only the second one has
+// array indexing. This makes sure that the qualifier for the declaration is set correctly if
+// transformations are applied to the declaration also in the case of ESSL output.
+TEST_P(GLSLTest_ES3, InitGlobalArrayWithArrayIndexing)
+{
+    const std::string vertexShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "in vec4 a_vec;\n"
+        "void main()\n"
+        "{\n"
+        "    gl_Position = vec4(a_vec);\n"
+        "}";
+
+    const std::string fragmentShaderSource =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "out vec4 my_FragColor;\n"
+        "const highp float f[2] = float[2](0.1, 0.2);\n"
+        "const highp float[2] g = float[2](0.3, 0.4), h = float[2](0.5, f[1]);\n"
+        "void main()\n"
+        "{\n"
+        "    my_FragColor = vec4(h[1]);\n"
+        "}";
+
+    GLuint program = CompileProgram(vertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(GLSLTest,
                        ES2_D3D9(),