Prefer identifying functions by using symbol ids

The shader translator code is now structured in a way that ensures
that all function definition, function prototype and function call
nodes store the integer symbol id for the function. This is guaranteed
regardless of whether the function node is added while parsing or as a
result of an AST transformation. TIntermAggregate nodes, which include
function calls and constructors can now only be created by calling one
of the TIntermAggregate::Create*() functions to ensure they have all
the necessary properties.

This makes it possible to keep track of functions using integer ids
instead of their mangled name strings when generating the call graph
and when using TLValueTrackingTraverser.

This commit includes a few other small cleanups to the CallDAG class
as well.

BUG=angleproject:1490
TEST=angle_unittests, angle_end2end_tests

Change-Id: Idd1013506cbe4c3380e20d90524a9cd09b890259
Reviewed-on: https://chromium-review.googlesource.com/459603
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ArrayReturnValueToOutParameter.cpp b/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
index 1c7e604..552abc5 100644
--- a/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
+++ b/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
@@ -50,9 +50,9 @@
         replacementArguments->push_back(arg);
     }
     replacementArguments->push_back(returnValueTarget);
-    TIntermAggregate *replacementCall =
-        new TIntermAggregate(TType(EbtVoid), EOpCallFunctionInAST, replacementArguments);
-    *replacementCall->getFunctionSymbolInfo() = *originalCall->getFunctionSymbolInfo();
+    TIntermAggregate *replacementCall = TIntermAggregate::CreateFunctionCall(
+        TType(EbtVoid), originalCall->getFunctionSymbolInfo()->getId(),
+        originalCall->getFunctionSymbolInfo()->getNameObj(), replacementArguments);
     replacementCall->setLine(originalCall->getLine());
     return replacementCall;
 }
@@ -110,7 +110,8 @@
     {
         // Replace the whole prototype node with another node that has the out parameter
         // added. Also set the function to return void.
-        TIntermFunctionPrototype *replacement = new TIntermFunctionPrototype(TType(EbtVoid));
+        TIntermFunctionPrototype *replacement =
+            new TIntermFunctionPrototype(TType(EbtVoid), node->getFunctionSymbolInfo()->getId());
         CopyAggregateChildren(node, replacement);
         replacement->getSequence()->push_back(CreateReturnValueOutSymbol(node->getType()));
         *replacement->getFunctionSymbolInfo() = *node->getFunctionSymbolInfo();
diff --git a/src/compiler/translator/CallDAG.cpp b/src/compiler/translator/CallDAG.cpp
index 6daa778..8557fdf 100644
--- a/src/compiler/translator/CallDAG.cpp
+++ b/src/compiler/translator/CallDAG.cpp
@@ -9,7 +9,9 @@
 // order.
 
 #include "compiler/translator/CallDAG.h"
+
 #include "compiler/translator/Diagnostics.h"
+#include "compiler/translator/SymbolTable.h"
 
 namespace sh
 {
@@ -78,7 +80,7 @@
                 record.callees.push_back(static_cast<int>(callee->index));
             }
 
-            (*idToIndex)[data.node->getFunctionSymbolInfo()->getId()] =
+            (*idToIndex)[data.node->getFunctionSymbolInfo()->getId().get()] =
                 static_cast<int>(data.index);
         }
     }
