Set correct symbol ids when referring to GLSL built-ins

src/compiler/translator/Compiler.cpp

@@ -432,7 +432,7 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
             parseContext.isMultiviewExtensionEnabled() && getShaderType() != GL_COMPUTE_SHADER)
         {
             DeclareAndInitBuiltinsForInstancedMultiview(root, mNumViews, shaderType, compileOptions,
-                                                        outputType);
+                                                        outputType, symbolTable);
         }
 
         // This pass might emit short circuits so keep it before the short circuit unfolding
@@ -517,7 +517,8 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
             compileResources.EXT_draw_buffers && compileResources.MaxDrawBuffers > 1 &&
             IsExtensionEnabled(extensionBehavior, "GL_EXT_draw_buffers"))
         {
-            EmulateGLFragColorBroadcast(root, compileResources.MaxDrawBuffers, &outputVariables);
+            EmulateGLFragColorBroadcast(root, compileResources.MaxDrawBuffers, &outputVariables,
+                                        symbolTable, shaderVersion);
         }
 
         if (success)
@@ -946,7 +947,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, shaderVersion, shaderSpec, extensionBehavior);
+    InitializeVariables(root, list, symbolTable, shaderVersion, extensionBehavior);
 }
 
 void TCompiler::useAllMembersInUnusedStandardAndSharedBlocks(TIntermBlock *root)
@@ -988,7 +989,7 @@ void TCompiler::initializeOutputVariables(TIntermBlock *root)
             list.push_back(var);
         }
     }
-    InitializeVariables(root, list, symbolTable, shaderVersion, shaderSpec, extensionBehavior);
+    InitializeVariables(root, list, symbolTable, shaderVersion, extensionBehavior);
 }
 
 const TExtensionBehavior &TCompiler::getExtensionBehavior() const

src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp

@@ -11,6 +11,7 @@
 
 #include "compiler/translator/FindMain.h"
 #include "compiler/translator/InitializeVariables.h"
+#include "compiler/translator/IntermNode_util.h"
 #include "compiler/translator/IntermTraverse.h"
 #include "compiler/translator/SymbolTable.h"
 #include "compiler/translator/util.h"
@@ -43,15 +44,16 @@ class ReplaceVariableTraverser : public TIntermTraverser
     TIntermSymbol *mNewSymbol;
 };
 
