Add symbol table function to get TFunction for a built-in op

src/compiler/translator/Compiler.cpp

@@ -373,7 +373,8 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
         bool multiview2 = IsExtensionEnabled(extensionBehavior, "GL_OVR_multiview2");
         if (success && compileResources.OVR_multiview && IsWebGLBasedSpec(shaderSpec) &&
             (IsExtensionEnabled(extensionBehavior, "GL_OVR_multiview") || multiview2))
-            success = ValidateMultiviewWebGL(root, shaderType, multiview2, &mDiagnostics);
+            success = ValidateMultiviewWebGL(root, shaderType, symbolTable, shaderVersion,
+                                             multiview2, &mDiagnostics);
 
         // Fail compilation if precision emulation not supported.
         if (success && getResources().WEBGL_debug_shader_precision &&

src/compiler/translator/IntermTraverse.cpp

@@ -680,26 +680,9 @@ void TLValueTrackingTraverser::traverseAggregate(TIntermAggregate *node)
             // Find the built-in function corresponding to this op so that we can determine the
             // in/out qualifiers of its parameters.
             TFunction *builtInFunc = nullptr;
-            TString opString       = GetOperatorString(node->getOp());
-            if (!node->isConstructor() && !opString.empty())
+            if (!node->isConstructor() && node->getOp() != EOpFunctionCall)
             {
-                // The return type doesn't affect the mangled name of the function, which is used
-                // to look it up from the symbol table.
-                TType dummyReturnType;
-                TFunction call(&opString, &dummyReturnType, node->getOp());
-                for (auto *child : *sequence)
-                {
-                    TType *paramType = child->getAsTyped()->getTypePointer();
-                    TConstParameter p(paramType);
-                    call.addParameter(p);
-                }
-
-                TSymbol *sym = mSymbolTable.findBuiltIn(call.getMangledName(), mShaderVersion);
-                if (sym != nullptr && sym->isFunction())
-                {
-                    builtInFunc = static_cast<TFunction *>(sym);
-                    ASSERT(builtInFunc->getParamCount() == sequence->size());
-                }
+                builtInFunc = mSymbolTable.findBuiltInOp(node, mShaderVersion);
             }
 
             size_t paramIndex = 0;

src/compiler/translator/SymbolTable.cpp

@@ -14,7 +14,9 @@
 #endif
 
 #include "compiler/translator/SymbolTable.h"
+
 #include "compiler/translator/Cache.h"
+#include "compiler/translator/IntermNode.h"
 
 #include <stdio.h>
 #include <algorithm>
@@ -153,6 +155,30 @@ TSymbol *TSymbolTable::findBuiltIn(const TString &name, int shaderVersion) const
     return 0;
 }
 