@@ -101,19 +103,20 @@
         // Create the record if need be and remember the node.
         if (visit == PreVisit)
         {
-            auto it = mFunctions.find(node->getFunctionSymbolInfo()->getName());
+            auto it = mFunctions.find(node->getFunctionSymbolInfo()->getId().get());
 
             if (it == mFunctions.end())
             {
-                mCurrentFunction = &mFunctions[node->getFunctionSymbolInfo()->getName()];
+                mCurrentFunction       = &mFunctions[node->getFunctionSymbolInfo()->getId().get()];
+                mCurrentFunction->name = node->getFunctionSymbolInfo()->getName();
             }
             else
             {
                 mCurrentFunction = &it->second;
+                ASSERT(mCurrentFunction->name == node->getFunctionSymbolInfo()->getName());
             }
 
             mCurrentFunction->node = node;
-            mCurrentFunction->name = node->getFunctionSymbolInfo()->getName();
         }
         else if (visit == PostVisit)
         {
@@ -125,8 +128,13 @@
     bool visitFunctionPrototype(Visit visit, TIntermFunctionPrototype *node) override
     {
         ASSERT(visit == PreVisit);
+        if (mCurrentFunction != nullptr)
+        {
+            return false;
+        }
+
         // Function declaration, create an empty record.
-        auto &record = mFunctions[node->getFunctionSymbolInfo()->getName()];
+        auto &record = mFunctions[node->getFunctionSymbolInfo()->getId().get()];
         record.name  = node->getFunctionSymbolInfo()->getName();
 
         // No need to traverse the parameters.
@@ -139,10 +147,12 @@
         if (visit == PreVisit && node->getOp() == EOpCallFunctionInAST)
         {
             // Function call, add the callees
-            auto it = mFunctions.find(node->getFunctionSymbolInfo()->getName());
+            auto it = mFunctions.find(node->getFunctionSymbolInfo()->getId().get());
             ASSERT(it != mFunctions.end());
 
-            // We might be in a top-level function call to set a global variable
+            // We might be traversing the initializer of a global variable. Even though function
+            // calls in global scope are forbidden by the parser, some subsequent AST
+            // transformations can add them to emulate particular features.
             if (mCurrentFunction)
             {
                 mCurrentFunction->callees.insert(&it->second);
@@ -259,7 +269,7 @@
 
     TDiagnostics *mDiagnostics;
 
-    std::map<TString, CreatorFunctionData> mFunctions;
+    std::map<int, CreatorFunctionData> mFunctions;
     CreatorFunctionData *mCurrentFunction;
     size_t mCurrentIndex;
 };
@@ -278,7 +288,7 @@
 
 size_t CallDAG::findIndex(const TFunctionSymbolInfo *functionInfo) const
 {
-    auto it = mFunctionIdToIndex.find(functionInfo->getId());
+    auto it = mFunctionIdToIndex.find(functionInfo->getId().get());
 
     if (it == mFunctionIdToIndex.end())
     {
diff --git a/src/compiler/translator/DeferGlobalInitializers.cpp b/src/compiler/translator/DeferGlobalInitializers.cpp
index 7353506..cf7ebb5 100644
--- a/src/compiler/translator/DeferGlobalInitializers.cpp
+++ b/src/compiler/translator/DeferGlobalInitializers.cpp
@@ -21,43 +21,6 @@
 namespace
 {
 
-void SetInternalFunctionName(TFunctionSymbolInfo *functionInfo, const char *name)
-{
-    TString nameStr(name);
-    nameStr = TFunction::mangleName(nameStr);
-    TName nameObj(nameStr);
-    nameObj.setInternal(true);
-    functionInfo->setNameObj(nameObj);
-}
-
-TIntermFunctionPrototype *CreateFunctionPrototypeNode(const char *name, const int functionId)
-{
-    TType returnType(EbtVoid);
-    TIntermFunctionPrototype *functionNode = new TIntermFunctionPrototype(returnType);
-
-    SetInternalFunctionName(functionNode->getFunctionSymbolInfo(), name);
-    functionNode->getFunctionSymbolInfo()->setId(functionId);
-    return functionNode;
-}
-
-TIntermFunctionDefinition *CreateFunctionDefinitionNode(const char *name,
-                                                        TIntermBlock *functionBody,
-                                                        const int functionId)
-{
-    TIntermFunctionPrototype *prototypeNode = CreateFunctionPrototypeNode(name, functionId);
-    return new TIntermFunctionDefinition(prototypeNode, functionBody);
-}
-
-TIntermAggregate *CreateFunctionCallNode(const char *name, const int functionId)
-{
-    TType returnType(EbtVoid);
-    TIntermAggregate *functionNode =
-        new TIntermAggregate(returnType, EOpCallFunctionInAST, nullptr);
-    SetInternalFunctionName(functionNode->getFunctionSymbolInfo(), name);
-    functionNode->getFunctionSymbolInfo()->setId(functionId);
-    return functionNode;
-}
-
 class DeferGlobalInitializersTraverser : public TIntermTraverser
 {
   public:
@@ -132,13 +95,13 @@
     {
         return;
     }
-    const int initFunctionId = TSymbolTable::nextUniqueId();
+    TSymbolUniqueId initFunctionId;
 
     const char *functionName = "initializeDeferredGlobals";
 
     // Add function prototype to the beginning of the shader
     TIntermFunctionPrototype *functionPrototypeNode =
-        CreateFunctionPrototypeNode(functionName, initFunctionId);
+        CreateInternalFunctionPrototypeNode(TType(EbtVoid), functionName, initFunctionId);
     root->getSequence()->insert(root->getSequence()->begin(), functionPrototypeNode);
 
     // Add function definition to the end of the shader
@@ -148,8 +111,8 @@
     {
         functionBody->push_back(deferredInit);
     }
-    TIntermFunctionDefinition *functionDefinition =
-        CreateFunctionDefinitionNode(functionName, functionBodyNode, initFunctionId);
+    TIntermFunctionDefinition *functionDefinition = CreateInternalFunctionDefinitionNode(
+        TType(EbtVoid), functionName, functionBodyNode, initFunctionId);
     root->getSequence()->push_back(functionDefinition);
 
     // Insert call into main function
@@ -158,8 +121,8 @@
         TIntermFunctionDefinition *nodeFunction = node->getAsFunctionDefinition();
         if (nodeFunction != nullptr && nodeFunction->getFunctionSymbolInfo()->isMain())
         {
-            TIntermAggregate *functionCallNode =
-                CreateFunctionCallNode(functionName, initFunctionId);
+            TIntermAggregate *functionCallNode = CreateInternalFunctionCallNode(
+                TType(EbtVoid), functionName, initFunctionId, nullptr);
 
             TIntermBlock *mainBody = nodeFunction->getBody();
             ASSERT(mainBody != nullptr);
diff --git a/src/compiler/translator/EmulatePrecision.cpp b/src/compiler/translator/EmulatePrecision.cpp
index b8a5c9a..888d336 100644
--- a/src/compiler/translator/EmulatePrecision.cpp
+++ b/src/compiler/translator/EmulatePrecision.cpp
@@ -433,7 +433,8 @@
 {
     TName nameObj(TFunction::GetMangledNameFromCall(name, *arguments));
     nameObj.setInternal(true);
-    TIntermAggregate *callNode = new TIntermAggregate(type, EOpCallInternalRawFunction, arguments);
+    TIntermAggregate *callNode =
+        TIntermAggregate::Create(type, EOpCallInternalRawFunction, arguments);
     callNode->getFunctionSymbolInfo()->setNameObj(nameObj);
     return callNode;
 }
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index e1f173c..3a8fdc0 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -272,6 +272,57 @@
     return true;
 }
 
+TIntermAggregate *TIntermAggregate::CreateFunctionCall(const TFunction &func,
+                                                       TIntermSequence *arguments)
+{
+    TIntermAggregate *callNode =
+        new TIntermAggregate(func.getReturnType(), EOpCallFunctionInAST, arguments);
+    callNode->getFunctionSymbolInfo()->setFromFunction(func);
+    return callNode;
+}
+
+TIntermAggregate *TIntermAggregate::CreateFunctionCall(const TType &type,
+                                                       const TSymbolUniqueId &id,
+                                                       const TName &name,
+                                                       TIntermSequence *arguments)
+{
+    TIntermAggregate *callNode = new TIntermAggregate(type, EOpCallFunctionInAST, arguments);
+    callNode->getFunctionSymbolInfo()->setId(id);
+    callNode->getFunctionSymbolInfo()->setNameObj(name);
+    return callNode;
+}
+
+TIntermAggregate *TIntermAggregate::CreateBuiltInFunctionCall(const TFunction &func,
+                                                              TIntermSequence *arguments)
+{
+    TIntermAggregate *callNode =
+        new TIntermAggregate(func.getReturnType(), EOpCallBuiltInFunction, arguments);
+    callNode->getFunctionSymbolInfo()->setFromFunction(func);
+    // Note that name needs to be set before texture function type is determined.
+    callNode->setBuiltInFunctionPrecision();
+    return callNode;
+}
+
+TIntermAggregate *TIntermAggregate::CreateConstructor(const TType &type,
+                                                      TOperator op,
+                                                      TIntermSequence *arguments)
+{
+    TIntermAggregate *constructorNode = new TIntermAggregate(type, op, arguments);
+    ASSERT(constructorNode->isConstructor());
+    return constructorNode;
+}
+
+TIntermAggregate *TIntermAggregate::Create(const TType &type,
+                                           TOperator op,
+                                           TIntermSequence *arguments)
+{
+    TIntermAggregate *node = new TIntermAggregate(type, op, arguments);
+    ASSERT(op != EOpCallFunctionInAST);    // Should use CreateFunctionCall
+    ASSERT(op != EOpCallBuiltInFunction);  // Should use CreateBuiltInFunctionCall
+    ASSERT(!node->isConstructor());        // Should use CreateConstructor
+    return node;
+}
+
 TIntermAggregate::TIntermAggregate(const TType &type, TOperator op, TIntermSequence *arguments)
     : TIntermOperator(op), mUseEmulatedFunction(false), mGotPrecisionFromChildren(false)
 {
@@ -557,7 +608,8 @@
         }
     }
 
-    return new TIntermAggregate(constType, sh::TypeToConstructorOperator(type), arguments);
+    return TIntermAggregate::CreateConstructor(constType, sh::TypeToConstructorOperator(type),
+                                               arguments);
 }
 
 // static
@@ -579,7 +631,45 @@
 void TFunctionSymbolInfo::setFromFunction(const TFunction &function)
 {
     setName(function.getMangledName());
-    setId(function.getUniqueId());
+    setId(TSymbolUniqueId(function));
+}
+
+TFunctionSymbolInfo::TFunctionSymbolInfo(const TSymbolUniqueId &id) : mId(new TSymbolUniqueId(id))
+{
+}
+
+TFunctionSymbolInfo::TFunctionSymbolInfo(const TFunctionSymbolInfo &info)
+    : mName(info.mName), mId(nullptr)
+{
+    if (info.mId)
+    {
+        mId = new TSymbolUniqueId(*info.mId);
+    }
+}
+
+TFunctionSymbolInfo &TFunctionSymbolInfo::operator=(const TFunctionSymbolInfo &info)
+{
+    mName = info.mName;
+    if (info.mId)
+    {
+        mId = new TSymbolUniqueId(*info.mId);
+    }
+    else
+    {
+        mId = nullptr;
+    }
+    return *this;
+}
+
+void TFunctionSymbolInfo::setId(const TSymbolUniqueId &id)
+{
+    mId = new TSymbolUniqueId(id);
+}
+
+const TSymbolUniqueId &TFunctionSymbolInfo::getId() const
+{
+    ASSERT(mId);
+    return *mId;
 }
 
 TIntermAggregate::TIntermAggregate(const TIntermAggregate &node)
@@ -597,6 +687,16 @@
     }
 }
 
+TIntermAggregate *TIntermAggregate::shallowCopy() const
+{
+    TIntermSequence *copySeq = new TIntermSequence();
+    copySeq->insert(copySeq->begin(), getSequence()->begin(), getSequence()->end());
+    TIntermAggregate *copyNode         = new TIntermAggregate(mType, mOp, copySeq);
+    *copyNode->getFunctionSymbolInfo() = mFunctionInfo;
+    copyNode->setLine(mLine);
+    return copyNode;
+}
+
 TIntermSwizzle::TIntermSwizzle(const TIntermSwizzle &node) : TIntermTyped(node)
 {
     TIntermTyped *operandCopy = node.mOperand->deepCopy();
@@ -3276,4 +3376,45 @@
     mReplacements.push_back(NodeUpdateEntry(parent, original, replacement, originalBecomesChild));
 }
 
+TName TIntermTraverser::GetInternalFunctionName(const char *name)
+{
+    TString nameStr(name);
+    nameStr = TFunction::mangleName(nameStr);
+    TName nameObj(nameStr);
+    nameObj.setInternal(true);
+    return nameObj;
+}
+
+TIntermFunctionPrototype *TIntermTraverser::CreateInternalFunctionPrototypeNode(
+    const TType &returnType,
+    const char *name,
+    const TSymbolUniqueId &functionId)
+{
+    TIntermFunctionPrototype *functionNode = new TIntermFunctionPrototype(returnType, functionId);
+    functionNode->getFunctionSymbolInfo()->setNameObj(GetInternalFunctionName(name));
+    return functionNode;
+}
+
+TIntermFunctionDefinition *TIntermTraverser::CreateInternalFunctionDefinitionNode(
+    const TType &returnType,
+    const char *name,
+    TIntermBlock *functionBody,
+    const TSymbolUniqueId &functionId)
+{
+    TIntermFunctionPrototype *prototypeNode =
+        CreateInternalFunctionPrototypeNode(returnType, name, functionId);
+    return new TIntermFunctionDefinition(prototypeNode, functionBody);
+}
+
+TIntermAggregate *TIntermTraverser::CreateInternalFunctionCallNode(
+    const TType &returnType,
+    const char *name,
+    const TSymbolUniqueId &functionId,
+    TIntermSequence *arguments)
+{
+    TIntermAggregate *functionNode = TIntermAggregate::CreateFunctionCall(
+        returnType, functionId, GetInternalFunctionName(name), arguments);
+    return functionNode;
+}
+
 }  // namespace sh
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 608aa05..513c111 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -56,6 +56,7 @@
 class TIntermBranch;
 
 class TSymbolTable;
+class TSymbolUniqueId;
 class TFunction;
 
 // Encapsulate an identifier string and track whether it is coming from the original shader code
@@ -414,7 +415,7 @@
 
     TIntermOperator(const TIntermOperator &) = default;
 
-    TOperator mOp;
+    const TOperator mOp;
 };
 
 // Node for vector swizzles.
@@ -535,10 +536,11 @@
 {
   public:
     POOL_ALLOCATOR_NEW_DELETE();
-    TFunctionSymbolInfo() : mId(0) {}
+    TFunctionSymbolInfo(const TSymbolUniqueId &id);
+    TFunctionSymbolInfo() : mId(nullptr) {}
 
-    TFunctionSymbolInfo(const TFunctionSymbolInfo &) = default;
-    TFunctionSymbolInfo &operator=(const TFunctionSymbolInfo &) = default;
+    TFunctionSymbolInfo(const TFunctionSymbolInfo &info);
+    TFunctionSymbolInfo &operator=(const TFunctionSymbolInfo &info);
 
     void setFromFunction(const TFunction &function);
 
@@ -550,11 +552,12 @@
     void setName(const TString &name) { mName.setString(name); }
     bool isMain() const { return mName.getString() == "main("; }
 
-    void setId(int functionId) { mId = functionId; }
-    int getId() const { return mId; }
+    void setId(const TSymbolUniqueId &functionId);
+    const TSymbolUniqueId &getId() const;
+
   private:
     TName mName;
-    int mId;
+    TSymbolUniqueId *mId;
 };
 
 typedef TVector<TIntermNode *> TIntermSequence;
@@ -593,12 +596,28 @@
 class TIntermAggregate : public TIntermOperator, public TIntermAggregateBase
 {
   public:
-    TIntermAggregate(const TType &type, TOperator op, TIntermSequence *arguments);
+    static TIntermAggregate *CreateFunctionCall(const TFunction &func, TIntermSequence *arguments);
+
+    // If using this, ensure that there's a consistent function definition with the same symbol id
+    // added to the AST.
+    static TIntermAggregate *CreateFunctionCall(const TType &type,
+                                                const TSymbolUniqueId &id,
+                                                const TName &name,
+                                                TIntermSequence *arguments);
+
+    static TIntermAggregate *CreateBuiltInFunctionCall(const TFunction &func,
+                                                       TIntermSequence *arguments);
+    static TIntermAggregate *CreateConstructor(const TType &type,
+                                               TOperator op,
+                                               TIntermSequence *arguments);
+    static TIntermAggregate *Create(const TType &type, TOperator op, TIntermSequence *arguments);
     ~TIntermAggregate() {}
 
     // Note: only supported for nodes that can be a part of an expression.
     TIntermTyped *deepCopy() const override { return new TIntermAggregate(*this); }
 
+    TIntermAggregate *shallowCopy() const;
+
     TIntermAggregate *getAsAggregate() override { return this; }
     void traverse(TIntermTraverser *it) override;
     bool replaceChildNode(TIntermNode *original, TIntermNode *replacement) override;
@@ -619,10 +638,6 @@
     TFunctionSymbolInfo *getFunctionSymbolInfo() { return &mFunctionInfo; }
     const TFunctionSymbolInfo *getFunctionSymbolInfo() const { return &mFunctionInfo; }
 
-    // Used for built-in functions under EOpCallBuiltInFunction. The function name in the symbol
-    // info needs to be set before calling this.
-    void setBuiltInFunctionPrecision();
-
   protected:
     TIntermSequence mArguments;
 
@@ -635,6 +650,8 @@
     TFunctionSymbolInfo mFunctionInfo;
 
   private:
+    TIntermAggregate(const TType &type, TOperator op, TIntermSequence *arguments);
+
     TIntermAggregate(const TIntermAggregate &node);  // note: not deleted, just private!
 
     void setTypePrecisionAndQualifier(const TType &type);
@@ -647,6 +664,10 @@
 
     // Returns true if precision was set according to special rules for this built-in.
     bool setPrecisionForSpecialBuiltInOp();
+
+    // Used for built-in functions under EOpCallBuiltInFunction. The function name in the symbol
+    // info needs to be set before calling this.
+    void setBuiltInFunctionPrecision();
 };
 
 // A list of statements. Either the root node which contains declarations and function definitions,
@@ -678,7 +699,10 @@
   public:
     // TODO(oetuaho@nvidia.com): See if TFunctionSymbolInfo could be added to constructor
     // parameters.
-    TIntermFunctionPrototype(const TType &type) : TIntermTyped(type) {}
+    TIntermFunctionPrototype(const TType &type, const TSymbolUniqueId &id)
+        : TIntermTyped(type), mFunctionInfo(id)
+    {
+    }
     ~TIntermFunctionPrototype() {}
 
     TIntermFunctionPrototype *getAsFunctionPrototypeNode() override { return this; }
@@ -967,6 +991,20 @@
     // Start creating temporary symbols from the given temporary symbol index + 1.
     void useTemporaryIndex(unsigned int *temporaryIndex);
 
+    static TIntermFunctionPrototype *CreateInternalFunctionPrototypeNode(
+        const TType &returnType,
+        const char *name,
+        const TSymbolUniqueId &functionId);
+    static TIntermFunctionDefinition *CreateInternalFunctionDefinitionNode(
+        const TType &returnType,
+        const char *name,
+        TIntermBlock *functionBody,
+        const TSymbolUniqueId &functionId);
+    static TIntermAggregate *CreateInternalFunctionCallNode(const TType &returnType,
+                                                            const char *name,
+                                                            const TSymbolUniqueId &functionId,
+                                                            TIntermSequence *arguments);
+
   protected:
     // Should only be called from traverse*() functions
     void incrementDepth(TIntermNode *current)
@@ -1112,6 +1150,8 @@
     std::vector<NodeInsertMultipleEntry> mInsertions;
 
   private:
+    static TName GetInternalFunctionName(const char *name);
+
     // To replace a single node with another on the parent node
     struct NodeUpdateEntry
     {
@@ -1195,7 +1235,7 @@
     bool operatorRequiresLValue() const { return mOperatorRequiresLValue; }
 
     // Add a function encountered during traversal to the function map.
-    void addToFunctionMap(const TName &name, TIntermSequence *paramSequence);
+    void addToFunctionMap(const TSymbolUniqueId &id, TIntermSequence *paramSequence);
 
     // Return true if the prototype or definition of the function being called has been encountered
     // during traversal.
@@ -1211,20 +1251,8 @@
     bool mOperatorRequiresLValue;
     bool mInFunctionCallOutParameter;
 
-    struct TNameComparator
-    {
-        bool operator()(const TName &a, const TName &b) const
-        {
-            int compareResult = a.getString().compare(b.getString());
-            if (compareResult != 0)
-                return compareResult < 0;
-            // Internal functions may have same names as non-internal functions.
-            return !a.isInternal() && b.isInternal();
-        }
-    };
-
-    // Map from mangled function names to their parameter sequences
-    TMap<TName, TIntermSequence *, TNameComparator> mFunctionMap;
+    // Map from function symbol id values to their parameter sequences
+    TMap<int, TIntermSequence *> mFunctionMap;
 
     const TSymbolTable &mSymbolTable;
     const int mShaderVersion;
diff --git a/src/compiler/translator/IntermTraverse.cpp b/src/compiler/translator/IntermTraverse.cpp
index 8980e69..552979d 100644
--- a/src/compiler/translator/IntermTraverse.cpp
+++ b/src/compiler/translator/IntermTraverse.cpp
@@ -228,22 +228,23 @@
     ++(*mTemporaryIndex);
 }
 
-void TLValueTrackingTraverser::addToFunctionMap(const TName &name, TIntermSequence *paramSequence)
+void TLValueTrackingTraverser::addToFunctionMap(const TSymbolUniqueId &id,
+                                                TIntermSequence *paramSequence)
 {
-    mFunctionMap[name] = paramSequence;
+    mFunctionMap[id.get()] = paramSequence;
 }
 
 bool TLValueTrackingTraverser::isInFunctionMap(const TIntermAggregate *callNode) const
 {
     ASSERT(callNode->getOp() == EOpCallFunctionInAST);
-    return (mFunctionMap.find(callNode->getFunctionSymbolInfo()->getNameObj()) !=
+    return (mFunctionMap.find(callNode->getFunctionSymbolInfo()->getId().get()) !=
             mFunctionMap.end());
 }
 
 TIntermSequence *TLValueTrackingTraverser::getFunctionParameters(const TIntermAggregate *callNode)
 {
     ASSERT(isInFunctionMap(callNode));
-    return mFunctionMap[callNode->getFunctionSymbolInfo()->getNameObj()];
+    return mFunctionMap[callNode->getFunctionSymbolInfo()->getId().get()];
 }
 
 void TLValueTrackingTraverser::setInFunctionCallOutParameter(bool inOutParameter)
@@ -623,7 +624,7 @@
 void TLValueTrackingTraverser::traverseFunctionPrototype(TIntermFunctionPrototype *node)
 {
     TIntermSequence *sequence = node->getSequence();
-    addToFunctionMap(node->getFunctionSymbolInfo()->getNameObj(), sequence);
+    addToFunctionMap(node->getFunctionSymbolInfo()->getId(), sequence);
 
     TIntermTraverser::traverseFunctionPrototype(node);
 }
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index bb08882..bdd68f3 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -2490,7 +2490,8 @@
     const TSourceLoc &location,
     bool insertParametersToSymbolTable)
 {
-    TIntermFunctionPrototype *prototype = new TIntermFunctionPrototype(function.getReturnType());
+    TIntermFunctionPrototype *prototype =
+        new TIntermFunctionPrototype(function.getReturnType(), TSymbolUniqueId(function));
     // TODO(oetuaho@nvidia.com): Instead of converting the function information here, the node could
     // point to the data that already exists in the symbol table.
     prototype->getFunctionSymbolInfo()->setFromFunction(function);
@@ -2803,9 +2804,8 @@
         return TIntermTyped::CreateZero(type);
     }
 
-    TIntermAggregate *constructorNode = new TIntermAggregate(type, op, arguments);
+    TIntermAggregate *constructorNode = TIntermAggregate::CreateConstructor(type, op, arguments);
     constructorNode->setLine(line);
-    ASSERT(constructorNode->isConstructor());
 
     TIntermTyped *constConstructor =
         intermediate.foldAggregateBuiltIn(constructorNode, mDiagnostics);
@@ -4533,7 +4533,7 @@
                 else
                 {
                     TIntermAggregate *callNode =
-                        new TIntermAggregate(fnCandidate->getReturnType(), op, arguments);
+                        TIntermAggregate::Create(fnCandidate->getReturnType(), op, arguments);
                     callNode->setLine(loc);
 
                     // Some built-in functions have out parameters too.
@@ -4561,19 +4561,13 @@
                 // This needs to happen after the function info including name is set.
                 if (builtIn)
                 {
-                    callNode = new TIntermAggregate(fnCandidate->getReturnType(),
-                                                    EOpCallBuiltInFunction, arguments);
-                    // Note that name needs to be set before texture function type is determined.
-                    callNode->getFunctionSymbolInfo()->setFromFunction(*fnCandidate);
-                    callNode->setBuiltInFunctionPrecision();
+                    callNode = TIntermAggregate::CreateBuiltInFunctionCall(*fnCandidate, arguments);
                     checkTextureOffsetConst(callNode);
                     checkImageMemoryAccessForBuiltinFunctions(callNode);
                 }
                 else
                 {
-                    callNode = new TIntermAggregate(fnCandidate->getReturnType(),
-                                                    EOpCallFunctionInAST, arguments);
-                    callNode->getFunctionSymbolInfo()->setFromFunction(*fnCandidate);
+                    callNode = TIntermAggregate::CreateFunctionCall(*fnCandidate, arguments);
                     checkImageMemoryAccessForUserDefinedFunctions(fnCandidate, callNode);
                 }
 
diff --git a/src/compiler/translator/RemoveDynamicIndexing.cpp b/src/compiler/translator/RemoveDynamicIndexing.cpp
index 7424f39..a8f6b51 100644
--- a/src/compiler/translator/RemoveDynamicIndexing.cpp
+++ b/src/compiler/translator/RemoveDynamicIndexing.cpp
@@ -20,7 +20,7 @@
 namespace
 {
 
-TName GetIndexFunctionName(const TType &type, bool write)
+std::string GetIndexFunctionName(const TType &type, bool write)
 {
     TInfoSinkBase nameSink;
     nameSink << "dyn_index_";
@@ -53,12 +53,7 @@
         }
         nameSink << type.getNominalSize();
     }
-    TString nameString = TFunction::mangleName(nameSink.c_str());
-    TName name(nameString);
-    // TODO(oetuaho@nvidia.com): would be better to have the parameter types in the mangled name as
-    // well.
-    name.setInternal(true);
-    return name;
+    return nameSink.str();
 }
 
 TIntermSymbol *CreateBaseSymbol(const TType &type, TQualifier qualifier)
@@ -115,7 +110,7 @@
 
     TIntermSequence *arguments = new TIntermSequence();
     arguments->push_back(node);
-    return new TIntermAggregate(TType(EbtInt), EOpConstructInt, arguments);
+    return TIntermAggregate::CreateConstructor(TType(EbtInt), EOpConstructInt, arguments);
 }
 
 TType GetFieldType(const TType &indexedType)
