Propagate correct type to the lvalue in an output variable initializer

src/compiler/translator/Compiler.cpp

@@ -937,7 +937,7 @@ void TCompiler::initializeGLPosition(TIntermBlock *root)
     sh::ShaderVariable var(GL_FLOAT_VEC4, 0);
     var.name = "gl_Position";
     list.push_back(var);
-    InitializeVariables(root, list, symbolTable);
+    InitializeVariables(root, list, symbolTable, shaderVersion, shaderSpec, extensionBehavior);
 }
 
 void TCompiler::useAllMembersInUnusedStandardAndSharedBlocks(TIntermBlock *root)
@@ -974,7 +974,7 @@ void TCompiler::initializeOutputVariables(TIntermBlock *root)
             list.push_back(var);
         }
     }
-    InitializeVariables(root, list, symbolTable);
+    InitializeVariables(root, list, symbolTable, shaderVersion, shaderSpec, extensionBehavior);
 }
 
 const TExtensionBehavior &TCompiler::getExtensionBehavior() const

src/compiler/translator/InitializeVariables.cpp

@@ -84,36 +84,50 @@ void AddArrayZeroInitSequence(const TIntermTyped *initializedNode, TIntermSequen
 
 void InsertInitCode(TIntermSequence *mainBody,
                     const InitVariableList &variables,
-                    const TSymbolTable &symbolTable)
+                    const TSymbolTable &symbolTable,
+                    int shaderVersion,
+                    ShShaderSpec shaderSpec,
+                    const TExtensionBehavior &extensionBehavior)
 {
     for (const auto &var : variables)
     {
         TString name = TString(var.name.c_str());
-
-        TIntermSymbol *initializedSymbol = nullptr;
-        if (var.isArray())
+        size_t pos   = name.find_last_of('[');
+        if (pos != TString::npos)
         {
-            size_t pos = name.find_last_of('[');
-            if (pos != TString::npos)
-            {
-                name = name.substr(0, pos);
-            }
-            TType arrayType = sh::GetShaderVariableBasicType(var);
-            arrayType.setArraySize(var.elementCount());
-            initializedSymbol = new TIntermSymbol(0, name, arrayType);
+            name = name.substr(0, pos);
         }
-        else if (var.isStruct())
-        {
-            TVariable *structInfo = reinterpret_cast<TVariable *>(symbolTable.findGlobal(name));
-            ASSERT(structInfo);
 
-            initializedSymbol = new TIntermSymbol(0, name, structInfo->getType());
+        const TVariable *symbolInfo = nullptr;
+        if (var.isBuiltIn())
+        {
+            symbolInfo =
+                reinterpret_cast<const TVariable *>(symbolTable.findBuiltIn(name, shaderVersion));
         }
         else
         {
-            TType type        = sh::GetShaderVariableBasicType(var);
-            initializedSymbol = new TIntermSymbol(0, name, type);
+            symbolInfo = reinterpret_cast<const TVariable *>(symbolTable.findGlobal(name));
         }
+        ASSERT(symbolInfo != nullptr);
+
+        TType type = symbolInfo->getType();
+        if (type.getQualifier() == EvqFragData &&
+            (shaderSpec == SH_WEBGL2_SPEC ||
+             !IsExtensionEnabled(extensionBehavior, "GL_EXT_draw_buffers")))
+        {
+            // Adjust the number of attachment indices which can be initialized according to the
+            // WebGL2 spec and extension behavior:
+            // - WebGL2 spec, Editor's draft May 31, 5.13 GLSL ES
+            //   1.00 Fragment Shader Output: "A fragment shader written in The OpenGL ES Shading
+            //   Language, Version 1.00, that statically assigns a value to gl_FragData[n] where n
+            //   does not equal constant value 0 must fail to compile in the WebGL 2.0 API.".
+            //   This excerpt limits the initialization of gl_FragData to only the 0th index.
+            // - If GL_EXT_draw_buffers is disabled, only the 0th index of gl_FragData can be
+            //   written to.
+            type.setArraySize(1u);
+        }
+
+        TIntermSymbol *initializedSymbol = new TIntermSymbol(0, name, type);
         TIntermSequence *initCode = CreateInitCode(initializedSymbol);
         mainBody->insert(mainBody->begin(), initCode->begin(), initCode->end());
     }
@@ -204,12 +218,17 @@ void InitializeUninitializedLocals(TIntermBlock *root, int shaderVersion)
 
 void InitializeVariables(TIntermBlock *root,
                          const InitVariableList &vars,
-                         const TSymbolTable &symbolTable)
+                         const TSymbolTable &symbolTable,
+                         int shaderVersion,
+                         ShShaderSpec shaderSpec,
+                         const TExtensionBehavior &extensionBehavior)
 {
+
     TIntermFunctionDefinition *main = FindMain(root);
     ASSERT(main != nullptr);
     TIntermBlock *body = main->getBody();
-    InsertInitCode(body->getSequence(), vars, symbolTable);
+    InsertInitCode(body->getSequence(), vars, symbolTable, shaderVersion, shaderSpec,
+                   extensionBehavior);
 }
 
 }  // namespace sh

