Mark some internal functions as not having side effects

src/compiler/translator/EmulatePrecision.cpp

@@ -448,7 +448,10 @@ TIntermAggregate *createRoundingFunctionCallNode(TIntermTyped *roundedChild)
         roundFunctionName      = "angle_frl";
     TIntermSequence *arguments = new TIntermSequence();
     arguments->push_back(roundedChild);
-    return createInternalFunctionCallNode(roundedChild->getType(), roundFunctionName, arguments);
+    TIntermAggregate *callNode =
+        createInternalFunctionCallNode(roundedChild->getType(), roundFunctionName, arguments);
+    callNode->getFunctionSymbolInfo()->setKnownToNotHaveSideEffects(true);
+    return callNode;
 }
 
 TIntermAggregate *createCompoundAssignmentFunctionCallNode(TIntermTyped *left,

src/compiler/translator/IntermNode.cpp

@@ -462,6 +462,23 @@ TString TIntermAggregate::getSymbolTableMangledName() const
     }
 }
 
+bool TIntermAggregate::hasSideEffects() const
+{
+    if (isFunctionCall() && mFunctionInfo.isKnownToNotHaveSideEffects())
+    {
+        for (TIntermNode *arg : mArguments)
+        {
+            if (arg->getAsTyped()->hasSideEffects())
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+    // Conservatively assume most aggregate operators have side-effects
+    return true;
+}
+
 void TIntermBlock::appendStatement(TIntermNode *statement)
 {
     // Declaration nodes with no children can appear if all the declarators just added constants to
@@ -652,12 +669,13 @@ void TFunctionSymbolInfo::setFromFunction(const TFunction &function)
     setId(TSymbolUniqueId(function));
 }
 
-TFunctionSymbolInfo::TFunctionSymbolInfo(const TSymbolUniqueId &id) : mId(new TSymbolUniqueId(id))
+TFunctionSymbolInfo::TFunctionSymbolInfo(const TSymbolUniqueId &id)
+    : mId(new TSymbolUniqueId(id)), mKnownToNotHaveSideEffects(false)
 {
 }
 
 TFunctionSymbolInfo::TFunctionSymbolInfo(const TFunctionSymbolInfo &info)
-    : mName(info.mName), mId(nullptr)
+    : mName(info.mName), mId(nullptr), mKnownToNotHaveSideEffects(info.mKnownToNotHaveSideEffects)
 {
     if (info.mId)
     {

src/compiler/translator/IntermNode.h

@@ -148,6 +148,9 @@ class TIntermTyped : public TIntermNode
 
     TIntermTyped *getAsTyped() override { return this; }
 
+    // True if executing the expression represented by this node affects state, like values of
+    // variables. False if the executing the expression only computes its return value without
+    // affecting state. May return true conservatively.
     virtual bool hasSideEffects() const = 0;
 
     void setType(const TType &t) { mType = t; }
@@ -534,7 +537,7 @@ class TFunctionSymbolInfo
   public:
     POOL_ALLOCATOR_NEW_DELETE();
     TFunctionSymbolInfo(const TSymbolUniqueId &id);
-    TFunctionSymbolInfo() : mId(nullptr) {}
+    TFunctionSymbolInfo() : mId(nullptr), mKnownToNotHaveSideEffects(false) {}
 
     TFunctionSymbolInfo(const TFunctionSymbolInfo &info);
     TFunctionSymbolInfo &operator=(const TFunctionSymbolInfo &info);
@@ -548,12 +551,19 @@ class TFunctionSymbolInfo
     void setName(const TString &name) { mName.setString(name); }
     bool isMain() const { return mName.getString() == "main"; }
 
+    void setKnownToNotHaveSideEffects(bool knownToNotHaveSideEffects)
+    {
+        mKnownToNotHaveSideEffects = knownToNotHaveSideEffects;
+    }
+    bool isKnownToNotHaveSideEffects() const { return mKnownToNotHaveSideEffects; }
+
     void setId(const TSymbolUniqueId &functionId);
     const TSymbolUniqueId &getId() const;
 
   private:
     TName mName;
     TSymbolUniqueId *mId;
+    bool mKnownToNotHaveSideEffects;
 };
 
 typedef TVector<TIntermNode *> TIntermSequence;
@@ -617,8 +627,8 @@ class TIntermAggregate : public TIntermOperator, public TIntermAggregateBase
     void traverse(TIntermTraverser *it) override;
     bool replaceChildNode(TIntermNode *original, TIntermNode *replacement) override;
 
-    // Conservatively assume function calls and other aggregate operators have side-effects
-    bool hasSideEffects() const override { return true; }
+    bool hasSideEffects() const override;
+
     TIntermTyped *fold(TDiagnostics *diagnostics);
 
     TIntermSequence *getSequence() override { return &mArguments; }

src/compiler/translator/RemoveDynamicIndexing.cpp

@@ -355,6 +355,7 @@ TIntermAggregate *CreateIndexFunctionCall(TIntermBinary *node,
     TIntermAggregate *indexingCall = TIntermTraverser::CreateInternalFunctionCallNode(
         fieldType, functionName.c_str(), functionId, arguments);
     indexingCall->setLine(node->getLine());
+    indexingCall->getFunctionSymbolInfo()->setKnownToNotHaveSideEffects(true);
     return indexingCall;
 }
 

src/tests/angle_unittests.gypi

@@ -110,6 +110,7 @@
         # TODO(jmadill): should probably call this windows sources
         'angle_unittests_hlsl_sources':
         [
+            '<(angle_path)/src/tests/compiler_tests/HLSLOutput_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/UnrollFlatten_test.cpp',
         ],
     },

src/tests/compiler_tests/DebugShaderPrecision_test.cpp

@@ -1059,3 +1059,19 @@ TEST_F(DebugShaderPrecisionTest, CompoundAssignmentInsideExpression)
     compile(shaderString);
     ASSERT_TRUE(foundInAllGLSLCode("abs(angle_compound_add_frm(f, 1.0))"));
 }
+
+// Test that having rounded values inside the right hand side of logical or doesn't trigger asserts
+// in HLSL output.
+TEST_F(DebugShaderPrecisionTest, RoundedValueOnRightSideOfLogicalOr)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision mediump float;\n"
+        "out vec4 my_FragColor;\n"
+        "uniform float u1, u2;\n"
+        "void main() {\n"
+        "   my_FragColor = vec4(u1 == 0.0 || u2 == 0.0);\n"
+        "}\n";
+    compile(shaderString);
+    ASSERT_TRUE(foundInHLSLCode("angle_frm(_u2) == 0.0"));
+}

