commit a22aa4eda1d8b164887a22151da3d58f8f8943c7
author Olli Etuaho <oetuaho@nvidia.com> Wed May 24 18:17:23 2017 +0300
committer Commit Bot <commit-bot@chromium.org> Fri May 26 15:56:54 2017 +0000
tree e05e9bc1ae26908fbbbfd3c38aaa7277f7bf52a5
parent ff77c359221485886b8207e8afd23c11e0768cf4

Mark some internal functions as not having side effects

Precision emulation rounding function calls and vector/matrix dynamic
indexing function calls now get a flag that indicates that running the
function body does not have side effects. This avoids triggering
asserts in OutputHLSL when these internal function calls end up on the
right hand side of a non-unfolded logical operator.

BUG=chromium:724870
TEST=angle_unittests

Change-Id: Id1a2b6b744f6a04c6cdb86a8f4109ccc12bc70b9
Reviewed-on: https://chromium-review.googlesource.com/516705
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

src/compiler/translator/EmulatePrecision.cpp

diff --git a/src/compiler/translator/EmulatePrecision.cpp b/src/compiler/translator/EmulatePrecision.cpp
index 4662929..9622569 100644
--- a/src/compiler/translator/EmulatePrecision.cpp
+++ b/src/compiler/translator/EmulatePrecision.cpp
@@ -448,7 +448,10 @@
         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

diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 3f4c6c1..67bdcd6 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -462,6 +462,23 @@
     }
 }
 
+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 @@
     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

diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 65c9bdc..2048022 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -148,6 +148,9 @@
 
     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 @@
   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 @@
     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 @@
     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

diff --git a/src/compiler/translator/RemoveDynamicIndexing.cpp b/src/compiler/translator/RemoveDynamicIndexing.cpp
index 5d136a3..32fe444 100644
--- a/src/compiler/translator/RemoveDynamicIndexing.cpp
+++ b/src/compiler/translator/RemoveDynamicIndexing.cpp
@@ -355,6 +355,7 @@
     TIntermAggregate *indexingCall = TIntermTraverser::CreateInternalFunctionCallNode(
         fieldType, functionName.c_str(), functionId, arguments);
     indexingCall->setLine(node->getLine());
+    indexingCall->getFunctionSymbolInfo()->setKnownToNotHaveSideEffects(true);
     return indexingCall;
 }