@@ -175,7 +170,9 @@
 //    base[1] = value;
 // }
 // Note that else is not used in above functions to avoid the RewriteElseBlocks transformation.
-TIntermFunctionDefinition *GetIndexFunctionDefinition(TType type, bool write)
+TIntermFunctionDefinition *GetIndexFunctionDefinition(TType type,
+                                                      bool write,
+                                                      const TSymbolUniqueId &functionId)
 {
     ASSERT(!type.isArray());
     // Conservatively use highp here, even if the indexed type is not highp. That way the code can't
@@ -195,16 +192,15 @@
         numCases = type.getNominalSize();
     }
 
-    TIntermFunctionPrototype *prototypeNode = nullptr;
-    if (write)
+    TType returnType(EbtVoid);
+    if (!write)
     {
-        prototypeNode = new TIntermFunctionPrototype(TType(EbtVoid));
+        returnType = fieldType;
     }
-    else
-    {
-        prototypeNode = new TIntermFunctionPrototype(fieldType);
-    }
-    prototypeNode->getFunctionSymbolInfo()->setNameObj(GetIndexFunctionName(type, write));
+
+    std::string functionName                = GetIndexFunctionName(type, write);
+    TIntermFunctionPrototype *prototypeNode = TIntermTraverser::CreateInternalFunctionPrototypeNode(
+        returnType, functionName.c_str(), functionId);
 
     TQualifier baseQualifier     = EvqInOut;
     if (!write)
