Detect when built-in modf requires an l-value in AST traversal

src/compiler/translator/EmulatePrecision.cpp

@@ -291,8 +291,9 @@ bool parentUsesResult(TIntermNode* parent, TIntermNode* node)
 
 }  // namespace anonymous
 
-EmulatePrecision::EmulatePrecision()
-    : TLValueTrackingTraverser(true, true, true), mDeclaringVariables(false)
+EmulatePrecision::EmulatePrecision(const TSymbolTable &symbolTable, int shaderVersion)
+    : TLValueTrackingTraverser(true, true, true, symbolTable, shaderVersion),
+      mDeclaringVariables(false)
 {}
 
 void EmulatePrecision::visitSymbol(TIntermSymbol *node)

src/compiler/translator/EmulatePrecision.h

@@ -21,7 +21,7 @@
 class EmulatePrecision : public TLValueTrackingTraverser
 {
   public:
-    EmulatePrecision();
+    EmulatePrecision(const TSymbolTable &symbolTable, int shaderVersion);
 
     virtual void visitSymbol(TIntermSymbol *node);
     virtual bool visitBinary(Visit visit, TIntermBinary *node);

src/compiler/translator/IntermNode.h

@@ -42,6 +42,8 @@ class TInfoSink;
 class TInfoSinkBase;
 class TIntermRaw;
 
+class TSymbolTable;
+
 //
 // Base class for the tree nodes
 //
@@ -797,10 +799,16 @@ class TIntermTraverser : angle::NonCopyable
 class TLValueTrackingTraverser : public TIntermTraverser
 {
   public:
-    TLValueTrackingTraverser(bool preVisit, bool inVisit, bool postVisit)
+    TLValueTrackingTraverser(bool preVisit,
+                             bool inVisit,
+                             bool postVisit,
+                             const TSymbolTable &symbolTable,
+                             int shaderVersion)
         : TIntermTraverser(preVisit, inVisit, postVisit),
           mOperatorRequiresLValue(false),
-          mInFunctionCallOutParameter(false)
+          mInFunctionCallOutParameter(false),
+          mSymbolTable(symbolTable),
+          mShaderVersion(shaderVersion)
     {
     }
     virtual ~TLValueTrackingTraverser() {}
@@ -849,6 +857,9 @@ class TLValueTrackingTraverser : public TIntermTraverser
 
     // Map from mangled function names to their parameter sequences
     TMap<TString, TIntermSequence *, TStringComparator> mFunctionMap;
+
+    const TSymbolTable &mSymbolTable;
+    const int mShaderVersion;
 };
 
 //

src/compiler/translator/IntermTraverse.cpp

@@ -6,6 +6,7 @@
 
 #include "compiler/translator/IntermNode.h"
 #include "compiler/translator/InfoSink.h"
+#include "compiler/translator/SymbolTable.h"
 
 void TIntermSymbol::traverse(TIntermTraverser *it)
 {
@@ -473,9 +474,41 @@ void TLValueTrackingTraverser::traverseAggregate(TIntermAggregate *node)
             if (node->getOp() == EOpSequence)
                 pushParentBlock(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())
+            {
+                // 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());
+                }
+            }
+
+            size_t paramIndex = 0;
+
             for (auto *child : *sequence)
             {
+                TQualifier qualifier = EvqIn;
+                if (builtInFunc != nullptr)
+                    qualifier = builtInFunc->getParam(paramIndex).type->getQualifier();
+                setInFunctionCallOutParameter(qualifier == EvqOut || qualifier == EvqInOut);
                 child->traverse(this);
+
                 if (visit && inVisit)
                 {
                     if (child != sequence->back())
@@ -484,8 +517,12 @@ void TLValueTrackingTraverser::traverseAggregate(TIntermAggregate *node)
 
                 if (node->getOp() == EOpSequence)
                     incrementParentBlockPos();
+
+                ++paramIndex;
             }
 
+            setInFunctionCallOutParameter(false);
+
             if (node->getOp() == EOpSequence)
                 popParentBlock();
         }

src/compiler/translator/TranslatorESSL.cpp

@@ -43,7 +43,7 @@ void TranslatorESSL::translate(TIntermNode *root, int) {
 
     if (precisionEmulation)
     {
-        EmulatePrecision emulatePrecision;
+        EmulatePrecision emulatePrecision(getSymbolTable(), shaderVer);
         root->traverse(&emulatePrecision);
         emulatePrecision.updateTree();
         emulatePrecision.writeEmulationHelpers(sink, SH_ESSL_OUTPUT);

src/compiler/translator/TranslatorGLSL.cpp

@@ -44,7 +44,7 @@ void TranslatorGLSL::translate(TIntermNode *root, int) {
 
     if (precisionEmulation)
     {
-        EmulatePrecision emulatePrecision;
+        EmulatePrecision emulatePrecision(getSymbolTable(), getShaderVersion());
         root->traverse(&emulatePrecision);
         emulatePrecision.updateTree();
         emulatePrecision.writeEmulationHelpers(sink, getOutputType());

src/tests/compiler_tests/DebugShaderPrecision_test.cpp

@@ -61,8 +61,8 @@ class DebugShaderPrecisionTest : public testing::Test
         ShBuiltInResources resources;
         ShInitBuiltInResources(&resources);
         resources.WEBGL_debug_shader_precision = 1;
-        return compileTestShader(GL_FRAGMENT_SHADER, SH_GLES2_SPEC, output, shaderString, &resources,
-                                 translatedCode, infoLog);
+        return compileTestShader(GL_FRAGMENT_SHADER, SH_GLES3_SPEC, output, shaderString,
+                                 &resources, translatedCode, infoLog);
     }
 
     std::string mESSLCode;
@@ -803,3 +803,20 @@ TEST_F(DebugShaderPrecisionTest, OpInIndexOfLValue)
     compile(shaderString);
     ASSERT_TRUE(foundInCode("angle_frm(exp2(angle_frm(u2)))"));
 }
+
+// Test that the out parameter of modf doesn't get rounded
+TEST_F(DebugShaderPrecisionTest, ModfOutParameter)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "uniform float u;\n"
+        "out vec4 my_FragColor;\n"
+        "void main() {\n"
+        "   float o;\n"
+        "   float f = modf(u, o);\n"
+        "   my_FragColor = vec4(f, o, 0, 1);\n"
+        "}\n";
+    compile(shaderString);
+    ASSERT_TRUE(foundInCode("modf(angle_frm(u), o)"));
+}