src/compiler/translator/InitializeVariables.h

@@ -9,6 +9,7 @@
 
 #include <GLSLANG/ShaderLang.h>
 
+#include "compiler/translator/ExtensionBehavior.h"
 #include "compiler/translator/IntermNode.h"
 
 namespace sh
@@ -24,15 +25,20 @@ TIntermSequence *CreateInitCode(const TIntermSymbol *initializedSymbol);
 // Initialize all uninitialized local variables, so that undefined behavior is avoided.
 void InitializeUninitializedLocals(TIntermBlock *root, int shaderVersion);
 
-// This function can initialize all the types that CreateInitCode is able to initialize. For struct
-// typed variables it requires that the struct is found from the symbolTable, which is usually not
-// the case for locally defined struct types.
-// For now it is used for the following two scenarios:
-//   1. initializing gl_Position;
-//   2. initializing ESSL 3.00 shaders' output variables.
+// This function can initialize all the types that CreateInitCode is able to initialize. All
+// variables must be globals which can be found in the symbol table. For now it is used for the
+// following two scenarios:
+//   1. Initializing gl_Position;
+//   2. Initializing output variables referred to in the shader source.
+// Note: The type of each lvalue in an initializer is retrieved from the symbol table. gl_FragData
+// requires special handling because the number of indices which can be initialized is determined by
+// the API spec and extension support.
 void InitializeVariables(TIntermBlock *root,
                          const InitVariableList &vars,
-                         const TSymbolTable &symbolTable);
+                         const TSymbolTable &symbolTable,
+                         int shaderVersion,
+                         ShShaderSpec shaderSpec,
+                         const TExtensionBehavior &extensionBehavior);
 
 }  // namespace sh
 

src/tests/angle_unittests.gypi

@@ -63,6 +63,7 @@
             '<(angle_path)/src/tests/compiler_tests/FloatLex_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/FragDepth_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/GLSLCompatibilityOutput_test.cpp',
+            '<(angle_path)/src/tests/compiler_tests/InitOutputVariables_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/IntermNode_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/NV_draw_buffers_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/Pack_Unpack_test.cpp',

src/tests/compiler_tests/InitOutputVariables_test.cpp