-TIntermSymbol *CreateGLInstanceIDSymbol()
+TIntermSymbol *CreateGLInstanceIDSymbol(const TSymbolTable &symbolTable)
 {
-    return new TIntermSymbol(0, "gl_InstanceID", TType(EbtInt, EbpHigh, EvqInstanceID));
+    return ReferenceBuiltInVariable("gl_InstanceID", symbolTable, 300);
 }
 
 // Adds the InstanceID and ViewID_OVR initializers to the end of the initializers' sequence.
 void InitializeViewIDAndInstanceID(TIntermTyped *viewIDSymbol,
                                    TIntermTyped *instanceIDSymbol,
                                    unsigned numberOfViews,
+                                   const TSymbolTable &symbolTable,
                                    TIntermSequence *initializers)
 {
     // Create a signed numberOfViews node.
@@ -62,7 +64,7 @@ void InitializeViewIDAndInstanceID(TIntermTyped *viewIDSymbol,
 
     // Create a gl_InstanceID / numberOfViews node.
     TIntermBinary *normalizedInstanceID =
-        new TIntermBinary(EOpDiv, CreateGLInstanceIDSymbol(), numberOfViewsIntSymbol);
+        new TIntermBinary(EOpDiv, CreateGLInstanceIDSymbol(symbolTable), numberOfViewsIntSymbol);
 
     // Create a InstanceID = gl_InstanceID / numberOfViews node.
     TIntermBinary *instanceIDInitializer =
@@ -71,7 +73,7 @@ void InitializeViewIDAndInstanceID(TIntermTyped *viewIDSymbol,
 
     // Create a uint(gl_InstanceID) node.
     TIntermSequence *glInstanceIDSymbolCastArguments = new TIntermSequence();
-    glInstanceIDSymbolCastArguments->push_back(CreateGLInstanceIDSymbol());
+    glInstanceIDSymbolCastArguments->push_back(CreateGLInstanceIDSymbol(symbolTable));
     TIntermAggregate *glInstanceIDAsUint = TIntermAggregate::CreateConstructor(
         TType(EbtUInt, EbpHigh, EvqTemporary), glInstanceIDSymbolCastArguments);
 
@@ -132,7 +134,8 @@ void DeclareAndInitBuiltinsForInstancedMultiview(TIntermBlock *root,
                                                  unsigned numberOfViews,
                                                  GLenum shaderType,
                                                  ShCompileOptions compileOptions,
-                                                 ShShaderOutput shaderOutput)
+                                                 ShShaderOutput shaderOutput,
+                                                 const TSymbolTable &symbolTable)
 {
     ASSERT(shaderType == GL_VERTEX_SHADER || shaderType == GL_FRAGMENT_SHADER);
 
@@ -154,7 +157,8 @@ void DeclareAndInitBuiltinsForInstancedMultiview(TIntermBlock *root,
         ReplaceSymbol(root, "gl_InstanceID", instanceIDSymbol);
 
         TIntermSequence *initializers = new TIntermSequence();
-        InitializeViewIDAndInstanceID(viewIDSymbol, instanceIDSymbol, numberOfViews, initializers);
+        InitializeViewIDAndInstanceID(viewIDSymbol, instanceIDSymbol, numberOfViews, symbolTable,
+                                      initializers);
 
         // The AST transformation which adds the expression to select the viewport index should
         // be done only for the GLSL and ESSL output.

src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.h

@@ -24,16 +24,18 @@
 #include "GLSLANG/ShaderLang.h"
 #include "angle_gl.h"
 
-class TIntermBlock;
-
 namespace sh
 {
 
+class TIntermBlock;
+class TSymbolTable;
+
 void DeclareAndInitBuiltinsForInstancedMultiview(TIntermBlock *root,
                                                  unsigned numberOfViews,
                                                  GLenum shaderType,
                                                  ShCompileOptions compileOptions,
-                                                 ShShaderOutput shaderOutput);
+                                                 ShShaderOutput shaderOutput,
+                                                 const TSymbolTable &symbolTable);
 
 }  // namespace sh
 

src/compiler/translator/EmulateGLFragColorBroadcast.cpp

@@ -26,10 +26,14 @@ namespace
 class GLFragColorBroadcastTraverser : public TIntermTraverser
 {
   public:
-    GLFragColorBroadcastTraverser(int maxDrawBuffers)
+    GLFragColorBroadcastTraverser(int maxDrawBuffers,
+                                  const TSymbolTable &symbolTable,
+                                  int shaderVersion)
         : TIntermTraverser(true, false, false),
           mGLFragColorUsed(false),
-          mMaxDrawBuffers(maxDrawBuffers)
+          mMaxDrawBuffers(maxDrawBuffers),
+          mSymbolTable(symbolTable),
+          mShaderVersion(shaderVersion)
     {
     }
 
@@ -46,14 +50,14 @@ class GLFragColorBroadcastTraverser : public TIntermTraverser
   private:
     bool mGLFragColorUsed;
     int mMaxDrawBuffers;
+    const TSymbolTable &mSymbolTable;
+    const int mShaderVersion;
 };
 
 TIntermBinary *GLFragColorBroadcastTraverser::constructGLFragDataNode(int index) const
 {
-    TType gl_FragDataType = TType(EbtFloat, EbpMedium, EvqFragData, 4);
-    gl_FragDataType.setArraySize(mMaxDrawBuffers);
-
-    TIntermSymbol *symbol   = new TIntermSymbol(0, "gl_FragData", gl_FragDataType);
+    TIntermSymbol *symbol =
+        ReferenceBuiltInVariable(TString("gl_FragData"), mSymbolTable, mShaderVersion);
     TIntermTyped *indexNode = CreateIndexNode(index);
 
     TIntermBinary *binary = new TIntermBinary(EOpIndexDirect, symbol, indexNode);
@@ -99,10 +103,12 @@ void GLFragColorBroadcastTraverser::broadcastGLFragColor(TIntermBlock *root)
 
 void EmulateGLFragColorBroadcast(TIntermBlock *root,
                                  int maxDrawBuffers,
-                                 std::vector<sh::OutputVariable> *outputVariables)
+                                 std::vector<sh::OutputVariable> *outputVariables,
+                                 const TSymbolTable &symbolTable,
+                                 int shaderVersion)
 {
     ASSERT(maxDrawBuffers > 1);
-    GLFragColorBroadcastTraverser traverser(maxDrawBuffers);
+    GLFragColorBroadcastTraverser traverser(maxDrawBuffers, symbolTable, shaderVersion);
     root->traverse(&traverser);
     if (traverser.isGLFragColorUsed())
     {

src/compiler/translator/EmulateGLFragColorBroadcast.h

@@ -16,13 +16,16 @@ namespace sh
 {
 struct OutputVariable;
 class TIntermBlock;
+class TSymbolTable;
 
 // Replace all gl_FragColor with gl_FragData[0], and in the end of main() function,
 // assign gl_FragData[1] ... gl_FragData[maxDrawBuffers - 1] with gl_FragData[0].
 // If gl_FragColor is in outputVariables, it is replaced by gl_FragData.
 void EmulateGLFragColorBroadcast(TIntermBlock *root,
                                  int maxDrawBuffers,
-                                 std::vector<OutputVariable> *outputVariables);
+                                 std::vector<OutputVariable> *outputVariables,
+                                 const TSymbolTable &symbolTable,
+                                 int shaderVersion);
 }
 
 #endif  // COMPILER_TRANSLATOR_EMULATEGLFRAGCOLORBROADCAST_H_

src/compiler/translator/Initialize.cpp

@@ -824,8 +824,15 @@ void IdentifyBuiltIns(sh::GLenum type,
             symbolTable.insert(ESSL1_BUILTINS,
                                new TVariable(NewPoolTString("gl_FragColor"),
                                              TType(EbtFloat, EbpMedium, EvqFragColor, 4)));
-            TType fragData(EbtFloat, EbpMedium, EvqFragData, 4, 1, true);
-            fragData.setArraySize(resources.MaxDrawBuffers);
+            TType fragData(EbtFloat, EbpMedium, EvqFragData, 4);
+            if (spec != SH_WEBGL2_SPEC && spec != SH_WEBGL3_SPEC)
+            {
+                fragData.setArraySize(resources.MaxDrawBuffers);
+            }
+            else
+            {
+                fragData.setArraySize(1u);
+            }
             symbolTable.insert(ESSL1_BUILTINS,
                                new TVariable(NewPoolTString("gl_FragData"), fragData));
 

src/compiler/translator/InitializeVariables.cpp

@@ -87,7 +87,6 @@ void InsertInitCode(TIntermSequence *mainBody,
                     const InitVariableList &variables,
                     const TSymbolTable &symbolTable,
                     int shaderVersion,
-                    ShShaderSpec shaderSpec,
                     const TExtensionBehavior &extensionBehavior)
 {
     for (const auto &var : variables)
@@ -99,36 +98,29 @@ void InsertInitCode(TIntermSequence *mainBody,
             name = name.substr(0, pos);
         }
 
-        const TVariable *symbolInfo = nullptr;
+        TIntermTyped *initializedSymbol = nullptr;
         if (var.isBuiltIn())
         {
-            symbolInfo =
-                reinterpret_cast<const TVariable *>(symbolTable.findBuiltIn(name, shaderVersion));
+            initializedSymbol = ReferenceBuiltInVariable(name, symbolTable, shaderVersion);
+            if (initializedSymbol->getQualifier() == EvqFragData &&
+                !IsExtensionEnabled(extensionBehavior, "GL_EXT_draw_buffers"))
+            {
+                // If GL_EXT_draw_buffers is disabled, only the 0th index of gl_FragData can be
+                // written to.
+                // TODO(oetuaho): This is a bit hacky and would be better to remove, if we came up
+                // with a good way to do it. Right now "gl_FragData" in symbol table is initialized
+                // to have the array size of MaxDrawBuffers, and the initialization happens before
+                // the shader sets the extensions it is using.
+                initializedSymbol =
+                    new TIntermBinary(EOpIndexDirect, initializedSymbol, CreateIndexNode(0));
+            }
         }
         else
         {
-            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);
+            initializedSymbol = ReferenceGlobalVariable(name, symbolTable);
         }
+        ASSERT(initializedSymbol != nullptr);
 
-        TIntermSymbol *initializedSymbol = new TIntermSymbol(0, name, type);
         TIntermSequence *initCode = CreateInitCode(initializedSymbol);
         mainBody->insert(mainBody->begin(), initCode->begin(), initCode->end());
     }
@@ -191,7 +183,7 @@ class InitializeLocalsTraverser : public TIntermTraverser
 
 }  // namespace anonymous
 
-TIntermSequence *CreateInitCode(const TIntermSymbol *initializedSymbol)
+TIntermSequence *CreateInitCode(const TIntermTyped *initializedSymbol)
 {
     TIntermSequence *initCode = new TIntermSequence();
     if (initializedSymbol->isArray())
@@ -221,12 +213,10 @@ void InitializeVariables(TIntermBlock *root,
                          const InitVariableList &vars,
                          const TSymbolTable &symbolTable,
                          int shaderVersion,
-                         ShShaderSpec shaderSpec,
                          const TExtensionBehavior &extensionBehavior)
 {
     TIntermBlock *body = FindMainBody(root);
-    InsertInitCode(body->getSequence(), vars, symbolTable, shaderVersion, shaderSpec,
-                   extensionBehavior);
+    InsertInitCode(body->getSequence(), vars, symbolTable, shaderVersion, extensionBehavior);
 }
 
 }  // namespace sh

src/compiler/translator/InitializeVariables.h

@@ -20,7 +20,7 @@ typedef std::vector<sh::ShaderVariable> InitVariableList;
 
 // Return a sequence of assignment operations to initialize "initializedSymbol". initializedSymbol
 // may be an array, struct or any combination of these, as long as it contains only basic types.
-TIntermSequence *CreateInitCode(const TIntermSymbol *initializedSymbol);
+TIntermSequence *CreateInitCode(const TIntermTyped *initializedSymbol);
 
 // Initialize all uninitialized local variables, so that undefined behavior is avoided.
 void InitializeUninitializedLocals(TIntermBlock *root, int shaderVersion);
@@ -32,12 +32,11 @@ void InitializeUninitializedLocals(TIntermBlock *root, int shaderVersion);
 //   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.
+// enabled extensions.
 void InitializeVariables(TIntermBlock *root,
                          const InitVariableList &vars,
                          const TSymbolTable &symbolTable,
                          int shaderVersion,
-                         ShShaderSpec shaderSpec,
                          const TExtensionBehavior &extensionBehavior);
 
 }  // namespace sh

src/compiler/translator/IntermNode_util.cpp

@@ -166,11 +166,21 @@ TIntermBlock *EnsureBlock(TIntermNode *node)
     return blockNode;
 }
 
-TIntermSymbol *ReferToGlobalSymbol(const TString &name, const TSymbolTable &symbolTable)
+TIntermSymbol *ReferenceGlobalVariable(const TString &name, const TSymbolTable &symbolTable)
 {
     TVariable *var = reinterpret_cast<TVariable *>(symbolTable.findGlobal(name));
     ASSERT(var);
     return new TIntermSymbol(var->getUniqueId(), name, var->getType());
 }
 
+TIntermSymbol *ReferenceBuiltInVariable(const TString &name,
+                                        const TSymbolTable &symbolTable,
+                                        int shaderVersion)
+{
+    const TVariable *var =
+        reinterpret_cast<const TVariable *>(symbolTable.findBuiltIn(name, shaderVersion));
+    ASSERT(var);
+    return new TIntermSymbol(var->getUniqueId(), name, var->getType());
+}
+
 }  // namespace sh