@@ -305,10 +301,11 @@
     bool usedTreeInsertion() const { return mUsedTreeInsertion; }
 
   protected:
-    // Sets of types that are indexed. Note that these can not store multiple variants
-    // of the same type with different precisions - only one precision gets stored.
-    std::set<TType> mIndexedVecAndMatrixTypes;
-    std::set<TType> mWrittenVecAndMatrixTypes;
+    // Maps of types that are indexed to the indexing function ids used for them. Note that these
+    // can not store multiple variants of the same type with different precisions - only one
+    // precision gets stored.
+    std::map<TType, TSymbolUniqueId> mIndexedVecAndMatrixTypes;
+    std::map<TType, TSymbolUniqueId> mWrittenVecAndMatrixTypes;
 
     bool mUsedTreeInsertion;
 
@@ -332,49 +329,51 @@
     TIntermBlock *rootBlock = root->getAsBlock();
     ASSERT(rootBlock != nullptr);
     TIntermSequence insertions;
-    for (TType type : mIndexedVecAndMatrixTypes)
+    for (auto &type : mIndexedVecAndMatrixTypes)
     {
-        insertions.push_back(GetIndexFunctionDefinition(type, false));
+        insertions.push_back(GetIndexFunctionDefinition(type.first, false, type.second));
     }
