diff options
Diffstat (limited to 'src/sksl')
36 files changed, 147 insertions, 773 deletions
diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp index f3e4f92396..31bace9fb7 100644 --- a/src/sksl/SkSLCFGGenerator.cpp +++ b/src/sksl/SkSLCFGGenerator.cpp @@ -55,7 +55,7 @@ void CFG::dump() { 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() : "<undefined>"); + *iter->second ? (*iter->second)->description().c_str() : "<undefined>"); separator = ", "; } printf("\nEntrances: "); @@ -67,9 +67,9 @@ 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 (%p): %s\n", (int) j, &n, n.fKind == BasicBlock::Node::kExpression_Kind + printf("Node %d: %s\n", (int) j, n.fKind == BasicBlock::Node::kExpression_Kind ? (*n.fExpression)->description().c_str() - : (*n.fStatement)->description().c_str()); + : n.fStatement->description().c_str()); } printf("Exits: "); separator = ""; @@ -81,203 +81,6 @@ void CFG::dump() { } } -bool BasicBlock::tryRemoveExpressionBefore(std::vector<BasicBlock::Node>::iterator* iter, - Expression* e) { - if (e->fKind == Expression::kTernary_Kind) { - return false; - } - bool result; - if ((*iter)->fKind == BasicBlock::Node::kExpression_Kind) { - ASSERT((*iter)->fExpression->get() != e); - Expression* old = (*iter)->fExpression->get(); - do { - if ((*iter) == fNodes.begin()) { - SkASSERT(false); - return false; - } - --(*iter); - } while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind || - (*iter)->fExpression->get() != e); - result = this->tryRemoveExpression(iter); - while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind || - (*iter)->fExpression->get() != old) { - ASSERT(*iter != fNodes.end()); - ++(*iter); - } - } else { - Statement* old = (*iter)->fStatement->get(); - do { - if ((*iter) == fNodes.begin()) { - SkASSERT(false); - return false; - } - --(*iter); - } while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind || - (*iter)->fExpression->get() != e); - result = this->tryRemoveExpression(iter); - while ((*iter)->fKind != BasicBlock::Node::kStatement_Kind || - (*iter)->fStatement->get() != old) { - ASSERT(*iter != fNodes.end()); - ++(*iter); - } - } - return result; -} - -bool BasicBlock::tryRemoveLValueBefore(std::vector<BasicBlock::Node>::iterator* iter, - Expression* lvalue) { - switch (lvalue->fKind) { - case Expression::kVariableReference_Kind: - return true; - case Expression::kSwizzle_Kind: - return this->tryRemoveLValueBefore(iter, ((Swizzle*) lvalue)->fBase.get()); - case Expression::kFieldAccess_Kind: - return this->tryRemoveLValueBefore(iter, ((FieldAccess*) lvalue)->fBase.get()); - case Expression::kIndex_Kind: - if (!this->tryRemoveLValueBefore(iter, ((IndexExpression*) lvalue)->fBase.get())) { - return false; - } - return this->tryRemoveExpressionBefore(iter, ((IndexExpression*) lvalue)->fIndex.get()); - default: - SkDebugf("invalid lvalue: %s\n", lvalue->description().c_str()); - ASSERT(false); - return false; - } -} - -bool BasicBlock::tryRemoveExpression(std::vector<BasicBlock::Node>::iterator* iter) { - ASSERT((*iter)->fKind == BasicBlock::Node::kExpression_Kind); - Expression* expr = (*iter)->fExpression->get(); - switch (expr->fKind) { - case Expression::kBinary_Kind: { - BinaryExpression* b = (BinaryExpression*) expr; - if (b->fOperator == Token::EQ) { - if (!this->tryRemoveLValueBefore(iter, b->fLeft.get())) { - return false; - } - } else if (!this->tryRemoveExpressionBefore(iter, b->fLeft.get())) { - return false; - } - if (!this->tryRemoveExpressionBefore(iter, b->fRight.get())) { - return false; - } - ASSERT((*iter)->fKind == BasicBlock::Node::kExpression_Kind && - (*iter)->fExpression->get() == expr); - *iter = fNodes.erase(*iter); - return true; - } - case Expression::kTernary_Kind: { - // ternaries cross basic block boundaries, must regenerate the CFG to remove it - return false; - } - case Expression::kFieldAccess_Kind: { - FieldAccess* f = (FieldAccess*) expr; - if (!this->tryRemoveExpressionBefore(iter, f->fBase.get())) { - return false; - } - *iter = fNodes.erase(*iter); - return true; - } - case Expression::kSwizzle_Kind: { - Swizzle* s = (Swizzle*) expr; - if (!this->tryRemoveExpressionBefore(iter, s->fBase.get())) { - return false; - } - *iter = fNodes.erase(*iter); - return true; - } - case Expression::kIndex_Kind: { - IndexExpression* idx = (IndexExpression*) expr; - if (!this->tryRemoveExpressionBefore(iter, idx->fBase.get())) { - return false; - } - if (!this->tryRemoveExpressionBefore(iter, idx->fIndex.get())) { - return false; - } - *iter = fNodes.erase(*iter); - return true; - } - case Expression::kConstructor_Kind: { - Constructor* c = (Constructor*) expr; - for (auto& arg : c->fArguments) { - if (!this->tryRemoveExpressionBefore(iter, arg.get())) { - return false; - } - ASSERT((*iter)->fKind == BasicBlock::Node::kExpression_Kind && - (*iter)->fExpression->get() == expr); - } - *iter = fNodes.erase(*iter); - return true; - } - case Expression::kFunctionCall_Kind: { - FunctionCall* f = (FunctionCall*) expr; - for (auto& arg : f->fArguments) { - if (!this->tryRemoveExpressionBefore(iter, arg.get())) { - return false; - } - ASSERT((*iter)->fKind == BasicBlock::Node::kExpression_Kind && - (*iter)->fExpression->get() == expr); - } - *iter = fNodes.erase(*iter); - return true; - } - case Expression::kPrefix_Kind: - if (!this->tryRemoveExpressionBefore(iter, - ((PrefixExpression*) expr)->fOperand.get())) { - return false; - } - *iter = fNodes.erase(*iter); - return true; - case Expression::kPostfix_Kind: - if (!this->tryRemoveExpressionBefore(iter, - ((PrefixExpression*) expr)->fOperand.get())) { - return false; - } - *iter = fNodes.erase(*iter); - return true; - case Expression::kBoolLiteral_Kind: // fall through - case Expression::kFloatLiteral_Kind: // fall through - case Expression::kIntLiteral_Kind: // fall through - case Expression::kVariableReference_Kind: - *iter = fNodes.erase(*iter); - return true; - default: - SkDebugf("unhandled expression: %s\n", expr->description().c_str()); - ASSERT(false); - return false; - } -} - -bool BasicBlock::tryInsertExpression(std::vector<BasicBlock::Node>::iterator* iter, - std::unique_ptr<Expression>* expr) { - switch ((*expr)->fKind) { - case Expression::kBinary_Kind: { - BinaryExpression* b = (BinaryExpression*) expr->get(); - if (!this->tryInsertExpression(iter, &b->fRight)) { - return false; - } - ++(*iter); - if (!this->tryInsertExpression(iter, &b->fLeft)) { - return false; - } - ++(*iter); - BasicBlock::Node node = { BasicBlock::Node::kExpression_Kind, true, expr, nullptr }; - *iter = fNodes.insert(*iter, node); - return true; - } - case Expression::kBoolLiteral_Kind: // fall through - case Expression::kFloatLiteral_Kind: // fall through - case Expression::kIntLiteral_Kind: // fall through - case Expression::kVariableReference_Kind: { - BasicBlock::Node node = { BasicBlock::Node::kExpression_Kind, true, expr, nullptr }; - *iter = fNodes.insert(*iter, node); - return true; - } - default: - return false; - } -} - void CFGGenerator::addExpression(CFG& cfg, std::unique_ptr<Expression>* e, bool constantPropagate) { ASSERT(e); switch ((*e)->fKind) { @@ -374,8 +177,6 @@ void CFGGenerator::addExpression(CFG& cfg, std::unique_ptr<Expression>* e, bool case Expression::kTernary_Kind: { TernaryExpression* t = (TernaryExpression*) e->get(); this->addExpression(cfg, &t->fTest, constantPropagate); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, - constantPropagate, e, nullptr }); BlockId start = cfg.fCurrent; cfg.newBlock(); this->addExpression(cfg, &t->fIfTrue, constantPropagate); @@ -417,26 +218,24 @@ void CFGGenerator::addLValue(CFG& cfg, std::unique_ptr<Expression>* e) { } } -void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) { - switch ((*s)->fKind) { +void CFGGenerator::addStatement(CFG& cfg, const Statement* s) { + switch (s->fKind) { case Statement::kBlock_Kind: - for (auto& child : ((Block&) **s).fStatements) { - addStatement(cfg, &child); + for (const auto& child : ((const Block*) s)->fStatements) { + addStatement(cfg, child.get()); } break; case Statement::kIf_Kind: { - IfStatement& ifs = (IfStatement&) **s; - this->addExpression(cfg, &ifs.fTest, true); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, - nullptr, s }); + IfStatement* ifs = (IfStatement*) s; + this->addExpression(cfg, &ifs->fTest, true); BlockId start = cfg.fCurrent; cfg.newBlock(); - this->addStatement(cfg, &ifs.fIfTrue); + this->addStatement(cfg, ifs->fIfTrue.get()); BlockId next = cfg.newBlock(); - if (ifs.fIfFalse) { + if (ifs->fIfFalse) { cfg.fCurrent = start; cfg.newBlock(); - this->addStatement(cfg, &ifs.fIfFalse); + this->addStatement(cfg, ifs->fIfFalse.get()); cfg.addExit(cfg.fCurrent, next); cfg.fCurrent = next; } else { @@ -445,16 +244,14 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) { break; } case Statement::kExpression_Kind: { - this->addExpression(cfg, &((ExpressionStatement&) **s).fExpression, true); - cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, - nullptr, s }); + this->addExpression(cfg, &((ExpressionStatement&) *s).fExpression, true); break; } case Statement::kVarDeclarations_Kind: { - VarDeclarationsStatement& decls = ((VarDeclarationsStatement&) **s); + VarDeclarationsStatement& decls = ((VarDeclarationsStatement&) *s); for (auto& vd : decls.fDeclaration->fVars) { - if (vd->fValue) { - this->addExpression(cfg, &vd->fValue, true); + if (vd.fValue) { + this->addExpression(cfg, &vd.fValue, true); } } cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false, @@ -467,7 +264,7 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) { cfg.fCurrent = cfg.newIsolatedBlock(); break; case Statement::kReturn_Kind: { - ReturnStatement& r = ((ReturnStatement&) **s); + ReturnStatement& r = ((ReturnStatement&) *s); if (r.fExpression) { this->addExpression(cfg, &r.fExpression, true); } @@ -489,16 +286,16 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) { cfg.fCurrent = cfg.newIsolatedBlock(); break; case Statement::kWhile_Kind: { - WhileStatement& w = (WhileStatement&) **s; + WhileStatement* w = (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, true); BlockId test = cfg.fCurrent; cfg.addExit(test, loopExit); cfg.newBlock(); - this->addStatement(cfg, &w.fStatement); + this->addStatement(cfg, w->fStatement.get()); cfg.addExit(cfg.fCurrent, loopStart); fLoopContinues.pop(); fLoopExits.pop(); @@ -506,13 +303,13 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) { break; } case Statement::kDo_Kind: { - DoStatement& d = (DoStatement&) **s; + DoStatement* d = (DoStatement*) s; BlockId loopStart = cfg.newBlock(); fLoopContinues.push(loopStart); BlockId loopExit = cfg.newIsolatedBlock(); fLoopExits.push(loopExit); - this->addStatement(cfg, &d.fStatement); - this->addExpression(cfg, &d.fTest, true); + this->addStatement(cfg, d->fStatement.get()); + this->addExpression(cfg, &d->fTest, true); cfg.addExit(cfg.fCurrent, loopExit); cfg.addExit(cfg.fCurrent, loopStart); fLoopContinues.pop(); @@ -521,26 +318,26 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) { break; } case Statement::kFor_Kind: { - ForStatement& f = (ForStatement&) **s; - if (f.fInitializer) { - this->addStatement(cfg, &f.fInitializer); + ForStatement* f = (ForStatement*) s; + if (f->fInitializer) { + this->addStatement(cfg, f->fInitializer.get()); } BlockId loopStart = cfg.newBlock(); BlockId next = cfg.newIsolatedBlock(); fLoopContinues.push(next); BlockId loopExit = cfg.newIsolatedBlock(); fLoopExits.push(loopExit); - if (f.fTest) { - this->addExpression(cfg, &f.fTest, true); + if (f->fTest) { + this->addExpression(cfg, &f->fTest, true); BlockId test = cfg.fCurrent; cfg.addExit(test, loopExit); } cfg.newBlock(); - this->addStatement(cfg, &f.fStatement); + this->addStatement(cfg, f->fStatement.get()); cfg.addExit(cfg.fCurrent, next); cfg.fCurrent = next; - if (f.fNext) { - this->addExpression(cfg, &f.fNext, true); + if (f->fNext) { + this->addExpression(cfg, &f->fNext, true); } cfg.addExit(cfg.fCurrent, loopStart); fLoopContinues.pop(); @@ -548,19 +345,17 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) { cfg.fCurrent = loopExit; break; } - case Statement::kNop_Kind: - break; default: - printf("statement: %s\n", (*s)->description().c_str()); + printf("statement: %s\n", s->description().c_str()); ABORT("unsupported statement kind"); } } -CFG CFGGenerator::getCFG(FunctionDefinition& f) { +CFG CFGGenerator::getCFG(const FunctionDefinition& f) { CFG result; result.fStart = result.newBlock(); result.fCurrent = result.fStart; - this->addStatement(result, &f.fBody); + this->addStatement(result, f.fBody.get()); result.newBlock(); result.fExit = result.fCurrent; return result; diff --git a/src/sksl/SkSLCFGGenerator.h b/src/sksl/SkSLCFGGenerator.h index 3fb7acbc17..337fdfac35 100644 --- a/src/sksl/SkSLCFGGenerator.h +++ b/src/sksl/SkSLCFGGenerator.h @@ -26,15 +26,6 @@ struct BasicBlock { kExpression_Kind }; - SkString description() const { - if (fKind == kStatement_Kind) { - return (*fStatement)->description(); - } else { - ASSERT(fKind == kExpression_Kind); - return (*fExpression)->description(); - } - } - 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 @@ -44,46 +35,10 @@ struct BasicBlock { // 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; - // we store pointers to the unique_ptrs so that we can replace expressions or statements - // during optimization without having to regenerate the entire tree std::unique_ptr<Expression>* fExpression; - std::unique_ptr<Statement>* fStatement; + const Statement* fStatement; }; - /** - * Attempts to remove the expression (and its subexpressions) pointed to by the iterator. If the - * expression can be cleanly removed, returns true and updates the iterator to point to the - * expression after the deleted expression. Otherwise returns false (and the CFG will need to be - * regenerated). - */ - bool SK_WARN_UNUSED_RESULT tryRemoveExpression(std::vector<BasicBlock::Node>::iterator* iter); - - /** - * Locates and attempts remove an expression occurring before the expression pointed to by iter. - * If the expression can be cleanly removed, returns true and resets iter to a valid iterator - * pointing to the same expression it did initially. Otherwise returns false (and the CFG will - * need to be regenerated). - */ - bool SK_WARN_UNUSED_RESULT tryRemoveExpressionBefore( - std::vector<BasicBlock::Node>::iterator* iter, - Expression* e); - - /** - * As tryRemoveExpressionBefore, but for lvalues. As lvalues are at most partially evaluated - * (for instance, x[i] = 0 evaluates i but not x) this will only look for the parts of the - * lvalue that are actually evaluated. - */ - bool SK_WARN_UNUSED_RESULT tryRemoveLValueBefore(std::vector<BasicBlock::Node>::iterator* iter, - Expression* lvalue); - - /** - * Attempts to inserts a new expression before the node pointed to by iter. If the - * expression can be cleanly inserted, returns true and updates the iterator to point to the - * newly inserted expression. Otherwise returns false (and the CFG will need to be regenerated). - */ - bool SK_WARN_UNUSED_RESULT tryInsertExpression(std::vector<BasicBlock::Node>::iterator* iter, - std::unique_ptr<Expression>* expr); - std::vector<Node> fNodes; std::set<BlockId> fEntrances; std::set<BlockId> fExits; @@ -126,10 +81,10 @@ class CFGGenerator { public: CFGGenerator() {} - CFG getCFG(FunctionDefinition& f); + CFG getCFG(const FunctionDefinition& f); private: - void addStatement(CFG& cfg, std::unique_ptr<Statement>* s); + void addStatement(CFG& cfg, const Statement* s); void addExpression(CFG& cfg, std::unique_ptr<Expression>* e, bool constantPropagate); diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 92261a489b..743745ad14 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -14,12 +14,9 @@ #include "SkSLParser.h" #include "SkSLSPIRVCodeGenerator.h" #include "ir/SkSLExpression.h" -#include "ir/SkSLExpressionStatement.h" #include "ir/SkSLIntLiteral.h" #include "ir/SkSLModifiersDeclaration.h" -#include "ir/SkSLNop.h" #include "ir/SkSLSymbolTable.h" -#include "ir/SkSLTernaryExpression.h" #include "ir/SkSLUnresolvedFunction.h" #include "ir/SkSLVarDeclarations.h" #include "SkMutex.h" @@ -236,30 +233,22 @@ void Compiler::addDefinitions(const BasicBlock::Node& node, p->fOperand.get(), (std::unique_ptr<Expression>*) &fContext.fDefined_Expression, definitions); + } break; } - case Expression::kVariableReference_Kind: { - const VariableReference* v = (VariableReference*) expr; - if (v->fRefKind != VariableReference::kRead_RefKind) { - this->addDefinition( - v, - (std::unique_ptr<Expression>*) &fContext.fDefined_Expression, - definitions); - } - } default: break; } break; } case BasicBlock::Node::kStatement_Kind: { - const Statement* stmt = (Statement*) node.fStatement->get(); + const Statement* stmt = (Statement*) node.fStatement; if (stmt->fKind == Statement::kVarDeclarations_Kind) { VarDeclarationsStatement* vd = (VarDeclarationsStatement*) stmt; - for (const auto& decl : vd->fDeclaration->fVars) { - if (decl->fValue) { - (*definitions)[decl->fVar] = &decl->fValue; + for (VarDeclaration& decl : vd->fDeclaration->fVars) { + if (decl.fValue) { + (*definitions)[decl.fVar] = &decl.fValue; } } } @@ -309,11 +298,11 @@ static DefinitionMap compute_start_state(const CFG& cfg) { for (const auto& node : block.fNodes) { if (node.fKind == BasicBlock::Node::kStatement_Kind) { ASSERT(node.fStatement); - const Statement* s = node.fStatement->get(); + const Statement* s = node.fStatement; if (s->fKind == Statement::kVarDeclarations_Kind) { const VarDeclarationsStatement* vd = (const VarDeclarationsStatement*) s; - for (const auto& decl : vd->fDeclaration->fVars) { - result[decl->fVar] = nullptr; + for (const VarDeclaration& decl : vd->fDeclaration->fVars) { + result[decl.fVar] = nullptr; } } } @@ -322,74 +311,20 @@ static DefinitionMap compute_start_state(const CFG& cfg) { return result; } -/** - * Returns true if assigning to this lvalue has no effect. - */ -static bool is_dead(const Expression& lvalue) { - switch (lvalue.fKind) { - case Expression::kVariableReference_Kind: - return ((VariableReference&) lvalue).fVariable.dead(); - case Expression::kSwizzle_Kind: - return is_dead(*((Swizzle&) lvalue).fBase); - case Expression::kFieldAccess_Kind: - return is_dead(*((FieldAccess&) lvalue).fBase); - case Expression::kIndex_Kind: { - const IndexExpression& idx = (IndexExpression&) lvalue; - return is_dead(*idx.fBase) && !idx.fIndex->hasSideEffects(); - } - default: - SkDebugf("invalid lvalue: %s\n", lvalue.description().c_str()); - ASSERT(false); - return false; - } -} - -/** - * Returns true if this is an assignment which can be collapsed down to just the right hand side due - * to a dead target and lack of side effects on the left hand side. - */ -static bool dead_assignment(const BinaryExpression& b) { - if (!Token::IsAssignment(b.fOperator)) { - return false; - } - return is_dead(*b.fLeft); -} +void Compiler::scanCFG(const FunctionDefinition& f) { + CFG cfg = CFGGenerator().getCFG(f); -void Compiler::computeDataFlow(CFG* cfg) { - cfg->fBlocks[cfg->fStart].fBefore = compute_start_state(*cfg); + // compute the data flow + cfg.fBlocks[cfg.fStart].fBefore = compute_start_state(cfg); std::set<BlockId> workList; - for (BlockId i = 0; i < cfg->fBlocks.size(); i++) { + for (BlockId i = 0; i < cfg.fBlocks.size(); i++) { workList.insert(i); } while (workList.size()) { BlockId next = *workList.begin(); workList.erase(workList.begin()); - this->scanCFG(cfg, next, &workList); - } -} - - -/** - * Attempts to replace the expression pointed to by iter with a new one (in both the CFG and the - * IR). If the expression can be cleanly removed, returns true and updates the iterator to point to - * the newly-inserted element. Otherwise updates only the IR and returns false (and the CFG will - * need to be regenerated). - */ -bool SK_WARN_UNUSED_RESULT try_replace_expression(BasicBlock* b, - std::vector<BasicBlock::Node>::iterator* iter, - std::unique_ptr<Expression> newExpression) { - std::unique_ptr<Expression>* target = (*iter)->fExpression; - if (!b->tryRemoveExpression(iter)) { - *target = std::move(newExpression); - return false; + this->scanCFG(&cfg, next, &workList); } - *target = std::move(newExpression); - return b->tryInsertExpression( iter, target); -} - -void Compiler::scanCFG(FunctionDefinition& f) { - CFG cfg = CFGGenerator().getCFG(f); - this->computeDataFlow(&cfg); // check for unreachable code for (size_t i = 0; i < cfg.fBlocks.size(); i++) { @@ -398,7 +333,7 @@ void Compiler::scanCFG(FunctionDefinition& f) { Position p; switch (cfg.fBlocks[i].fNodes[0].fKind) { case BasicBlock::Node::kStatement_Kind: - p = (*cfg.fBlocks[i].fNodes[0].fStatement)->fPosition; + p = cfg.fBlocks[i].fNodes[0].fStatement->fPosition; break; case BasicBlock::Node::kExpression_Kind: p = (*cfg.fBlocks[i].fNodes[0].fExpression)->fPosition; @@ -411,170 +346,33 @@ void Compiler::scanCFG(FunctionDefinition& f) { return; } - // check for dead code & undefined variables, perform constant propagation - std::unordered_set<const Variable*> undefinedVariables; - bool updated; - bool needsRescan = false; - do { - if (needsRescan) { - cfg = CFGGenerator().getCFG(f); - this->computeDataFlow(&cfg); - needsRescan = false; - } - - updated = false; - for (BasicBlock& b : cfg.fBlocks) { - DefinitionMap definitions = b.fBefore; - - for (auto iter = b.fNodes.begin(); iter != b.fNodes.end() && !needsRescan; ++iter) { - if (iter->fKind == BasicBlock::Node::kExpression_Kind) { - Expression* expr = iter->fExpression->get(); - ASSERT(expr); - if (iter->fConstantPropagation) { - std::unique_ptr<Expression> optimized = expr->constantPropagate( - *fIRGenerator, - definitions); - if (optimized) { - if (!try_replace_expression(&b, &iter, std::move(optimized))) { - needsRescan = true; - } - ASSERT(iter->fKind == BasicBlock::Node::kExpression_Kind); - expr = iter->fExpression->get(); - updated = true; - } - } - switch (expr->fKind) { - case Expression::kVariableReference_Kind: { - const Variable& var = ((VariableReference*) expr)->fVariable; - if (var.fStorage == Variable::kLocal_Storage && - !definitions[&var] && - undefinedVariables.find(&var) == undefinedVariables.end()) { - undefinedVariables.insert(&var); - this->error(expr->fPosition, - "'" + var.fName + "' has not been assigned"); - } - break; - } - case Expression::kTernary_Kind: { - TernaryExpression* t = (TernaryExpression*) expr; - if (t->fTest->fKind == Expression::kBoolLiteral_Kind) { - // ternary has a constant test, replace it with either the true or - // false branch - if (((BoolLiteral&) *t->fTest).fValue) { - *iter->fExpression = std::move(t->fIfTrue); - } else { - *iter->fExpression = std::move(t->fIfFalse); - } - updated = true; - needsRescan = true; - } - break; - } - default: - break; + // check for undefined variables, perform constant propagation + for (BasicBlock& b : cfg.fBlocks) { + DefinitionMap definitions = b.fBefore; + for (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<Expression> optimized = expr->constantPropagate(*fIRGenerator, + definitions); + if (optimized) { + n.fExpression->reset(optimized.release()); + expr = n.fExpression->get(); } - } else { - ASSERT(iter->fKind == BasicBlock::Node::kStatement_Kind); - Statement* stmt = iter->fStatement->get(); - switch (stmt->fKind) { - case Statement::kVarDeclarations_Kind: { - VarDeclarations& vd = *((VarDeclarationsStatement&) *stmt).fDeclaration; - for (auto varIter = vd.fVars.begin(); varIter != vd.fVars.end(); ) { - const auto& varDecl = **varIter; - if (varDecl.fVar->dead() && - (!varDecl.fValue || - !varDecl.fValue->hasSideEffects())) { - if (varDecl.fValue) { - ASSERT(iter->fKind == BasicBlock::Node::kStatement_Kind && - iter->fStatement->get() == stmt); - if (!b.tryRemoveExpressionBefore( - &iter, - varDecl.fValue.get())) { - needsRescan = true; - } - } - varIter = vd.fVars.erase(varIter); - updated = true; - } else { - ++varIter; - } - } - if (vd.fVars.size() == 0) { - iter->fStatement->reset(new Nop()); - } - break; - } - case Statement::kIf_Kind: { - IfStatement& i = (IfStatement&) *stmt; - if (i.fIfFalse && i.fIfFalse->isEmpty()) { - // else block doesn't do anything, remove it - i.fIfFalse.reset(); - updated = true; - needsRescan = true; - } - if (!i.fIfFalse && i.fIfTrue->isEmpty()) { - // if block doesn't do anything, no else block - if (i.fTest->hasSideEffects()) { - // test has side effects, keep it - iter->fStatement->reset(new ExpressionStatement( - std::move(i.fTest))); - } else { - // no if, no else, no test side effects, kill the whole if - // statement - iter->fStatement->reset(new Nop()); - } - updated = true; - needsRescan = true; - } - break; - } - case Statement::kExpression_Kind: { - ExpressionStatement& e = (ExpressionStatement&) *stmt; - ASSERT(iter->fStatement->get() == &e); - if (e.fExpression->fKind == Expression::kBinary_Kind) { - BinaryExpression& bin = (BinaryExpression&) *e.fExpression; - if (dead_assignment(bin)) { - if (!b.tryRemoveExpressionBefore(&iter, &bin)) { - needsRescan = true; - } - if (bin.fRight->hasSideEffects()) { - // still have to evaluate the right due to side effects, - // replace the binary expression with just the right side - e.fExpression = std::move(bin.fRight); - if (!b.tryInsertExpression(&iter, &e.fExpression)) { - needsRescan = true; - } - } else { - // no side effects, kill the whole statement - ASSERT(iter->fKind == BasicBlock::Node::kStatement_Kind && - iter->fStatement->get() == stmt); - iter->fStatement->reset(new Nop()); - } - updated = true; - break; - } - } - if (!e.fExpression->hasSideEffects()) { - // Expression statement with no side effects, kill it - if (!b.tryRemoveExpressionBefore(&iter, e.fExpression.get())) { - needsRescan = true; - } - ASSERT(iter->fKind == BasicBlock::Node::kStatement_Kind && - iter->fStatement->get() == stmt); - iter->fStatement->reset(new Nop()); - updated = true; - } - break; - } - default: - break; + } + if (expr->fKind == Expression::kVariableReference_Kind) { + const Variable& var = ((VariableReference*) expr)->fVariable; + if (var.fStorage == Variable::kLocal_Storage && + !definitions[&var]) { + this->error(expr->fPosition, + "'" + var.fName + "' has not been assigned"); } } - this->addDefinitions(*iter, &definitions); } + this->addDefinitions(n, &definitions); } - } while (updated); - ASSERT(!needsRescan); + } // check for missing return if (f.fDeclaration.fReturnType != *fContext.fVoid_Type) { diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h index 0b915632d7..fdca12d2cf 100644 --- a/src/sksl/SkSLCompiler.h +++ b/src/sksl/SkSLCompiler.h @@ -67,9 +67,7 @@ private: void scanCFG(CFG* cfg, BlockId block, std::set<BlockId>* workList); - void computeDataFlow(CFG* cfg); - - void scanCFG(FunctionDefinition& f); + void scanCFG(const FunctionDefinition& f); void internalConvertProgram(SkString text, Modifiers::Flag* defaultPrecision, diff --git a/src/sksl/SkSLContext.h b/src/sksl/SkSLContext.h index 8fced1cd7a..18de3367bc 100644 --- a/src/sksl/SkSLContext.h +++ b/src/sksl/SkSLContext.h @@ -272,11 +272,7 @@ private: Defined(const Type& type) : INHERITED(Position(), kDefined_Kind, type) {} - bool hasSideEffects() const override { - return false; - } - - SkString description() const override { + virtual SkString description() const override { return SkString("<defined>"); } diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp index 47225e13d0..dd8c0cf312 100644 --- a/src/sksl/SkSLGLSLCodeGenerator.cpp +++ b/src/sksl/SkSLGLSLCodeGenerator.cpp @@ -16,7 +16,6 @@ #include "ir/SkSLExtension.h" #include "ir/SkSLIndexExpression.h" #include "ir/SkSLModifiersDeclaration.h" -#include "ir/SkSLNop.h" #include "ir/SkSLVariableReference.h" namespace SkSL { @@ -494,7 +493,10 @@ void GLSLCodeGenerator::writeFunction(const FunctionDefinition& f) { SkDynamicMemoryWStream buffer; fOut = &buffer; fIndentation++; - this->writeStatements(((Block&) *f.fBody).fStatements); + for (const auto& s : f.fBody->fStatements) { + this->writeStatement(*s); + this->writeLine(); + } fIndentation--; this->writeLine("}"); @@ -587,26 +589,26 @@ void GLSLCodeGenerator::writeInterfaceBlock(const InterfaceBlock& intf) { void GLSLCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, bool global) { ASSERT(decl.fVars.size() > 0); - this->writeModifiers(decl.fVars[0]->fVar->fModifiers, global); + this->writeModifiers(decl.fVars[0].fVar->fModifiers, global); this->writeType(decl.fBaseType); SkString separator(" "); for (const auto& var : decl.fVars) { - ASSERT(var->fVar->fModifiers == decl.fVars[0]->fVar->fModifiers); + ASSERT(var.fVar->fModifiers == decl.fVars[0].fVar->fModifiers); this->write(separator); separator = SkString(", "); - this->write(var->fVar->fName); - for (const auto& size : var->fSizes) { + this->write(var.fVar->fName); + for (const auto& size : var.fSizes) { this->write("["); if (size) { this->writeExpression(*size, kTopLevel_Precedence); } this->write("]"); } - if (var->fValue) { + if (var.fValue) { this->write(" = "); - this->writeExpression(*var->fValue, kTopLevel_Precedence); + this->writeExpression(*var.fValue, kTopLevel_Precedence); } - if (!fFoundImageDecl && var->fVar->fType == *fContext.fImage2D_Type) { + if (!fFoundImageDecl && var.fVar->fType == *fContext.fImage2D_Type) { if (fProgram.fSettings.fCaps->imageLoadStoreExtensionString()) { fHeader.writeText("#extension "); fHeader.writeText(fProgram.fSettings.fCaps->imageLoadStoreExtensionString()); @@ -654,27 +656,18 @@ void GLSLCodeGenerator::writeStatement(const Statement& s) { case Statement::kDiscard_Kind: this->write("discard;"); break; - case Statement::kNop_Kind: - this->write(";"); - break; default: ABORT("unsupported statement: %s", s.description().c_str()); } } -void GLSLCodeGenerator::writeStatements(const std::vector<std::unique_ptr<Statement>>& statements) { - for (const auto& s : statements) { - if (!s->isEmpty()) { - this->writeStatement(*s); - this->writeLine(); - } - } -} - void GLSLCodeGenerator::writeBlock(const Block& b) { this->writeLine("{"); fIndentation++; - this->writeStatements(b.fStatements); + for (const auto& s : b.fStatements) { + this->writeStatement(*s); + this->writeLine(); + } fIndentation--; this->write("}"); } @@ -770,7 +763,7 @@ bool GLSLCodeGenerator::generateCode() { case ProgramElement::kVar_Kind: { VarDeclarations& decl = (VarDeclarations&) *e; if (decl.fVars.size() > 0) { - int builtin = decl.fVars[0]->fVar->fModifiers.fLayout.fBuiltin; + int builtin = decl.fVars[0].fVar->fModifiers.fLayout.fBuiltin; if (builtin == -1) { // normal var this->writeVarDeclarations(decl, true); diff --git a/src/sksl/SkSLGLSLCodeGenerator.h b/src/sksl/SkSLGLSLCodeGenerator.h index 73c8b6fd35..0ae2c5c585 100644 --- a/src/sksl/SkSLGLSLCodeGenerator.h +++ b/src/sksl/SkSLGLSLCodeGenerator.h @@ -145,8 +145,6 @@ private: void writeStatement(const Statement& s); - void writeStatements(const std::vector<std::unique_ptr<Statement>>& statements); - void writeBlock(const Block& b); void writeIfStatement(const IfStatement& stmt); diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index b9a30d3971..55d9d2c8d6 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -190,7 +190,7 @@ std::unique_ptr<Statement> IRGenerator::convertVarDeclarationStatement( std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVarDeclarations& decl, Variable::Storage storage) { - std::vector<std::unique_ptr<VarDeclaration>> variables; + std::vector<VarDeclaration> variables; const Type* baseType = this->convertType(*decl.fType); if (!baseType) { return nullptr; @@ -235,7 +235,6 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVa return nullptr; } value = this->coerce(std::move(value), *type); - var->fWriteCount = 1; } if (storage == Variable::kGlobal_Storage && varDecl.fName == SkString("sk_FragColor") && (*fSymbolTable)[varDecl.fName]) { @@ -247,8 +246,7 @@ std::unique_ptr<VarDeclarations> IRGenerator::convertVarDeclarations(const ASTVa Variable* old = (Variable*) (*fSymbolTable)[varDecl.fName]; old->fModifiers = var->fModifiers; } else { - variables.emplace_back(new VarDeclaration(var.get(), std::move(sizes), - std::move(value))); + variables.emplace_back(var.get(), std::move(sizes), std::move(value)); fSymbolTable->add(varDecl.fName, std::move(var)); } } @@ -537,16 +535,16 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte return nullptr; } for (const auto& var : decl->fVars) { - fields.push_back(Type::Field(var->fVar->fModifiers, var->fVar->fName, - &var->fVar->fType)); - if (var->fValue) { + fields.push_back(Type::Field(var.fVar->fModifiers, var.fVar->fName, + &var.fVar->fType)); + if (var.fValue) { fErrors.error(decl->fPosition, "initializers are not permitted on interface block fields"); } - if (var->fVar->fModifiers.fFlags & (Modifiers::kIn_Flag | - Modifiers::kOut_Flag | - Modifiers::kUniform_Flag | - Modifiers::kConst_Flag)) { + if (var.fVar->fModifiers.fFlags & (Modifiers::kIn_Flag | + Modifiers::kOut_Flag | + Modifiers::kUniform_Flag | + Modifiers::kConst_Flag)) { fErrors.error(decl->fPosition, "interface block fields may not have storage qualifiers"); } @@ -1030,8 +1028,7 @@ std::unique_ptr<Expression> IRGenerator::call(Position position, return nullptr; } if (arguments[i] && (function.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag)) { - this->markWrittenTo(*arguments[i], - function.fParameters[i]->fModifiers.fFlags & Modifiers::kIn_Flag); + this->markWrittenTo(*arguments[i], true); } } return std::unique_ptr<FunctionCall>(new FunctionCall(position, *returnType, function, diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp index 7352cf2ab9..9c8a5d0001 100644 --- a/src/sksl/SkSLSPIRVCodeGenerator.cpp +++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp @@ -2430,7 +2430,7 @@ SpvId SPIRVCodeGenerator::writeFunction(const FunctionDefinition& f, SkWStream& write_data(*fGlobalInitializersBuffer.detachAsData(), out); } SkDynamicMemoryWStream bodyBuffer; - this->writeBlock((Block&) *f.fBody, bodyBuffer); + this->writeBlock(*f.fBody, bodyBuffer); write_data(*fVariableBuffer.detachAsData(), out); write_data(*bodyBuffer.detachAsData(), out); if (fCurrentBlock) { @@ -2524,7 +2524,7 @@ SpvId SPIRVCodeGenerator::writeInterfaceBlock(const InterfaceBlock& intf) { void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclarations& decl, SkWStream& out) { for (size_t i = 0; i < decl.fVars.size(); i++) { - const VarDeclaration& varDecl = *decl.fVars[i]; + const VarDeclaration& varDecl = decl.fVars[i]; const Variable* var = varDecl.fVar; // These haven't been implemented in our SPIR-V generator yet and we only currently use them // in the OpenGL backend. @@ -2586,7 +2586,7 @@ void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclaratio void SPIRVCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, SkWStream& out) { for (const auto& varDecl : decl.fVars) { - const Variable* var = varDecl->fVar; + const Variable* var = varDecl.fVar; // These haven't been implemented in our SPIR-V generator yet and we only currently use them // in the OpenGL backend. ASSERT(!(var->fModifiers.fFlags & (Modifiers::kReadOnly_Flag | @@ -2599,8 +2599,8 @@ void SPIRVCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, SkWSt SpvId type = this->getPointerType(var->fType, SpvStorageClassFunction); this->writeInstruction(SpvOpVariable, type, id, SpvStorageClassFunction, fVariableBuffer); this->writeInstruction(SpvOpName, id, var->fName.c_str(), fNameBuffer); - if (varDecl->fValue) { - SpvId value = this->writeExpression(*varDecl->fValue, out); + if (varDecl.fValue) { + SpvId value = this->writeExpression(*varDecl.fValue, out); this->writeInstruction(SpvOpStore, id, value, out); } } @@ -2608,8 +2608,6 @@ void SPIRVCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, SkWSt void SPIRVCodeGenerator::writeStatement(const Statement& s, SkWStream& out) { switch (s.fKind) { - case Statement::kNop_Kind: - break; case Statement::kBlock_Kind: this->writeBlock((Block&) s, out); break; diff --git a/src/sksl/ir/SkSLBinaryExpression.h b/src/sksl/ir/SkSLBinaryExpression.h index 4837d18093..de85e4812b 100644 --- a/src/sksl/ir/SkSLBinaryExpression.h +++ b/src/sksl/ir/SkSLBinaryExpression.h @@ -34,11 +34,6 @@ struct BinaryExpression : public Expression { *fRight); } - virtual bool hasSideEffects() const override { - return Token::IsAssignment(fOperator) || fLeft->hasSideEffects() || - fRight->hasSideEffects(); - } - virtual SkString description() const override { return "(" + fLeft->description() + " " + Token::OperatorName(fOperator) + " " + fRight->description() + ")"; diff --git a/src/sksl/ir/SkSLBlock.h b/src/sksl/ir/SkSLBlock.h index f00c146725..17970fd561 100644 --- a/src/sksl/ir/SkSLBlock.h +++ b/src/sksl/ir/SkSLBlock.h @@ -23,15 +23,6 @@ struct Block : public Statement { , fSymbols(std::move(symbols)) , fStatements(std::move(statements)) {} - virtual bool isEmpty() const override { - for (const auto& s : fStatements) { - if (!s->isEmpty()) { - return false; - } - } - return true; - } - SkString description() const override { SkString result("{"); for (size_t i = 0; i < fStatements.size(); i++) { @@ -45,7 +36,7 @@ struct Block : public Statement { // 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; - std::vector<std::unique_ptr<Statement>> fStatements; + const std::vector<std::unique_ptr<Statement>> fStatements; typedef Statement INHERITED; }; diff --git a/src/sksl/ir/SkSLBoolLiteral.h b/src/sksl/ir/SkSLBoolLiteral.h index 5ca7c7428a..b372f2f3ff 100644 --- a/src/sksl/ir/SkSLBoolLiteral.h +++ b/src/sksl/ir/SkSLBoolLiteral.h @@ -25,10 +25,6 @@ struct BoolLiteral : public Expression { return SkString(fValue ? "true" : "false"); } - bool hasSideEffects() const override { - return false; - } - bool isConstant() const override { return true; } diff --git a/src/sksl/ir/SkSLConstructor.h b/src/sksl/ir/SkSLConstructor.h index 5ecb74eb72..691bea123a 100644 --- a/src/sksl/ir/SkSLConstructor.h +++ b/src/sksl/ir/SkSLConstructor.h @@ -24,36 +24,20 @@ struct Constructor : public Expression { : INHERITED(position, kConstructor_Kind, type) , fArguments(std::move(arguments)) {} - std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator, - const DefinitionMap& definitions) override { - if (fArguments.size() == 1 && fArguments[0]->fKind == Expression::kIntLiteral_Kind) { - if (fType == *irGenerator.fContext.fFloat_Type) { - // promote float(1) to 1.0 - int64_t intValue = ((IntLiteral&) *fArguments[0]).fValue; - return std::unique_ptr<Expression>(new FloatLiteral(irGenerator.fContext, - fPosition, - intValue)); - } else if (fType == *irGenerator.fContext.fUInt_Type) { - // promote uint(1) to 1u - int64_t intValue = ((IntLiteral&) *fArguments[0]).fValue; - return std::unique_ptr<Expression>(new IntLiteral(irGenerator.fContext, - fPosition, - intValue, - &fType)); - } + 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; } - bool hasSideEffects() const override { - for (const auto& arg : fArguments) { - if (arg->hasSideEffects()) { - return true; - } - } - return false; - } - SkString description() const override { SkString result = fType.description() + "("; SkString separator; diff --git a/src/sksl/ir/SkSLDoStatement.h b/src/sksl/ir/SkSLDoStatement.h index 1b233f9889..e26d3dc974 100644 --- a/src/sksl/ir/SkSLDoStatement.h +++ b/src/sksl/ir/SkSLDoStatement.h @@ -27,7 +27,7 @@ struct DoStatement : public Statement { return "do " + fStatement->description() + " while (" + fTest->description() + ");"; } - std::unique_ptr<Statement> fStatement; + const std::unique_ptr<Statement> fStatement; std::unique_ptr<Expression> fTest; typedef Statement INHERITED; diff --git a/src/sksl/ir/SkSLExpression.h b/src/sksl/ir/SkSLExpression.h index 5db9ddf96f..f87d810fc0 100644 --- a/src/sksl/ir/SkSLExpression.h +++ b/src/sksl/ir/SkSLExpression.h @@ -53,13 +53,6 @@ struct Expression : public IRNode { } /** - * Returns true if evaluating the expression potentially has side effects. Expressions may never - * return false if they actually have side effects, but it is legal (though suboptimal) to - * return true if there are not actually any side effects. - */ - virtual bool hasSideEffects() const = 0; - - /** * 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. diff --git a/src/sksl/ir/SkSLFieldAccess.h b/src/sksl/ir/SkSLFieldAccess.h index b4f5695b63..de26a3f626 100644 --- a/src/sksl/ir/SkSLFieldAccess.h +++ b/src/sksl/ir/SkSLFieldAccess.h @@ -24,18 +24,14 @@ struct FieldAccess : public Expression { kAnonymousInterfaceBlock_OwnerKind }; - FieldAccess(std::unique_ptr<Expression> base, int fieldIndex, + FieldAccess(std::unique_ptr<Expression> base, int fieldIndex, OwnerKind ownerKind = kDefault_OwnerKind) : INHERITED(base->fPosition, kFieldAccess_Kind, *base->fType.fields()[fieldIndex].fType) , fBase(std::move(base)) , fFieldIndex(fieldIndex) , fOwnerKind(ownerKind) {} - bool hasSideEffects() const override { - return fBase->hasSideEffects(); - } - - SkString description() const override { + virtual SkString description() const override { return fBase->description() + "." + fBase->fType.fields()[fFieldIndex].fName; } diff --git a/src/sksl/ir/SkSLFloatLiteral.h b/src/sksl/ir/SkSLFloatLiteral.h index 8fdf1af603..8a1a5ad63a 100644 --- a/src/sksl/ir/SkSLFloatLiteral.h +++ b/src/sksl/ir/SkSLFloatLiteral.h @@ -21,14 +21,10 @@ struct FloatLiteral : public Expression { : INHERITED(position, kFloatLiteral_Kind, *context.fFloat_Type) , fValue(value) {} - SkString description() const override { + virtual SkString description() const override { return to_string(fValue); } - bool hasSideEffects() const override { - return false; - } - bool isConstant() const override { return true; } diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h index 4f2228a42f..f2bf880ddd 100644 --- a/src/sksl/ir/SkSLForStatement.h +++ b/src/sksl/ir/SkSLForStatement.h @@ -48,10 +48,10 @@ struct ForStatement : public Statement { // it's important to keep fSymbols defined first (and thus destroyed last) because destroying // the other fields can update symbol reference counts const std::shared_ptr<SymbolTable> fSymbols; - std::unique_ptr<Statement> fInitializer; + const std::unique_ptr<Statement> fInitializer; std::unique_ptr<Expression> fTest; std::unique_ptr<Expression> fNext; - std::unique_ptr<Statement> fStatement; + const std::unique_ptr<Statement> fStatement; typedef Statement INHERITED; }; diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h index 73a174bc2d..1838076796 100644 --- a/src/sksl/ir/SkSLFunctionCall.h +++ b/src/sksl/ir/SkSLFunctionCall.h @@ -23,21 +23,6 @@ struct FunctionCall : public Expression { , fFunction(std::move(function)) , fArguments(std::move(arguments)) {} - bool hasSideEffects() const override { - if (!fFunction.fBuiltin) { - // conservatively assume that user-defined functions have side effects - return true; - } - for (const auto& arg : fArguments) { - if (arg->hasSideEffects()) { - return true; - } - } - // Note that we are assuming no builtin functions have side effects. This is true for the - // moment, but could change as our support for GLSL functions expands. - return false; - } - SkString description() const override { SkString result = fFunction.fName + "("; SkString separator; diff --git a/src/sksl/ir/SkSLFunctionDefinition.h b/src/sksl/ir/SkSLFunctionDefinition.h index 7e7b115188..bae882525a 100644 --- a/src/sksl/ir/SkSLFunctionDefinition.h +++ b/src/sksl/ir/SkSLFunctionDefinition.h @@ -18,8 +18,8 @@ namespace SkSL { * A function definition (a declaration plus an associated block of code). */ struct FunctionDefinition : public ProgramElement { - FunctionDefinition(Position position, const FunctionDeclaration& declaration, - std::unique_ptr<Statement> body) + FunctionDefinition(Position position, const FunctionDeclaration& declaration, + std::unique_ptr<Block> body) : INHERITED(position, kFunction_Kind) , fDeclaration(declaration) , fBody(std::move(body)) {} @@ -29,7 +29,7 @@ struct FunctionDefinition : public ProgramElement { } const FunctionDeclaration& fDeclaration; - std::unique_ptr<Statement> fBody; + const std::unique_ptr<Block> fBody; typedef ProgramElement INHERITED; }; diff --git a/src/sksl/ir/SkSLFunctionReference.h b/src/sksl/ir/SkSLFunctionReference.h index 1465de9bf3..ec1fc3804c 100644 --- a/src/sksl/ir/SkSLFunctionReference.h +++ b/src/sksl/ir/SkSLFunctionReference.h @@ -23,10 +23,6 @@ struct FunctionReference : public Expression { : INHERITED(position, kFunctionReference_Kind, *context.fInvalid_Type) , fFunctions(function) {} - bool hasSideEffects() const override { - return false; - } - virtual SkString description() const override { ASSERT(false); return SkString("<function>"); diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h index ee2612c305..8667e932ec 100644 --- a/src/sksl/ir/SkSLIfStatement.h +++ b/src/sksl/ir/SkSLIfStatement.h @@ -33,9 +33,8 @@ struct IfStatement : public Statement { } std::unique_ptr<Expression> fTest; - std::unique_ptr<Statement> fIfTrue; - // may be null - std::unique_ptr<Statement> fIfFalse; + const std::unique_ptr<Statement> fIfTrue; + const std::unique_ptr<Statement> fIfFalse; typedef Statement INHERITED; }; diff --git a/src/sksl/ir/SkSLIndexExpression.h b/src/sksl/ir/SkSLIndexExpression.h index 0b6879378d..d255c7daf6 100644 --- a/src/sksl/ir/SkSLIndexExpression.h +++ b/src/sksl/ir/SkSLIndexExpression.h @@ -43,7 +43,7 @@ static const Type& index_type(const Context& context, const Type& type) { * An expression which extracts a value from an array or matrix, as in 'm[2]'. */ struct IndexExpression : public Expression { - IndexExpression(const Context& context, std::unique_ptr<Expression> base, + IndexExpression(const Context& context, std::unique_ptr<Expression> base, std::unique_ptr<Expression> index) : INHERITED(base->fPosition, kIndex_Kind, index_type(context, base->fType)) , fBase(std::move(base)) @@ -51,10 +51,6 @@ struct IndexExpression : public Expression { ASSERT(fIndex->fType == *context.fInt_Type || fIndex->fType == *context.fUInt_Type); } - bool hasSideEffects() const override { - return fBase->hasSideEffects() || fIndex->hasSideEffects(); - } - SkString description() const override { return fBase->description() + "[" + fIndex->description() + "]"; } diff --git a/src/sksl/ir/SkSLIntLiteral.h b/src/sksl/ir/SkSLIntLiteral.h index 2488a6a1dc..23325e65fb 100644 --- a/src/sksl/ir/SkSLIntLiteral.h +++ b/src/sksl/ir/SkSLIntLiteral.h @@ -22,15 +22,11 @@ struct IntLiteral : public Expression { : INHERITED(position, kIntLiteral_Kind, type ? *type : *context.fInt_Type) , fValue(value) {} - SkString description() const override { + virtual SkString description() const override { return to_string(fValue); } - bool hasSideEffects() const override { - return false; - } - - bool isConstant() const override { + bool isConstant() const override { return true; } diff --git a/src/sksl/ir/SkSLNop.h b/src/sksl/ir/SkSLNop.h deleted file mode 100644 index 6b13e140b6..0000000000 --- a/src/sksl/ir/SkSLNop.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2016 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef SKSL_NOP -#define SKSL_NOP - -#include "SkSLStatement.h" -#include "SkSLSymbolTable.h" - -namespace SkSL { - -/** - * A no-op statement that does nothing. - */ -struct Nop : public Statement { - Nop() - : INHERITED(Position(), kNop_Kind) {} - - virtual bool isEmpty() const override { - return true; - } - - SkString description() const override { - return SkString(";"); - } - - typedef Statement INHERITED; -}; - -} // namespace - -#endif diff --git a/src/sksl/ir/SkSLPostfixExpression.h b/src/sksl/ir/SkSLPostfixExpression.h index 1c400f9fee..6c9fafe5a0 100644 --- a/src/sksl/ir/SkSLPostfixExpression.h +++ b/src/sksl/ir/SkSLPostfixExpression.h @@ -21,11 +21,7 @@ struct PostfixExpression : public Expression { , fOperand(std::move(operand)) , fOperator(op) {} - bool hasSideEffects() const override { - return true; - } - - SkString description() const override { + virtual SkString description() const override { return fOperand->description() + Token::OperatorName(fOperator); } diff --git a/src/sksl/ir/SkSLPrefixExpression.h b/src/sksl/ir/SkSLPrefixExpression.h index 6138bb523b..b7db99a0a4 100644 --- a/src/sksl/ir/SkSLPrefixExpression.h +++ b/src/sksl/ir/SkSLPrefixExpression.h @@ -21,12 +21,7 @@ struct PrefixExpression : public Expression { , fOperand(std::move(operand)) , fOperator(op) {} - bool hasSideEffects() const override { - return fOperator == Token::PLUSPLUS || fOperator == Token::MINUSMINUS || - fOperand->hasSideEffects(); - } - - SkString description() const override { + virtual SkString description() const override { return Token::OperatorName(fOperator) + fOperand->description(); } diff --git a/src/sksl/ir/SkSLStatement.h b/src/sksl/ir/SkSLStatement.h index 02caac1f6c..012311fdd3 100644 --- a/src/sksl/ir/SkSLStatement.h +++ b/src/sksl/ir/SkSLStatement.h @@ -25,9 +25,7 @@ struct Statement : public IRNode { kDo_Kind, kExpression_Kind, kFor_Kind, - kGroup_Kind, kIf_Kind, - kNop_Kind, kReturn_Kind, kVarDeclarations_Kind, kWhile_Kind @@ -37,10 +35,6 @@ struct Statement : public IRNode { : INHERITED(position) , fKind(kind) {} - virtual bool isEmpty() const { - return false; - } - const Kind fKind; typedef IRNode INHERITED; diff --git a/src/sksl/ir/SkSLSwizzle.h b/src/sksl/ir/SkSLSwizzle.h index a77397aa14..8ad9001ada 100644 --- a/src/sksl/ir/SkSLSwizzle.h +++ b/src/sksl/ir/SkSLSwizzle.h @@ -68,10 +68,6 @@ struct Swizzle : public Expression { ASSERT(fComponents.size() >= 1 && fComponents.size() <= 4); } - bool hasSideEffects() const override { - return fBase->hasSideEffects(); - } - SkString description() const override { SkString result = fBase->description() + "."; for (int x : fComponents) { diff --git a/src/sksl/ir/SkSLTernaryExpression.h b/src/sksl/ir/SkSLTernaryExpression.h index a9e8560aa1..02750049d4 100644 --- a/src/sksl/ir/SkSLTernaryExpression.h +++ b/src/sksl/ir/SkSLTernaryExpression.h @@ -26,12 +26,8 @@ struct TernaryExpression : public Expression { ASSERT(fIfTrue->fType == fIfFalse->fType); } - bool hasSideEffects() const override { - return fTest->hasSideEffects() || fIfTrue->hasSideEffects() || fIfFalse->hasSideEffects(); - } - SkString description() const override { - return "(" + fTest->description() + " ? " + fIfTrue->description() + " : " + + return "(" + fTest->description() + " ? " + fIfTrue->description() + " : " + fIfFalse->description() + ")"; } diff --git a/src/sksl/ir/SkSLTypeReference.h b/src/sksl/ir/SkSLTypeReference.h index 1ee216d0d1..1c6f16ce58 100644 --- a/src/sksl/ir/SkSLTypeReference.h +++ b/src/sksl/ir/SkSLTypeReference.h @@ -22,10 +22,6 @@ struct TypeReference : public Expression { : INHERITED(position, kTypeReference_Kind, *context.fInvalid_Type) , fValue(type) {} - bool hasSideEffects() const override { - return false; - } - SkString description() const override { return fValue.name(); } diff --git a/src/sksl/ir/SkSLUnresolvedFunction.h b/src/sksl/ir/SkSLUnresolvedFunction.h index 4047df63ef..76741cfca8 100644 --- a/src/sksl/ir/SkSLUnresolvedFunction.h +++ b/src/sksl/ir/SkSLUnresolvedFunction.h @@ -26,7 +26,7 @@ struct UnresolvedFunction : public Symbol { #endif } - SkString description() const override { + virtual SkString description() const override { return fName; } diff --git a/src/sksl/ir/SkSLVarDeclarations.h b/src/sksl/ir/SkSLVarDeclarations.h index 498a32d46e..490259a081 100644 --- a/src/sksl/ir/SkSLVarDeclarations.h +++ b/src/sksl/ir/SkSLVarDeclarations.h @@ -52,7 +52,7 @@ struct VarDeclaration { */ struct VarDeclarations : public ProgramElement { VarDeclarations(Position position, const Type* baseType, - std::vector<std::unique_ptr<VarDeclaration>> vars) + std::vector<VarDeclaration> vars) : INHERITED(position, kVar_Kind) , fBaseType(*baseType) , fVars(std::move(vars)) {} @@ -61,18 +61,18 @@ struct VarDeclarations : public ProgramElement { if (!fVars.size()) { return SkString(); } - SkString result = fVars[0]->fVar->fModifiers.description() + fBaseType.description() + " "; + SkString result = fVars[0].fVar->fModifiers.description() + fBaseType.description() + " "; SkString separator; for (const auto& var : fVars) { result += separator; separator = ", "; - result += var->description(); + result += var.description(); } return result; } const Type& fBaseType; - std::vector<std::unique_ptr<VarDeclaration>> fVars; + std::vector<VarDeclaration> fVars; typedef ProgramElement INHERITED; }; diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h index 8daf2bd6bf..2c3391dfa2 100644 --- a/src/sksl/ir/SkSLVariable.h +++ b/src/sksl/ir/SkSLVariable.h @@ -40,10 +40,6 @@ struct Variable : public Symbol { return fModifiers.description() + fType.fName + " " + fName; } - bool dead() const { - return !fWriteCount || (!fReadCount && !(fModifiers.fFlags & Modifiers::kOut_Flag)); - } - mutable Modifiers fModifiers; const Type& fType; const Storage fStorage; diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h index 75b7f3dcd4..fecb04e2e5 100644 --- a/src/sksl/ir/SkSLVariableReference.h +++ b/src/sksl/ir/SkSLVariableReference.h @@ -38,7 +38,7 @@ struct VariableReference : public Expression { } } - ~VariableReference() override { + virtual ~VariableReference() override { if (fRefKind != kWrite_RefKind) { fVariable.fReadCount--; } @@ -64,19 +64,13 @@ struct VariableReference : public Expression { fRefKind = refKind; } - bool hasSideEffects() const override { - return false; - } - SkString description() const override { return fVariable.fName; } - std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator, - const DefinitionMap& definitions) override { - if (fRefKind != kRead_RefKind) { - return nullptr; - } + 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(); @@ -91,11 +85,6 @@ struct VariableReference : public Expression { irGenerator.fContext, Position(), ((FloatLiteral*) expr)->fValue)); - case Expression::kBoolLiteral_Kind: - return std::unique_ptr<Expression>(new BoolLiteral( - irGenerator.fContext, - Position(), - ((BoolLiteral*) expr)->fValue)); default: break; } @@ -104,9 +93,10 @@ struct VariableReference : public Expression { } const Variable& fVariable; - RefKind fRefKind; private: + RefKind fRefKind; + typedef Expression INHERITED; }; diff --git a/src/sksl/ir/SkSLWhileStatement.h b/src/sksl/ir/SkSLWhileStatement.h index d4fc5878ae..a741a0441d 100644 --- a/src/sksl/ir/SkSLWhileStatement.h +++ b/src/sksl/ir/SkSLWhileStatement.h @@ -28,7 +28,7 @@ struct WhileStatement : public Statement { } std::unique_ptr<Expression> fTest; - std::unique_ptr<Statement> fStatement; + const std::unique_ptr<Statement> fStatement; typedef Statement INHERITED; }; |