From e1d9cb82bf9004eb05831f34bb3e9e708ae0617f Mon Sep 17 00:00:00 2001 From: Ethan Nicholas Date: Mon, 6 Feb 2017 18:53:07 +0000 Subject: [PATCH] Revert "Added dead variable / code elimination to skslc." This reverts commit 113628d76176a1ab3e6719c59efff23cd10ab213. Reason for revert: Looks to have caused https://bugs.chromium.org/p/chromium/issues/detail?id=688939 Original change's description: > Added dead variable / code elimination to skslc. > > BUG=skia: > > Change-Id: Ib037730803a8f222f099de0e001fe06ad452a22c > Reviewed-on: https://skia-review.googlesource.com/7584 > Commit-Queue: Ethan Nicholas > Reviewed-by: Ben Wagner > TBR=egdaniel@google.com,benjaminwagner@google.com,ethannicholas@google.com,reviews@skia.org # Not skipping CQ checks because original CL landed > 1 day ago. BUG=skia: Change-Id: I85599e4ca2bc6bfd782edc163f67b64195d6ae65 Reviewed-on: https://skia-review.googlesource.com/8077 Commit-Queue: Ethan Nicholas Reviewed-by: Ethan Nicholas --- src/sksl/SkSLCFGGenerator.cpp | 273 +++++----------------------------- src/sksl/SkSLCFGGenerator.h | 51 +------ src/sksl/SkSLCompiler.cpp | 276 +++++------------------------------ src/sksl/SkSLCompiler.h | 4 +- src/sksl/SkSLContext.h | 6 +- src/sksl/SkSLGLSLCodeGenerator.cpp | 39 ++--- src/sksl/SkSLGLSLCodeGenerator.h | 2 - src/sksl/SkSLIRGenerator.cpp | 23 ++- src/sksl/SkSLSPIRVCodeGenerator.cpp | 12 +- src/sksl/ir/SkSLBinaryExpression.h | 5 - src/sksl/ir/SkSLBlock.h | 11 +- src/sksl/ir/SkSLBoolLiteral.h | 4 - src/sksl/ir/SkSLConstructor.h | 36 ++--- src/sksl/ir/SkSLDoStatement.h | 2 +- src/sksl/ir/SkSLExpression.h | 7 - src/sksl/ir/SkSLFieldAccess.h | 8 +- src/sksl/ir/SkSLFloatLiteral.h | 6 +- src/sksl/ir/SkSLForStatement.h | 4 +- src/sksl/ir/SkSLFunctionCall.h | 15 -- src/sksl/ir/SkSLFunctionDefinition.h | 6 +- src/sksl/ir/SkSLFunctionReference.h | 4 - src/sksl/ir/SkSLIfStatement.h | 5 +- src/sksl/ir/SkSLIndexExpression.h | 6 +- src/sksl/ir/SkSLIntLiteral.h | 8 +- src/sksl/ir/SkSLNop.h | 36 ----- src/sksl/ir/SkSLPostfixExpression.h | 6 +- src/sksl/ir/SkSLPrefixExpression.h | 7 +- src/sksl/ir/SkSLStatement.h | 6 - src/sksl/ir/SkSLSwizzle.h | 4 - src/sksl/ir/SkSLTernaryExpression.h | 6 +- src/sksl/ir/SkSLTypeReference.h | 4 - src/sksl/ir/SkSLUnresolvedFunction.h | 2 +- src/sksl/ir/SkSLVarDeclarations.h | 8 +- src/sksl/ir/SkSLVariable.h | 4 - src/sksl/ir/SkSLVariableReference.h | 22 +-- src/sksl/ir/SkSLWhileStatement.h | 2 +- tests/SkSLGLSLTest.cpp | 239 ++++++++++-------------------- 37 files changed, 222 insertions(+), 937 deletions(-) delete mode 100644 src/sksl/ir/SkSLNop.h diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp index f3e4f92..31bace9 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() : ""); + *iter->second ? (*iter->second)->description().c_str() : ""); 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::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::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::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::iterator* iter, - std::unique_ptr* 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* e, bool constantPropagate) { ASSERT(e); switch ((*e)->fKind) { @@ -374,8 +177,6 @@ void CFGGenerator::addExpression(CFG& cfg, std::unique_ptr* 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* e) { } } -void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr* 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* 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* 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* 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* 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* 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* 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 3fb7acb..337fdfa 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* fExpression; - std::unique_ptr* 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::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::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::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::iterator* iter, - std::unique_ptr* expr); - std::vector fNodes; std::set fEntrances; std::set 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* s); + void addStatement(CFG& cfg, const Statement* s); void addExpression(CFG& cfg, std::unique_ptr* e, bool constantPropagate); diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 92261a4..743745a 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*) &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*) &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 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::iterator* iter, - std::unique_ptr newExpression) { - std::unique_ptr* 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 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 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 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 0b91563..fdca12d 100644 --- a/src/sksl/SkSLCompiler.h +++ b/src/sksl/SkSLCompiler.h @@ -67,9 +67,7 @@ private: void scanCFG(CFG* cfg, BlockId block, std::set* 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 8fced1c..18de336 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(""); } diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp index 47225e1..dd8c0cf 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>& 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 73c8b6f..0ae2c5c 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>& statements); - void writeBlock(const Block& b); void writeIfStatement(const IfStatement& stmt); diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index b9a30d3..55d9d2c 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -190,7 +190,7 @@ std::unique_ptr IRGenerator::convertVarDeclarationStatement( std::unique_ptr IRGenerator::convertVarDeclarations(const ASTVarDeclarations& decl, Variable::Storage storage) { - std::vector> variables; + std::vector variables; const Type* baseType = this->convertType(*decl.fType); if (!baseType) { return nullptr; @@ -235,7 +235,6 @@ std::unique_ptr 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 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 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 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(new FunctionCall(position, *returnType, function, diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp index 7352cf2..9c8a5d0 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 4837d18..de85e48 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 f00c146..17970fd 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 fSymbols; - std::vector> fStatements; + const std::vector> fStatements; typedef Statement INHERITED; }; diff --git a/src/sksl/ir/SkSLBoolLiteral.h b/src/sksl/ir/SkSLBoolLiteral.h index 5ca7c74..b372f2f 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 5ecb74e..691bea1 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 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(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(new IntLiteral(irGenerator.fContext, - fPosition, - intValue, - &fType)); - } + 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; } - 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 1b233f9..e26d3dc 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 fStatement; + const std::unique_ptr fStatement; std::unique_ptr fTest; typedef Statement INHERITED; diff --git a/src/sksl/ir/SkSLExpression.h b/src/sksl/ir/SkSLExpression.h index 5db9ddf..f87d810 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 b4f5695..de26a3f 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 base, int fieldIndex, + FieldAccess(std::unique_ptr 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 8fdf1af..8a1a5ad 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 4f2228a..f2bf880 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 fSymbols; - std::unique_ptr fInitializer; + const std::unique_ptr fInitializer; std::unique_ptr fTest; std::unique_ptr fNext; - std::unique_ptr fStatement; + const std::unique_ptr fStatement; typedef Statement INHERITED; }; diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h index 73a174b..1838076 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 7e7b115..bae8825 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 body) + FunctionDefinition(Position position, const FunctionDeclaration& declaration, + std::unique_ptr 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 fBody; + const std::unique_ptr fBody; typedef ProgramElement INHERITED; }; diff --git a/src/sksl/ir/SkSLFunctionReference.h b/src/sksl/ir/SkSLFunctionReference.h index 1465de9..ec1fc38 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(""); diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h index ee2612c..8667e93 100644 --- a/src/sksl/ir/SkSLIfStatement.h +++ b/src/sksl/ir/SkSLIfStatement.h @@ -33,9 +33,8 @@ struct IfStatement : public Statement { } std::unique_ptr fTest; - std::unique_ptr fIfTrue; - // may be null - std::unique_ptr fIfFalse; + const std::unique_ptr fIfTrue; + const std::unique_ptr fIfFalse; typedef Statement INHERITED; }; diff --git a/src/sksl/ir/SkSLIndexExpression.h b/src/sksl/ir/SkSLIndexExpression.h index 0b68793..d255c7d 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 base, + IndexExpression(const Context& context, std::unique_ptr base, std::unique_ptr 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 2488a6a..23325e6 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 6b13e14..0000000 --- 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 1c400f9..6c9fafe 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 6138bb5..b7db99a 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 02caac1..012311f 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 a77397a..8ad9001 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 a9e8560..0275004 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 1ee216d..1c6f16c 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 4047df6..76741cf 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 498a32d..490259a 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> vars) + std::vector 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> fVars; + std::vector fVars; typedef ProgramElement INHERITED; }; diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h index 8daf2bd..2c3391d 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 75b7f3d..fecb04e 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 constantPropagate(const IRGenerator& irGenerator, - const DefinitionMap& definitions) override { - if (fRefKind != kRead_RefKind) { - return nullptr; - } + 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(); @@ -91,11 +85,6 @@ struct VariableReference : public Expression { irGenerator.fContext, Position(), ((FloatLiteral*) expr)->fValue)); - case Expression::kBoolLiteral_Kind: - return std::unique_ptr(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 d4fc587..a741a04 100644 --- a/src/sksl/ir/SkSLWhileStatement.h +++ b/src/sksl/ir/SkSLWhileStatement.h @@ -28,7 +28,7 @@ struct WhileStatement : public Statement { } std::unique_ptr fTest; - std::unique_ptr fStatement; + const std::unique_ptr fStatement; typedef Statement INHERITED; }; diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp index 2a8cc3f..1501dc5 100644 --- a/tests/SkSLGLSLTest.cpp +++ b/tests/SkSLGLSLTest.cpp @@ -11,13 +11,6 @@ #if SK_SUPPORT_GPU -// Note that the optimizer will aggressively kill dead code and substitute constants in place of -// variables, so we have to jump through a few hoops to ensure that the code in these tests has the -// necessary side-effects to remain live. In some cases we rely on the optimizer not (yet) being -// smart enough to optimize around certain constructs; as the optimizer gets smarter it will -// undoubtedly end up breaking some of these tests. That is a good thing, as long as the new code is -// equivalent! - static void test(skiatest::Reporter* r, const char* src, const SkSL::Program::Settings& settings, const char* expected, SkSL::Program::Inputs* inputs) { SkSL::Compiler compiler; @@ -65,7 +58,7 @@ DEF_TEST(SkSLControl, r) { "void main() {" "if (sqrt(2) > 5) { sk_FragColor = vec4(0.75); } else { discard; }" "int i = 0;" - "while (i < 10) { sk_FragColor *= 0.5; i++; }" + "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;" @@ -82,10 +75,7 @@ DEF_TEST(SkSLControl, r) { " discard;\n" " }\n" " int i = 0;\n" - " while (i < 10) {\n" - " sk_FragColor *= 0.5;\n" - " i++;\n" - " }\n" + " while (true) sk_FragColor *= 0.5;\n" " do {\n" " sk_FragColor += 0.01;\n" " } while (sk_FragColor.x < 0.75);\n" @@ -116,8 +106,8 @@ DEF_TEST(SkSLFunctions, r) { "}\n" "void main() {\n" " float x = 10.0;\n" - " bar(x);\n" - " sk_FragColor = vec4(x);\n" + " bar(10.0);\n" + " sk_FragColor = vec4(10.0);\n" "}\n"); } @@ -176,7 +166,6 @@ DEF_TEST(SkSLMatrices, r) { "mat3x4 z = x * y;" "vec3 v1 = mat3(1) * vec3(1);" "vec3 v2 = vec3(1) * mat3(1);" - "sk_FragColor = vec4(z[0].x, v1 + v2);" "}", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" @@ -187,7 +176,6 @@ DEF_TEST(SkSLMatrices, r) { " mat3x4 z = x * y;\n" " vec3 v1 = mat3(1.0) * vec3(1.0);\n" " vec3 v2 = vec3(1.0) * mat3(1.0);\n" - " sk_FragColor = vec4(z[0].x, v1 + v2);\n" "}\n"); } @@ -268,21 +256,16 @@ DEF_TEST(SkSLVersion, r) { DEF_TEST(SkSLUsesPrecisionModifiers, r) { test(r, - "void main() { float x = 0.75; highp float y = 1; x++; y++;" - "sk_FragColor.rg = vec2(x, y); }", + "void main() { float x = 0.75; highp float y = 1; }", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" " float x = 0.75;\n" " float y = 1.0;\n" - " x++;\n" - " y++;\n" - " sk_FragColor.xy = vec2(x, y);\n" - "}\n"); + "}\n"); test(r, - "void main() { float x = 0.75; highp float y = 1; x++; y++;" - "sk_FragColor.rg = vec2(x, y); }", + "void main() { float x = 0.75; highp float y = 1; }", *SkSL::ShaderCapsFactory::UsesPrecisionModifiers(), "#version 400\n" "precision highp float;\n" @@ -290,29 +273,27 @@ DEF_TEST(SkSLUsesPrecisionModifiers, r) { "void main() {\n" " float x = 0.75;\n" " highp float y = 1.0;\n" - " x++;\n" - " y++;\n" - " sk_FragColor.xy = vec2(x, y);\n" - "}\n"); + "}\n"); } DEF_TEST(SkSLMinAbs, r) { test(r, "void main() {" "float x = -5;" - "sk_FragColor.r = min(abs(x), 6);" + "x = min(abs(x), 6);" "}", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" - " sk_FragColor.x = min(abs(-5.0), 6.0);\n" + " float x = -5.0;\n" + " x = min(abs(-5.0), 6.0);\n" "}\n"); test(r, "void main() {" "float x = -5.0;" - "sk_FragColor.r = min(abs(x), 6.0);" + "x = min(abs(x), 6.0);" "}", *SkSL::ShaderCapsFactory::CannotUseMinAndAbsTogether(), "#version 400\n" @@ -320,29 +301,30 @@ DEF_TEST(SkSLMinAbs, r) { "void main() {\n" " float minAbsHackVar0;\n" " float minAbsHackVar1;\n" - " sk_FragColor.x = ((minAbsHackVar0 = abs(-5.0)) < (minAbsHackVar1 = 6.0) ? " - "minAbsHackVar0 : minAbsHackVar1);\n" + " float x = -5.0;\n" + " x = ((minAbsHackVar0 = abs(-5.0)) < (minAbsHackVar1 = 6.0) ? minAbsHackVar0 : " + "minAbsHackVar1);\n" "}\n"); } DEF_TEST(SkSLNegatedAtan, r) { test(r, - "void main() { vec2 x = vec2(1, 2); sk_FragColor.r = atan(x.x, -(2 * x.y)); }", + "void main() { vec2 x = vec2(1, 2); float y = atan(x.x, -(2 * x.y)); }", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" " vec2 x = vec2(1.0, 2.0);\n" - " sk_FragColor.x = atan(x.x, -(2.0 * x.y));\n" + " float y = atan(x.x, -(2.0 * x.y));\n" "}\n"); test(r, - "void main() { vec2 x = vec2(1, 2); sk_FragColor.r = atan(x.x, -(2 * x.y)); }", + "void main() { vec2 x = vec2(1, 2); float y = atan(x.x, -(2 * x.y)); }", *SkSL::ShaderCapsFactory::MustForceNegatedAtanParamToFloat(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" " vec2 x = vec2(1.0, 2.0);\n" - " sk_FragColor.x = atan(x.x, -1.0 * (2.0 * x.y));\n" + " float y = atan(x.x, -1.0 * (2.0 * x.y));\n" "}\n"); } @@ -362,46 +344,28 @@ DEF_TEST(SkSLHex, r) { test(r, "void main() {" "int i1 = 0x0;" - "i1++;" "int i2 = 0x1234abcd;" - "i2++;" "int i3 = 0x7fffffff;" - "i3++;" "int i4 = 0xffffffff;" - "i4++;" "int i5 = -0xbeef;" - "i5++;" "uint u1 = 0x0;" - "u1++;" "uint u2 = 0x1234abcd;" - "u2++;" "uint u3 = 0x7fffffff;" - "u3++;" "uint u4 = 0xffffffff;" - "u4++;" "}", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" " int i1 = 0;\n" - " i1++;\n" " int i2 = 305441741;\n" - " i2++;\n" " int i3 = 2147483647;\n" - " i3++;\n" " int i4 = -1;\n" - " i4++;\n" " int i5 = -48879;\n" - " i5++;\n" " uint u1 = 0u;\n" - " u1++;\n" " uint u2 = 305441741u;\n" - " u2++;\n" " uint u3 = 2147483647u;\n" - " u3++;\n" " uint u4 = 4294967295u;\n" - " u4++;\n" "}\n"); } @@ -445,29 +409,29 @@ DEF_TEST(SkSLArrayConstructors, r) { DEF_TEST(SkSLDerivatives, r) { test(r, - "void main() { sk_FragColor.r = dFdx(1); }", + "void main() { float x = dFdx(1); }", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" - " sk_FragColor.x = dFdx(1.0);\n" + " float x = dFdx(1.0);\n" "}\n"); test(r, - "void main() { sk_FragColor.r = 1; }", + "void main() { float x = 1; }", *SkSL::ShaderCapsFactory::ShaderDerivativeExtensionString(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" - " sk_FragColor.x = 1.0;\n" + " float x = 1.0;\n" "}\n"); test(r, - "void main() { sk_FragColor.r = dFdx(1); }", + "void main() { float x = dFdx(1); }", *SkSL::ShaderCapsFactory::ShaderDerivativeExtensionString(), "#version 400\n" "#extension GL_OES_standard_derivatives : require\n" "out vec4 sk_FragColor;\n" "void main() {\n" - " sk_FragColor.x = dFdx(1.0);\n" + " float x = dFdx(1.0);\n" "}\n"); } @@ -475,109 +439,76 @@ DEF_TEST(SkSLConstantFolding, r) { test(r, "void main() {" "float f_add = 32 + 2;" - "sk_FragColor.r = f_add;" "float f_sub = 32 - 2;" - "sk_FragColor.r = f_sub;" "float f_mul = 32 * 2;" - "sk_FragColor.r = f_mul;" "float f_div = 32 / 2;" - "sk_FragColor.r = f_div;" "float mixed = (12 > 2.0) ? (10 * 2 / 5 + 18 - 3) : 0;" - "sk_FragColor.r = mixed;" "int i_add = 32 + 2;" - "sk_FragColor.r = i_add;" "int i_sub = 32 - 2;" - "sk_FragColor.r = i_sub;" "int i_mul = 32 * 2;" - "sk_FragColor.r = i_mul;" "int i_div = 32 / 2;" - "sk_FragColor.r = i_div;" "int i_or = 12 | 6;" - "sk_FragColor.r = i_or;" "int i_and = 254 & 7;" - "sk_FragColor.r = i_and;" "int i_xor = 2 ^ 7;" - "sk_FragColor.r = i_xor;" "int i_shl = 1 << 4;" - "sk_FragColor.r = i_shl;" "int i_shr = 128 >> 2;" - "sk_FragColor.r = i_shr;" "bool gt_it = 6 > 5;" - "sk_FragColor.r = true ? 1 : 0;" "bool gt_if = 6 > 6;" - "sk_FragColor.r = gt_if ? 1 : 0;" "bool gt_ft = 6.0 > 5.0;" - "sk_FragColor.r = gt_ft ? 1 : 0;" "bool gt_ff = 6.0 > 6.0;" - "sk_FragColor.r = gt_ff ? 1 : 0;" "bool gte_it = 6 >= 6;" - "sk_FragColor.r = gte_it ? 1 : 0;" "bool gte_if = 6 >= 7;" - "sk_FragColor.r = gte_if ? 1 : 0;" "bool gte_ft = 6.0 >= 6.0;" - "sk_FragColor.r = gte_ft ? 1 : 0;" "bool gte_ff = 6.0 >= 7.0;" - "sk_FragColor.r = gte_ff ? 1 : 0;" "bool lte_it = 6 <= 6;" - "sk_FragColor.r = lte_it ? 1 : 0;" "bool lte_if = 6 <= 5;" - "sk_FragColor.r = lte_if ? 1 : 0;" "bool lte_ft = 6.0 <= 6.0;" - "sk_FragColor.r = lte_ft ? 1 : 0;" "bool lte_ff = 6.0 <= 5.0;" - "sk_FragColor.r = lte_ff ? 1 : 0;" "bool or_t = 1 == 1 || 2 == 8;" - "sk_FragColor.r = or_t ? 1 : 0;" "bool or_f = 1 > 1 || 2 == 8;" - "sk_FragColor.r = or_f ? 1 : 0;" "bool and_t = 1 == 1 && 2 <= 8;" - "sk_FragColor.r = and_t ? 1 : 0;" "bool and_f = 1 == 2 && 2 == 8;" - "sk_FragColor.r = and_f ? 1 : 0;" "bool xor_t = 1 == 1 ^^ 1 != 1;" - "sk_FragColor.r = xor_t ? 1 : 0;" "bool xor_f = 1 == 1 ^^ 1 == 1;" - "sk_FragColor.r = xor_f ? 1 : 0;" "int ternary = 10 > 5 ? 10 : 5;" - "sk_FragColor.r = ternary;" "}", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" - " sk_FragColor.x = 34.0;\n" - " sk_FragColor.x = 30.0;\n" - " sk_FragColor.x = 64.0;\n" - " sk_FragColor.x = 16.0;\n" - " sk_FragColor.x = 19.0;\n" - " sk_FragColor.x = 34.0;\n" - " sk_FragColor.x = 30.0;\n" - " sk_FragColor.x = 64.0;\n" - " sk_FragColor.x = 16.0;\n" - " sk_FragColor.x = 14.0;\n" - " sk_FragColor.x = 6.0;\n" - " sk_FragColor.x = 5.0;\n" - " sk_FragColor.x = 16.0;\n" - " sk_FragColor.x = 32.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 1.0;\n" - " sk_FragColor.x = 0.0;\n" - " sk_FragColor.x = 10.0;\n" + " float f_add = 34.0;\n" + " float f_sub = 30.0;\n" + " float f_mul = 64.0;\n" + " float f_div = 16.0;\n" + " float mixed = 19.0;\n" + " int i_add = 34;\n" + " int i_sub = 30;\n" + " int i_mul = 64;\n" + " int i_div = 16;\n" + " int i_or = 14;\n" + " int i_and = 6;\n" + " int i_xor = 5;\n" + " int i_shl = 16;\n" + " int i_shr = 32;\n" + " bool gt_it = true;\n" + " bool gt_if = false;\n" + " bool gt_ft = true;\n" + " bool gt_ff = false;\n" + " bool gte_it = true;\n" + " bool gte_if = false;\n" + " bool gte_ft = true;\n" + " bool gte_ff = false;\n" + " bool lte_it = true;\n" + " bool lte_if = false;\n" + " bool lte_ft = true;\n" + " bool lte_ff = false;\n" + " bool or_t = true;\n" + " bool or_f = false;\n" + " bool and_t = true;\n" + " bool and_f = false;\n" + " bool xor_t = true;\n" + " bool xor_f = false;\n" + " int ternary = 10;\n" "}\n"); } @@ -589,34 +520,40 @@ DEF_TEST(SkSLStaticIf, r) { "if (2 > 1) x = 2; else x = 3;" "if (1 > 2) x = 4; else x = 5;" "if (false) x = 6;" - "sk_FragColor.r = x;" "}", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" - " sk_FragColor.x = 5.0;\n" + " int x;\n" + " x = 1;\n" + " x = 2;\n" + " x = 5;\n" + " {\n" + " }\n" "}\n"); } DEF_TEST(SkSLCaps, r) { test(r, "void main() {" - "int x = 0;" - "int y = 0;" - "int z = 0;" - "int w = 0;" + "int x;" "if (sk_Caps.externalTextureSupport) x = 1;" - "if (sk_Caps.fbFetchSupport) y = 1;" - "if (sk_Caps.dropsTileOnZeroDivide && sk_Caps.texelFetchSupport) z = 1;" - "if (sk_Caps.dropsTileOnZeroDivide && sk_Caps.canUseAnyFunctionInShader) w = 1;" - "sk_FragColor = vec4(x, y, z, w);" + "if (sk_Caps.fbFetchSupport) x = 2;" + "if (sk_Caps.dropsTileOnZeroDivide && sk_Caps.texelFetchSupport) x = 3;" + "if (sk_Caps.dropsTileOnZeroDivide && sk_Caps.canUseAnyFunctionInShader) x = 4;" "}", *SkSL::ShaderCapsFactory::VariousCaps(), "#version 400\n" "out vec4 sk_FragColor;\n" "void main() {\n" - " sk_FragColor = vec4(1.0, 0.0, 1.0, 0.0);\n" + " int x;\n" + " x = 1;\n" + " {\n" + " }\n" + " x = 3;\n" + " {\n" + " }\n" "}\n"); } @@ -629,7 +566,6 @@ DEF_TEST(SkSLTexture, r) { "vec4 b = texture(two, vec2(0));" "vec4 c = texture(one, vec2(0));" "vec4 d = texture(two, vec3(0));" - "sk_FragColor = vec4(a.x, b.x, c.x, d.x);" "}", *SkSL::ShaderCapsFactory::Default(), "#version 400\n" @@ -641,7 +577,6 @@ DEF_TEST(SkSLTexture, r) { " vec4 b = texture(two, vec2(0.0));\n" " vec4 c = textureProj(one, vec2(0.0));\n" " vec4 d = textureProj(two, vec3(0.0));\n" - " sk_FragColor = vec4(a.x, b.x, c.x, d.x);\n" "}\n"); test(r, "uniform sampler1D one;" @@ -651,7 +586,6 @@ DEF_TEST(SkSLTexture, r) { "vec4 b = texture(two, vec2(0));" "vec4 c = texture(one, vec2(0));" "vec4 d = texture(two, vec3(0));" - "sk_FragColor = vec4(a.x, b.x, c.x, d.x);" "}", *SkSL::ShaderCapsFactory::Version110(), "#version 110\n" @@ -662,7 +596,6 @@ DEF_TEST(SkSLTexture, r) { " vec4 b = texture2D(two, vec2(0.0));\n" " vec4 c = texture1DProj(one, vec2(0.0));\n" " vec4 d = texture2DProj(two, vec3(0.0));\n" - " gl_FragColor = vec4(a.x, b.x, c.x, d.x);\n" "}\n"); } @@ -744,26 +677,4 @@ DEF_TEST(SkSLFragCoord, r) { REPORTER_ASSERT(r, !inputs.fRTHeight); } -DEF_TEST(SkSLUnusedVars, r) { - test(r, - "void main() {" - "float a = 1, b = 2, c = 3;" - "float d = c;" - "float e = d;" - "b++;" - "d++;" - "sk_FragColor = vec4(b, b, d, d);" - "}", - *SkSL::ShaderCapsFactory::Default(), - "#version 400\n" - "out vec4 sk_FragColor;\n" - "void main() {\n" - " float b = 2.0;\n" - " float d = 3.0;\n" - " b++;\n" - " d++;\n" - " sk_FragColor = vec4(b, b, d, d);\n" - "}\n"); -} - #endif -- 2.7.4