Record precision of constant variables when needed

src/compiler.gypi

@@ -80,6 +80,8 @@
             'compiler/translator/Pragma.h',
             'compiler/translator/PruneEmptyDeclarations.cpp',
             'compiler/translator/PruneEmptyDeclarations.h',
+            'compiler/translator/RecordConstantPrecision.cpp',
+            'compiler/translator/RecordConstantPrecision.h',
             'compiler/translator/RegenerateStructNames.cpp',
             'compiler/translator/RegenerateStructNames.h',
             'compiler/translator/RemovePow.cpp',

src/compiler/translator/IntermNode.cpp

@@ -295,6 +295,7 @@ bool TIntermAggregate::insertChildNodes(TIntermSequence::size_type position, TIn
 
 void TIntermAggregate::setPrecisionFromChildren()
 {
+    mGotPrecisionFromChildren = true;
     if (getBasicType() == EbtBool)
     {
         mType.setPrecision(EbpUndefined);

src/compiler/translator/IntermNode.h

@@ -429,10 +429,16 @@ class TIntermAggregate : public TIntermOperator
     TIntermAggregate()
         : TIntermOperator(EOpNull),
           mUserDefined(false),
-          mUseEmulatedFunction(false) { }
+          mUseEmulatedFunction(false),
+          mGotPrecisionFromChildren(false)
+    {
+    }
     TIntermAggregate(TOperator op)
         : TIntermOperator(op),
-          mUseEmulatedFunction(false) { }
+          mUseEmulatedFunction(false),
+          mGotPrecisionFromChildren(false)
+    {
+    }
     ~TIntermAggregate() { }
 
     virtual TIntermAggregate *getAsAggregate() { return this; }
@@ -467,6 +473,9 @@ class TIntermAggregate : public TIntermOperator
     void setPrecisionFromChildren();
     void setBuiltInFunctionPrecision();
 
+    // Returns true if changing parameter precision may affect the return value.
+    bool gotPrecisionFromChildren() const { return mGotPrecisionFromChildren; }
+
   protected:
     TIntermAggregate(const TIntermAggregate &); // disallow copy constructor
     TIntermAggregate &operator=(const TIntermAggregate &); // disallow assignment operator
@@ -481,6 +490,8 @@ class TIntermAggregate : public TIntermOperator
     // If set to true, replace the built-in function call with an emulated one
     // to work around driver bugs.
     bool mUseEmulatedFunction;
+
+    bool mGotPrecisionFromChildren;
 };
 
 //
@@ -728,10 +739,14 @@ class TIntermTraverser : angle::NonCopyable
     // supported.
     void insertStatementsInParentBlock(const TIntermSequence &insertions);
 
+    // Helper to create a temporary symbol node with the given qualifier.
+    TIntermSymbol *createTempSymbol(const TType &type, TQualifier qualifier);
     // Helper to create a temporary symbol node.
     TIntermSymbol *createTempSymbol(const TType &type);
     // Create a node that declares but doesn't initialize a temporary symbol.
     TIntermAggregate *createTempDeclaration(const TType &type);
+    // Create a node that initializes the current temporary symbol with initializer having the given qualifier.
+    TIntermAggregate *createTempInitDeclaration(TIntermTyped *initializer, TQualifier qualifier);
     // Create a node that initializes the current temporary symbol with initializer.
     TIntermAggregate *createTempInitDeclaration(TIntermTyped *initializer);
     // Create a node that assigns rightNode to the current temporary symbol.

src/compiler/translator/IntermTraverse.cpp

@@ -30,7 +30,7 @@ void TIntermTraverser::insertStatementsInParentBlock(const TIntermSequence &inse
     mInsertions.push_back(insert);
 }
 
-TIntermSymbol *TIntermTraverser::createTempSymbol(const TType &type)
+TIntermSymbol *TIntermTraverser::createTempSymbol(const TType &type, TQualifier qualifier)
 {
     // Each traversal uses at most one temporary variable, so the index stays the same within a single traversal.
     TInfoSinkBase symbolNameOut;
@@ -40,10 +40,15 @@ TIntermSymbol *TIntermTraverser::createTempSymbol(const TType &type)
 
     TIntermSymbol *node = new TIntermSymbol(0, symbolName, type);
     node->setInternal(true);
-    node->getTypePointer()->setQualifier(EvqTemporary);
+    node->getTypePointer()->setQualifier(qualifier);
     return node;
 }
 
+TIntermSymbol *TIntermTraverser::createTempSymbol(const TType &type)
+{
+    return createTempSymbol(type, EvqTemporary);
+}
+
 TIntermAggregate *TIntermTraverser::createTempDeclaration(const TType &type)
 {
     TIntermAggregate *tempDeclaration = new TIntermAggregate(EOpDeclaration);
@@ -51,10 +56,10 @@ TIntermAggregate *TIntermTraverser::createTempDeclaration(const TType &type)
     return tempDeclaration;
 }
 
-TIntermAggregate *TIntermTraverser::createTempInitDeclaration(TIntermTyped *initializer)
+TIntermAggregate *TIntermTraverser::createTempInitDeclaration(TIntermTyped *initializer, TQualifier qualifier)
 {
     ASSERT(initializer != nullptr);
-    TIntermSymbol *tempSymbol = createTempSymbol(initializer->getType());
+    TIntermSymbol *tempSymbol = createTempSymbol(initializer->getType(), qualifier);
     TIntermAggregate *tempDeclaration = new TIntermAggregate(EOpDeclaration);
     TIntermBinary *tempInit = new TIntermBinary(EOpInitialize);
     tempInit->setLeft(tempSymbol);
@@ -64,6 +69,11 @@ TIntermAggregate *TIntermTraverser::createTempInitDeclaration(TIntermTyped *init
     return tempDeclaration;
 }
 
+TIntermAggregate *TIntermTraverser::createTempInitDeclaration(TIntermTyped *initializer)
+{
+    return createTempInitDeclaration(initializer, EvqTemporary);
+}
+
 TIntermBinary *TIntermTraverser::createTempAssignment(TIntermTyped *rightNode)
 {
     ASSERT(rightNode != nullptr);

src/compiler/translator/RecordConstantPrecision.cpp

+//
+// Copyright (c) 2002-2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// During parsing, all constant expressions are folded to constant union nodes. The expressions that have been
+// folded may have had precision qualifiers, which should affect the precision of the consuming operation.
+// If the folded constant union nodes are written to output as such they won't have any precision qualifiers,
+// and their effect on the precision of the consuming operation is lost.
+//
+// RecordConstantPrecision is an AST traverser that inspects the precision qualifiers of constants and hoists
+// the constants outside the containing expression as precision qualified named variables in case that is
+// required for correct precision propagation.
+//
+
+#include "compiler/translator/RecordConstantPrecision.h"
+
+#include "compiler/translator/InfoSink.h"
+#include "compiler/translator/IntermNode.h"
+
+namespace
+{
+
+class RecordConstantPrecisionTraverser : public TIntermTraverser
+{
+  public:
+    RecordConstantPrecisionTraverser();
+
+    void visitConstantUnion(TIntermConstantUnion *node) override;
+
+    void nextIteration();
+
+    bool foundHigherPrecisionConstant() const { return mFoundHigherPrecisionConstant; }
+  protected:
+    bool operandAffectsParentOperationPrecision(TIntermTyped *operand);
+
+    bool mFoundHigherPrecisionConstant;
+};
+
+RecordConstantPrecisionTraverser::RecordConstantPrecisionTraverser()
+    : TIntermTraverser(true, false, true),
+      mFoundHigherPrecisionConstant(false)
+{
+}
+
+bool RecordConstantPrecisionTraverser::operandAffectsParentOperationPrecision(TIntermTyped *operand)
+{
+    const TIntermBinary *parentAsBinary = getParentNode()->getAsBinaryNode();
+    if (parentAsBinary != nullptr)
+    {
+        // If the constant is assigned or is used to initialize a variable, or if it's an index,
+        // its precision has no effect.
+        switch (parentAsBinary->getOp())
+        {
+          case EOpInitialize:
+          case EOpAssign:
+          case EOpIndexDirect:
+          case EOpIndexDirectStruct:
+          case EOpIndexDirectInterfaceBlock:
+          case EOpIndexIndirect:
+            return false;
+          default:
+            break;
+        }
+
+        TIntermTyped *otherOperand = parentAsBinary->getRight();
+        if (otherOperand == operand)
+        {
+            otherOperand = parentAsBinary->getLeft();
+        }
+        // If the precision of the other child is at least as high as the precision of the constant, the precision of
+        // the constant has no effect.
+        if (otherOperand->getAsConstantUnion() == nullptr && otherOperand->getPrecision() >= operand->getPrecision())
+        {
+            return false;
+        }
+    }
+
+    TIntermAggregate *parentAsAggregate = getParentNode()->getAsAggregate();
+    if (parentAsAggregate != nullptr)
+    {
+        if (!parentAsAggregate->gotPrecisionFromChildren())
+        {
+            // This can be either:
+            // * a call to an user-defined function
+            // * a call to a texture function
+            // * some other kind of aggregate
+            // In any of these cases the constant precision has no effect.
+            return false;
+        }
+        if (parentAsAggregate->isConstructor() && parentAsAggregate->getBasicType() == EbtBool)
+        {
+            return false;
+        }
+        // If the precision of operands does affect the result, but the precision of any of the other children
+        // has a precision that's at least as high as the precision of the constant, the precision of the constant
+        // has no effect.
+        TIntermSequence *parameters = parentAsAggregate->getSequence();
+        for (TIntermNode *parameter : *parameters)
+        {
+            const TIntermTyped *typedParameter = parameter->getAsTyped();
+            if (parameter != operand && typedParameter != nullptr && parameter->getAsConstantUnion() == nullptr &&
+                typedParameter->getPrecision() >= operand->getPrecision())
+            {
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
+void RecordConstantPrecisionTraverser::visitConstantUnion(TIntermConstantUnion *node)
+{
+    if (mFoundHigherPrecisionConstant)
+        return;
+
+    // If the constant has lowp or undefined precision, it can't increase the precision of consuming operations.
+    if (node->getPrecision() < EbpMedium)
+        return;
+
+    // It's possible the node has no effect on the precision of the consuming expression, depending on the
+    // consuming expression, and the precision of the other parameters of the expression.
+    if (!operandAffectsParentOperationPrecision(node))
+        return;
+
+    // Make the constant a precision-qualified named variable to make sure it affects the precision of the consuming
+    // expression.
+    TIntermSequence insertions;
+    insertions.push_back(createTempInitDeclaration(node, EvqConst));
+    insertStatementsInParentBlock(insertions);
+    mReplacements.push_back(NodeUpdateEntry(getParentNode(), node, createTempSymbol(node->getType()), false));
+    mFoundHigherPrecisionConstant = true;
+}
+
+void RecordConstantPrecisionTraverser::nextIteration()
+{
+    nextTemporaryIndex();
+    mFoundHigherPrecisionConstant = false;
+}
+
+} // namespace
+
+void RecordConstantPrecision(TIntermNode *root, unsigned int *temporaryIndex)
+{
+    RecordConstantPrecisionTraverser traverser;
+    ASSERT(temporaryIndex != nullptr);
+    traverser.useTemporaryIndex(temporaryIndex);
+    // Iterate as necessary, and reset the traverser between iterations.
+    do
+    {
+        traverser.nextIteration();
+        root->traverse(&traverser);
+        if (traverser.foundHigherPrecisionConstant())
+            traverser.updateTree();
+    }
+    while (traverser.foundHigherPrecisionConstant());
+}

src/compiler/translator/RecordConstantPrecision.h

+//
+// Copyright (c) 2002-2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// During parsing, all constant expressions are folded to constant union nodes. The expressions that have been
+// folded may have had precision qualifiers, which should affect the precision of the consuming operation.
+// If the folded constant union nodes are written to output as such they won't have any precision qualifiers,
+// and their effect on the precision of the consuming operation is lost.
+//
+// RecordConstantPrecision is an AST traverser that inspects the precision qualifiers of constants and hoists
+// the constants outside the containing expression as precision qualified named variables in case that is
+// required for correct precision propagation.
+//
+
+#ifndef COMPILER_TRANSLATOR_RECORDCONSTANTPRECISION_H_
+#define COMPILER_TRANSLATOR_RECORDCONSTANTPRECISION_H_
+
+class TIntermNode;
+
+void RecordConstantPrecision(TIntermNode *root, unsigned int *temporaryIndex);
+
+#endif   // COMPILER_TRANSLATOR_RECORDCONSTANTPRECISION_H_

src/compiler/translator/TranslatorESSL.cpp

@@ -8,6 +8,7 @@
 
 #include "compiler/translator/BuiltInFunctionEmulatorGLSL.h"
 #include "compiler/translator/EmulatePrecision.h"
+#include "compiler/translator/RecordConstantPrecision.h"
 #include "compiler/translator/OutputESSL.h"
 #include "angle_gl.h"
 
@@ -46,6 +47,10 @@ void TranslatorESSL::translate(TIntermNode *root, int) {
         emulatePrecision.writeEmulationHelpers(sink, SH_ESSL_OUTPUT);
     }
 
+    unsigned int temporaryIndex = 0;
+
+    RecordConstantPrecision(root, &temporaryIndex);
+
     // Write emulated built-in functions if needed.
     if (!getBuiltInFunctionEmulator().IsOutputEmpty())
     {

src/tests/angle_unittests.gypi

@@ -38,6 +38,7 @@
             '<(angle_path)/src/tests/compiler_tests/MalformedShader_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/NV_draw_buffers_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/PruneUnusedFunctions_test.cpp',
+            '<(angle_path)/src/tests/compiler_tests/RecordConstantPrecision_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/RemovePow_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderExtension_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderVariable_test.cpp',

src/tests/compiler_tests/RecordConstantPrecision_test.cpp

+//
+// Copyright (c) 2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// RecordConstantPrecision_test.cpp:
+//   Test for recording constant variable precision when it affects consuming expression.
+//
+
+#include "angle_gl.h"
+#include "gtest/gtest.h"
+#include "GLSLANG/ShaderLang.h"
+#include "tests/test_utils/compiler_test.h"
+
+class RecordConstantPrecisionTest : public testing::Test
+{
+  public:
+    RecordConstantPrecisionTest() {}
+
+  protected:
+    void compile(const std::string &shaderString)
+    {
+        std::string infoLog;
+        bool compilationSuccess = compileTestShader(GL_FRAGMENT_SHADER, SH_GLES2_SPEC, SH_ESSL_OUTPUT,
+                                                    shaderString, &mTranslatedCode, &infoLog);
+        if (!compilationSuccess)
+        {
+            FAIL() << "Shader compilation into ESSL failed " << infoLog;
+        }
+    }
+
+    bool foundInCode(const char *stringToFind)
+    {
+        return mTranslatedCode.find(stringToFind) != std::string::npos;
+    }
+  private:
+    std::string mTranslatedCode;
+};
+
+// The constant cannot be folded if its precision is higher than the other operands, since it
+// increases the precision of the consuming expression.
+TEST_F(RecordConstantPrecisionTest, HigherPrecisionConstantAsParameter)
+{
+    const std::string &shaderString =
+        "uniform mediump float u;"
+        "void main()\n"
+        "{\n"
+        "    const highp float a = 4096.5;\n"
+        "    mediump float b = fract(a + u);\n"
+        "    gl_FragColor = vec4(b);\n"
+        "}\n";
+    compile(shaderString);
+    ASSERT_TRUE(foundInCode("const highp float s"));
+    ASSERT_FALSE(foundInCode("fract(4096.5"));
+    ASSERT_FALSE(foundInCode("fract((4096.5"));
+}
+
+// The constant can be folded if its precision is equal to the other operands, as it does not
+// increase the precision of the consuming expression.
+TEST_F(RecordConstantPrecisionTest, EqualPrecisionConstantAsParameter)
+{
+    const std::string &shaderString =
+        "uniform mediump float u;"
+        "void main()\n"
+        "{\n"
+        "    const mediump float a = 4096.5;\n"
+        "    mediump float b = fract(a + u);\n"
+        "    gl_FragColor = vec4(b);\n"
+        "}\n";
+    compile(shaderString);
+    ASSERT_FALSE(foundInCode("const mediump float s"));
+    ASSERT_TRUE(foundInCode("fract((4096.5"));
+}
+
+// The constant cannot be folded if its precision is higher than the other operands, since it
+// increases the precision of the consuming expression. This applies also when the constant is
+// part of a constant expression that can be folded.
+TEST_F(RecordConstantPrecisionTest, FoldedBinaryConstantPrecisionIsHigher)
+{
+    const std::string &shaderString =
+        "uniform mediump float u;"
+        "void main()\n"
+        "{\n"
+        "    const highp float a = 4095.5;\n"
+        "    mediump float b = fract((a + 1.0) + u);\n"
+        "    gl_FragColor = vec4(b);\n"
+        "}\n";
+    compile(shaderString);
+    ASSERT_TRUE(foundInCode("const highp float s"));
+    ASSERT_FALSE(foundInCode("fract(4096.5"));
+    ASSERT_FALSE(foundInCode("fract((4096.5"));
+}
+
+
+// The constant cannot be folded if its precision is higher than the other operands, since it
+// increases the precision of the consuming expression. This applies also when the constant is
+// part of a constant expression that can be folded.
+TEST_F(RecordConstantPrecisionTest, FoldedUnaryConstantPrecisionIsHigher)
+{
+    const std::string &shaderString =
+        "uniform mediump float u;"
+        "void main()\n"
+        "{\n"
+        "    const highp float a = 0.5;\n"
+        "    mediump float b = sin(fract(a) + u);\n"
+        "    gl_FragColor = vec4(b);\n"
+        "}\n";
+    compile(shaderString);
+    ASSERT_TRUE(foundInCode("const highp float s"));
+    ASSERT_FALSE(foundInCode("sin(0.5"));
+    ASSERT_FALSE(foundInCode("sin((0.5"));
+}

src/tests/compiler_tests/compiler_tests.gypi

@@ -14,6 +14,7 @@
         'MalformedShader_test.cpp',
         'NV_draw_buffers_test.cpp',
         'PruneUnusedFunctions_test.cpp',
+        'RecordConstantPrecision_test.cpp',
         'RemovePow_test.cpp',
         'ShaderExtension_test.cpp',
         'ShaderVariable_test.cpp',