src/compiler/translator/IntermNode_util.h

@@ -35,7 +35,10 @@ TIntermConstantUnion *CreateBoolNode(bool value);
 // If the input node is not a block node, put it inside a block node and return that.
 TIntermBlock *EnsureBlock(TIntermNode *node);
 
-TIntermSymbol *ReferToGlobalSymbol(const TString &name, const TSymbolTable &symbolTable);
+TIntermSymbol *ReferenceGlobalVariable(const TString &name, const TSymbolTable &symbolTable);
+TIntermSymbol *ReferenceBuiltInVariable(const TString &name,
+                                        const TSymbolTable &symbolTable,
+                                        int shaderVersion);
 
 }  // namespace sh
 

src/compiler/translator/ParseContext.cpp

@@ -3521,16 +3521,7 @@ TIntermTyped *TParseContext::addIndexExpression(TIntermTyped *baseExpression,
         {
             if (baseExpression->getQualifier() == EvqFragData && index > 0)
             {
-                if (mShaderSpec == SH_WEBGL2_SPEC)
-                {
-                    // Error has been already generated if index is not const.
-                    if (indexExpression->getQualifier() == EvqConst)
-                    {
-                        error(location, "array index for gl_FragData must be constant zero", "[");
-                    }
-                    safeIndex = 0;
-                }
-                else if (!isExtensionEnabled("GL_EXT_draw_buffers"))
+                if (!isExtensionEnabled("GL_EXT_draw_buffers"))
                 {
                     outOfRangeError(outOfRangeIndexIsError, location,
                                     "array index for gl_FragData must be zero when "

src/compiler/translator/UseInterfaceBlockFields.cpp

@@ -35,7 +35,7 @@ void AddFieldUseStatements(const ShaderVariable &var,
             name = name.substr(0, pos);
         }
     }
-    TIntermSymbol *symbol = ReferToGlobalSymbol(name, symbolTable);
+    TIntermSymbol *symbol = ReferenceGlobalVariable(name, symbolTable);
     if (var.isArray())
     {
         for (unsigned int i = 0u; i < var.arraySize; ++i)
@@ -77,7 +77,7 @@ void InsertUseCode(TIntermSequence *sequence,
         else if (block.arraySize > 0u)
         {
             TString name(block.instanceName.c_str());
-            TIntermSymbol *arraySymbol = ReferToGlobalSymbol(name, symbolTable);
+            TIntermSymbol *arraySymbol = ReferenceGlobalVariable(name, symbolTable);
             for (unsigned int i = 0u; i < block.arraySize; ++i)
             {
                 TIntermBinary *elementSymbol =
@@ -88,7 +88,7 @@ void InsertUseCode(TIntermSequence *sequence,
         else
         {
             TString name(block.instanceName.c_str());
-            TIntermSymbol *blockSymbol = ReferToGlobalSymbol(name, symbolTable);
+            TIntermSymbol *blockSymbol = ReferenceGlobalVariable(name, symbolTable);
             InsertUseCode(block, blockSymbol, sequence);
         }
     }

src/tests/compiler_tests/InitOutputVariables_test.cpp

@@ -361,8 +361,10 @@ TEST_F(InitOutputVariablesWebGL1FragmentShaderTest, FragData)
     compileAssumeSuccess(shaderString);
     VerifyOutputVariableInitializers verifier(mASTRoot);
 
+    // In the symbol table, gl_FragData array has 2 elements. However, only the 1st one should be
+    // initialized.
     ExpectedLValues expectedLValues =
-        CreateIndexedLValueNodeList("gl_FragData", TType(EbtFloat, EbpMedium, EvqFragData, 4), 1);
+        CreateIndexedLValueNodeList("gl_FragData", TType(EbtFloat, EbpMedium, EvqFragData, 4), 2);
     EXPECT_TRUE(verifier.isExpectedLValueFound(expectedLValues[0]));
     EXPECT_EQ(1u, verifier.getCandidates().size());
 }