-    for (TType type : mWrittenVecAndMatrixTypes)
+    for (auto &type : mWrittenVecAndMatrixTypes)
     {
-        insertions.push_back(GetIndexFunctionDefinition(type, true));
+        insertions.push_back(GetIndexFunctionDefinition(type.first, true, type.second));
     }
     mInsertions.push_back(NodeInsertMultipleEntry(rootBlock, 0, insertions, TIntermSequence()));
 }
 
 // Create a call to dyn_index_*() based on an indirect indexing op node
 TIntermAggregate *CreateIndexFunctionCall(TIntermBinary *node,
-                                          TIntermTyped *indexedNode,
-                                          TIntermTyped *index)
+                                          TIntermTyped *index,
+                                          const TSymbolUniqueId &functionId)
 {
     ASSERT(node->getOp() == EOpIndexIndirect);
     TIntermSequence *arguments = new TIntermSequence();
-    arguments->push_back(indexedNode);
+    arguments->push_back(node->getLeft());
     arguments->push_back(index);
 
-    TType fieldType = GetFieldType(indexedNode->getType());
-    TIntermAggregate *indexingCall =
-        new TIntermAggregate(fieldType, EOpCallFunctionInAST, arguments);
+    TType fieldType                = GetFieldType(node->getLeft()->getType());
+    std::string functionName       = GetIndexFunctionName(node->getLeft()->getType(), false);
+    TIntermAggregate *indexingCall = TIntermTraverser::CreateInternalFunctionCallNode(
+        fieldType, functionName.c_str(), functionId, arguments);
     indexingCall->setLine(node->getLine());
-    indexingCall->getFunctionSymbolInfo()->setNameObj(
-        GetIndexFunctionName(indexedNode->getType(), false));
     return indexingCall;
 }
 
 TIntermAggregate *CreateIndexedWriteFunctionCall(TIntermBinary *node,
                                                  TIntermTyped *index,
-                                                 TIntermTyped *writtenValue)
+                                                 TIntermTyped *writtenValue,
+                                                 const TSymbolUniqueId &functionId)
 {
-    // Deep copy the left node so that two pointers to the same node don't end up in the tree.
-    TIntermNode *leftCopy = node->getLeft()->deepCopy();
-    ASSERT(leftCopy != nullptr && leftCopy->getAsTyped() != nullptr);
-    TIntermAggregate *indexedWriteCall =
-        CreateIndexFunctionCall(node, leftCopy->getAsTyped(), index);
-    indexedWriteCall->getFunctionSymbolInfo()->setNameObj(
-        GetIndexFunctionName(node->getLeft()->getType(), true));
-    indexedWriteCall->setType(TType(EbtVoid));
-    indexedWriteCall->getSequence()->push_back(writtenValue);
+    ASSERT(node->getOp() == EOpIndexIndirect);
+    TIntermSequence *arguments = new TIntermSequence();
+    // Deep copy the child nodes so that two pointers to the same node don't end up in the tree.
+    arguments->push_back(node->getLeft()->deepCopy());
+    arguments->push_back(index->deepCopy());
+    arguments->push_back(writtenValue);
+
+    std::string functionName           = GetIndexFunctionName(node->getLeft()->getType(), true);
+    TIntermAggregate *indexedWriteCall = TIntermTraverser::CreateInternalFunctionCallNode(
+        TType(EbtVoid), functionName.c_str(), functionId, arguments);
+    indexedWriteCall->setLine(node->getLine());
     return indexedWriteCall;
 }
 