src/tests/compiler_tests/HLSLOutput_test.cpp

+//
+// Copyright (c) 2017 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.
+//
+// HLSLOutput_test.cpp:
+//   Tests for HLSL output.
+//
+
+#include "GLSLANG/ShaderLang.h"
+#include "angle_gl.h"
+#include "gtest/gtest.h"
+#include "tests/test_utils/compiler_test.h"
+
+using namespace sh;
+
+class HLSLOutputTest : public MatchOutputCodeTest
+{
+  public:
+    HLSLOutputTest() : MatchOutputCodeTest(GL_FRAGMENT_SHADER, 0, SH_HLSL_4_1_OUTPUT) {}
+};
+
+// Test that having dynamic indexing of a vector inside the right hand side of logical or doesn't
+// trigger asserts in HLSL output.
+TEST_F(HLSLOutputTest, DynamicIndexingOfVectorOnRightSideOfLogicalOr)
+{
+    const std::string &shaderString =
+        "#version 300 es\n"
+        "precision highp float;\n"
+        "out vec4 my_FragColor;\n"
+        "uniform int u1;\n"
+        "void main() {\n"
+        "   bvec4 v = bvec4(true, true, true, false);\n"
+        "   my_FragColor = vec4(v[u1 + 1] || v[u1]);\n"
+        "}\n";
+    compile(shaderString);
+}