+TFunction *TSymbolTable::findBuiltInOp(TIntermAggregate *callNode, int shaderVersion) const
+{
+    ASSERT(!callNode->isConstructor());
+    ASSERT(callNode->getOp() != EOpFunctionCall);
+    TString opString = GetOperatorString(callNode->getOp());
+    // The return type doesn't affect the mangled name of the function, which is used to look it up.
+    TType dummyReturnType;
+    TFunction call(&opString, &dummyReturnType, callNode->getOp());
+    TIntermSequence *sequence = callNode->getSequence();
+    for (auto *child : *sequence)
+    {
+        TType *paramType = child->getAsTyped()->getTypePointer();
+        TConstParameter p(paramType);
+        call.addParameter(p);
+    }
+
+    TSymbol *sym = findBuiltIn(call.getMangledName(), shaderVersion);
+    ASSERT(sym != nullptr && sym->isFunction());
+
+    TFunction *builtInFunc = static_cast<TFunction *>(sym);
+    ASSERT(builtInFunc->getParamCount() == sequence->size());
+    return builtInFunc;
+}
+
 TSymbolTable::~TSymbolTable()
 {
     while (table.size() > 0)

src/compiler/translator/SymbolTable.h

@@ -41,6 +41,8 @@
 namespace sh
 {
 
+class TIntermAggregate;
+
 // Symbol base class. (Can build functions or variables out of these...)
 class TSymbol : angle::NonCopyable
 {
@@ -424,6 +426,10 @@ class TSymbolTable : angle::NonCopyable
 
     TSymbol *findBuiltIn(const TString &name, int shaderVersion) const;
 
+    // Helper front-end for regular findBuiltIn that constructs the mangled function name from
+    // callNode.
+    TFunction *findBuiltInOp(TIntermAggregate *callNode, int shaderVersion) const;
+
     TSymbolTableLevel *getOuterLevel()
     {
         assert(currentLevel() >= 1);

src/compiler/translator/ValidateMultiviewWebGL.cpp

@@ -31,6 +31,8 @@ class ValidateMultiviewTraverser : public TIntermTraverser
     // errors.
     static bool validate(TIntermBlock *root,
                          GLenum shaderType,
+                         const TSymbolTable &symbolTable,
+                         int shaderVersion,
                          bool multiview2,
                          TDiagnostics *diagnostics);
 
@@ -44,7 +46,11 @@ class ValidateMultiviewTraverser : public TIntermTraverser
     bool visitAggregate(Visit visit, TIntermAggregate *node) override;
 
   private:
-    ValidateMultiviewTraverser(GLenum shaderType, bool multiview2, TDiagnostics *diagnostics);
+    ValidateMultiviewTraverser(GLenum shaderType,
+                               const TSymbolTable &symbolTable,
+                               int shaderVersion,
+                               bool multiview2,
+                               TDiagnostics *diagnostics);
 
     static bool IsGLPosition(TIntermNode *node);
     static bool IsGLViewIDOVR(TIntermNode *node);
@@ -56,6 +62,8 @@ class ValidateMultiviewTraverser : public TIntermTraverser
     bool mValid;
     bool mMultiview2;
     GLenum mShaderType;
+    const TSymbolTable &mSymbolTable;
+    const int mShaderVersion;
 
     bool mInsideViewIDConditional;  // Only set if mMultiview2 is false.
     bool mInsideRestrictedAssignment;
@@ -67,22 +75,29 @@ class ValidateMultiviewTraverser : public TIntermTraverser
 
 bool ValidateMultiviewTraverser::validate(TIntermBlock *root,
                                           GLenum shaderType,
+                                          const TSymbolTable &symbolTable,
+                                          int shaderVersion,
                                           bool multiview2,
                                           TDiagnostics *diagnostics)
 {
-    ValidateMultiviewTraverser validate(shaderType, multiview2, diagnostics);
+    ValidateMultiviewTraverser validate(shaderType, symbolTable, shaderVersion, multiview2,
+                                        diagnostics);
     ASSERT(root);
     root->traverse(&validate);
     return validate.isValid();
 }
 
 ValidateMultiviewTraverser::ValidateMultiviewTraverser(GLenum shaderType,
+                                                       const TSymbolTable &symbolTable,
+                                                       int shaderVersion,
                                                        bool multiview2,
                                                        TDiagnostics *diagnostics)
     : TIntermTraverser(true, true, true),
       mValid(true),
       mMultiview2(multiview2),
       mShaderType(shaderType),
+      mSymbolTable(symbolTable),
+      mShaderVersion(shaderVersion),
       mInsideViewIDConditional(false),
       mInsideRestrictedAssignment(false),
       mGLPositionAllowed(multiview2),
@@ -361,17 +376,21 @@ bool ValidateMultiviewTraverser::visitAggregate(Visit visit, TIntermAggregate *n
                 mValid = false;
             }
         }
-        else if (node->getOp() == EOpModf)
+        else if (!node->isConstructor())
         {
-            // TODO(oetuaho@nvidia.com): It's quite hacky to hard-code modf - should maybe refactor
-            // out parameter detecting functionality in LValueTrackingTraverser so that it could be
-            // used here as well?
-            // LValueTrackingTraverser itself seems like a bad fit with the needs of this traverser.
-            mDiagnostics->error(node->getLine(),
-                                "Disallowed use of a function with an out parameter inside "
-                                "assignment to gl_Position.x when using OVR_multiview",
-                                GetOperatorString(node->getOp()));
-            mValid = false;
+            TFunction *builtInFunc = mSymbolTable.findBuiltInOp(node, mShaderVersion);
+            for (size_t paramIndex = 0u; paramIndex < builtInFunc->getParamCount(); ++paramIndex)
+            {
+                TQualifier qualifier = builtInFunc->getParam(paramIndex).type->getQualifier();
+                if (qualifier == EvqOut || qualifier == EvqInOut)
+                {
+                    mDiagnostics->error(node->getLine(),
+                                        "Disallowed use of a function with an out parameter inside "
+                                        "assignment to gl_Position.x when using OVR_multiview",
+                                        GetOperatorString(node->getOp()));
+                    mValid = false;
+                }
+            }
         }
     }
     return true;
@@ -381,6 +400,8 @@ bool ValidateMultiviewTraverser::visitAggregate(Visit visit, TIntermAggregate *n
 
 bool ValidateMultiviewWebGL(TIntermBlock *root,
                             GLenum shaderType,
+                            const TSymbolTable &symbolTable,
+                            int shaderVersion,
                             bool multiview2,
                             TDiagnostics *diagnostics)
 {
@@ -388,7 +409,8 @@ bool ValidateMultiviewWebGL(TIntermBlock *root,
     {
         return true;
     }
-    return ValidateMultiviewTraverser::validate(root, shaderType, multiview2, diagnostics);
+    return ValidateMultiviewTraverser::validate(root, shaderType, symbolTable, shaderVersion,
+                                                multiview2, diagnostics);
 }
 
 }  // namespace sh

src/compiler/translator/ValidateMultiviewWebGL.h

@@ -17,11 +17,14 @@ namespace sh
 {
 class TDiagnostics;
 class TIntermBlock;
+class TSymbolTable;
 
 // Check for errors and output error messages with diagnostics.
 // Returns true if there are no errors.
 bool ValidateMultiviewWebGL(TIntermBlock *root,
                             sh::GLenum shaderType,
+                            const TSymbolTable &symbolTable,
+                            int shaderVersion,
                             bool multiview2,
                             TDiagnostics *diagnostics);