@@ -415,8 +414,16 @@
             ASSERT(matcher.match(node, getParentNode(), isLValueRequiredHere()) == write);
 #endif
 
-            TType type = node->getLeft()->getType();
-            mIndexedVecAndMatrixTypes.insert(type);
+            const TType &type = node->getLeft()->getType();
+            TSymbolUniqueId indexingFunctionId;
+            if (mIndexedVecAndMatrixTypes.find(type) == mIndexedVecAndMatrixTypes.end())
+            {
+                mIndexedVecAndMatrixTypes[type] = indexingFunctionId;
+            }
+            else
+            {
+                indexingFunctionId = mIndexedVecAndMatrixTypes[type];
+            }
 
             if (write)
             {
@@ -450,7 +457,15 @@
                 // TODO(oetuaho@nvidia.com): This is not optimal if the expression using the value
                 // only writes it and doesn't need the previous value. http://anglebug.com/1116
 
-                mWrittenVecAndMatrixTypes.insert(type);
+                TSymbolUniqueId indexedWriteFunctionId;
+                if (mWrittenVecAndMatrixTypes.find(type) == mWrittenVecAndMatrixTypes.end())
+                {
+                    mWrittenVecAndMatrixTypes[type] = indexedWriteFunctionId;
+                }
+                else
+                {
+                    indexedWriteFunctionId = mWrittenVecAndMatrixTypes[type];
+                }
                 TType fieldType = GetFieldType(type);
 
                 TIntermSequence insertionsBefore;
@@ -462,19 +477,19 @@
                 initIndex->setLine(node->getLine());
                 insertionsBefore.push_back(initIndex);
 
-                TIntermAggregate *indexingCall = CreateIndexFunctionCall(
-                    node, node->getLeft(), createTempSymbol(indexInitializer->getType()));
-
                 // Create a node for referring to the index after the nextTemporaryIndex() call
                 // below.
                 TIntermSymbol *tempIndex = createTempSymbol(indexInitializer->getType());
 
+                TIntermAggregate *indexingCall =
+                    CreateIndexFunctionCall(node, tempIndex, indexingFunctionId);
+
                 nextTemporaryIndex();  // From now on, creating temporary symbols that refer to the
                                        // field value.
                 insertionsBefore.push_back(createTempInitDeclaration(indexingCall));
 
-                TIntermAggregate *indexedWriteCall =
-                    CreateIndexedWriteFunctionCall(node, tempIndex, createTempSymbol(fieldType));
+                TIntermAggregate *indexedWriteCall = CreateIndexedWriteFunctionCall(
+                    node, tempIndex, createTempSymbol(fieldType), indexedWriteFunctionId);
                 insertionsAfter.push_back(indexedWriteCall);
                 insertStatementsInParentBlock(insertionsBefore, insertionsAfter);
                 queueReplacement(node, createTempSymbol(fieldType), OriginalNode::IS_DROPPED);
@@ -489,7 +504,7 @@
                 // If the index_expr is unsigned, we'll convert it to signed.
                 ASSERT(!mRemoveIndexSideEffectsInSubtree);
                 TIntermAggregate *indexingCall = CreateIndexFunctionCall(
-                    node, node->getLeft(), EnsureSignedInt(node->getRight()));
+                    node, EnsureSignedInt(node->getRight()), indexingFunctionId);
                 queueReplacement(node, indexingCall, OriginalNode::IS_DROPPED);
             }
         }
