Revert "Added constant propagation and better variable liveness tracking to"

This reverts commit f54b07121f81a56145fb118a2e18841fc135717d.

Reason for revert: ASAN failure

Original change's description:
> Added constant propagation and better variable liveness tracking to
> skslc. 
> 
> This allows skslc to track the values of variables with constant
> values across multiple statements and replace variable references with
> constant values where appropriate.
> 
> The improved liveness tracking allows skslc to realize that a
> variable is no longer alive if all references to it have been
> replaced. It is not yet doing much with this information; better
> dead code elimination is coming in a followup change.
> 
> BUG=skia:
> 
> Change-Id: I6bf267d478b769caf0063ac3597dc16bbe618cb4
> Reviewed-on: https://skia-review.googlesource.com/7033
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> 

TBR=egdaniel@google.com,ethannicholas@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Change-Id: Id2e26bce96b27df73948f8b32d3dff2e358ae0d6
Reviewed-on: https://skia-review.googlesource.com/7274
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

22 files changed, 39 insertions, 160 deletions

diff --git a/src/sksl/ir/SkSLBinaryExpression.h b/src/sksl/ir/SkSLBinaryExpression.h
index de85e4812b..132513e7f7 100644
--- a/src/sksl/ir/SkSLBinaryExpression.h
+++ b/src/sksl/ir/SkSLBinaryExpression.h
@@ -4,19 +4,17 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
-
+ 
 #ifndef SKSL_BINARYEXPRESSION
 #define SKSL_BINARYEXPRESSION
 
 #include "SkSLExpression.h"