+//
+// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// InitOutputVariables_test.cpp: Tests correctness of the AST pass enabled through
+// SH_INIT_OUTPUT_VARIABLES.
+//
+
+#include "common/angleutils.h"
+
+#include "compiler/translator/FindMain.h"
+#include "compiler/translator/IntermNode.h"
+#include "tests/test_utils/ShaderCompileTreeTest.h"
+
+#include <algorithm>
+
+namespace sh
+{
+
+namespace
+{
+
+typedef std::vector<TIntermTyped *> ExpectedLValues;
+
+bool AreSymbolsTheSame(const TIntermSymbol *expected, const TIntermSymbol *candidate)
+{
+    if (expected == nullptr || candidate == nullptr)
+    {
+        return false;
+    }
+    const TType &expectedType  = expected->getType();
+    const TType &candidateType = candidate->getType();
+    const bool sameTypes       = expectedType == candidateType &&
+                           expectedType.getPrecision() == candidateType.getPrecision() &&
+                           expectedType.getQualifier() == candidateType.getQualifier();
+    const bool sameSymbols = expected->getSymbol() == candidate->getSymbol();
+    return sameSymbols && sameTypes;
+}
+
+bool AreLValuesTheSame(TIntermTyped *expected, TIntermTyped *candidate)
+{
+    const TIntermBinary *expectedBinary = expected->getAsBinaryNode();
+    if (expectedBinary)
+    {
+        ASSERT(expectedBinary->getOp() == EOpIndexDirect);
+        const TIntermBinary *candidateBinary = candidate->getAsBinaryNode();
+        if (candidateBinary == nullptr || candidateBinary->getOp() != EOpIndexDirect)
+        {
+            return false;
+        }
+        if (expectedBinary->getRight()->getAsConstantUnion()->getIConst(0) !=
+            candidateBinary->getRight()->getAsConstantUnion()->getIConst(0))
+        {
+            return false;
+        }
+        return AreSymbolsTheSame(expectedBinary->getLeft()->getAsSymbolNode(),
+                                 candidateBinary->getLeft()->getAsSymbolNode());
+    }
+    return AreSymbolsTheSame(expected->getAsSymbolNode(), candidate->getAsSymbolNode());
+}
+
+TIntermTyped *CreateLValueNode(const TString &lValueName, const TType &type)
+{
+    return new TIntermSymbol(0, lValueName, type);
+}
+
+ExpectedLValues CreateIndexedLValueNodeList(const TString &lValueName,
+                                            TType elementType,
+                                            unsigned arraySize)
+{
+    ASSERT(elementType.isArray() == false);
+    elementType.setArraySize(arraySize);
+
+    ExpectedLValues expected(arraySize);
+    for (unsigned index = 0u; index < arraySize; ++index)
+    {
+        expected[index] =
+            new TIntermBinary(EOpIndexDirect, new TIntermSymbol(0, lValueName, elementType),
+                              TIntermTyped::CreateIndexNode(static_cast<int>(index)));
+    }
+    return expected;
+}
+
+void ReleaseExpectedLValuesMemory(ExpectedLValues &expectedLValues)
+{
+    for (size_t i = 0u; i < expectedLValues.size(); ++i)
+    {
+        delete expectedLValues[i];
+    }
+    expectedLValues.clear();
+}
+
+class ConstantsAreZerosTraverser final : public TIntermTraverser
+{
+  public:
+    ConstantsAreZerosTraverser() : TIntermTraverser(true, false, false), mAllConstantsAreZeros(true)
+    {
+    }
+
+    void visitConstantUnion(TIntermConstantUnion *node)
+    {
+        if (!mAllConstantsAreZeros)
+        {
+            return;
+        }
+
+        const TType &type = node->getType();
+        size_t objectSize = type.getObjectSize();
+        for (size_t i = 0u; i < objectSize && mAllConstantsAreZeros; ++i)
+        {
+            switch (type.getBasicType())
+            {
+                case EbtFloat:
+                    mAllConstantsAreZeros = (node->getFConst(i) == 0.0f);
+                    break;
+                case EbtInt:
+                    mAllConstantsAreZeros = (node->getIConst(i) == 0);
+                    break;
+                case EbtUInt:
+                    mAllConstantsAreZeros = (node->getUConst(i) == 0u);
+                    break;
+                default:
+                    // Cannot handle.
+                    mAllConstantsAreZeros = false;
+            }
+        }
+    }
+
+    bool areAllConstantsZeros() const { return mAllConstantsAreZeros; }
+
+  private:
+    bool mAllConstantsAreZeros;
+};
+
+// Returns true if all visited constants in subtree are zeros.
+bool AreAllConstantsInSubtreeZeros(TIntermNode *subtree)
+{
+    ConstantsAreZerosTraverser traverser;
+    subtree->traverse(&traverser);
+    return traverser.areAllConstantsZeros();
+}
+
+// VerifyOutputVariableInitializers traverses the subtree covering main and collects the lvalues in
+// assignments for which the rvalue is an expression containing only zero constants.
+class VerifyOutputVariableInitializers final : public TIntermTraverser
+{
+  public:
+    VerifyOutputVariableInitializers(TIntermBlock *root) : TIntermTraverser(true, false, false)
+    {
+        ASSERT(root != nullptr);
+
+        // The traversal starts in the body of main because this is where the varyings and output
+        // variables are initialized.
+        sh::TIntermFunctionDefinition *main = FindMain(root);
+        ASSERT(main != nullptr);
+        main->traverse(this);
+    }
+
+    bool visitBinary(Visit visit, TIntermBinary *node) override
+    {
+        if (node->getOp() == EOpAssign && AreAllConstantsInSubtreeZeros(node->getRight()))
+        {
+            mCandidateLValues.push_back(node->getLeft());
+            return false;
+        }
+        return true;
+    }
+
+    // The collected lvalues are considered valid if every expected lvalue in expectedLValues is
+    // matched by name and type with any lvalue in mCandidateLValues.
+    bool areAllExpectedLValuesFound(const ExpectedLValues &expectedLValues) const
+    {
+        for (size_t i = 0u; i < expectedLValues.size(); ++i)
+        {
+            if (!isExpectedLValueFound(expectedLValues[i]))
+            {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    bool isExpectedLValueFound(TIntermTyped *expectedLValue) const
+    {
+        bool isFound = false;
+        for (size_t j = 0; j < mCandidateLValues.size() && !isFound; ++j)
+        {
+            isFound = AreLValuesTheSame(expectedLValue, mCandidateLValues[j]);
+        }
+        return isFound;
+    }
+
+    const ExpectedLValues &getCandidates() const { return mCandidateLValues; }
+
+  private:
+    ExpectedLValues mCandidateLValues;
+};
+
+// Traverses the AST and records a pointer to a structure with a given name.
+class FindStructByName final : public TIntermTraverser
+{
+  public:
+    FindStructByName(const TString &structName)
+        : TIntermTraverser(true, false, false), mStructName(structName), mStructure(nullptr)
+    {
+    }
+
+    void visitSymbol(TIntermSymbol *symbol) override
+    {
+        if (isStructureFound())
+        {
+            return;
+        }
+
+        TStructure *structure = symbol->getType().getStruct();
+
+        if (structure != nullptr && structure->name() == mStructName)
+        {
+            mStructure = structure;
+        }
+    }
+
+    bool isStructureFound() const { return mStructure != nullptr; };
+    TStructure *getStructure() const { return mStructure; }
+
+  private:
+    TString mStructName;
+    TStructure *mStructure;
+};
+
+}  // namespace
+
+class InitOutputVariablesWebGL2Test : public ShaderCompileTreeTest
+{
+  public:
+    InitOutputVariablesWebGL2Test()
+    {
+        mExtraCompileOptions |= SH_VARIABLES;
+        mExtraCompileOptions |= SH_INIT_OUTPUT_VARIABLES;
+    }
+
+  protected:
+    ShShaderSpec getShaderSpec() const override { return SH_WEBGL2_SPEC; }
+};
+
+class InitOutputVariablesWebGL2VertexShaderTest : public InitOutputVariablesWebGL2Test
+{
+  protected:
+    ::GLenum getShaderType() const override { return GL_VERTEX_SHADER; }
+};
+
+class InitOutputVariablesWebGL2FragmentShaderTest : public InitOutputVariablesWebGL2Test
+{
+  protected:
+    ::GLenum getShaderType() const override { return GL_FRAGMENT_SHADER; }
+    void initResources(ShBuiltInResources *resources)
+    {
+        resources->EXT_draw_buffers = 1;
+        resources->MaxDrawBuffers   = 2;
+    }
+};
+
+class InitOutputVariablesWebGL1FragmentShaderTest : public ShaderCompileTreeTest
+{
+  public:
+    InitOutputVariablesWebGL1FragmentShaderTest()
+    {
+        mExtraCompileOptions |= SH_VARIABLES;
+        mExtraCompileOptions |= SH_INIT_OUTPUT_VARIABLES;
+    }
+
+  protected:
+    ::GLenum getShaderType() const override { return GL_FRAGMENT_SHADER; }
+    ShShaderSpec getShaderSpec() const override { return SH_WEBGL_SPEC; }
+    void initResources(ShBuiltInResources *resources)
+    {
+        resources->EXT_draw_buffers = 1;
+        resources->MaxDrawBuffers   = 2;
+    }
+};
+
+// Test the initialization of output variables with various qualifiers in a vertex shader.
+TEST_F(InitOutputVariablesWebGL2VertexShaderTest, OutputAllQualifiers)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "precision lowp int;\n"
+        "out vec4 out1;\n"
+        "flat out int out2;\n"
+        "centroid out float out3;\n"
+        "smooth out float out4;\n"
+        "void main() {\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    ExpectedLValues expectedLValues = {
+        CreateLValueNode("out1", TType(EbtFloat, EbpMedium, EvqVertexOut, 4)),
+        CreateLValueNode("out2", TType(EbtInt, EbpLow, EvqFlatOut)),
+        CreateLValueNode("out3", TType(EbtFloat, EbpMedium, EvqCentroidOut)),
+        CreateLValueNode("out4", TType(EbtFloat, EbpMedium, EvqSmoothOut))};
+    EXPECT_TRUE(verifier.areAllExpectedLValuesFound(expectedLValues));
+    ReleaseExpectedLValuesMemory(expectedLValues);
+}
+
+// Test the initialization of an output array in a vertex shader.
+TEST_F(InitOutputVariablesWebGL2VertexShaderTest, OutputArray)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "out float out1[2];\n"
+        "void main() {\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    ExpectedLValues expectedLValues =
+        CreateIndexedLValueNodeList("out1", TType(EbtFloat, EbpMedium, EvqVertexOut), 2);
+    EXPECT_TRUE(verifier.areAllExpectedLValuesFound(expectedLValues));
+    ReleaseExpectedLValuesMemory(expectedLValues);
+}
+
+// Test the initialization of a struct output variable in a vertex shader.
+TEST_F(InitOutputVariablesWebGL2VertexShaderTest, OutputStruct)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "struct MyS{\n"
+        "   float a;\n"
+        "   float b;\n"
+        "};\n"
+        "out MyS out1;\n"
+        "void main() {\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    FindStructByName findStruct("MyS");
+    mASTRoot->traverse(&findStruct);
+    ASSERT(findStruct.isStructureFound());
+
+    TType type(EbtStruct, EbpUndefined, EvqVertexOut);
+    type.setStruct(findStruct.getStructure());
+
+    TIntermTyped *expectedLValue = CreateLValueNode("out1", type);
+    EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValue));
+    delete expectedLValue;
+}
+
+// Test the initialization of a varying variable in an ESSL1 vertex shader.
+TEST_F(InitOutputVariablesWebGL2VertexShaderTest, OutputFromESSL1Shader)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "varying vec4 out1;\n"
+        "void main() {\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    TIntermTyped *expectedLValue =
+        CreateLValueNode("out1", TType(EbtFloat, EbpMedium, EvqVaryingOut, 4));
+    EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValue));
+    delete expectedLValue;
+}
+
+// Test the initialization of output variables in a fragment shader.
+TEST_F(InitOutputVariablesWebGL2FragmentShaderTest, Output)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "out vec4 out1;\n"
+        "void main() {\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    TIntermTyped *expectedLValue =
+        CreateLValueNode("out1", TType(EbtFloat, EbpMedium, EvqFragmentOut, 4));
+    EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValue));
+    delete expectedLValue;
+}
+
+// Test the initialization of gl_FragData in a WebGL2 ESSL1 fragment shader. Only writes to
+// gl_FragData[0] should be found.
+TEST_F(InitOutputVariablesWebGL2FragmentShaderTest, FragData)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "void main() {\n"
+        "   gl_FragData[0] = vec4(1.);\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    ExpectedLValues expectedLValues =
+        CreateIndexedLValueNodeList("gl_FragData", TType(EbtFloat, EbpMedium, EvqFragData, 4), 1);
+    EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValues[0]));
+    EXPECT_EQ(1u, verifier.getCandidates().size());
+    ReleaseExpectedLValuesMemory(expectedLValues);
+}
+
+// Test the initialization of gl_FragData in a WebGL1 ESSL1 fragment shader. Only writes to
+// gl_FragData[0] should be found.
+TEST_F(InitOutputVariablesWebGL1FragmentShaderTest, FragData)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "void main() {\n"
+        "   gl_FragData[0] = vec4(1.);\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    ExpectedLValues expectedLValues =
+        CreateIndexedLValueNodeList("gl_FragData", TType(EbtFloat, EbpMedium, EvqFragData, 4), 1);
+    EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValues[0]));
+    EXPECT_EQ(1u, verifier.getCandidates().size());
+    ReleaseExpectedLValuesMemory(expectedLValues);
+}
+
+// Test the initialization of gl_FragData in a WebGL1 ESSL1 fragment shader with GL_EXT_draw_buffers
+// enabled. All attachment slots should be initialized.
+TEST_F(InitOutputVariablesWebGL1FragmentShaderTest, FragDataWithDrawBuffersExtEnabled)
+{
+    const std::string &shaderString =
+        "#extension GL_EXT_draw_buffers : enable\n"
+        "precision mediump float;\n"
+        "void main() {\n"
+        "   gl_FragData[0] = vec4(1.);\n"
+        "}\n";
+    compileAssumeSuccess(shaderString);
+    VerifyOutputVariableInitializers verifier(mASTRoot);
+
+    ExpectedLValues expectedLValues =
+        CreateIndexedLValueNodeList("gl_FragData", TType(EbtFloat, EbpMedium, EvqFragData, 4), 2);
+    EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValues[0]));
+    EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValues[1]));
+    EXPECT_EQ(2u, verifier.getCandidates().size());
+    ReleaseExpectedLValuesMemory(expectedLValues);
+}
+
+}  // namespace sh
\ No newline at end of file