diff --git a/src/compiler/translator/RewriteTexelFetchOffset.cpp b/src/compiler/translator/RewriteTexelFetchOffset.cpp
index b1ccf0c..3be83df 100644
--- a/src/compiler/translator/RewriteTexelFetchOffset.cpp
+++ b/src/compiler/translator/RewriteTexelFetchOffset.cpp
@@ -89,8 +89,7 @@
         16, node->getFunctionSymbolInfo()->getName().length() - 20);
     TString newName           = "texelFetch" + newArgs;
     TSymbol *texelFetchSymbol = symbolTable->findBuiltIn(newName, shaderVersion);
-    ASSERT(texelFetchSymbol);
-    int uniqueId = texelFetchSymbol->getUniqueId();
+    ASSERT(texelFetchSymbol && texelFetchSymbol->isFunction());
 
     // Create new node that represents the call of function texelFetch.
     // Its argument list will be: texelFetch(sampler, Position+offset, lod).
@@ -117,8 +116,8 @@
         TIntermTyped *zeroNode = TIntermTyped::CreateZero(TType(EbtInt));
         constructOffsetIvecArguments->push_back(zeroNode);
 
-        offsetNode = new TIntermAggregate(texCoordNode->getType(), EOpConstructIVec3,
-                                          constructOffsetIvecArguments);
+        offsetNode = TIntermAggregate::CreateConstructor(texCoordNode->getType(), EOpConstructIVec3,
+                                                         constructOffsetIvecArguments);
         offsetNode->setLine(texCoordNode->getLine());
     }
     else
@@ -136,10 +135,8 @@
 
     ASSERT(texelFetchArguments->size() == 3u);
 