-#include "SkSLExpression.h"
-#include "../SkSLIRGenerator.h"
 #include "../SkSLToken.h"
 
 namespace SkSL {
 
 /**
- * A binary operation.
+ * A binary operation. 
  */
 struct BinaryExpression : public Expression {
     BinaryExpression(Position position, std::unique_ptr<Expression> left, Token::Kind op,
@@ -26,22 +24,14 @@ struct BinaryExpression : public Expression {
     , fOperator(op)
     , fRight(std::move(right)) {}
 
-    virtual std::unique_ptr<Expression> constantPropagate(
-                                                        const IRGenerator& irGenerator,
-                                                        const DefinitionMap& definitions) override {
-        return irGenerator.constantFold(*fLeft,
-                                        fOperator,
-                                        *fRight);
-    }
-
     virtual SkString description() const override {
         return "(" + fLeft->description() + " " + Token::OperatorName(fOperator) + " " +
                fRight->description() + ")";
     }
 
-    std::unique_ptr<Expression> fLeft;
+    const std::unique_ptr<Expression> fLeft;
     const Token::Kind fOperator;
-    std::unique_ptr<Expression> fRight;
+    const std::unique_ptr<Expression> fRight;
 
     typedef Expression INHERITED;
 };
diff --git a/src/sksl/ir/SkSLBlock.h b/src/sksl/ir/SkSLBlock.h
index 17970fd561..f975d160a0 100644
--- a/src/sksl/ir/SkSLBlock.h
+++ b/src/sksl/ir/SkSLBlock.h
@@ -20,8 +20,8 @@ struct Block : public Statement {
     Block(Position position, std::vector<std::unique_ptr<Statement>> statements,
           const std::shared_ptr<SymbolTable> symbols)
     : INHERITED(position, kBlock_Kind)
-    , fSymbols(std::move(symbols))
-    , fStatements(std::move(statements)) {}
+    , fStatements(std::move(statements))
+    , fSymbols(std::move(symbols)) {}
 
     SkString description() const override {
         SkString result("{");
@@ -33,10 +33,8 @@ struct Block : public Statement {
         return result;
     }
 
-    // it's important to keep fStatements defined after (and thus destroyed before) fSymbols,
-    // because destroying statements can modify reference counts in symbols
-    const std::shared_ptr<SymbolTable> fSymbols;
     const std::vector<std::unique_ptr<Statement>> fStatements;
+    const std::shared_ptr<SymbolTable> fSymbols;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLConstructor.h b/src/sksl/ir/SkSLConstructor.h
index 691bea123a..63c692b88e 100644
--- a/src/sksl/ir/SkSLConstructor.h
+++ b/src/sksl/ir/SkSLConstructor.h
@@ -9,9 +9,6 @@
 #define SKSL_CONSTRUCTOR
 
 #include "SkSLExpression.h"
-#include "SkSLFloatLiteral.h"
-#include "SkSLIntLiteral.h"
-#include "SkSLIRGenerator.h"
 
 namespace SkSL {
 
@@ -24,20 +21,6 @@ struct Constructor : public Expression {
     : INHERITED(position, kConstructor_Kind, type)
     , fArguments(std::move(arguments)) {}
 
-    virtual std::unique_ptr<Expression> constantPropagate(
-                                                        const IRGenerator& irGenerator,
-                                                        const DefinitionMap& definitions) override {
-        if (fArguments.size() == 1 && fArguments[0]->fKind == Expression::kIntLiteral_Kind &&
-            // promote float(1) to 1.0
-            fType == *irGenerator.fContext.fFloat_Type) {
-            int64_t intValue = ((IntLiteral&) *fArguments[0]).fValue;
-            return std::unique_ptr<Expression>(new FloatLiteral(irGenerator.fContext,
-                                                                fPosition,
-                                                                intValue));
-        }
-        return nullptr;
-    }
-
     SkString description() const override {
         SkString result = fType.description() + "(";
         SkString separator;
@@ -59,7 +42,7 @@ struct Constructor : public Expression {
         return true;
     }
 
-    std::vector<std::unique_ptr<Expression>> fArguments;
+    const std::vector<std::unique_ptr<Expression>> fArguments;
 
     typedef Expression INHERITED;
 };
diff --git a/src/sksl/ir/SkSLDoStatement.h b/src/sksl/ir/SkSLDoStatement.h
index e26d3dc974..78c0a1b768 100644
--- a/src/sksl/ir/SkSLDoStatement.h
+++ b/src/sksl/ir/SkSLDoStatement.h
@@ -28,7 +28,7 @@ struct DoStatement : public Statement {
     }
 
     const std::unique_ptr<Statement> fStatement;
-    std::unique_ptr<Expression> fTest;
+    const std::unique_ptr<Expression> fTest;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLExpression.h b/src/sksl/ir/SkSLExpression.h
index f87d810fc0..b4ed37c09a 100644
--- a/src/sksl/ir/SkSLExpression.h
+++ b/src/sksl/ir/SkSLExpression.h
@@ -4,24 +4,17 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
-
+ 
 #ifndef SKSL_EXPRESSION
 #define SKSL_EXPRESSION
 
+#include "SkSLIRNode.h"
 #include "SkSLType.h"
-#include "SkSLVariable.h"
-
-#include <unordered_map>
 
 namespace SkSL {
 
-struct Expression;
-class IRGenerator;
-
-typedef std::unordered_map<const Variable*, std::unique_ptr<Expression>*> DefinitionMap;
-
 /**
- * Abstract supertype of all expressions.
+ * Abstract supertype of all expressions. 
  */
 struct Expression : public IRNode {
     enum Kind {
@@ -52,18 +45,6 @@ struct Expression : public IRNode {
         return false;
     }
 
-    /**
-     * Given a map of known constant variable values, substitute them in for references to those
-     * variables occurring in this expression and its subexpressions.  Similar simplifications, such
-     * as folding a constant binary expression down to a single value, may also be performed.
-     * Returns a new expression which replaces this expression, or null if no replacements were
-     * made. If a new expression is returned, this expression is no longer valid.
-     */
-    virtual std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
-                                                          const DefinitionMap& definitions) {
-        return nullptr;
-    }
-
     const Kind fKind;
     const Type& fType;
 
diff --git a/src/sksl/ir/SkSLExpressionStatement.h b/src/sksl/ir/SkSLExpressionStatement.h
index 088b1c9ad1..677c647587 100644
--- a/src/sksl/ir/SkSLExpressionStatement.h
+++ b/src/sksl/ir/SkSLExpressionStatement.h
@@ -25,7 +25,7 @@ struct ExpressionStatement : public Statement {
         return fExpression->description() + ";";
     }
 
-    std::unique_ptr<Expression> fExpression;
+    const std::unique_ptr<Expression> fExpression;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLFieldAccess.h b/src/sksl/ir/SkSLFieldAccess.h
index de26a3f626..fb727e017e 100644
--- a/src/sksl/ir/SkSLFieldAccess.h
+++ b/src/sksl/ir/SkSLFieldAccess.h
@@ -35,7 +35,7 @@ struct FieldAccess : public Expression {
         return fBase->description() + "." + fBase->fType.fields()[fFieldIndex].fName;
     }
 
-    std::unique_ptr<Expression> fBase;
+    const std::unique_ptr<Expression> fBase;
     const int fFieldIndex;
     const OwnerKind fOwnerKind;
 
diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h
index 6f03e2bb36..ff03d0d7f9 100644
--- a/src/sksl/ir/SkSLForStatement.h
+++ b/src/sksl/ir/SkSLForStatement.h
@@ -46,8 +46,8 @@ struct ForStatement : public Statement {
     }
 
     const std::unique_ptr<Statement> fInitializer;
-    std::unique_ptr<Expression> fTest;
-    std::unique_ptr<Expression> fNext;
+    const std::unique_ptr<Expression> fTest;
+    const std::unique_ptr<Expression> fNext;
     const std::unique_ptr<Statement> fStatement;
     const std::shared_ptr<SymbolTable> fSymbols;
 
diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h
index 1838076796..971af366b9 100644
--- a/src/sksl/ir/SkSLFunctionCall.h
+++ b/src/sksl/ir/SkSLFunctionCall.h
@@ -36,7 +36,7 @@ struct FunctionCall : public Expression {
     }
 
     const FunctionDeclaration& fFunction;
-    std::vector<std::unique_ptr<Expression>> fArguments;
+    const std::vector<std::unique_ptr<Expression>> fArguments;
 
     typedef Expression INHERITED;
 };
diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h
index 8667e932ec..f8beded9e8 100644
--- a/src/sksl/ir/SkSLIfStatement.h
+++ b/src/sksl/ir/SkSLIfStatement.h
@@ -32,7 +32,7 @@ struct IfStatement : public Statement {
         return result;
     }
 
-    std::unique_ptr<Expression> fTest;
+    const std::unique_ptr<Expression> fTest;
     const std::unique_ptr<Statement> fIfTrue;
     const std::unique_ptr<Statement> fIfFalse;
 
diff --git a/src/sksl/ir/SkSLIndexExpression.h b/src/sksl/ir/SkSLIndexExpression.h
index d255c7daf6..079dde5e53 100644
--- a/src/sksl/ir/SkSLIndexExpression.h
+++ b/src/sksl/ir/SkSLIndexExpression.h
@@ -55,8 +55,8 @@ struct IndexExpression : public Expression {
         return fBase->description() + "[" + fIndex->description() + "]";
     }
 
-    std::unique_ptr<Expression> fBase;
-    std::unique_ptr<Expression> fIndex;
+    const std::unique_ptr<Expression> fBase;
+    const std::unique_ptr<Expression> fIndex;
 
     typedef Expression INHERITED;
 };
diff --git a/src/sksl/ir/SkSLPostfixExpression.h b/src/sksl/ir/SkSLPostfixExpression.h
index 6c9fafe5a0..01671b5b88 100644
--- a/src/sksl/ir/SkSLPostfixExpression.h
+++ b/src/sksl/ir/SkSLPostfixExpression.h
@@ -25,7 +25,7 @@ struct PostfixExpression : public Expression {
         return fOperand->description() + Token::OperatorName(fOperator);
     }
 
-    std::unique_ptr<Expression> fOperand;
+    const std::unique_ptr<Expression> fOperand;
     const Token::Kind fOperator;
 
     typedef Expression INHERITED;
diff --git a/src/sksl/ir/SkSLPrefixExpression.h b/src/sksl/ir/SkSLPrefixExpression.h
index b7db99a0a4..790c5ab47a 100644
--- a/src/sksl/ir/SkSLPrefixExpression.h
+++ b/src/sksl/ir/SkSLPrefixExpression.h
@@ -25,7 +25,7 @@ struct PrefixExpression : public Expression {
         return Token::OperatorName(fOperator) + fOperand->description();
     }
 
-    std::unique_ptr<Expression> fOperand;
+    const std::unique_ptr<Expression> fOperand;
     const Token::Kind fOperator;
 
     typedef Expression INHERITED;
diff --git a/src/sksl/ir/SkSLProgram.h b/src/sksl/ir/SkSLProgram.h
index 6a73be6983..ac49d6dcc7 100644
--- a/src/sksl/ir/SkSLProgram.h
+++ b/src/sksl/ir/SkSLProgram.h
@@ -59,8 +59,8 @@ struct Program {
     , fSettings(settings)
     , fDefaultPrecision(defaultPrecision)
     , fContext(context)
-    , fSymbols(symbols)
     , fElements(std::move(elements))
+    , fSymbols(symbols)
     , fInputs(inputs) {}
 
     Kind fKind;
@@ -68,10 +68,8 @@ struct Program {
     // FIXME handle different types; currently it assumes this is for floats
     Modifiers::Flag fDefaultPrecision;
     Context* fContext;
-    // it's important to keep fElements defined after (and thus destroyed before) fSymbols,
-    // because destroying elements can modify reference counts in symbols
-    std::shared_ptr<SymbolTable> fSymbols;
     std::vector<std::unique_ptr<ProgramElement>> fElements;
+    std::shared_ptr<SymbolTable> fSymbols;
     Inputs fInputs;
 };
 
diff --git a/src/sksl/ir/SkSLReturnStatement.h b/src/sksl/ir/SkSLReturnStatement.h
index dc5ec9aa9c..c83b45066e 100644
--- a/src/sksl/ir/SkSLReturnStatement.h
+++ b/src/sksl/ir/SkSLReturnStatement.h
@@ -32,7 +32,7 @@ struct ReturnStatement : public Statement {
         }
     }
 
-    std::unique_ptr<Expression> fExpression;
+    const std::unique_ptr<Expression> fExpression;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLSwizzle.h b/src/sksl/ir/SkSLSwizzle.h
index 8ad9001ada..c9397aec7f 100644
--- a/src/sksl/ir/SkSLSwizzle.h
+++ b/src/sksl/ir/SkSLSwizzle.h
@@ -76,7 +76,7 @@ struct Swizzle : public Expression {
         return result;
     }
 
-    std::unique_ptr<Expression> fBase;
+    const std::unique_ptr<Expression> fBase;
     const std::vector<int> fComponents;
 
     typedef Expression INHERITED;
diff --git a/src/sksl/ir/SkSLTernaryExpression.h b/src/sksl/ir/SkSLTernaryExpression.h
index 02750049d4..4a352536e3 100644
--- a/src/sksl/ir/SkSLTernaryExpression.h
+++ b/src/sksl/ir/SkSLTernaryExpression.h
@@ -31,9 +31,9 @@ struct TernaryExpression : public Expression {
                fIfFalse->description() + ")";
     }
 
-    std::unique_ptr<Expression> fTest;
-    std::unique_ptr<Expression> fIfTrue;
-    std::unique_ptr<Expression> fIfFalse;
+    const std::unique_ptr<Expression> fTest;
+    const std::unique_ptr<Expression> fIfTrue;
+    const std::unique_ptr<Expression> fIfFalse;
 
     typedef Expression INHERITED;
 };
diff --git a/src/sksl/ir/SkSLVarDeclarations.h b/src/sksl/ir/SkSLVarDeclarations.h
index 490259a081..295c0b6997 100644
--- a/src/sksl/ir/SkSLVarDeclarations.h
+++ b/src/sksl/ir/SkSLVarDeclarations.h
@@ -72,7 +72,7 @@ struct VarDeclarations : public ProgramElement {
     }
 
     const Type& fBaseType;
-    std::vector<VarDeclaration> fVars;
+    const std::vector<VarDeclaration> fVars;
 
     typedef ProgramElement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLVarDeclarationsStatement.h b/src/sksl/ir/SkSLVarDeclarationsStatement.h
index 66b570f853..7a29656593 100644
--- a/src/sksl/ir/SkSLVarDeclarationsStatement.h
+++ b/src/sksl/ir/SkSLVarDeclarationsStatement.h
@@ -18,14 +18,14 @@ namespace SkSL {
  */
 struct VarDeclarationsStatement : public Statement {
     VarDeclarationsStatement(std::unique_ptr<VarDeclarations> decl)
-    : INHERITED(decl->fPosition, kVarDeclarations_Kind)
+    : INHERITED(decl->fPosition, kVarDeclarations_Kind) 
     , fDeclaration(std::move(decl)) {}
 
     SkString description() const override {
         return fDeclaration->description();
     }
 
-    std::shared_ptr<VarDeclarations> fDeclaration;
+    const std::shared_ptr<VarDeclarations> fDeclaration;
 
     typedef Statement INHERITED;
 };
diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h
index 2c3391dfa2..39b8482a7b 100644
--- a/src/sksl/ir/SkSLVariable.h
+++ b/src/sksl/ir/SkSLVariable.h
@@ -33,8 +33,8 @@ struct Variable : public Symbol {
     , fModifiers(modifiers)
     , fType(type)
     , fStorage(storage)
-    , fReadCount(0)
-    , fWriteCount(0) {}
+    , fIsReadFrom(false)
+    , fIsWrittenTo(false) {}
 
     virtual SkString description() const override {
         return fModifiers.description() + fType.fName + " " + fName;
@@ -44,12 +44,8 @@ struct Variable : public Symbol {
     const Type& fType;
     const Storage fStorage;
 
-    // Tracks how many sites read from the variable. If this is zero for a non-out variable (or
-    // becomes zero during optimization), the variable is dead and may be eliminated.
-    mutable int fReadCount;
-    // Tracks how many sites write to the variable. If this is zero, the variable is dead and may be
-    // eliminated.
-    mutable int fWriteCount;
+    mutable bool fIsReadFrom;
+    mutable bool fIsWrittenTo;
 
     typedef Symbol INHERITED;
 };
diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h
index fecb04e2e5..c6a2ea0511 100644
--- a/src/sksl/ir/SkSLVariableReference.h
+++ b/src/sksl/ir/SkSLVariableReference.h
@@ -20,83 +20,16 @@ namespace SkSL {
  * there is only one Variable 'x', but two VariableReferences to it.
  */
 struct VariableReference : public Expression {
-    enum RefKind {
-        kRead_RefKind,
-        kWrite_RefKind,
-        kReadWrite_RefKind
-    };
-
-    VariableReference(Position position, const Variable& variable, RefKind refKind = kRead_RefKind)
+    VariableReference(Position position, const Variable& variable)
     : INHERITED(position, kVariableReference_Kind, variable.fType)
-    , fVariable(variable)
-    , fRefKind(refKind) {
-        if (refKind != kRead_RefKind) {
-            fVariable.fWriteCount++;
-        }
-        if (refKind != kWrite_RefKind) {
-            fVariable.fReadCount++;
-        }
-    }
-
-    virtual ~VariableReference() override {
-        if (fRefKind != kWrite_RefKind) {
-            fVariable.fReadCount--;
-        }
-    }
-
-    RefKind refKind() {
-        return fRefKind;
-    }
-
-    void setRefKind(RefKind refKind) {
-        if (fRefKind != kRead_RefKind) {
-            fVariable.fWriteCount--;
-        }
-        if (fRefKind != kWrite_RefKind) {
-            fVariable.fReadCount--;
-        }
-        if (refKind != kRead_RefKind) {
-            fVariable.fWriteCount++;
-        }
-        if (refKind != kWrite_RefKind) {
-            fVariable.fReadCount++;
-        }
-        fRefKind = refKind;
-    }
+    , fVariable(variable) {}
 
     SkString description() const override {
         return fVariable.fName;
     }
 
-    virtual std::unique_ptr<Expression> constantPropagate(
-                                                        const IRGenerator& irGenerator,
-                                                        const DefinitionMap& definitions) override {
-        auto exprIter = definitions.find(&fVariable);
-        if (exprIter != definitions.end() && exprIter->second) {
-            const Expression* expr = exprIter->second->get();
-            switch (expr->fKind) {
-                case Expression::kIntLiteral_Kind:
-                    return std::unique_ptr<Expression>(new IntLiteral(
-                                                                     irGenerator.fContext,
-                                                                     Position(),
-                                                                     ((IntLiteral*) expr)->fValue));
-                case Expression::kFloatLiteral_Kind:
-                    return std::unique_ptr<Expression>(new FloatLiteral(
-                                                                   irGenerator.fContext,
-                                                                   Position(),
-                                                                   ((FloatLiteral*) expr)->fValue));
-                default:
-                    break;
-            }
-        }
-        return nullptr;
-    }
-
     const Variable& fVariable;
 
-private:
-    RefKind fRefKind;
-
     typedef Expression INHERITED;
 };
 
diff --git a/src/sksl/ir/SkSLWhileStatement.h b/src/sksl/ir/SkSLWhileStatement.h
index a741a0441d..7c6a2907c4 100644
--- a/src/sksl/ir/SkSLWhileStatement.h
+++ b/src/sksl/ir/SkSLWhileStatement.h
@@ -27,7 +27,7 @@ struct WhileStatement : public Statement {
         return "while (" + fTest->description() + ") " + fStatement->description();
     }
 
-    std::unique_ptr<Expression> fTest;
+    const std::unique_ptr<Expression> fTest;
     const std::unique_ptr<Statement> fStatement;
 
     typedef Statement INHERITED;