From 6415e0d2417d133af28ac523400d3f958d2bcd1c Mon Sep 17 00:00:00 2001 From: Ethan Nicholas Date: Thu, 19 Jan 2017 16:31:32 +0000 Subject: [PATCH] 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 > Reviewed-by: Greg Daniel > 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 Reviewed-by: Ethan Nicholas --- src/sksl/SkSLCFGGenerator.cpp | 159 +++++++++------------ src/sksl/SkSLCFGGenerator.h | 19 +-- src/sksl/SkSLCompiler.cpp | 119 +++++---------- src/sksl/SkSLCompiler.h | 7 +- src/sksl/SkSLIRGenerator.cpp | 78 ++++++---- src/sksl/SkSLIRGenerator.h | 18 ++- src/sksl/SkSLSPIRVCodeGenerator.cpp | 2 +- src/sksl/SkSLToken.h | 22 --- src/sksl/ir/SkSLBinaryExpression.h | 18 +-- src/sksl/ir/SkSLBlock.h | 8 +- src/sksl/ir/SkSLConstructor.h | 19 +-- src/sksl/ir/SkSLDoStatement.h | 2 +- src/sksl/ir/SkSLExpression.h | 25 +--- src/sksl/ir/SkSLExpressionStatement.h | 2 +- src/sksl/ir/SkSLFieldAccess.h | 2 +- src/sksl/ir/SkSLForStatement.h | 4 +- src/sksl/ir/SkSLFunctionCall.h | 2 +- src/sksl/ir/SkSLIfStatement.h | 2 +- src/sksl/ir/SkSLIndexExpression.h | 4 +- src/sksl/ir/SkSLPostfixExpression.h | 2 +- src/sksl/ir/SkSLPrefixExpression.h | 2 +- src/sksl/ir/SkSLProgram.h | 6 +- src/sksl/ir/SkSLReturnStatement.h | 2 +- src/sksl/ir/SkSLSwizzle.h | 2 +- src/sksl/ir/SkSLTernaryExpression.h | 6 +- src/sksl/ir/SkSLVarDeclarations.h | 2 +- src/sksl/ir/SkSLVarDeclarationsStatement.h | 4 +- src/sksl/ir/SkSLVariable.h | 12 +- src/sksl/ir/SkSLVariableReference.h | 71 +-------- src/sksl/ir/SkSLWhileStatement.h | 2 +- tests/SkSLGLSLTest.cpp | 24 ++-- 31 files changed, 223 insertions(+), 424 deletions(-) diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp index 31bace9fb7..964a8dc84a 100644 --- a/src/sksl/SkSLCFGGenerator.cpp +++ b/src/sksl/SkSLCFGGenerator.cpp @@ -54,8 +54,8 @@ void CFG::dump() { printf("Block %d\n-------\nBefore: ", (int) i); const char* separator = ""; for (auto iter = fBlocks[i].fBefore.begin(); iter != fBlocks[i].fBefore.end(); iter++) { - printf("%s%s = %s", separator, iter->first->description().c_str(), - *iter->second ? (*iter->second)->description().c_str() : ""); + printf("%s%s = %s", separator, iter->first->description().c_str(), + iter->second ? iter->second->description().c_str() : ""); separator = ", "; } printf("\nEntrances: "); @@ -66,10 +66,7 @@ void CFG::dump() { } printf("\n"); for (size_t j = 0; j < fBlocks[i].fNodes.size(); j++) { - BasicBlock::Node& n = fBlocks[i].fNodes[j]; - printf("Node %d: %s\n", (int) j, n.fKind == BasicBlock::Node::kExpression_Kind - ? (*n.fExpression)->description().c_str() - : n.fStatement->description().c_str()); + printf("Node %d: %s\n", (int) j, fBlocks[i].fNodes[j].fNode->description().c_str()); } printf("Exits: "); separator = ""; @@ -81,109 +78,96 @@ void CFG::dump() { } } -void CFGGenerator::addExpression(CFG& cfg, std::unique_ptr* e, bool constantPropagate) { - ASSERT(e); - switch ((*e)->fKind) { +void CFGGenerator::addExpression(CFG& cfg, const Expression* e) { + switch (e->fKind) { case Expression::kBinary_Kind: { - BinaryExpression* b = (BinaryExpression*) e->get(); + const BinaryExpression* b = (const BinaryExpression*) e; switch (b->fOperator) { case Token::LOGICALAND: // fall through case Token::LOGICALOR: { // this isn't as precise as it could be -- we don't bother to track that if we // early exit from a logical and/or, we know which branch of an 'if' we're going // to hit -- but it won't make much difference in practice. - this->addExpression(cfg, &b->fLeft, constantPropagate); + this->addExpression(cfg, b->fLeft.get()); BlockId start = cfg.fCurrent; cfg.newBlock(); - this->addExpression(cfg, &b->fRight, constantPropagate); + this->addExpression(cfg, b->fRight.get()); cfg.newBlock(); cfg.addExit(start, cfg.fCurrent); break; } case Token::EQ: { - this->addExpression(cfg, &b->fRight, constantPropagate); - this->addLValue(cfg, &b->fLeft); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ - BasicBlock::Node::kExpression_Kind, - constantPropagate, - e, - nullptr + this->addExpression(cfg, b->fRight.get()); + this->addLValue(cfg, b->fLeft.get()); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ + BasicBlock::Node::kExpression_Kind, + b }); break; } default: - this->addExpression(cfg, &b->fLeft, !Token::IsAssignment(b->fOperator)); - this->addExpression(cfg, &b->fRight, constantPropagate); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ - BasicBlock::Node::kExpression_Kind, - constantPropagate, - e, - nullptr + this->addExpression(cfg, b->fLeft.get()); + this->addExpression(cfg, b->fRight.get()); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ + BasicBlock::Node::kExpression_Kind, + b }); } break; } case Expression::kConstructor_Kind: { - Constructor* c = (Constructor*) e->get(); - for (auto& arg : c->fArguments) { - this->addExpression(cfg, &arg, constantPropagate); + const Constructor* c = (const Constructor*) e; + for (const auto& arg : c->fArguments) { + this->addExpression(cfg, arg.get()); } - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, c }); break; } case Expression::kFunctionCall_Kind: { - FunctionCall* c = (FunctionCall*) e->get(); - for (auto& arg : c->fArguments) { - this->addExpression(cfg, &arg, constantPropagate); + const FunctionCall* c = (const FunctionCall*) e; + for (const auto& arg : c->fArguments) { + this->addExpression(cfg, arg.get()); } - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, c }); break; } case Expression::kFieldAccess_Kind: - this->addExpression(cfg, &((FieldAccess*) e->get())->fBase, constantPropagate); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + this->addExpression(cfg, ((const FieldAccess*) e)->fBase.get()); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e }); break; case Expression::kIndex_Kind: - this->addExpression(cfg, &((IndexExpression*) e->get())->fBase, constantPropagate); - this->addExpression(cfg, &((IndexExpression*) e->get())->fIndex, constantPropagate); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + this->addExpression(cfg, ((const IndexExpression*) e)->fBase.get()); + this->addExpression(cfg, ((const IndexExpression*) e)->fIndex.get()); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e }); break; case Expression::kPrefix_Kind: - this->addExpression(cfg, &((PrefixExpression*) e->get())->fOperand, constantPropagate); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + this->addExpression(cfg, ((const PrefixExpression*) e)->fOperand.get()); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e }); break; case Expression::kPostfix_Kind: - this->addExpression(cfg, &((PostfixExpression*) e->get())->fOperand, constantPropagate); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + this->addExpression(cfg, ((const PostfixExpression*) e)->fOperand.get()); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e }); break; case Expression::kSwizzle_Kind: - this->addExpression(cfg, &((Swizzle*) e->get())->fBase, constantPropagate); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + this->addExpression(cfg, ((const Swizzle*) e)->fBase.get()); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e }); break; case Expression::kBoolLiteral_Kind: // fall through case Expression::kFloatLiteral_Kind: // fall through case Expression::kIntLiteral_Kind: // fall through case Expression::kVariableReference_Kind: - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e }); break; case Expression::kTernary_Kind: { - TernaryExpression* t = (TernaryExpression*) e->get(); - this->addExpression(cfg, &t->fTest, constantPropagate); + const TernaryExpression* t = (const TernaryExpression*) e; + this->addExpression(cfg, t->fTest.get()); BlockId start = cfg.fCurrent; cfg.newBlock(); - this->addExpression(cfg, &t->fIfTrue, constantPropagate); + this->addExpression(cfg, t->fIfTrue.get()); BlockId next = cfg.newBlock(); cfg.fCurrent = start; cfg.newBlock(); - this->addExpression(cfg, &t->fIfFalse, constantPropagate); + this->addExpression(cfg, t->fIfFalse.get()); cfg.addExit(cfg.fCurrent, next); cfg.fCurrent = next; break; @@ -197,17 +181,17 @@ void CFGGenerator::addExpression(CFG& cfg, std::unique_ptr* e, bool } // adds expressions that are evaluated as part of resolving an lvalue -void CFGGenerator::addLValue(CFG& cfg, std::unique_ptr* e) { - switch ((*e)->fKind) { +void CFGGenerator::addLValue(CFG& cfg, const Expression* e) { + switch (e->fKind) { case Expression::kFieldAccess_Kind: - this->addLValue(cfg, &((FieldAccess&) **e).fBase); + this->addLValue(cfg, ((const FieldAccess*) e)->fBase.get()); break; case Expression::kIndex_Kind: - this->addLValue(cfg, &((IndexExpression&) **e).fBase); - this->addExpression(cfg, &((IndexExpression&) **e).fIndex, true); + this->addLValue(cfg, ((const IndexExpression*) e)->fBase.get()); + this->addExpression(cfg, ((const IndexExpression*) e)->fIndex.get()); break; case Expression::kSwizzle_Kind: - this->addLValue(cfg, &((Swizzle&) **e).fBase); + this->addLValue(cfg, ((const Swizzle*) e)->fBase.get()); break; case Expression::kVariableReference_Kind: break; @@ -226,8 +210,8 @@ void CFGGenerator::addStatement(CFG& cfg, const Statement* s) { } break; case Statement::kIf_Kind: { - IfStatement* ifs = (IfStatement*) s; - this->addExpression(cfg, &ifs->fTest, true); + const IfStatement* ifs = (const IfStatement*) s; + this->addExpression(cfg, ifs->fTest.get()); BlockId start = cfg.fCurrent; cfg.newBlock(); this->addStatement(cfg, ifs->fIfTrue.get()); @@ -244,54 +228,49 @@ void CFGGenerator::addStatement(CFG& cfg, const Statement* s) { break; } case Statement::kExpression_Kind: { - this->addExpression(cfg, &((ExpressionStatement&) *s).fExpression, true); + this->addExpression(cfg, ((ExpressionStatement&) *s).fExpression.get()); break; } case Statement::kVarDeclarations_Kind: { - VarDeclarationsStatement& decls = ((VarDeclarationsStatement&) *s); - for (auto& vd : decls.fDeclaration->fVars) { + const VarDeclarationsStatement& decls = ((VarDeclarationsStatement&) *s); + for (const auto& vd : decls.fDeclaration->fVars) { if (vd.fValue) { - this->addExpression(cfg, &vd.fValue, true); + this->addExpression(cfg, vd.fValue.get()); } } - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, - nullptr, s }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s }); break; } case Statement::kDiscard_Kind: - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, - nullptr, s }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s }); cfg.fCurrent = cfg.newIsolatedBlock(); break; case Statement::kReturn_Kind: { - ReturnStatement& r = ((ReturnStatement&) *s); + const ReturnStatement& r = ((ReturnStatement&) *s); if (r.fExpression) { - this->addExpression(cfg, &r.fExpression, true); + this->addExpression(cfg, r.fExpression.get()); } - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, - nullptr, s }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s }); cfg.fCurrent = cfg.newIsolatedBlock(); break; } case Statement::kBreak_Kind: - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, - nullptr, s }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s }); cfg.addExit(cfg.fCurrent, fLoopExits.top()); cfg.fCurrent = cfg.newIsolatedBlock(); break; case Statement::kContinue_Kind: - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, - nullptr, s }); + cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s }); cfg.addExit(cfg.fCurrent, fLoopContinues.top()); cfg.fCurrent = cfg.newIsolatedBlock(); break; case Statement::kWhile_Kind: { - WhileStatement* w = (WhileStatement*) s; + const WhileStatement* w = (const WhileStatement*) s; BlockId loopStart = cfg.newBlock(); fLoopContinues.push(loopStart); BlockId loopExit = cfg.newIsolatedBlock(); fLoopExits.push(loopExit); - this->addExpression(cfg, &w->fTest, true); + this->addExpression(cfg, w->fTest.get()); BlockId test = cfg.fCurrent; cfg.addExit(test, loopExit); cfg.newBlock(); @@ -303,13 +282,13 @@ void CFGGenerator::addStatement(CFG& cfg, const Statement* s) { break; } case Statement::kDo_Kind: { - DoStatement* d = (DoStatement*) s; + const DoStatement* d = (const DoStatement*) s; BlockId loopStart = cfg.newBlock(); fLoopContinues.push(loopStart); BlockId loopExit = cfg.newIsolatedBlock(); fLoopExits.push(loopExit); this->addStatement(cfg, d->fStatement.get()); - this->addExpression(cfg, &d->fTest, true); + this->addExpression(cfg, d->fTest.get()); cfg.addExit(cfg.fCurrent, loopExit); cfg.addExit(cfg.fCurrent, loopStart); fLoopContinues.pop(); @@ -318,7 +297,7 @@ void CFGGenerator::addStatement(CFG& cfg, const Statement* s) { break; } case Statement::kFor_Kind: { - ForStatement* f = (ForStatement*) s; + const ForStatement* f = (const ForStatement*) s; if (f->fInitializer) { this->addStatement(cfg, f->fInitializer.get()); } @@ -328,7 +307,7 @@ void CFGGenerator::addStatement(CFG& cfg, const Statement* s) { BlockId loopExit = cfg.newIsolatedBlock(); fLoopExits.push(loopExit); if (f->fTest) { - this->addExpression(cfg, &f->fTest, true); + this->addExpression(cfg, f->fTest.get()); BlockId test = cfg.fCurrent; cfg.addExit(test, loopExit); } @@ -337,9 +316,9 @@ void CFGGenerator::addStatement(CFG& cfg, const Statement* s) { cfg.addExit(cfg.fCurrent, next); cfg.fCurrent = next; if (f->fNext) { - this->addExpression(cfg, &f->fNext, true); + this->addExpression(cfg, f->fNext.get()); } - cfg.addExit(cfg.fCurrent, loopStart); + cfg.addExit(next, loopStart); fLoopContinues.pop(); fLoopExits.pop(); cfg.fCurrent = loopExit; diff --git a/src/sksl/SkSLCFGGenerator.h b/src/sksl/SkSLCFGGenerator.h index 337fdfac35..c37850112c 100644 --- a/src/sksl/SkSLCFGGenerator.h +++ b/src/sksl/SkSLCFGGenerator.h @@ -27,23 +27,14 @@ struct BasicBlock { }; Kind fKind; - // if false, this node should not be subject to constant propagation. This happens with - // compound assignment (i.e. x *= 2), in which the value x is used as an rvalue for - // multiplication by 2 and then as an lvalue for assignment purposes. Since there is only - // one "x" node, replacing it with a constant would break the assignment and we suppress - // it. Down the road, we should handle this more elegantly by substituting a regular - // assignment if the target is constant (i.e. x = 1; x *= 2; should become x = 1; x = 1 * 2; - // and then collapse down to a simple x = 2;). - bool fConstantPropagation; - std::unique_ptr* fExpression; - const Statement* fStatement; + const IRNode* fNode; }; - + std::vector fNodes; std::set fEntrances; std::set fExits; // variable definitions upon entering this basic block (null expression = undefined) - DefinitionMap fBefore; + std::unordered_map fBefore; }; struct CFG { @@ -86,9 +77,9 @@ public: private: void addStatement(CFG& cfg, const Statement* s); - void addExpression(CFG& cfg, std::unique_ptr* e, bool constantPropagate); + void addExpression(CFG& cfg, const Expression* e); - void addLValue(CFG& cfg, std::unique_ptr* e); + void addLValue(CFG& cfg, const Expression* e); std::stack fLoopContinues; std::stack fLoopExits; diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 743745ad14..9faf836156 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -156,8 +156,8 @@ Compiler::~Compiler() { } // add the definition created by assigning to the lvalue to the definition set -void Compiler::addDefinition(const Expression* lvalue, std::unique_ptr* expr, - DefinitionMap* definitions) { +void Compiler::addDefinition(const Expression* lvalue, const Expression* expr, + std::unordered_map* definitions) { switch (lvalue->fKind) { case Expression::kVariableReference_Kind: { const Variable& var = ((VariableReference*) lvalue)->fVariable; @@ -174,19 +174,19 @@ void Compiler::addDefinition(const Expression* lvalue, std::unique_ptraddDefinition(((Swizzle*) lvalue)->fBase.get(), - (std::unique_ptr*) &fContext.fDefined_Expression, + fContext.fDefined_Expression.get(), definitions); break; case Expression::kIndex_Kind: // see comments in Swizzle this->addDefinition(((IndexExpression*) lvalue)->fBase.get(), - (std::unique_ptr*) &fContext.fDefined_Expression, + fContext.fDefined_Expression.get(), definitions); break; case Expression::kFieldAccess_Kind: // see comments in Swizzle this->addDefinition(((FieldAccess*) lvalue)->fBase.get(), - (std::unique_ptr*) &fContext.fDefined_Expression, + fContext.fDefined_Expression.get(), definitions); break; default: @@ -197,58 +197,25 @@ void Compiler::addDefinition(const Expression* lvalue, std::unique_ptr* definitions) { switch (node.fKind) { case BasicBlock::Node::kExpression_Kind: { - ASSERT(node.fExpression); - const Expression* expr = (Expression*) node.fExpression->get(); - switch (expr->fKind) { - case Expression::kBinary_Kind: { - BinaryExpression* b = (BinaryExpression*) expr; - if (b->fOperator == Token::EQ) { - this->addDefinition(b->fLeft.get(), &b->fRight, definitions); - } else if (Token::IsAssignment(b->fOperator)) { - this->addDefinition( - b->fLeft.get(), - (std::unique_ptr*) &fContext.fDefined_Expression, - definitions); - - } - break; + const Expression* expr = (Expression*) node.fNode; + if (expr->fKind == Expression::kBinary_Kind) { + const BinaryExpression* b = (BinaryExpression*) expr; + if (b->fOperator == Token::EQ) { + this->addDefinition(b->fLeft.get(), b->fRight.get(), definitions); } - case Expression::kPrefix_Kind: { - const PrefixExpression* p = (PrefixExpression*) expr; - if (p->fOperator == Token::MINUSMINUS || p->fOperator == Token::PLUSPLUS) { - this->addDefinition( - p->fOperand.get(), - (std::unique_ptr*) &fContext.fDefined_Expression, - definitions); - } - break; - } - case Expression::kPostfix_Kind: { - const PostfixExpression* p = (PostfixExpression*) expr; - if (p->fOperator == Token::MINUSMINUS || p->fOperator == Token::PLUSPLUS) { - this->addDefinition( - p->fOperand.get(), - (std::unique_ptr*) &fContext.fDefined_Expression, - definitions); - - } - break; - } - default: - break; } break; } case BasicBlock::Node::kStatement_Kind: { - const Statement* stmt = (Statement*) node.fStatement; + const Statement* stmt = (Statement*) node.fNode; if (stmt->fKind == Statement::kVarDeclarations_Kind) { - VarDeclarationsStatement* vd = (VarDeclarationsStatement*) stmt; - for (VarDeclaration& decl : vd->fDeclaration->fVars) { + const VarDeclarationsStatement* vd = (VarDeclarationsStatement*) stmt; + for (const VarDeclaration& decl : vd->fDeclaration->fVars) { if (decl.fValue) { - (*definitions)[decl.fVar] = &decl.fValue; + (*definitions)[decl.fVar] = decl.fValue.get(); } } } @@ -261,7 +228,7 @@ void Compiler::scanCFG(CFG* cfg, BlockId blockId, std::set* workList) { BasicBlock& block = cfg->fBlocks[blockId]; // compute definitions after this block - DefinitionMap after = block.fBefore; + std::unordered_map after = block.fBefore; for (const BasicBlock::Node& n : block.fNodes) { this->addDefinitions(n, &after); } @@ -270,20 +237,19 @@ void Compiler::scanCFG(CFG* cfg, BlockId blockId, std::set* workList) { for (BlockId exitId : block.fExits) { BasicBlock& exit = cfg->fBlocks[exitId]; for (const auto& pair : after) { - std::unique_ptr* e1 = pair.second; - auto found = exit.fBefore.find(pair.first); - if (found == exit.fBefore.end()) { - // exit has no definition for it, just copy it - workList->insert(exitId); + const Expression* e1 = pair.second; + if (exit.fBefore.find(pair.first) == exit.fBefore.end()) { exit.fBefore[pair.first] = e1; } else { - // exit has a (possibly different) value already defined - std::unique_ptr* e2 = exit.fBefore[pair.first]; + const Expression* e2 = exit.fBefore[pair.first]; if (e1 != e2) { // definition has changed, merge and add exit block to worklist workList->insert(exitId); - exit.fBefore[pair.first] = - (std::unique_ptr*) &fContext.fDefined_Expression; + if (!e1 || !e2) { + exit.fBefore[pair.first] = nullptr; + } else { + exit.fBefore[pair.first] = fContext.fDefined_Expression.get(); + } } } } @@ -292,13 +258,12 @@ void Compiler::scanCFG(CFG* cfg, BlockId blockId, std::set* workList) { // returns a map which maps all local variables in the function to null, indicating that their value // is initially unknown -static DefinitionMap compute_start_state(const CFG& cfg) { - DefinitionMap result; +static std::unordered_map compute_start_state(const CFG& cfg) { + std::unordered_map result; for (const auto& block : cfg.fBlocks) { for (const auto& node : block.fNodes) { if (node.fKind == BasicBlock::Node::kStatement_Kind) { - ASSERT(node.fStatement); - const Statement* s = node.fStatement; + const Statement* s = (Statement*) node.fNode; if (s->fKind == Statement::kVarDeclarations_Kind) { const VarDeclarationsStatement* vd = (const VarDeclarationsStatement*) s; for (const VarDeclaration& decl : vd->fDeclaration->fVars) { @@ -330,37 +295,19 @@ void Compiler::scanCFG(const FunctionDefinition& f) { for (size_t i = 0; i < cfg.fBlocks.size(); i++) { if (i != cfg.fStart && !cfg.fBlocks[i].fEntrances.size() && cfg.fBlocks[i].fNodes.size()) { - Position p; - switch (cfg.fBlocks[i].fNodes[0].fKind) { - case BasicBlock::Node::kStatement_Kind: - p = cfg.fBlocks[i].fNodes[0].fStatement->fPosition; - break; - case BasicBlock::Node::kExpression_Kind: - p = (*cfg.fBlocks[i].fNodes[0].fExpression)->fPosition; - break; - } - this->error(p, SkString("unreachable")); + this->error(cfg.fBlocks[i].fNodes[0].fNode->fPosition, SkString("unreachable")); } } if (fErrorCount) { return; } - // check for undefined variables, perform constant propagation - for (BasicBlock& b : cfg.fBlocks) { - DefinitionMap definitions = b.fBefore; - for (BasicBlock::Node& n : b.fNodes) { + // check for undefined variables + for (const BasicBlock& b : cfg.fBlocks) { + std::unordered_map definitions = b.fBefore; + for (const BasicBlock::Node& n : b.fNodes) { if (n.fKind == BasicBlock::Node::kExpression_Kind) { - ASSERT(n.fExpression); - Expression* expr = n.fExpression->get(); - if (n.fConstantPropagation) { - std::unique_ptr optimized = expr->constantPropagate(*fIRGenerator, - definitions); - if (optimized) { - n.fExpression->reset(optimized.release()); - expr = n.fExpression->get(); - } - } + const Expression* expr = (const Expression*) n.fNode; if (expr->fKind == Expression::kVariableReference_Kind) { const Variable& var = ((VariableReference*) expr)->fVariable; if (var.fStorage == Variable::kLocal_Storage && diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h index fdca12d2cf..0f893f7e64 100644 --- a/src/sksl/SkSLCompiler.h +++ b/src/sksl/SkSLCompiler.h @@ -60,10 +60,11 @@ public: } private: - void addDefinition(const Expression* lvalue, std::unique_ptr* expr, - DefinitionMap* definitions); + void addDefinition(const Expression* lvalue, const Expression* expr, + std::unordered_map* definitions); - void addDefinitions(const BasicBlock::Node& node, DefinitionMap* definitions); + void addDefinitions(const BasicBlock::Node& node, + std::unordered_map* definitions); void scanCFG(CFG* cfg, BlockId block, std::set* workList); diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index 55d9d2c8d6..9f06c97d11 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -551,11 +551,11 @@ std::unique_ptr IRGenerator::convertInterfaceBlock(const ASTInte } } Type* type = new Type(intf.fPosition, intf.fInterfaceName, fields); - old->takeOwnership(type); + fSymbolTable->takeOwnership(type); SkString name = intf.fValueName.size() > 0 ? intf.fValueName : intf.fInterfaceName; Variable* var = new Variable(intf.fPosition, intf.fModifiers, name, *type, Variable::kGlobal_Storage); - old->takeOwnership(var); + fSymbolTable->takeOwnership(var); if (intf.fValueName.size()) { old->addWithoutOwnership(intf.fValueName, var); } else { @@ -624,22 +624,19 @@ std::unique_ptr IRGenerator::convertIdentifier(const ASTIdentifier& f->fFunctions)); } case Symbol::kVariable_Kind: { - Variable* var = (Variable*) result; + const Variable* var = (const Variable*) result; + this->markReadFrom(*var); if (var->fModifiers.fLayout.fBuiltin == SK_FRAGCOORD_BUILTIN && fSettings->fFlipY && (!fSettings->fCaps || !fSettings->fCaps->fragCoordConventionsExtensionString())) { fInputs.fRTHeight = true; } - // default to kRead_RefKind; this will be corrected later if the variable is written to - return std::unique_ptr(new VariableReference( - identifier.fPosition, - *var, - VariableReference::kRead_RefKind)); + return std::unique_ptr(new VariableReference(identifier.fPosition, + *var)); } case Symbol::kField_Kind: { const Field* field = (const Field*) result; - VariableReference* base = new VariableReference(identifier.fPosition, field->fOwner, - VariableReference::kRead_RefKind); + VariableReference* base = new VariableReference(identifier.fPosition, field->fOwner); return std::unique_ptr(new FieldAccess( std::unique_ptr(base), field->fFieldIndex, @@ -693,6 +690,28 @@ static bool is_matrix_multiply(const Type& left, const Type& right) { return left.kind() == Type::kVector_Kind && right.kind() == Type::kMatrix_Kind; } +static bool is_assignment(Token::Kind op) { + switch (op) { + case Token::EQ: // fall through + case Token::PLUSEQ: // fall through + case Token::MINUSEQ: // fall through + case Token::STAREQ: // fall through + case Token::SLASHEQ: // fall through + case Token::PERCENTEQ: // fall through + case Token::SHLEQ: // fall through + case Token::SHREQ: // fall through + case Token::BITWISEOREQ: // fall through + case Token::BITWISEXOREQ: // fall through + case Token::BITWISEANDEQ: // fall through + case Token::LOGICALOREQ: // fall through + case Token::LOGICALXOREQ: // fall through + case Token::LOGICALANDEQ: + return true; + default: + return false; + } +} + /** * Determines the operand and result types of a binary expression. Returns true if the expression is * legal, false otherwise. If false, the values of the out parameters are undefined. @@ -823,9 +842,14 @@ static bool determine_binary_type(const Context& context, return false; } +/** + * If both operands are compile-time constants and can be folded, returns an expression representing + * the folded value. Otherwise, returns null. Note that unlike most other functions here, null does + * not represent a compilation error. + */ std::unique_ptr IRGenerator::constantFold(const Expression& left, Token::Kind op, - const Expression& right) const { + const Expression& right) { // Note that we expressly do not worry about precision and overflow here -- we use the maximum // precision to calculate the results and hope the result makes sense. The plan is to move the // Skia caps into SkSL, so we have access to all of them including the precisions of the various @@ -919,16 +943,15 @@ std::unique_ptr IRGenerator::convertBinaryExpression( const Type* rightType; const Type* resultType; if (!determine_binary_type(fContext, expression.fOperator, left->fType, right->fType, &leftType, - &rightType, &resultType, - !Token::IsAssignment(expression.fOperator))) { + &rightType, &resultType, !is_assignment(expression.fOperator))) { fErrors.error(expression.fPosition, "type mismatch: '" + Token::OperatorName(expression.fOperator) + "' cannot operate on '" + left->fType.fName + "', '" + right->fType.fName + "'"); return nullptr; } - if (Token::IsAssignment(expression.fOperator)) { - this->markWrittenTo(*left, expression.fOperator != Token::EQ); + if (is_assignment(expression.fOperator)) { + this->markWrittenTo(*left); } left = this->coerce(std::move(left), *leftType); right = this->coerce(std::move(right), *rightType); @@ -1028,7 +1051,7 @@ std::unique_ptr IRGenerator::call(Position position, return nullptr; } if (arguments[i] && (function.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag)) { - this->markWrittenTo(*arguments[i], true); + this->markWrittenTo(*arguments[i]); } } return std::unique_ptr(new FunctionCall(position, *returnType, function, @@ -1238,7 +1261,7 @@ std::unique_ptr IRGenerator::convertPrefixExpression( "' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->markWrittenTo(*base); break; case Token::MINUSMINUS: if (!base->fType.isNumber()) { @@ -1247,7 +1270,7 @@ std::unique_ptr IRGenerator::convertPrefixExpression( "' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->markWrittenTo(*base); break; case Token::LOGICALNOT: if (base->fType != *fContext.fBool_Type) { @@ -1441,7 +1464,7 @@ std::unique_ptr IRGenerator::convertSuffixExpression( "'++' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->markWrittenTo(*base); return std::unique_ptr(new PostfixExpression(std::move(base), Token::PLUSPLUS)); case ASTSuffix::kPostDecrement_Kind: @@ -1450,7 +1473,7 @@ std::unique_ptr IRGenerator::convertSuffixExpression( "'--' cannot operate on '" + base->fType.description() + "'"); return nullptr; } - this->markWrittenTo(*base, true); + this->markWrittenTo(*base); return std::unique_ptr(new PostfixExpression(std::move(base), Token::MINUSMINUS)); default: @@ -1473,6 +1496,10 @@ void IRGenerator::checkValid(const Expression& expr) { } } +void IRGenerator::markReadFrom(const Variable& var) { + var.fIsReadFrom = true; +} + static bool has_duplicates(const Swizzle& swizzle) { int bits = 0; for (int idx : swizzle.fComponents) { @@ -1486,7 +1513,7 @@ static bool has_duplicates(const Swizzle& swizzle) { return false; } -void IRGenerator::markWrittenTo(const Expression& expr, bool readWrite) { +void IRGenerator::markWrittenTo(const Expression& expr) { switch (expr.fKind) { case Expression::kVariableReference_Kind: { const Variable& var = ((VariableReference&) expr).fVariable; @@ -1494,22 +1521,21 @@ void IRGenerator::markWrittenTo(const Expression& expr, bool readWrite) { fErrors.error(expr.fPosition, "cannot modify immutable variable '" + var.fName + "'"); } - ((VariableReference&) expr).setRefKind(readWrite ? VariableReference::kReadWrite_RefKind - : VariableReference::kWrite_RefKind); + var.fIsWrittenTo = true; break; } case Expression::kFieldAccess_Kind: - this->markWrittenTo(*((FieldAccess&) expr).fBase, readWrite); + this->markWrittenTo(*((FieldAccess&) expr).fBase); break; case Expression::kSwizzle_Kind: if (has_duplicates((Swizzle&) expr)) { fErrors.error(expr.fPosition, "cannot write to the same swizzle field more than once"); } - this->markWrittenTo(*((Swizzle&) expr).fBase, readWrite); + this->markWrittenTo(*((Swizzle&) expr).fBase); break; case Expression::kIndex_Kind: - this->markWrittenTo(*((IndexExpression&) expr).fBase, readWrite); + this->markWrittenTo(*((IndexExpression&) expr).fBase); break; default: fErrors.error(expr.fPosition, "cannot assign to '" + expr.description() + "'"); diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h index 2ffcb0df26..13b20fbbcc 100644 --- a/src/sksl/SkSLIRGenerator.h +++ b/src/sksl/SkSLIRGenerator.h @@ -88,16 +88,7 @@ public: std::unique_ptr convertModifiersDeclaration( const ASTModifiersDeclaration& m); - /** - * If both operands are compile-time constants and can be folded, returns an expression - * representing the folded value. Otherwise, returns null. Note that unlike most other functions - * here, null does not represent a compilation error. - */ - std::unique_ptr constantFold(const Expression& left, - Token::Kind op, - const Expression& right) const; Program::Inputs fInputs; - const Context& fContext; private: /** @@ -133,6 +124,11 @@ private: std::unique_ptr convertDiscard(const ASTDiscardStatement& d); std::unique_ptr convertDo(const ASTDoStatement& d); std::unique_ptr convertBinaryExpression(const ASTBinaryExpression& expression); + // Returns null if it cannot fold the expression. Note that unlike most other functions here, a + // null return does not represent a compilation error. + std::unique_ptr constantFold(const Expression& left, + Token::Kind op, + const Expression& right); std::unique_ptr convertExtension(const ASTExtension& e); std::unique_ptr convertExpressionStatement(const ASTExpressionStatement& s); std::unique_ptr convertFor(const ASTForStatement& f); @@ -155,8 +151,10 @@ private: std::unique_ptr convertWhile(const ASTWhileStatement& w); void checkValid(const Expression& expr); - void markWrittenTo(const Expression& expr, bool readWrite); + void markReadFrom(const Variable& var); + void markWrittenTo(const Expression& expr); + const Context& fContext; const FunctionDeclaration* fCurrentFunction; const Program::Settings* fSettings; std::unordered_map fCapsMap; diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp index d43e4c4035..8afd13688c 100644 --- a/src/sksl/SkSLSPIRVCodeGenerator.cpp +++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp @@ -2540,7 +2540,7 @@ void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclaratio kind != Program::kFragment_Kind) { continue; } - if (!var->fReadCount && !var->fWriteCount && + if (!var->fIsReadFrom && !var->fIsWrittenTo && !(var->fModifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kOut_Flag | Modifiers::kUniform_Flag))) { diff --git a/src/sksl/SkSLToken.h b/src/sksl/SkSLToken.h index 197781f2a0..5c8c2bd215 100644 --- a/src/sksl/SkSLToken.h +++ b/src/sksl/SkSLToken.h @@ -160,28 +160,6 @@ struct Token { , fKind(kind) , fText(std::move(text)) {} - static bool IsAssignment(Token::Kind op) { - switch (op) { - case Token::EQ: // fall through - case Token::PLUSEQ: // fall through - case Token::MINUSEQ: // fall through - case Token::STAREQ: // fall through - case Token::SLASHEQ: // fall through - case Token::PERCENTEQ: // fall through - case Token::SHLEQ: // fall through - case Token::SHREQ: // fall through - case Token::BITWISEOREQ: // fall through - case Token::BITWISEXOREQ: // fall through - case Token::BITWISEANDEQ: // fall through - case Token::LOGICALOREQ: // fall through - case Token::LOGICALXOREQ: // fall through - case Token::LOGICALANDEQ: - return true; - default: - return false; - } - } - Position fPosition; Kind fKind; // will be the empty string unless the token has variable text content (identifiers, numeric 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 left, Token::Kind op, @@ -26,22 +24,14 @@ struct BinaryExpression : public Expression { , fOperator(op) , fRight(std::move(right)) {} - virtual std::unique_ptr 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 fLeft; + const std::unique_ptr fLeft; const Token::Kind fOperator; - std::unique_ptr fRight; + const std::unique_ptr 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> statements, const std::shared_ptr 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 fSymbols; const std::vector> fStatements; + const std::shared_ptr 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 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(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> fArguments; + const std::vector> 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 fStatement; - std::unique_ptr fTest; + const std::unique_ptr 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 namespace SkSL { -struct Expression; -class IRGenerator; - -typedef std::unordered_map*> 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 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 fExpression; + const std::unique_ptr 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 fBase; + const std::unique_ptr 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 fInitializer; - std::unique_ptr fTest; - std::unique_ptr fNext; + const std::unique_ptr fTest; + const std::unique_ptr fNext; const std::unique_ptr fStatement; const std::shared_ptr 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> fArguments; + const std::vector> 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 fTest; + const std::unique_ptr fTest; const std::unique_ptr fIfTrue; const std::unique_ptr 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 fBase; - std::unique_ptr fIndex; + const std::unique_ptr fBase; + const std::unique_ptr 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 fOperand; + const std::unique_ptr 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 fOperand; + const std::unique_ptr 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 fSymbols; std::vector> fElements; + std::shared_ptr 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 fExpression; + const std::unique_ptr 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 fBase; + const std::unique_ptr fBase; const std::vector 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 fTest; - std::unique_ptr fIfTrue; - std::unique_ptr fIfFalse; + const std::unique_ptr fTest; + const std::unique_ptr fIfTrue; + const std::unique_ptr 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 fVars; + const std::vector 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 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 fDeclaration; + const std::shared_ptr 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 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(new IntLiteral( - irGenerator.fContext, - Position(), - ((IntLiteral*) expr)->fValue)); - case Expression::kFloatLiteral_Kind: - return std::unique_ptr(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 fTest; + const std::unique_ptr fTest; const std::unique_ptr fStatement; typedef Statement INHERITED; diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp index 1501dc5677..12ac4d1101 100644 --- a/tests/SkSLGLSLTest.cpp +++ b/tests/SkSLGLSLTest.cpp @@ -61,7 +61,7 @@ DEF_TEST(SkSLControl, r) { "while (i < 10) sk_FragColor *= 0.5;" "do { sk_FragColor += 0.01; } while (sk_FragColor.x < 0.75);" "for (int i = 0; i < 10; i++) {" - "if (i % 2 == 1) break; else continue;" + "if (i % 0 == 1) break; else continue;" "}" "return;" "}", @@ -75,12 +75,12 @@ DEF_TEST(SkSLControl, r) { " discard;\n" " }\n" " int i = 0;\n" - " while (true) sk_FragColor *= 0.5;\n" + " while (i < 10) sk_FragColor *= 0.5;\n" " do {\n" " sk_FragColor += 0.01;\n" " } while (sk_FragColor.x < 0.75);\n" " for (int i = 0;i < 10; i++) {\n" - " if (i % 2 == 1) break; else continue;\n" + " if (i % 0 == 1) break; else continue;\n" " }\n" " return;\n" "}\n"); @@ -106,8 +106,8 @@ DEF_TEST(SkSLFunctions, r) { "}\n" "void main() {\n" " float x = 10.0;\n" - " bar(10.0);\n" - " sk_FragColor = vec4(10.0);\n" + " bar(x);\n" + " sk_FragColor = vec4(x);\n" "}\n"); } @@ -116,7 +116,7 @@ DEF_TEST(SkSLOperators, r) { "void main() {" "float x = 1, y = 2;" "int z = 3;" - "x = x - x + y * z * x * (y - z);" + "x = x + y * z * x * (y - z);" "y = x / y / z;" "z = (z / 2 % 3 << 4) >> 2 << 1;" "bool b = (x > 4) == x < 2 || 2 >= sqrt(2) && y <= z;" @@ -139,10 +139,10 @@ DEF_TEST(SkSLOperators, r) { "void main() {\n" " float x = 1.0, y = 2.0;\n" " int z = 3;\n" - " x = -6.0;\n" - " y = -1.0;\n" - " z = 8;\n" - " bool b = false == true || 2.0 >= sqrt(2.0) && true;\n" + " x = x + ((y * float(z)) * x) * (y - float(z));\n" + " y = (x / y) / float(z);\n" + " z = (((z / 2) % 3 << 4) >> 2) << 1;\n" + " bool b = x > 4.0 == x < 2.0 || 2.0 >= sqrt(2.0) && y <= float(z);\n" " x += 12.0;\n" " x -= 12.0;\n" " x *= (y /= float(z = 10));\n" @@ -287,7 +287,7 @@ DEF_TEST(SkSLMinAbs, r) { "out vec4 sk_FragColor;\n" "void main() {\n" " float x = -5.0;\n" - " x = min(abs(-5.0), 6.0);\n" + " x = min(abs(x), 6.0);\n" "}\n"); test(r, @@ -302,7 +302,7 @@ DEF_TEST(SkSLMinAbs, r) { " float minAbsHackVar0;\n" " float minAbsHackVar1;\n" " float x = -5.0;\n" - " x = ((minAbsHackVar0 = abs(-5.0)) < (minAbsHackVar1 = 6.0) ? minAbsHackVar0 : " + " x = ((minAbsHackVar0 = abs(x)) < (minAbsHackVar1 = 6.0) ? minAbsHackVar0 : " "minAbsHackVar1);\n" "}\n"); }