Revert "Added constant propagation and better variable liveness tracking to"
This reverts commit f54b07121f81a56145fb118a2e18841fc135717d.
Reason for revert: ASAN failure
Original change's description:
> Added constant propagation and better variable liveness tracking to
> skslc.
>
> This allows skslc to track the values of variables with constant
> values across multiple statements and replace variable references with
> constant values where appropriate.
>
> The improved liveness tracking allows skslc to realize that a
> variable is no longer alive if all references to it have been
> replaced. It is not yet doing much with this information; better
> dead code elimination is coming in a followup change.
>
> BUG=skia:
>
> Change-Id: I6bf267d478b769caf0063ac3597dc16bbe618cb4
> Reviewed-on: https://skia-review.googlesource.com/7033
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
>
TBR=egdaniel@google.com,ethannicholas@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Change-Id: Id2e26bce96b27df73948f8b32d3dff2e358ae0d6
Reviewed-on: https://skia-review.googlesource.com/7274
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
index 31bace9..964a8dc 100644
--- a/src/sksl/SkSLCFGGenerator.cpp
+++ b/src/sksl/SkSLCFGGenerator.cpp
@@ -54,8 +54,8 @@
printf("Block %d\n-------\nBefore: ", (int) i);
const char* separator = "";
for (auto iter = fBlocks[i].fBefore.begin(); iter != fBlocks[i].fBefore.end(); iter++) {
- printf("%s%s = %s", separator, iter->first->description().c_str(),
- *iter->second ? (*iter->second)->description().c_str() : "<undefined>");
+ printf("%s%s = %s", separator, iter->first->description().c_str(),
+ iter->second ? iter->second->description().c_str() : "<undefined>");
separator = ", ";
}
printf("\nEntrances: ");
@@ -66,10 +66,7 @@
}
printf("\n");
for (size_t j = 0; j < fBlocks[i].fNodes.size(); j++) {
- BasicBlock::Node& n = fBlocks[i].fNodes[j];
- printf("Node %d: %s\n", (int) j, n.fKind == BasicBlock::Node::kExpression_Kind
- ? (*n.fExpression)->description().c_str()
- : n.fStatement->description().c_str());
+ printf("Node %d: %s\n", (int) j, fBlocks[i].fNodes[j].fNode->description().c_str());
}
printf("Exits: ");
separator = "";
@@ -81,109 +78,96 @@
}
}
-void CFGGenerator::addExpression(CFG& cfg, std::unique_ptr<Expression>* e, bool constantPropagate) {
- ASSERT(e);
- switch ((*e)->fKind) {
+void CFGGenerator::addExpression(CFG& cfg, const Expression* e) {
+ switch (e->fKind) {
case Expression::kBinary_Kind: {
- BinaryExpression* b = (BinaryExpression*) e->get();
+ const BinaryExpression* b = (const BinaryExpression*) e;
switch (b->fOperator) {
case Token::LOGICALAND: // fall through
case Token::LOGICALOR: {
// this isn't as precise as it could be -- we don't bother to track that if we
// early exit from a logical and/or, we know which branch of an 'if' we're going
// to hit -- but it won't make much difference in practice.
- this->addExpression(cfg, &b->fLeft, constantPropagate);
+ this->addExpression(cfg, b->fLeft.get());
BlockId start = cfg.fCurrent;
cfg.newBlock();
- this->addExpression(cfg, &b->fRight, constantPropagate);
+ this->addExpression(cfg, b->fRight.get());
cfg.newBlock();
cfg.addExit(start, cfg.fCurrent);
break;
}
case Token::EQ: {
- this->addExpression(cfg, &b->fRight, constantPropagate);
- this->addLValue(cfg, &b->fLeft);
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
- BasicBlock::Node::kExpression_Kind,
- constantPropagate,
- e,
- nullptr
+ this->addExpression(cfg, b->fRight.get());
+ this->addLValue(cfg, b->fLeft.get());
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
+ BasicBlock::Node::kExpression_Kind,
+ b
});
break;
}
default:
- this->addExpression(cfg, &b->fLeft, !Token::IsAssignment(b->fOperator));
- this->addExpression(cfg, &b->fRight, constantPropagate);
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
- BasicBlock::Node::kExpression_Kind,
- constantPropagate,
- e,
- nullptr
+ this->addExpression(cfg, b->fLeft.get());
+ this->addExpression(cfg, b->fRight.get());
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
+ BasicBlock::Node::kExpression_Kind,
+ b
});
}
break;
}
case Expression::kConstructor_Kind: {
- Constructor* c = (Constructor*) e->get();
- for (auto& arg : c->fArguments) {
- this->addExpression(cfg, &arg, constantPropagate);
+ const Constructor* c = (const Constructor*) e;
+ for (const auto& arg : c->fArguments) {
+ this->addExpression(cfg, arg.get());
}
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, c });
break;
}
case Expression::kFunctionCall_Kind: {
- FunctionCall* c = (FunctionCall*) e->get();
- for (auto& arg : c->fArguments) {
- this->addExpression(cfg, &arg, constantPropagate);
+ const FunctionCall* c = (const FunctionCall*) e;
+ for (const auto& arg : c->fArguments) {
+ this->addExpression(cfg, arg.get());
}
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, c });
break;
}
case Expression::kFieldAccess_Kind:
- this->addExpression(cfg, &((FieldAccess*) e->get())->fBase, constantPropagate);
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ this->addExpression(cfg, ((const FieldAccess*) e)->fBase.get());
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e });
break;
case Expression::kIndex_Kind:
- this->addExpression(cfg, &((IndexExpression*) e->get())->fBase, constantPropagate);
- this->addExpression(cfg, &((IndexExpression*) e->get())->fIndex, constantPropagate);
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ this->addExpression(cfg, ((const IndexExpression*) e)->fBase.get());
+ this->addExpression(cfg, ((const IndexExpression*) e)->fIndex.get());
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e });
break;
case Expression::kPrefix_Kind:
- this->addExpression(cfg, &((PrefixExpression*) e->get())->fOperand, constantPropagate);
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ this->addExpression(cfg, ((const PrefixExpression*) e)->fOperand.get());
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e });
break;
case Expression::kPostfix_Kind:
- this->addExpression(cfg, &((PostfixExpression*) e->get())->fOperand, constantPropagate);
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ this->addExpression(cfg, ((const PostfixExpression*) e)->fOperand.get());
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e });
break;
case Expression::kSwizzle_Kind:
- this->addExpression(cfg, &((Swizzle*) e->get())->fBase, constantPropagate);
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ this->addExpression(cfg, ((const Swizzle*) e)->fBase.get());
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e });
break;
case Expression::kBoolLiteral_Kind: // fall through
case Expression::kFloatLiteral_Kind: // fall through
case Expression::kIntLiteral_Kind: // fall through
case Expression::kVariableReference_Kind:
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind,
- constantPropagate, e, nullptr });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kExpression_Kind, e });
break;
case Expression::kTernary_Kind: {
- TernaryExpression* t = (TernaryExpression*) e->get();
- this->addExpression(cfg, &t->fTest, constantPropagate);
+ const TernaryExpression* t = (const TernaryExpression*) e;
+ this->addExpression(cfg, t->fTest.get());
BlockId start = cfg.fCurrent;
cfg.newBlock();
- this->addExpression(cfg, &t->fIfTrue, constantPropagate);
+ this->addExpression(cfg, t->fIfTrue.get());
BlockId next = cfg.newBlock();
cfg.fCurrent = start;
cfg.newBlock();
- this->addExpression(cfg, &t->fIfFalse, constantPropagate);
+ this->addExpression(cfg, t->fIfFalse.get());
cfg.addExit(cfg.fCurrent, next);
cfg.fCurrent = next;
break;
@@ -197,17 +181,17 @@
}
// adds expressions that are evaluated as part of resolving an lvalue
-void CFGGenerator::addLValue(CFG& cfg, std::unique_ptr<Expression>* e) {
- switch ((*e)->fKind) {
+void CFGGenerator::addLValue(CFG& cfg, const Expression* e) {
+ switch (e->fKind) {
case Expression::kFieldAccess_Kind:
- this->addLValue(cfg, &((FieldAccess&) **e).fBase);
+ this->addLValue(cfg, ((const FieldAccess*) e)->fBase.get());
break;
case Expression::kIndex_Kind:
- this->addLValue(cfg, &((IndexExpression&) **e).fBase);
- this->addExpression(cfg, &((IndexExpression&) **e).fIndex, true);
+ this->addLValue(cfg, ((const IndexExpression*) e)->fBase.get());
+ this->addExpression(cfg, ((const IndexExpression*) e)->fIndex.get());
break;
case Expression::kSwizzle_Kind:
- this->addLValue(cfg, &((Swizzle&) **e).fBase);
+ this->addLValue(cfg, ((const Swizzle*) e)->fBase.get());
break;
case Expression::kVariableReference_Kind:
break;
@@ -226,8 +210,8 @@
}
break;
case Statement::kIf_Kind: {
- IfStatement* ifs = (IfStatement*) s;
- this->addExpression(cfg, &ifs->fTest, true);
+ const IfStatement* ifs = (const IfStatement*) s;
+ this->addExpression(cfg, ifs->fTest.get());
BlockId start = cfg.fCurrent;
cfg.newBlock();
this->addStatement(cfg, ifs->fIfTrue.get());
@@ -244,54 +228,49 @@
break;
}
case Statement::kExpression_Kind: {
- this->addExpression(cfg, &((ExpressionStatement&) *s).fExpression, true);
+ this->addExpression(cfg, ((ExpressionStatement&) *s).fExpression.get());
break;
}
case Statement::kVarDeclarations_Kind: {
- VarDeclarationsStatement& decls = ((VarDeclarationsStatement&) *s);
- for (auto& vd : decls.fDeclaration->fVars) {
+ const VarDeclarationsStatement& decls = ((VarDeclarationsStatement&) *s);
+ for (const auto& vd : decls.fDeclaration->fVars) {
if (vd.fValue) {
- this->addExpression(cfg, &vd.fValue, true);
+ this->addExpression(cfg, vd.fValue.get());
}
}
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
- nullptr, s });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s });
break;
}
case Statement::kDiscard_Kind:
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
- nullptr, s });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s });
cfg.fCurrent = cfg.newIsolatedBlock();
break;
case Statement::kReturn_Kind: {
- ReturnStatement& r = ((ReturnStatement&) *s);
+ const ReturnStatement& r = ((ReturnStatement&) *s);
if (r.fExpression) {
- this->addExpression(cfg, &r.fExpression, true);
+ this->addExpression(cfg, r.fExpression.get());
}
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
- nullptr, s });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s });
cfg.fCurrent = cfg.newIsolatedBlock();
break;
}
case Statement::kBreak_Kind:
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
- nullptr, s });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s });
cfg.addExit(cfg.fCurrent, fLoopExits.top());
cfg.fCurrent = cfg.newIsolatedBlock();
break;
case Statement::kContinue_Kind:
- cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
- nullptr, s });
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, s });
cfg.addExit(cfg.fCurrent, fLoopContinues.top());
cfg.fCurrent = cfg.newIsolatedBlock();
break;
case Statement::kWhile_Kind: {
- WhileStatement* w = (WhileStatement*) s;
+ const WhileStatement* w = (const WhileStatement*) s;
BlockId loopStart = cfg.newBlock();
fLoopContinues.push(loopStart);
BlockId loopExit = cfg.newIsolatedBlock();
fLoopExits.push(loopExit);
- this->addExpression(cfg, &w->fTest, true);
+ this->addExpression(cfg, w->fTest.get());
BlockId test = cfg.fCurrent;
cfg.addExit(test, loopExit);
cfg.newBlock();
@@ -303,13 +282,13 @@
break;
}
case Statement::kDo_Kind: {
- DoStatement* d = (DoStatement*) s;
+ const DoStatement* d = (const DoStatement*) s;
BlockId loopStart = cfg.newBlock();
fLoopContinues.push(loopStart);
BlockId loopExit = cfg.newIsolatedBlock();
fLoopExits.push(loopExit);
this->addStatement(cfg, d->fStatement.get());
- this->addExpression(cfg, &d->fTest, true);
+ this->addExpression(cfg, d->fTest.get());
cfg.addExit(cfg.fCurrent, loopExit);
cfg.addExit(cfg.fCurrent, loopStart);
fLoopContinues.pop();
@@ -318,7 +297,7 @@
break;
}
case Statement::kFor_Kind: {
- ForStatement* f = (ForStatement*) s;
+ const ForStatement* f = (const ForStatement*) s;
if (f->fInitializer) {
this->addStatement(cfg, f->fInitializer.get());
}
@@ -328,7 +307,7 @@
BlockId loopExit = cfg.newIsolatedBlock();
fLoopExits.push(loopExit);
if (f->fTest) {
- this->addExpression(cfg, &f->fTest, true);
+ this->addExpression(cfg, f->fTest.get());
BlockId test = cfg.fCurrent;
cfg.addExit(test, loopExit);
}
@@ -337,9 +316,9 @@
cfg.addExit(cfg.fCurrent, next);
cfg.fCurrent = next;
if (f->fNext) {
- this->addExpression(cfg, &f->fNext, true);
+ this->addExpression(cfg, f->fNext.get());
}
- cfg.addExit(cfg.fCurrent, loopStart);
+ cfg.addExit(next, loopStart);
fLoopContinues.pop();
fLoopExits.pop();
cfg.fCurrent = loopExit;
diff --git a/src/sksl/SkSLCFGGenerator.h b/src/sksl/SkSLCFGGenerator.h
index 337fdfa..c378501 100644
--- a/src/sksl/SkSLCFGGenerator.h
+++ b/src/sksl/SkSLCFGGenerator.h
@@ -27,23 +27,14 @@
};
Kind fKind;
- // if false, this node should not be subject to constant propagation. This happens with
- // compound assignment (i.e. x *= 2), in which the value x is used as an rvalue for
- // multiplication by 2 and then as an lvalue for assignment purposes. Since there is only
- // one "x" node, replacing it with a constant would break the assignment and we suppress
- // it. Down the road, we should handle this more elegantly by substituting a regular
- // assignment if the target is constant (i.e. x = 1; x *= 2; should become x = 1; x = 1 * 2;
- // and then collapse down to a simple x = 2;).
- bool fConstantPropagation;
- std::unique_ptr<Expression>* fExpression;
- const Statement* fStatement;
+ const IRNode* fNode;
};
-
+
std::vector<Node> fNodes;
std::set<BlockId> fEntrances;
std::set<BlockId> fExits;
// variable definitions upon entering this basic block (null expression = undefined)
- DefinitionMap fBefore;
+ std::unordered_map<const Variable*, const Expression*> fBefore;
};
struct CFG {
@@ -86,9 +77,9 @@
private:
void addStatement(CFG& cfg, const Statement* s);
- void addExpression(CFG& cfg, std::unique_ptr<Expression>* e, bool constantPropagate);
+ void addExpression(CFG& cfg, const Expression* e);
- void addLValue(CFG& cfg, std::unique_ptr<Expression>* e);
+ void addLValue(CFG& cfg, const Expression* e);
std::stack<BlockId> fLoopContinues;
std::stack<BlockId> fLoopExits;
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 743745a..9faf836 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -156,8 +156,8 @@
}
// add the definition created by assigning to the lvalue to the definition set
-void Compiler::addDefinition(const Expression* lvalue, std::unique_ptr<Expression>* expr,
- DefinitionMap* definitions) {
+void Compiler::addDefinition(const Expression* lvalue, const Expression* expr,
+ std::unordered_map<const Variable*, const Expression*>* definitions) {
switch (lvalue->fKind) {
case Expression::kVariableReference_Kind: {
const Variable& var = ((VariableReference*) lvalue)->fVariable;
@@ -174,19 +174,19 @@
// but since we pass foo as a whole it is flagged as an error) unless we perform a much
// more complicated whole-program analysis. This is probably good enough.
this->addDefinition(((Swizzle*) lvalue)->fBase.get(),
- (std::unique_ptr<Expression>*) &fContext.fDefined_Expression,
+ fContext.fDefined_Expression.get(),
definitions);
break;
case Expression::kIndex_Kind:
// see comments in Swizzle
this->addDefinition(((IndexExpression*) lvalue)->fBase.get(),
- (std::unique_ptr<Expression>*) &fContext.fDefined_Expression,
+ fContext.fDefined_Expression.get(),
definitions);
break;
case Expression::kFieldAccess_Kind:
// see comments in Swizzle
this->addDefinition(((FieldAccess*) lvalue)->fBase.get(),
- (std::unique_ptr<Expression>*) &fContext.fDefined_Expression,
+ fContext.fDefined_Expression.get(),
definitions);
break;
default:
@@ -197,58 +197,25 @@
// add local variables defined by this node to the set
void Compiler::addDefinitions(const BasicBlock::Node& node,
- DefinitionMap* definitions) {
+ std::unordered_map<const Variable*, const Expression*>* definitions) {
switch (node.fKind) {
case BasicBlock::Node::kExpression_Kind: {
- ASSERT(node.fExpression);
- const Expression* expr = (Expression*) node.fExpression->get();
- switch (expr->fKind) {
- case Expression::kBinary_Kind: {
- BinaryExpression* b = (BinaryExpression*) expr;
- if (b->fOperator == Token::EQ) {
- this->addDefinition(b->fLeft.get(), &b->fRight, definitions);
- } else if (Token::IsAssignment(b->fOperator)) {
- this->addDefinition(
- b->fLeft.get(),
- (std::unique_ptr<Expression>*) &fContext.fDefined_Expression,
- definitions);
-
- }
- break;
+ const Expression* expr = (Expression*) node.fNode;
+ if (expr->fKind == Expression::kBinary_Kind) {
+ const BinaryExpression* b = (BinaryExpression*) expr;
+ if (b->fOperator == Token::EQ) {
+ this->addDefinition(b->fLeft.get(), b->fRight.get(), definitions);
}
- case Expression::kPrefix_Kind: {
- const PrefixExpression* p = (PrefixExpression*) expr;
- if (p->fOperator == Token::MINUSMINUS || p->fOperator == Token::PLUSPLUS) {
- this->addDefinition(
- p->fOperand.get(),
- (std::unique_ptr<Expression>*) &fContext.fDefined_Expression,
- definitions);
- }
- break;
- }
- case Expression::kPostfix_Kind: {
- const PostfixExpression* p = (PostfixExpression*) expr;
- if (p->fOperator == Token::MINUSMINUS || p->fOperator == Token::PLUSPLUS) {
- this->addDefinition(
- p->fOperand.get(),
- (std::unique_ptr<Expression>*) &fContext.fDefined_Expression,
- definitions);
-
- }
- break;
- }
- default:
- break;
}
break;
}
case BasicBlock::Node::kStatement_Kind: {
- const Statement* stmt = (Statement*) node.fStatement;
+ const Statement* stmt = (Statement*) node.fNode;
if (stmt->fKind == Statement::kVarDeclarations_Kind) {
- VarDeclarationsStatement* vd = (VarDeclarationsStatement*) stmt;
- for (VarDeclaration& decl : vd->fDeclaration->fVars) {
+ const VarDeclarationsStatement* vd = (VarDeclarationsStatement*) stmt;
+ for (const VarDeclaration& decl : vd->fDeclaration->fVars) {
if (decl.fValue) {
- (*definitions)[decl.fVar] = &decl.fValue;
+ (*definitions)[decl.fVar] = decl.fValue.get();
}
}
}
@@ -261,7 +228,7 @@
BasicBlock& block = cfg->fBlocks[blockId];
// compute definitions after this block
- DefinitionMap after = block.fBefore;
+ std::unordered_map<const Variable*, const Expression*> after = block.fBefore;
for (const BasicBlock::Node& n : block.fNodes) {
this->addDefinitions(n, &after);
}
@@ -270,20 +237,19 @@
for (BlockId exitId : block.fExits) {
BasicBlock& exit = cfg->fBlocks[exitId];
for (const auto& pair : after) {
- std::unique_ptr<Expression>* e1 = pair.second;
- auto found = exit.fBefore.find(pair.first);
- if (found == exit.fBefore.end()) {
- // exit has no definition for it, just copy it
- workList->insert(exitId);
+ const Expression* e1 = pair.second;
+ if (exit.fBefore.find(pair.first) == exit.fBefore.end()) {
exit.fBefore[pair.first] = e1;
} else {
- // exit has a (possibly different) value already defined
- std::unique_ptr<Expression>* e2 = exit.fBefore[pair.first];
+ const Expression* e2 = exit.fBefore[pair.first];
if (e1 != e2) {
// definition has changed, merge and add exit block to worklist
workList->insert(exitId);
- exit.fBefore[pair.first] =
- (std::unique_ptr<Expression>*) &fContext.fDefined_Expression;
+ if (!e1 || !e2) {
+ exit.fBefore[pair.first] = nullptr;
+ } else {
+ exit.fBefore[pair.first] = fContext.fDefined_Expression.get();
+ }
}
}
}
@@ -292,13 +258,12 @@
// returns a map which maps all local variables in the function to null, indicating that their value
// is initially unknown
-static DefinitionMap compute_start_state(const CFG& cfg) {
- DefinitionMap result;
+static std::unordered_map<const Variable*, const Expression*> compute_start_state(const CFG& cfg) {
+ std::unordered_map<const Variable*, const Expression*> result;
for (const auto& block : cfg.fBlocks) {
for (const auto& node : block.fNodes) {
if (node.fKind == BasicBlock::Node::kStatement_Kind) {
- ASSERT(node.fStatement);
- const Statement* s = node.fStatement;
+ const Statement* s = (Statement*) node.fNode;
if (s->fKind == Statement::kVarDeclarations_Kind) {
const VarDeclarationsStatement* vd = (const VarDeclarationsStatement*) s;
for (const VarDeclaration& decl : vd->fDeclaration->fVars) {
@@ -330,37 +295,19 @@
for (size_t i = 0; i < cfg.fBlocks.size(); i++) {
if (i != cfg.fStart && !cfg.fBlocks[i].fEntrances.size() &&
cfg.fBlocks[i].fNodes.size()) {
- Position p;
- switch (cfg.fBlocks[i].fNodes[0].fKind) {
- case BasicBlock::Node::kStatement_Kind:
- p = cfg.fBlocks[i].fNodes[0].fStatement->fPosition;
- break;
- case BasicBlock::Node::kExpression_Kind:
- p = (*cfg.fBlocks[i].fNodes[0].fExpression)->fPosition;
- break;
- }
- this->error(p, SkString("unreachable"));
+ this->error(cfg.fBlocks[i].fNodes[0].fNode->fPosition, SkString("unreachable"));
}
}
if (fErrorCount) {
return;
}
- // check for undefined variables, perform constant propagation
- for (BasicBlock& b : cfg.fBlocks) {
- DefinitionMap definitions = b.fBefore;
- for (BasicBlock::Node& n : b.fNodes) {
+ // check for undefined variables
+ for (const BasicBlock& b : cfg.fBlocks) {
+ std::unordered_map<const Variable*, const Expression*> definitions = b.fBefore;
+ for (const BasicBlock::Node& n : b.fNodes) {
if (n.fKind == BasicBlock::Node::kExpression_Kind) {
- ASSERT(n.fExpression);
- Expression* expr = n.fExpression->get();
- if (n.fConstantPropagation) {
- std::unique_ptr<Expression> optimized = expr->constantPropagate(*fIRGenerator,
- definitions);
- if (optimized) {
- n.fExpression->reset(optimized.release());
- expr = n.fExpression->get();
- }
- }
+ const Expression* expr = (const Expression*) n.fNode;
if (expr->fKind == Expression::kVariableReference_Kind) {
const Variable& var = ((VariableReference*) expr)->fVariable;
if (var.fStorage == Variable::kLocal_Storage &&
diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h
index fdca12d..0f893f7 100644
--- a/src/sksl/SkSLCompiler.h
+++ b/src/sksl/SkSLCompiler.h
@@ -60,10 +60,11 @@
}
private:
- void addDefinition(const Expression* lvalue, std::unique_ptr<Expression>* expr,
- DefinitionMap* definitions);
+ void addDefinition(const Expression* lvalue, const Expression* expr,
+ std::unordered_map<const Variable*, const Expression*>* definitions);
- void addDefinitions(const BasicBlock::Node& node, DefinitionMap* definitions);
+ void addDefinitions(const BasicBlock::Node& node,
+ std::unordered_map<const Variable*, const Expression*>* definitions);
void scanCFG(CFG* cfg, BlockId block, std::set<BlockId>* workList);
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 55d9d2c..9f06c97 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -551,11 +551,11 @@
}
}
Type* type = new Type(intf.fPosition, intf.fInterfaceName, fields);
- old->takeOwnership(type);
+ fSymbolTable->takeOwnership(type);
SkString name = intf.fValueName.size() > 0 ? intf.fValueName : intf.fInterfaceName;
Variable* var = new Variable(intf.fPosition, intf.fModifiers, name, *type,
Variable::kGlobal_Storage);
- old->takeOwnership(var);
+ fSymbolTable->takeOwnership(var);
if (intf.fValueName.size()) {
old->addWithoutOwnership(intf.fValueName, var);
} else {
@@ -624,22 +624,19 @@
f->fFunctions));
}
case Symbol::kVariable_Kind: {
- Variable* var = (Variable*) result;
+ const Variable* var = (const Variable*) result;
+ this->markReadFrom(*var);
if (var->fModifiers.fLayout.fBuiltin == SK_FRAGCOORD_BUILTIN &&
fSettings->fFlipY &&
(!fSettings->fCaps || !fSettings->fCaps->fragCoordConventionsExtensionString())) {
fInputs.fRTHeight = true;
}
- // default to kRead_RefKind; this will be corrected later if the variable is written to
- return std::unique_ptr<VariableReference>(new VariableReference(
- identifier.fPosition,
- *var,
- VariableReference::kRead_RefKind));
+ return std::unique_ptr<VariableReference>(new VariableReference(identifier.fPosition,
+ *var));
}
case Symbol::kField_Kind: {
const Field* field = (const Field*) result;
- VariableReference* base = new VariableReference(identifier.fPosition, field->fOwner,
- VariableReference::kRead_RefKind);
+ VariableReference* base = new VariableReference(identifier.fPosition, field->fOwner);
return std::unique_ptr<Expression>(new FieldAccess(
std::unique_ptr<Expression>(base),
field->fFieldIndex,
@@ -693,6 +690,28 @@
return left.kind() == Type::kVector_Kind && right.kind() == Type::kMatrix_Kind;
}
+static bool is_assignment(Token::Kind op) {
+ switch (op) {
+ case Token::EQ: // fall through
+ case Token::PLUSEQ: // fall through
+ case Token::MINUSEQ: // fall through
+ case Token::STAREQ: // fall through
+ case Token::SLASHEQ: // fall through
+ case Token::PERCENTEQ: // fall through
+ case Token::SHLEQ: // fall through
+ case Token::SHREQ: // fall through
+ case Token::BITWISEOREQ: // fall through
+ case Token::BITWISEXOREQ: // fall through
+ case Token::BITWISEANDEQ: // fall through
+ case Token::LOGICALOREQ: // fall through
+ case Token::LOGICALXOREQ: // fall through
+ case Token::LOGICALANDEQ:
+ return true;
+ default:
+ return false;
+ }
+}
+
/**
* Determines the operand and result types of a binary expression. Returns true if the expression is
* legal, false otherwise. If false, the values of the out parameters are undefined.
@@ -823,9 +842,14 @@
return false;
}
+/**
+ * If both operands are compile-time constants and can be folded, returns an expression representing
+ * the folded value. Otherwise, returns null. Note that unlike most other functions here, null does
+ * not represent a compilation error.
+ */
std::unique_ptr<Expression> IRGenerator::constantFold(const Expression& left,
Token::Kind op,
- const Expression& right) const {
+ const Expression& right) {
// Note that we expressly do not worry about precision and overflow here -- we use the maximum
// precision to calculate the results and hope the result makes sense. The plan is to move the
// Skia caps into SkSL, so we have access to all of them including the precisions of the various
@@ -919,16 +943,15 @@
const Type* rightType;
const Type* resultType;
if (!determine_binary_type(fContext, expression.fOperator, left->fType, right->fType, &leftType,
- &rightType, &resultType,
- !Token::IsAssignment(expression.fOperator))) {
+ &rightType, &resultType, !is_assignment(expression.fOperator))) {
fErrors.error(expression.fPosition, "type mismatch: '" +
Token::OperatorName(expression.fOperator) +
"' cannot operate on '" + left->fType.fName +
"', '" + right->fType.fName + "'");
return nullptr;
}
- if (Token::IsAssignment(expression.fOperator)) {
- this->markWrittenTo(*left, expression.fOperator != Token::EQ);
+ if (is_assignment(expression.fOperator)) {
+ this->markWrittenTo(*left);
}
left = this->coerce(std::move(left), *leftType);
right = this->coerce(std::move(right), *rightType);
@@ -1028,7 +1051,7 @@
return nullptr;
}
if (arguments[i] && (function.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag)) {
- this->markWrittenTo(*arguments[i], true);
+ this->markWrittenTo(*arguments[i]);
}
}
return std::unique_ptr<FunctionCall>(new FunctionCall(position, *returnType, function,
@@ -1238,7 +1261,7 @@
"' cannot operate on '" + base->fType.description() + "'");
return nullptr;
}
- this->markWrittenTo(*base, true);
+ this->markWrittenTo(*base);
break;
case Token::MINUSMINUS:
if (!base->fType.isNumber()) {
@@ -1247,7 +1270,7 @@
"' cannot operate on '" + base->fType.description() + "'");
return nullptr;
}
- this->markWrittenTo(*base, true);
+ this->markWrittenTo(*base);
break;
case Token::LOGICALNOT:
if (base->fType != *fContext.fBool_Type) {
@@ -1441,7 +1464,7 @@
"'++' cannot operate on '" + base->fType.description() + "'");
return nullptr;
}
- this->markWrittenTo(*base, true);
+ this->markWrittenTo(*base);
return std::unique_ptr<Expression>(new PostfixExpression(std::move(base),
Token::PLUSPLUS));
case ASTSuffix::kPostDecrement_Kind:
@@ -1450,7 +1473,7 @@
"'--' cannot operate on '" + base->fType.description() + "'");
return nullptr;
}
- this->markWrittenTo(*base, true);
+ this->markWrittenTo(*base);
return std::unique_ptr<Expression>(new PostfixExpression(std::move(base),
Token::MINUSMINUS));
default:
@@ -1473,6 +1496,10 @@
}
}
+void IRGenerator::markReadFrom(const Variable& var) {
+ var.fIsReadFrom = true;
+}
+
static bool has_duplicates(const Swizzle& swizzle) {
int bits = 0;
for (int idx : swizzle.fComponents) {
@@ -1486,7 +1513,7 @@
return false;
}
-void IRGenerator::markWrittenTo(const Expression& expr, bool readWrite) {
+void IRGenerator::markWrittenTo(const Expression& expr) {
switch (expr.fKind) {
case Expression::kVariableReference_Kind: {
const Variable& var = ((VariableReference&) expr).fVariable;
@@ -1494,22 +1521,21 @@
fErrors.error(expr.fPosition,
"cannot modify immutable variable '" + var.fName + "'");
}
- ((VariableReference&) expr).setRefKind(readWrite ? VariableReference::kReadWrite_RefKind
- : VariableReference::kWrite_RefKind);
+ var.fIsWrittenTo = true;
break;
}
case Expression::kFieldAccess_Kind:
- this->markWrittenTo(*((FieldAccess&) expr).fBase, readWrite);
+ this->markWrittenTo(*((FieldAccess&) expr).fBase);
break;
case Expression::kSwizzle_Kind:
if (has_duplicates((Swizzle&) expr)) {
fErrors.error(expr.fPosition,
"cannot write to the same swizzle field more than once");
}
- this->markWrittenTo(*((Swizzle&) expr).fBase, readWrite);
+ this->markWrittenTo(*((Swizzle&) expr).fBase);
break;
case Expression::kIndex_Kind:
- this->markWrittenTo(*((IndexExpression&) expr).fBase, readWrite);
+ this->markWrittenTo(*((IndexExpression&) expr).fBase);
break;
default:
fErrors.error(expr.fPosition, "cannot assign to '" + expr.description() + "'");
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index 2ffcb0d..13b20fb 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -88,16 +88,7 @@
std::unique_ptr<ModifiersDeclaration> convertModifiersDeclaration(
const ASTModifiersDeclaration& m);
- /**
- * If both operands are compile-time constants and can be folded, returns an expression
- * representing the folded value. Otherwise, returns null. Note that unlike most other functions
- * here, null does not represent a compilation error.
- */
- std::unique_ptr<Expression> constantFold(const Expression& left,
- Token::Kind op,
- const Expression& right) const;
Program::Inputs fInputs;
- const Context& fContext;
private:
/**
@@ -133,6 +124,11 @@
std::unique_ptr<Statement> convertDiscard(const ASTDiscardStatement& d);
std::unique_ptr<Statement> convertDo(const ASTDoStatement& d);
std::unique_ptr<Expression> convertBinaryExpression(const ASTBinaryExpression& expression);
+ // Returns null if it cannot fold the expression. Note that unlike most other functions here, a
+ // null return does not represent a compilation error.
+ std::unique_ptr<Expression> constantFold(const Expression& left,
+ Token::Kind op,
+ const Expression& right);
std::unique_ptr<Extension> convertExtension(const ASTExtension& e);
std::unique_ptr<Statement> convertExpressionStatement(const ASTExpressionStatement& s);
std::unique_ptr<Statement> convertFor(const ASTForStatement& f);
@@ -155,8 +151,10 @@
std::unique_ptr<Statement> convertWhile(const ASTWhileStatement& w);
void checkValid(const Expression& expr);
- void markWrittenTo(const Expression& expr, bool readWrite);
+ void markReadFrom(const Variable& var);
+ void markWrittenTo(const Expression& expr);
+ const Context& fContext;
const FunctionDeclaration* fCurrentFunction;
const Program::Settings* fSettings;
std::unordered_map<SkString, CapValue> fCapsMap;
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index d43e4c4..8afd136 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2540,7 +2540,7 @@
kind != Program::kFragment_Kind) {
continue;
}
- if (!var->fReadCount && !var->fWriteCount &&
+ if (!var->fIsReadFrom && !var->fIsWrittenTo &&
!(var->fModifiers.fFlags & (Modifiers::kIn_Flag |
Modifiers::kOut_Flag |
Modifiers::kUniform_Flag))) {
diff --git a/src/sksl/SkSLToken.h b/src/sksl/SkSLToken.h
index 197781f..5c8c2bd 100644
--- a/src/sksl/SkSLToken.h
+++ b/src/sksl/SkSLToken.h
@@ -160,28 +160,6 @@
, fKind(kind)
, fText(std::move(text)) {}
- static bool IsAssignment(Token::Kind op) {
- switch (op) {
- case Token::EQ: // fall through
- case Token::PLUSEQ: // fall through
- case Token::MINUSEQ: // fall through
- case Token::STAREQ: // fall through
- case Token::SLASHEQ: // fall through
- case Token::PERCENTEQ: // fall through
- case Token::SHLEQ: // fall through
- case Token::SHREQ: // fall through
- case Token::BITWISEOREQ: // fall through
- case Token::BITWISEXOREQ: // fall through
- case Token::BITWISEANDEQ: // fall through
- case Token::LOGICALOREQ: // fall through
- case Token::LOGICALXOREQ: // fall through
- case Token::LOGICALANDEQ:
- return true;
- default:
- return false;
- }
- }
-
Position fPosition;
Kind fKind;
// will be the empty string unless the token has variable text content (identifiers, numeric
diff --git a/src/sksl/ir/SkSLBinaryExpression.h b/src/sksl/ir/SkSLBinaryExpression.h
index de85e48..132513e 100644
--- a/src/sksl/ir/SkSLBinaryExpression.h
+++ b/src/sksl/ir/SkSLBinaryExpression.h
@@ -4,19 +4,17 @@
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
-
+
#ifndef SKSL_BINARYEXPRESSION
#define SKSL_BINARYEXPRESSION
#include "SkSLExpression.h"
-#include "SkSLExpression.h"
-#include "../SkSLIRGenerator.h"
#include "../SkSLToken.h"
namespace SkSL {
/**
- * A binary operation.
+ * A binary operation.
*/
struct BinaryExpression : public Expression {
BinaryExpression(Position position, std::unique_ptr<Expression> left, Token::Kind op,
@@ -26,22 +24,14 @@
, fOperator(op)
, fRight(std::move(right)) {}
- virtual std::unique_ptr<Expression> constantPropagate(
- const IRGenerator& irGenerator,
- const DefinitionMap& definitions) override {
- return irGenerator.constantFold(*fLeft,
- fOperator,
- *fRight);
- }
-
virtual SkString description() const override {
return "(" + fLeft->description() + " " + Token::OperatorName(fOperator) + " " +
fRight->description() + ")";
}
- std::unique_ptr<Expression> fLeft;
+ const std::unique_ptr<Expression> fLeft;
const Token::Kind fOperator;
- std::unique_ptr<Expression> fRight;
+ const std::unique_ptr<Expression> fRight;
typedef Expression INHERITED;
};
diff --git a/src/sksl/ir/SkSLBlock.h b/src/sksl/ir/SkSLBlock.h
index 17970fd..f975d16 100644
--- a/src/sksl/ir/SkSLBlock.h
+++ b/src/sksl/ir/SkSLBlock.h
@@ -20,8 +20,8 @@
Block(Position position, std::vector<std::unique_ptr<Statement>> statements,
const std::shared_ptr<SymbolTable> symbols)
: INHERITED(position, kBlock_Kind)
- , fSymbols(std::move(symbols))
- , fStatements(std::move(statements)) {}
+ , fStatements(std::move(statements))
+ , fSymbols(std::move(symbols)) {}
SkString description() const override {
SkString result("{");
@@ -33,10 +33,8 @@
return result;
}
- // it's important to keep fStatements defined after (and thus destroyed before) fSymbols,
- // because destroying statements can modify reference counts in symbols
- const std::shared_ptr<SymbolTable> fSymbols;
const std::vector<std::unique_ptr<Statement>> fStatements;
+ const std::shared_ptr<SymbolTable> fSymbols;
typedef Statement INHERITED;
};
diff --git a/src/sksl/ir/SkSLConstructor.h b/src/sksl/ir/SkSLConstructor.h
index 691bea1..63c692b 100644
--- a/src/sksl/ir/SkSLConstructor.h
+++ b/src/sksl/ir/SkSLConstructor.h
@@ -9,9 +9,6 @@
#define SKSL_CONSTRUCTOR
#include "SkSLExpression.h"
-#include "SkSLFloatLiteral.h"
-#include "SkSLIntLiteral.h"
-#include "SkSLIRGenerator.h"
namespace SkSL {
@@ -24,20 +21,6 @@
: INHERITED(position, kConstructor_Kind, type)
, fArguments(std::move(arguments)) {}
- virtual std::unique_ptr<Expression> constantPropagate(
- const IRGenerator& irGenerator,
- const DefinitionMap& definitions) override {
- if (fArguments.size() == 1 && fArguments[0]->fKind == Expression::kIntLiteral_Kind &&
- // promote float(1) to 1.0
- fType == *irGenerator.fContext.fFloat_Type) {
- int64_t intValue = ((IntLiteral&) *fArguments[0]).fValue;
- return std::unique_ptr<Expression>(new FloatLiteral(irGenerator.fContext,
- fPosition,
- intValue));
- }
- return nullptr;
- }
-
SkString description() const override {
SkString result = fType.description() + "(";
SkString separator;
@@ -59,7 +42,7 @@
return true;
}
- std::vector<std::unique_ptr<Expression>> fArguments;
+ const std::vector<std::unique_ptr<Expression>> fArguments;
typedef Expression INHERITED;
};
diff --git a/src/sksl/ir/SkSLDoStatement.h b/src/sksl/ir/SkSLDoStatement.h
index e26d3dc..78c0a1b 100644
--- a/src/sksl/ir/SkSLDoStatement.h
+++ b/src/sksl/ir/SkSLDoStatement.h
@@ -28,7 +28,7 @@
}
const std::unique_ptr<Statement> fStatement;
- std::unique_ptr<Expression> fTest;
+ const std::unique_ptr<Expression> fTest;
typedef Statement INHERITED;
};
diff --git a/src/sksl/ir/SkSLExpression.h b/src/sksl/ir/SkSLExpression.h
index f87d810..b4ed37c 100644
--- a/src/sksl/ir/SkSLExpression.h
+++ b/src/sksl/ir/SkSLExpression.h
@@ -4,24 +4,17 @@
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
-
+
#ifndef SKSL_EXPRESSION
#define SKSL_EXPRESSION
+#include "SkSLIRNode.h"
#include "SkSLType.h"
-#include "SkSLVariable.h"
-
-#include <unordered_map>
namespace SkSL {
-struct Expression;
-class IRGenerator;
-
-typedef std::unordered_map<const Variable*, std::unique_ptr<Expression>*> DefinitionMap;
-
/**
- * Abstract supertype of all expressions.
+ * Abstract supertype of all expressions.
*/
struct Expression : public IRNode {
enum Kind {
@@ -52,18 +45,6 @@
return false;
}
- /**
- * Given a map of known constant variable values, substitute them in for references to those
- * variables occurring in this expression and its subexpressions. Similar simplifications, such
- * as folding a constant binary expression down to a single value, may also be performed.
- * Returns a new expression which replaces this expression, or null if no replacements were
- * made. If a new expression is returned, this expression is no longer valid.
- */
- virtual std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
- const DefinitionMap& definitions) {
- return nullptr;
- }
-
const Kind fKind;
const Type& fType;
diff --git a/src/sksl/ir/SkSLExpressionStatement.h b/src/sksl/ir/SkSLExpressionStatement.h
index 088b1c9..677c647 100644
--- a/src/sksl/ir/SkSLExpressionStatement.h
+++ b/src/sksl/ir/SkSLExpressionStatement.h
@@ -25,7 +25,7 @@
return fExpression->description() + ";";
}
- std::unique_ptr<Expression> fExpression;
+ const std::unique_ptr<Expression> fExpression;
typedef Statement INHERITED;
};
diff --git a/src/sksl/ir/SkSLFieldAccess.h b/src/sksl/ir/SkSLFieldAccess.h
index de26a3f..fb727e0 100644
--- a/src/sksl/ir/SkSLFieldAccess.h
+++ b/src/sksl/ir/SkSLFieldAccess.h
@@ -35,7 +35,7 @@
return fBase->description() + "." + fBase->fType.fields()[fFieldIndex].fName;
}
- std::unique_ptr<Expression> fBase;
+ const std::unique_ptr<Expression> fBase;
const int fFieldIndex;
const OwnerKind fOwnerKind;
diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h
index 6f03e2b..ff03d0d 100644
--- a/src/sksl/ir/SkSLForStatement.h
+++ b/src/sksl/ir/SkSLForStatement.h
@@ -46,8 +46,8 @@
}
const std::unique_ptr<Statement> fInitializer;
- std::unique_ptr<Expression> fTest;
- std::unique_ptr<Expression> fNext;
+ const std::unique_ptr<Expression> fTest;
+ const std::unique_ptr<Expression> fNext;
const std::unique_ptr<Statement> fStatement;
const std::shared_ptr<SymbolTable> fSymbols;
diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h
index 1838076..971af36 100644
--- a/src/sksl/ir/SkSLFunctionCall.h
+++ b/src/sksl/ir/SkSLFunctionCall.h
@@ -36,7 +36,7 @@
}
const FunctionDeclaration& fFunction;
- std::vector<std::unique_ptr<Expression>> fArguments;
+ const std::vector<std::unique_ptr<Expression>> fArguments;
typedef Expression INHERITED;
};
diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h
index 8667e93..f8beded 100644
--- a/src/sksl/ir/SkSLIfStatement.h
+++ b/src/sksl/ir/SkSLIfStatement.h
@@ -32,7 +32,7 @@
return result;
}
- std::unique_ptr<Expression> fTest;
+ const std::unique_ptr<Expression> fTest;
const std::unique_ptr<Statement> fIfTrue;
const std::unique_ptr<Statement> fIfFalse;
diff --git a/src/sksl/ir/SkSLIndexExpression.h b/src/sksl/ir/SkSLIndexExpression.h
index d255c7d..079dde5 100644
--- a/src/sksl/ir/SkSLIndexExpression.h
+++ b/src/sksl/ir/SkSLIndexExpression.h
@@ -55,8 +55,8 @@
return fBase->description() + "[" + fIndex->description() + "]";
}
- std::unique_ptr<Expression> fBase;
- std::unique_ptr<Expression> fIndex;
+ const std::unique_ptr<Expression> fBase;
+ const std::unique_ptr<Expression> fIndex;
typedef Expression INHERITED;
};
diff --git a/src/sksl/ir/SkSLPostfixExpression.h b/src/sksl/ir/SkSLPostfixExpression.h
index 6c9fafe..01671b5 100644
--- a/src/sksl/ir/SkSLPostfixExpression.h
+++ b/src/sksl/ir/SkSLPostfixExpression.h
@@ -25,7 +25,7 @@
return fOperand->description() + Token::OperatorName(fOperator);
}
- std::unique_ptr<Expression> fOperand;
+ const std::unique_ptr<Expression> fOperand;
const Token::Kind fOperator;
typedef Expression INHERITED;
diff --git a/src/sksl/ir/SkSLPrefixExpression.h b/src/sksl/ir/SkSLPrefixExpression.h
index b7db99a..790c5ab 100644
--- a/src/sksl/ir/SkSLPrefixExpression.h
+++ b/src/sksl/ir/SkSLPrefixExpression.h
@@ -25,7 +25,7 @@
return Token::OperatorName(fOperator) + fOperand->description();
}
- std::unique_ptr<Expression> fOperand;
+ const std::unique_ptr<Expression> fOperand;
const Token::Kind fOperator;
typedef Expression INHERITED;
diff --git a/src/sksl/ir/SkSLProgram.h b/src/sksl/ir/SkSLProgram.h
index 6a73be6..ac49d6d 100644
--- a/src/sksl/ir/SkSLProgram.h
+++ b/src/sksl/ir/SkSLProgram.h
@@ -59,8 +59,8 @@
, fSettings(settings)
, fDefaultPrecision(defaultPrecision)
, fContext(context)
- , fSymbols(symbols)
, fElements(std::move(elements))
+ , fSymbols(symbols)
, fInputs(inputs) {}
Kind fKind;
@@ -68,10 +68,8 @@
// FIXME handle different types; currently it assumes this is for floats
Modifiers::Flag fDefaultPrecision;
Context* fContext;
- // it's important to keep fElements defined after (and thus destroyed before) fSymbols,
- // because destroying elements can modify reference counts in symbols
- std::shared_ptr<SymbolTable> fSymbols;
std::vector<std::unique_ptr<ProgramElement>> fElements;
+ std::shared_ptr<SymbolTable> fSymbols;
Inputs fInputs;
};
diff --git a/src/sksl/ir/SkSLReturnStatement.h b/src/sksl/ir/SkSLReturnStatement.h
index dc5ec9a..c83b450 100644
--- a/src/sksl/ir/SkSLReturnStatement.h
+++ b/src/sksl/ir/SkSLReturnStatement.h
@@ -32,7 +32,7 @@
}
}
- std::unique_ptr<Expression> fExpression;
+ const std::unique_ptr<Expression> fExpression;
typedef Statement INHERITED;
};
diff --git a/src/sksl/ir/SkSLSwizzle.h b/src/sksl/ir/SkSLSwizzle.h
index 8ad9001..c9397ae 100644
--- a/src/sksl/ir/SkSLSwizzle.h
+++ b/src/sksl/ir/SkSLSwizzle.h
@@ -76,7 +76,7 @@
return result;
}
- std::unique_ptr<Expression> fBase;
+ const std::unique_ptr<Expression> fBase;
const std::vector<int> fComponents;
typedef Expression INHERITED;
diff --git a/src/sksl/ir/SkSLTernaryExpression.h b/src/sksl/ir/SkSLTernaryExpression.h
index 0275004..4a35253 100644
--- a/src/sksl/ir/SkSLTernaryExpression.h
+++ b/src/sksl/ir/SkSLTernaryExpression.h
@@ -31,9 +31,9 @@
fIfFalse->description() + ")";
}
- std::unique_ptr<Expression> fTest;
- std::unique_ptr<Expression> fIfTrue;
- std::unique_ptr<Expression> fIfFalse;
+ const std::unique_ptr<Expression> fTest;
+ const std::unique_ptr<Expression> fIfTrue;
+ const std::unique_ptr<Expression> fIfFalse;
typedef Expression INHERITED;
};
diff --git a/src/sksl/ir/SkSLVarDeclarations.h b/src/sksl/ir/SkSLVarDeclarations.h
index 490259a..295c0b6 100644
--- a/src/sksl/ir/SkSLVarDeclarations.h
+++ b/src/sksl/ir/SkSLVarDeclarations.h
@@ -72,7 +72,7 @@
}
const Type& fBaseType;
- std::vector<VarDeclaration> fVars;
+ const std::vector<VarDeclaration> fVars;
typedef ProgramElement INHERITED;
};
diff --git a/src/sksl/ir/SkSLVarDeclarationsStatement.h b/src/sksl/ir/SkSLVarDeclarationsStatement.h
index 66b570f..7a29656 100644
--- a/src/sksl/ir/SkSLVarDeclarationsStatement.h
+++ b/src/sksl/ir/SkSLVarDeclarationsStatement.h
@@ -18,14 +18,14 @@
*/
struct VarDeclarationsStatement : public Statement {
VarDeclarationsStatement(std::unique_ptr<VarDeclarations> decl)
- : INHERITED(decl->fPosition, kVarDeclarations_Kind)
+ : INHERITED(decl->fPosition, kVarDeclarations_Kind)
, fDeclaration(std::move(decl)) {}
SkString description() const override {
return fDeclaration->description();
}
- std::shared_ptr<VarDeclarations> fDeclaration;
+ const std::shared_ptr<VarDeclarations> fDeclaration;
typedef Statement INHERITED;
};
diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h
index 2c3391d..39b8482 100644
--- a/src/sksl/ir/SkSLVariable.h
+++ b/src/sksl/ir/SkSLVariable.h
@@ -33,8 +33,8 @@
, fModifiers(modifiers)
, fType(type)
, fStorage(storage)
- , fReadCount(0)
- , fWriteCount(0) {}
+ , fIsReadFrom(false)
+ , fIsWrittenTo(false) {}
virtual SkString description() const override {
return fModifiers.description() + fType.fName + " " + fName;
@@ -44,12 +44,8 @@
const Type& fType;
const Storage fStorage;
- // Tracks how many sites read from the variable. If this is zero for a non-out variable (or
- // becomes zero during optimization), the variable is dead and may be eliminated.
- mutable int fReadCount;
- // Tracks how many sites write to the variable. If this is zero, the variable is dead and may be
- // eliminated.
- mutable int fWriteCount;
+ mutable bool fIsReadFrom;
+ mutable bool fIsWrittenTo;
typedef Symbol INHERITED;
};
diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h
index fecb04e..c6a2ea0 100644
--- a/src/sksl/ir/SkSLVariableReference.h
+++ b/src/sksl/ir/SkSLVariableReference.h
@@ -20,83 +20,16 @@
* there is only one Variable 'x', but two VariableReferences to it.
*/
struct VariableReference : public Expression {
- enum RefKind {
- kRead_RefKind,
- kWrite_RefKind,
- kReadWrite_RefKind
- };
-
- VariableReference(Position position, const Variable& variable, RefKind refKind = kRead_RefKind)
+ VariableReference(Position position, const Variable& variable)
: INHERITED(position, kVariableReference_Kind, variable.fType)
- , fVariable(variable)
- , fRefKind(refKind) {
- if (refKind != kRead_RefKind) {
- fVariable.fWriteCount++;
- }
- if (refKind != kWrite_RefKind) {
- fVariable.fReadCount++;
- }
- }
-
- virtual ~VariableReference() override {
- if (fRefKind != kWrite_RefKind) {
- fVariable.fReadCount--;
- }
- }
-
- RefKind refKind() {
- return fRefKind;
- }
-
- void setRefKind(RefKind refKind) {
- if (fRefKind != kRead_RefKind) {
- fVariable.fWriteCount--;
- }
- if (fRefKind != kWrite_RefKind) {
- fVariable.fReadCount--;
- }
- if (refKind != kRead_RefKind) {
- fVariable.fWriteCount++;
- }
- if (refKind != kWrite_RefKind) {
- fVariable.fReadCount++;
- }
- fRefKind = refKind;
- }
+ , fVariable(variable) {}
SkString description() const override {
return fVariable.fName;
}
- virtual std::unique_ptr<Expression> constantPropagate(
- const IRGenerator& irGenerator,
- const DefinitionMap& definitions) override {
- auto exprIter = definitions.find(&fVariable);
- if (exprIter != definitions.end() && exprIter->second) {
- const Expression* expr = exprIter->second->get();
- switch (expr->fKind) {
- case Expression::kIntLiteral_Kind:
- return std::unique_ptr<Expression>(new IntLiteral(
- irGenerator.fContext,
- Position(),
- ((IntLiteral*) expr)->fValue));
- case Expression::kFloatLiteral_Kind:
- return std::unique_ptr<Expression>(new FloatLiteral(
- irGenerator.fContext,
- Position(),
- ((FloatLiteral*) expr)->fValue));
- default:
- break;
- }
- }
- return nullptr;
- }
-
const Variable& fVariable;
-private:
- RefKind fRefKind;
-
typedef Expression INHERITED;
};
diff --git a/src/sksl/ir/SkSLWhileStatement.h b/src/sksl/ir/SkSLWhileStatement.h
index a741a04..7c6a290 100644
--- a/src/sksl/ir/SkSLWhileStatement.h
+++ b/src/sksl/ir/SkSLWhileStatement.h
@@ -27,7 +27,7 @@
return "while (" + fTest->description() + ") " + fStatement->description();
}
- std::unique_ptr<Expression> fTest;
+ const std::unique_ptr<Expression> fTest;
const std::unique_ptr<Statement> fStatement;
typedef Statement INHERITED;
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index 1501dc5..12ac4d1 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -61,7 +61,7 @@
"while (i < 10) sk_FragColor *= 0.5;"
"do { sk_FragColor += 0.01; } while (sk_FragColor.x < 0.75);"
"for (int i = 0; i < 10; i++) {"
- "if (i % 2 == 1) break; else continue;"
+ "if (i % 0 == 1) break; else continue;"
"}"
"return;"
"}",
@@ -75,12 +75,12 @@
" discard;\n"
" }\n"
" int i = 0;\n"
- " while (true) sk_FragColor *= 0.5;\n"
+ " while (i < 10) sk_FragColor *= 0.5;\n"
" do {\n"
" sk_FragColor += 0.01;\n"
" } while (sk_FragColor.x < 0.75);\n"
" for (int i = 0;i < 10; i++) {\n"
- " if (i % 2 == 1) break; else continue;\n"
+ " if (i % 0 == 1) break; else continue;\n"
" }\n"
" return;\n"
"}\n");
@@ -106,8 +106,8 @@
"}\n"
"void main() {\n"
" float x = 10.0;\n"
- " bar(10.0);\n"
- " sk_FragColor = vec4(10.0);\n"
+ " bar(x);\n"
+ " sk_FragColor = vec4(x);\n"
"}\n");
}
@@ -116,7 +116,7 @@
"void main() {"
"float x = 1, y = 2;"
"int z = 3;"
- "x = x - x + y * z * x * (y - z);"
+ "x = x + y * z * x * (y - z);"
"y = x / y / z;"
"z = (z / 2 % 3 << 4) >> 2 << 1;"
"bool b = (x > 4) == x < 2 || 2 >= sqrt(2) && y <= z;"
@@ -139,10 +139,10 @@
"void main() {\n"
" float x = 1.0, y = 2.0;\n"
" int z = 3;\n"
- " x = -6.0;\n"
- " y = -1.0;\n"
- " z = 8;\n"
- " bool b = false == true || 2.0 >= sqrt(2.0) && true;\n"
+ " x = x + ((y * float(z)) * x) * (y - float(z));\n"
+ " y = (x / y) / float(z);\n"
+ " z = (((z / 2) % 3 << 4) >> 2) << 1;\n"
+ " bool b = x > 4.0 == x < 2.0 || 2.0 >= sqrt(2.0) && y <= float(z);\n"
" x += 12.0;\n"
" x -= 12.0;\n"
" x *= (y /= float(z = 10));\n"
@@ -287,7 +287,7 @@
"out vec4 sk_FragColor;\n"
"void main() {\n"
" float x = -5.0;\n"
- " x = min(abs(-5.0), 6.0);\n"
+ " x = min(abs(x), 6.0);\n"
"}\n");
test(r,
@@ -302,7 +302,7 @@
" float minAbsHackVar0;\n"
" float minAbsHackVar1;\n"
" float x = -5.0;\n"
- " x = ((minAbsHackVar0 = abs(-5.0)) < (minAbsHackVar1 = 6.0) ? minAbsHackVar0 : "
+ " x = ((minAbsHackVar0 = abs(x)) < (minAbsHackVar1 = 6.0) ? minAbsHackVar0 : "
"minAbsHackVar1);\n"
"}\n");
}