-    TIntermAggregate *texelFetchNode =
-        new TIntermAggregate(node->getType(), EOpCallBuiltInFunction, texelFetchArguments);
-    texelFetchNode->getFunctionSymbolInfo()->setName(newName);
-    texelFetchNode->getFunctionSymbolInfo()->setId(uniqueId);
+    TIntermAggregate *texelFetchNode = TIntermAggregate::CreateBuiltInFunctionCall(
+        *static_cast<const TFunction *>(texelFetchSymbol), texelFetchArguments);
     texelFetchNode->setLine(node->getLine());
 
     // Replace the old node by this new node.
diff --git a/src/compiler/translator/SeparateExpressionsReturningArrays.cpp b/src/compiler/translator/SeparateExpressionsReturningArrays.cpp
index 6489a91..1e7000b 100644
--- a/src/compiler/translator/SeparateExpressionsReturningArrays.cpp
+++ b/src/compiler/translator/SeparateExpressionsReturningArrays.cpp
@@ -55,16 +55,6 @@
     return new TIntermBinary(node->getOp(), node->getLeft(), node->getRight());
 }
 
-// Performs a shallow copy of a constructor/function call node.
-TIntermAggregate *CopyAggregateNode(TIntermAggregate *node)
-{
-    TIntermSequence *copySeq = new TIntermSequence();
-    copySeq->insert(copySeq->begin(), node->getSequence()->begin(), node->getSequence()->end());
-    TIntermAggregate *copyNode = new TIntermAggregate(node->getType(), node->getOp(), copySeq);
-    *copyNode->getFunctionSymbolInfo() = *node->getFunctionSymbolInfo();
-    return copyNode;
-}
-
 bool SeparateExpressionsTraverser::visitBinary(Visit visit, TIntermBinary *node)
 {
     if (mFoundArrayExpression)
@@ -104,7 +94,7 @@
     mFoundArrayExpression = true;
 
     TIntermSequence insertions;
-    insertions.push_back(createTempInitDeclaration(CopyAggregateNode(node)));
+    insertions.push_back(createTempInitDeclaration(node->shallowCopy()));
     insertStatementsInParentBlock(insertions);
 
     queueReplacement(node, createTempSymbol(node->getType()), OriginalNode::IS_DROPPED);
diff --git a/src/compiler/translator/SymbolTable.cpp b/src/compiler/translator/SymbolTable.cpp
index b246a0f..4ec871e 100644
--- a/src/compiler/translator/SymbolTable.cpp
+++ b/src/compiler/translator/SymbolTable.cpp
@@ -26,6 +26,19 @@
 
 int TSymbolTable::uniqueIdCounter = 0;
 
+TSymbolUniqueId::TSymbolUniqueId() : mId(TSymbolTable::nextUniqueId())
+{
+}
+
+TSymbolUniqueId::TSymbolUniqueId(const TSymbol &symbol) : mId(symbol.getUniqueId())
+{
+}
+
+int TSymbolUniqueId::get() const
+{
+    return mId;
+}
+
 TSymbol::TSymbol(const TString *n) : uniqueId(TSymbolTable::nextUniqueId()), name(n)
 {
 }
diff --git a/src/compiler/translator/SymbolTable.h b/src/compiler/translator/SymbolTable.h
index 671b183..5f3e5d3 100644
--- a/src/compiler/translator/SymbolTable.h
+++ b/src/compiler/translator/SymbolTable.h
@@ -41,6 +41,22 @@
 namespace sh
 {
 
+// Encapsulates a unique id for a symbol.
+class TSymbolUniqueId
+{
+  public:
+    POOL_ALLOCATOR_NEW_DELETE();
+    TSymbolUniqueId();
+    TSymbolUniqueId(const TSymbol &symbol);
+    TSymbolUniqueId(const TSymbolUniqueId &) = default;
+    TSymbolUniqueId &operator=(const TSymbolUniqueId &) = default;
+
+    int get() const;
+
+  private:
+    int mId;
+};
+
 // Symbol base class. (Can build functions or variables out of these...)
 class TSymbol : angle::NonCopyable
 {
diff --git a/src/compiler/translator/intermOut.cpp b/src/compiler/translator/intermOut.cpp
index af2c199..2b71efa 100644
--- a/src/compiler/translator/intermOut.cpp
+++ b/src/compiler/translator/intermOut.cpp
@@ -17,7 +17,7 @@
 {
     const char *internal = info->getNameObj().isInternal() ? " (internal function)" : "";
     out << str << internal << ": " << info->getNameObj().getString() << " (symbol id "
-        << info->getId() << ")";
+        << info->getId().get() << ")";
 }
 
 //
diff --git a/src/tests/compiler_tests/IntermNode_test.cpp b/src/tests/compiler_tests/IntermNode_test.cpp
index a277956..67ef370 100644
--- a/src/tests/compiler_tests/IntermNode_test.cpp
+++ b/src/tests/compiler_tests/IntermNode_test.cpp
@@ -189,7 +189,7 @@
     originalSeq->push_back(createTestSymbol());
     originalSeq->push_back(createTestSymbol());
     TIntermAggregate *original =
-        new TIntermAggregate(originalSeq->at(0)->getAsTyped()->getType(), EOpMix, originalSeq);
+        TIntermAggregate::Create(originalSeq->at(0)->getAsTyped()->getType(), EOpMix, originalSeq);
     original->setLine(getTestSourceLoc());
 
     TIntermTyped *copyTyped = original->deepCopy();
diff --git a/src/tests/compiler_tests/PruneUnusedFunctions_test.cpp b/src/tests/compiler_tests/PruneUnusedFunctions_test.cpp
index fe710d8..d9ccedc 100644
--- a/src/tests/compiler_tests/PruneUnusedFunctions_test.cpp
+++ b/src/tests/compiler_tests/PruneUnusedFunctions_test.cpp
@@ -69,7 +69,7 @@
     EXPECT_TRUE(foundInCode("main(", 1));
 }
 
-// Check that used functions are not prunued (duh)
+// Check that used functions are not pruned (duh)
 TEST_F(PruneUnusedFunctionsTest, UsedFunction)
 {
     const std::string &shaderString =