Simplify built-in function node initialization

src/compiler/translator/IntermNode.cpp

@@ -352,11 +352,9 @@ TIntermAggregate *TIntermAggregate::CreateRawFunctionCall(const TFunction &func,
 TIntermAggregate *TIntermAggregate::CreateBuiltInFunctionCall(const TFunction &func,
                                                               TIntermSequence *arguments)
 {
-    TIntermAggregate *callNode =
-        new TIntermAggregate(&func, func.getReturnType(), EOpCallBuiltInFunction, arguments);
-    // Note that name needs to be set before texture function type is determined.
-    callNode->setBuiltInFunctionPrecision();
-    return callNode;
+    // op should be either EOpCallBuiltInFunction or a specific math op.
+    ASSERT(func.getBuiltInOp() != EOpNull);
+    return new TIntermAggregate(&func, func.getReturnType(), func.getBuiltInOp(), arguments);
 }
 
 TIntermAggregate *TIntermAggregate::CreateConstructor(const TType &type,
@@ -365,22 +363,11 @@ TIntermAggregate *TIntermAggregate::CreateConstructor(const TType &type,
     return new TIntermAggregate(nullptr, type, EOpConstruct, arguments);
 }
 
-TIntermAggregate *TIntermAggregate::Create(const TFunction &func,
-                                           TOperator op,
-                                           TIntermSequence *arguments)
-{
-    ASSERT(op != EOpCallFunctionInAST);    // Should use CreateFunctionCall
-    ASSERT(op != EOpCallInternalRawFunction);  // Should use CreateRawFunctionCall
-    ASSERT(op != EOpCallBuiltInFunction);  // Should use CreateBuiltInFunctionCall
-    ASSERT(op != EOpConstruct);            // Should use CreateConstructor
-    return new TIntermAggregate(&func, func.getReturnType(), op, arguments);
-}
-
 TIntermAggregate::TIntermAggregate(const TFunction *func,
                                    const TType &type,
                                    TOperator op,
                                    TIntermSequence *arguments)
-    : TIntermOperator(op),
+    : TIntermOperator(op, type),
       mUseEmulatedFunction(false),
       mGotPrecisionFromChildren(false),
       mFunction(func)
@@ -390,14 +377,17 @@ TIntermAggregate::TIntermAggregate(const TFunction *func,
         mArguments.swap(*arguments);
     }
     ASSERT(mFunction == nullptr || mFunction->symbolType() != SymbolType::Empty);
-    setTypePrecisionAndQualifier(type);
+    setPrecisionAndQualifier();
 }
 
-void TIntermAggregate::setTypePrecisionAndQualifier(const TType &type)
+void TIntermAggregate::setPrecisionAndQualifier()
 {
-    setType(type);
     mType.setQualifier(EvqTemporary);
-    if (!isFunctionCall())
+    if (mOp == EOpCallBuiltInFunction)
+    {
+        setBuiltInFunctionPrecision();
+    }
+    else if (!isFunctionCall())
     {
         if (isConstructor())
         {

src/compiler/translator/IntermNode.h

@@ -564,13 +564,11 @@ class TIntermAggregate : public TIntermOperator, public TIntermAggregateBase
     static TIntermAggregate *CreateRawFunctionCall(const TFunction &func,
                                                    TIntermSequence *arguments);
 
+    // This covers all built-in function calls - whether they are associated with an op or not.
     static TIntermAggregate *CreateBuiltInFunctionCall(const TFunction &func,
                                                        TIntermSequence *arguments);
     static TIntermAggregate *CreateConstructor(const TType &type,
                                                TIntermSequence *arguments);
-    static TIntermAggregate *Create(const TFunction &func,
-                                    TOperator op,
-                                    TIntermSequence *arguments);
     ~TIntermAggregate() {}
 
     // Note: only supported for nodes that can be a part of an expression.
@@ -622,7 +620,7 @@ class TIntermAggregate : public TIntermOperator, public TIntermAggregateBase
 
     TIntermAggregate(const TIntermAggregate &node);  // note: not deleted, just private!
 
-    void setTypePrecisionAndQualifier(const TType &type);
+    void setPrecisionAndQualifier();
 
     bool areChildrenConstQualified();
 

src/compiler/translator/IntermNode_util.cpp

@@ -259,13 +259,9 @@ TIntermTyped *CreateBuiltInFunctionCallNode(const char *name,
     const TFunction *fn = LookUpBuiltInFunction(name, arguments, symbolTable, shaderVersion);
     ASSERT(fn);
     TOperator op = fn->getBuiltInOp();
-    if (op != EOpNull)
+    if (op != EOpCallBuiltInFunction && arguments->size() == 1)
     {
-        if (arguments->size() == 1)
-        {
-            return new TIntermUnary(op, arguments->at(0)->getAsTyped());
-        }
-        return TIntermAggregate::Create(*fn, op, arguments);
+        return new TIntermUnary(op, arguments->at(0)->getAsTyped());
     }
     return TIntermAggregate::CreateBuiltInFunctionCall(*fn, arguments);
 }

src/compiler/translator/ParseContext.cpp

@@ -5808,23 +5808,39 @@ TIntermTyped *TParseContext::addNonConstructorFunctionCall(TFunctionLookup *fnCa
     }
     else
     {
-        symbol = symbolTable.find(fnCall->getMangledName(), mShaderVersion);
+        // There are no inner functions, so it's enough to look for user-defined functions in the
+        // global scope.
+        symbol = symbolTable.findGlobal(fnCall->getMangledName());
+        if (symbol != nullptr)
+        {
+            // A user-defined function - could be an overloaded built-in as well.
+            ASSERT(symbol->symbolType() == SymbolType::UserDefined);
+            const TFunction *fnCandidate = static_cast<const TFunction *>(symbol);
+            TIntermAggregate *callNode =
+                TIntermAggregate::CreateFunctionCall(*fnCandidate, &fnCall->arguments());
+            callNode->setLine(loc);
+            checkImageMemoryAccessForUserDefinedFunctions(fnCandidate, callNode);
+            functionCallRValueLValueErrorCheck(fnCandidate, callNode);
+            return callNode;
+        }
+
+        symbol = symbolTable.findBuiltIn(fnCall->getMangledName(), mShaderVersion);
         if (symbol == nullptr)
         {
             error(loc, "no matching overloaded function found", fnCall->name());
         }
         else
         {
+            // A built-in function.
+            ASSERT(symbol->symbolType() == SymbolType::BuiltIn);
             const TFunction *fnCandidate = static_cast<const TFunction *>(symbol);
-            //
-            // A declared function.
-            //
+
             if (fnCandidate->extension() != TExtension::UNDEFINED)
             {
                 checkCanUseExtension(loc, fnCandidate->extension());
             }
             TOperator op = fnCandidate->getBuiltInOp();
-            if (op != EOpNull)
+            if (op != EOpCallBuiltInFunction)
             {
                 // A function call mapped to a built-in operation.
                 if (fnCandidate->getParamCount() == 1)
@@ -5835,50 +5851,28 @@ TIntermTyped *TParseContext::addNonConstructorFunctionCall(TFunctionLookup *fnCa
                     ASSERT(callNode != nullptr);
                     return callNode;
                 }
-                else
-                {
-                    TIntermAggregate *callNode =
-                        TIntermAggregate::Create(*fnCandidate, op, &fnCall->arguments());
-                    callNode->setLine(loc);
-
-                    // Some built-in functions have out parameters too.
-                    functionCallRValueLValueErrorCheck(fnCandidate, callNode);
-
-                    // See if we can constant fold a built-in. Note that this may be possible
-                    // even if it is not const-qualified.
-                    return callNode->fold(mDiagnostics);
-                }
-            }
-            else
-            {
-                // This is a real function call.
-                TIntermAggregate *callNode = nullptr;
-
-                // If the symbol type is not BuiltIn, the function is user defined - could be an
-                // overloaded built-in as well. if the symbol type is BuiltIn, it's a built-in
-                // function with no op associated with it.
-                if (fnCandidate->symbolType() == SymbolType::BuiltIn)
-                {
-                    callNode = TIntermAggregate::CreateBuiltInFunctionCall(*fnCandidate,
-                                                                           &fnCall->arguments());
-                    checkTextureOffsetConst(callNode);
-                    checkTextureGather(callNode);
-                    checkImageMemoryAccessForBuiltinFunctions(callNode);
-                    checkAtomicMemoryBuiltinFunctions(callNode);
-                }
-                else
-                {
-                    callNode =
-                        TIntermAggregate::CreateFunctionCall(*fnCandidate, &fnCall->arguments());
-                    checkImageMemoryAccessForUserDefinedFunctions(fnCandidate, callNode);
-                }
+                TIntermAggregate *callNode =
+                    TIntermAggregate::CreateBuiltInFunctionCall(*fnCandidate, &fnCall->arguments());
+                callNode->setLine(loc);
 
+                // Some built-in functions have out parameters too.
                 functionCallRValueLValueErrorCheck(fnCandidate, callNode);
 
-                callNode->setLine(loc);
-
-                return callNode;
+                // See if we can constant fold a built-in. Note that this may be possible
+                // even if it is not const-qualified.
+                return callNode->fold(mDiagnostics);
             }
+
+            // This is a built-in function with no op associated with it.
+            TIntermAggregate *callNode =
+                TIntermAggregate::CreateBuiltInFunctionCall(*fnCandidate, &fnCall->arguments());
+            callNode->setLine(loc);
+            checkTextureOffsetConst(callNode);
+            checkTextureGather(callNode);
+            checkImageMemoryAccessForBuiltinFunctions(callNode);
+            checkAtomicMemoryBuiltinFunctions(callNode);
+            functionCallRValueLValueErrorCheck(fnCandidate, callNode);
+            return callNode;
         }
     }
 

src/compiler/translator/SymbolTable.h

@@ -196,8 +196,8 @@ class TSymbolTable : angle::NonCopyable
                        const TType *ptype5 = 0)
     {
         insertUnmangledBuiltInName(name, level);
-        insertBuiltIn(level, EOpNull, TExtension::UNDEFINED, rvalue, name, ptype1, ptype2, ptype3,
-                      ptype4, ptype5);
+        insertBuiltIn(level, EOpCallBuiltInFunction, TExtension::UNDEFINED, rvalue, name, ptype1,
+                      ptype2, ptype3, ptype4, ptype5);
     }
 
     void insertBuiltIn(ESymbolLevel level,
@@ -211,7 +211,8 @@ class TSymbolTable : angle::NonCopyable
                        const TType *ptype5 = 0)
     {
         insertUnmangledBuiltInName(name, level);
-        insertBuiltIn(level, EOpNull, ext, rvalue, name, ptype1, ptype2, ptype3, ptype4, ptype5);
+        insertBuiltIn(level, EOpCallBuiltInFunction, ext, rvalue, name, ptype1, ptype2, ptype3,
+                      ptype4, ptype5);
     }
 
     void insertBuiltInOp(ESymbolLevel level,

src/tests/compiler_tests/IntermNode_test.cpp

@@ -60,13 +60,18 @@ class IntermNodeTest : public testing::Test
         return createTestSymbol(type);
     }
 
-    TFunction *createTestBuiltInFunction(const TType &returnType, const TIntermSequence &args)
+    TFunction *createTestFunction(const TType &returnType, const TIntermSequence &args)
     {
         // We're using a dummy symbol table similarly as for creating symbol nodes.
         const ImmutableString name("testFunc");
         TSymbolTable symbolTable;
-        TFunction *func =
-            new TFunction(&symbolTable, name, new TType(returnType), SymbolType::BuiltIn, true);
+        TFunction *func = new TFunction(&symbolTable, name, new TType(returnType),
+                                        SymbolType::UserDefined, false);
+        for (TIntermNode *arg : args)
+        {
+            const TType *type = new TType(arg->getAsTyped()->getType());
+            func->addParameter(TConstParameter(type));
+        }
         return func;
     }
 
@@ -211,10 +216,10 @@ TEST_F(IntermNodeTest, DeepCopyAggregateNode)
     originalSeq->push_back(createTestSymbol());
     originalSeq->push_back(createTestSymbol());
 
-    TFunction *mix =
-        createTestBuiltInFunction(originalSeq->back()->getAsTyped()->getType(), *originalSeq);
+    TFunction *testFunc =
+        createTestFunction(originalSeq->back()->getAsTyped()->getType(), *originalSeq);
 
-    TIntermAggregate *original = TIntermAggregate::Create(*mix, EOpMix, originalSeq);
+    TIntermAggregate *original = TIntermAggregate::CreateFunctionCall(*testFunc, originalSeq);
     original->setLine(getTestSourceLoc());
 
     TIntermTyped *copyTyped = original->deepCopy();