commit a60e0805721f62c28a55faf2df74472cc5fc91fc
author Jamie Madill <jmadill@chromium.org> Thu Nov 07 16:03:40 2013
committer Jamie Madill <jmadill@chromium.org> Thu Nov 07 16:03:40 2013
tree 4b3b922f189efdabe827df2868d4ef49ebb63483
parent d1ffb561a2773f9675641abd5e7d408ae93c5846

Fix issues with the conditional discard workarounds to do with assignments.

The old modifiesState method really checked if an operator was an assignment,
so restored that behaviour and use the new side effects detection only for
the new code.

ANGLEBUG=486
BUG=
R=nicolascapens@chromium.org, zmo@chromium.org

Review URL: https://codereview.appspot.com/22130043

src/compiler/Intermediate.cpp

diff --git a/src/compiler/Intermediate.cpp b/src/compiler/Intermediate.cpp
index b3cb3dd..ef85821 100644
--- a/src/compiler/Intermediate.cpp
+++ b/src/compiler/Intermediate.cpp
@@ -804,9 +804,7 @@
 //
 // Say whether or not an operation node changes the value of a variable.
 //
-// Returns true if state is modified.
-//
-bool TIntermOperator::hasSideEffects() const
+bool TIntermOperator::isAssignment() const
 {
     switch (op) {
         case EOpPostIncrement:

src/compiler/ValidateLimitations.cpp

diff --git a/src/compiler/ValidateLimitations.cpp b/src/compiler/ValidateLimitations.cpp
index 3f3260b..64969c4 100644
--- a/src/compiler/ValidateLimitations.cpp
+++ b/src/compiler/ValidateLimitations.cpp
@@ -457,7 +457,7 @@
 bool ValidateLimitations::validateOperation(TIntermOperator* node,
                                             TIntermNode* operand) {
     // Check if loop index is modified in the loop body.
-    if (!withinLoopBody() || !node->hasSideEffects())
+    if (!withinLoopBody() || !node->isAssignment())
         return true;
 
     const TIntermSymbol* symbol = operand->getAsSymbolNode();

src/compiler/depgraph/DependencyGraphBuilder.cpp

diff --git a/src/compiler/depgraph/DependencyGraphBuilder.cpp b/src/compiler/depgraph/DependencyGraphBuilder.cpp
index 069e963..026e6d5 100644
--- a/src/compiler/depgraph/DependencyGraphBuilder.cpp
+++ b/src/compiler/depgraph/DependencyGraphBuilder.cpp
@@ -94,7 +94,7 @@
 bool TDependencyGraphBuilder::visitBinary(Visit visit, TIntermBinary* intermBinary)
 {
     TOperator op = intermBinary->getOp();
-    if (op == EOpInitialize || intermBinary->hasSideEffects())
+    if (op == EOpInitialize || intermBinary->isAssignment())
         visitAssignment(intermBinary);
     else if (op == EOpLogicalAnd || op == EOpLogicalOr)
         visitLogicalOp(intermBinary);

src/compiler/intermediate.h

diff --git a/src/compiler/intermediate.h b/src/compiler/intermediate.h
index 4ddfdab..14e39fd 100644
--- a/src/compiler/intermediate.h
+++ b/src/compiler/intermediate.h
@@ -406,9 +406,11 @@
     TOperator getOp() const { return op; }
     void setOp(TOperator o) { op = o; }
 
-    virtual bool hasSideEffects() const;
+    bool isAssignment() const;
     bool isConstructor() const;
 
+    virtual bool hasSideEffects() const { return isAssignment(); }
+
 protected:
     TIntermOperator(TOperator o) : TIntermTyped(TType(EbtFloat, EbpUndefined)), op(o) {}
     TIntermOperator(TOperator o, TType& t) : TIntermTyped(t), op(o) {}   
@@ -427,7 +429,7 @@
     virtual bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
-    virtual bool hasSideEffects() const { return (TIntermOperator::hasSideEffects() || left->hasSideEffects() || right->hasSideEffects()); }
+    virtual bool hasSideEffects() const { return (isAssignment() || left->hasSideEffects() || right->hasSideEffects()); }
 
     void setLeft(TIntermTyped* n) { left = n; }
     void setRight(TIntermTyped* n) { right = n; }
@@ -459,7 +461,7 @@
     virtual bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
-    virtual bool hasSideEffects() const { return (TIntermOperator::hasSideEffects() || operand->hasSideEffects()); }
+    virtual bool hasSideEffects() const { return (isAssignment() || operand->hasSideEffects()); }
 
     void setOperand(TIntermTyped* o) { operand = o; }
     TIntermTyped* getOperand() { return operand; }