Set correct symbol ids when referring to GLSL built-ins

The symbol ids are fetched from the symbol table. A new utility
function is added to make this more convenient.

BUG=angleproject:1490
TEST=angle_unittests, angle_end2end_tests

Change-Id: I780430e3386f6599503d8290c568ca9bc9cad147
Reviewed-on: https://chromium-review.googlesource.com/559535
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 92c850a..17acebf 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -432,7 +432,7 @@
             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 @@
             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 @@
     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 @@
             list.push_back(var);
         }
     }
-    InitializeVariables(root, list, symbolTable, shaderVersion, shaderSpec, extensionBehavior);
+    InitializeVariables(root, list, symbolTable, shaderVersion, extensionBehavior);
 }
 
 const TExtensionBehavior &TCompiler::getExtensionBehavior() const
diff --git a/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp b/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp
index 597ebbf..ff72cc2 100644
--- a/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp
+++ b/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 @@
     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 @@
 
     // 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 @@
 
     // 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 @@
                                                  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 @@
         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.
diff --git a/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.h b/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.h
index e9dc8fa..379d127 100644
--- a/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.h
+++ b/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
 
diff --git a/src/compiler/translator/EmulateGLFragColorBroadcast.cpp b/src/compiler/translator/EmulateGLFragColorBroadcast.cpp
index 5432f67..d65e826 100644
--- a/src/compiler/translator/EmulateGLFragColorBroadcast.cpp
+++ b/src/compiler/translator/EmulateGLFragColorBroadcast.cpp
@@ -26,10 +26,14 @@
 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 @@
   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 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())
     {
diff --git a/src/compiler/translator/EmulateGLFragColorBroadcast.h b/src/compiler/translator/EmulateGLFragColorBroadcast.h
index f14ee70..9b410fd 100644
--- a/src/compiler/translator/EmulateGLFragColorBroadcast.h
+++ b/src/compiler/translator/EmulateGLFragColorBroadcast.h
@@ -16,13 +16,16 @@
 {
 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_
diff --git a/src/compiler/translator/Initialize.cpp b/src/compiler/translator/Initialize.cpp
index f5941ea..4275837 100644
--- a/src/compiler/translator/Initialize.cpp
+++ b/src/compiler/translator/Initialize.cpp
@@ -824,8 +824,15 @@
             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));
 
diff --git a/src/compiler/translator/InitializeVariables.cpp b/src/compiler/translator/InitializeVariables.cpp
index d8f9de5..6b015e2 100644
--- a/src/compiler/translator/InitializeVariables.cpp
+++ b/src/compiler/translator/InitializeVariables.cpp
@@ -87,7 +87,6 @@
                     const InitVariableList &variables,
                     const TSymbolTable &symbolTable,
                     int shaderVersion,
-                    ShShaderSpec shaderSpec,
                     const TExtensionBehavior &extensionBehavior)
 {
     for (const auto &var : variables)
@@ -99,36 +98,29 @@
             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));
+            initializedSymbol = ReferenceGlobalVariable(name, symbolTable);
         }
-        ASSERT(symbolInfo != nullptr);
+        ASSERT(initializedSymbol != 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());
     }
@@ -191,7 +183,7 @@
 
 }  // namespace anonymous
 
-TIntermSequence *CreateInitCode(const TIntermSymbol *initializedSymbol)
+TIntermSequence *CreateInitCode(const TIntermTyped *initializedSymbol)
 {
     TIntermSequence *initCode = new TIntermSequence();
     if (initializedSymbol->isArray())
@@ -221,12 +213,10 @@
                          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
diff --git a/src/compiler/translator/InitializeVariables.h b/src/compiler/translator/InitializeVariables.h
index cc6a7ae..536d0cc 100644
--- a/src/compiler/translator/InitializeVariables.h
+++ b/src/compiler/translator/InitializeVariables.h
@@ -20,7 +20,7 @@
 
 // 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 @@
 //   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
diff --git a/src/compiler/translator/IntermNode_util.cpp b/src/compiler/translator/IntermNode_util.cpp
index 1321217..cdf91ab 100644
--- a/src/compiler/translator/IntermNode_util.cpp
+++ b/src/compiler/translator/IntermNode_util.cpp
@@ -166,11 +166,21 @@
     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
diff --git a/src/compiler/translator/IntermNode_util.h b/src/compiler/translator/IntermNode_util.h
index 222d427..72f5bdf 100644
--- a/src/compiler/translator/IntermNode_util.h
+++ b/src/compiler/translator/IntermNode_util.h
@@ -35,7 +35,10 @@
 // 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
 
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 4842f89..cb41d00 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -3521,16 +3521,7 @@
         {
             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 "
diff --git a/src/compiler/translator/UseInterfaceBlockFields.cpp b/src/compiler/translator/UseInterfaceBlockFields.cpp
index bd9666f..78529dc 100644
--- a/src/compiler/translator/UseInterfaceBlockFields.cpp
+++ b/src/compiler/translator/UseInterfaceBlockFields.cpp
@@ -35,7 +35,7 @@
             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 @@
         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 @@
         else
         {
             TString name(block.instanceName.c_str());
-            TIntermSymbol *blockSymbol = ReferToGlobalSymbol(name, symbolTable);
+            TIntermSymbol *blockSymbol = ReferenceGlobalVariable(name, symbolTable);
             InsertUseCode(block, blockSymbol, sequence);
         }
     }