skslc comma operator and optimizer fixes
Bug: skia:
Change-Id: I732d4fba843c06af570d4a56cadfaa1cc565808c
Reviewed-on: https://skia-review.googlesource.com/17125
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ben Wagner <benjaminwagner@google.com>
diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
index b756f2d..02e4e70 100644
--- a/src/sksl/SkSLCFGGenerator.cpp
+++ b/src/sksl/SkSLCFGGenerator.cpp
@@ -93,8 +93,7 @@
Expression* old = (*iter)->expression()->get();
do {
if ((*iter) == fNodes.begin()) {
- ABORT("couldn't find %s before %s\n", e->description().c_str(),
- old->description().c_str());
+ return false;
}
--(*iter);
} while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
@@ -109,8 +108,7 @@
Statement* old = (*iter)->statement()->get();
do {
if ((*iter) == fNodes.begin()) {
- ABORT("couldn't find %s before %s\n", e->description().c_str(),
- old->description().c_str());
+ return false;
}
--(*iter);
} while ((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
@@ -300,6 +298,12 @@
this->addExpression(cfg, &b->fRight, constantPropagate);
cfg.newBlock();
cfg.addExit(start, cfg.fCurrent);
+ cfg.fBlocks[cfg.fCurrent].fNodes.push_back({
+ BasicBlock::Node::kExpression_Kind,
+ constantPropagate,
+ e,
+ nullptr
+ });
break;
}
case Token::EQ: {
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index a283e30..f63ef97 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -436,20 +436,27 @@
BinaryExpression& bin = (BinaryExpression&) **target;
bool result;
if (bin.fOperator == Token::EQ) {
- result = !b->tryRemoveLValueBefore(iter, bin.fLeft.get());
+ result = b->tryRemoveLValueBefore(iter, bin.fLeft.get());
} else {
- result = !b->tryRemoveExpressionBefore(iter, bin.fLeft.get());
+ result = b->tryRemoveExpressionBefore(iter, bin.fLeft.get());
}
+ *target = std::move(bin.fRight);
if (!result) {
- *target = std::move(bin.fRight);
+ *outNeedsRescan = true;
+ return;
+ }
+ if (*iter == b->fNodes.begin()) {
*outNeedsRescan = true;
return;
}
--(*iter);
- ASSERT((*iter)->expression() == &bin.fRight);
+ if ((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
+ (*iter)->expression() != &bin.fRight) {
+ *outNeedsRescan = true;
+ return;
+ }
*iter = b->fNodes.erase(*iter);
ASSERT((*iter)->expression() == target);
- *target = std::move(bin.fRight);
}
/**
@@ -469,11 +476,19 @@
*outNeedsRescan = true;
return;
}
+ *target = std::move(bin.fLeft);
+ if (*iter == b->fNodes.begin()) {
+ *outNeedsRescan = true;
+ return;
+ }
--(*iter);
- ASSERT((*iter)->expression() == &bin.fLeft);
+ if (((*iter)->fKind != BasicBlock::Node::kExpression_Kind ||
+ (*iter)->expression() != &bin.fLeft)) {
+ *outNeedsRescan = true;
+ return;
+ }
*iter = b->fNodes.erase(*iter);
ASSERT((*iter)->expression() == target);
- *target = std::move(bin.fLeft);
}
/**
@@ -569,12 +584,13 @@
if ((*iter)->fConstantPropagation) {
std::unique_ptr<Expression> optimized = expr->constantPropagate(*fIRGenerator, definitions);
if (optimized) {
+ *outUpdated = true;
if (!try_replace_expression(&b, iter, &optimized)) {
*outNeedsRescan = true;
+ return;
}
ASSERT((*iter)->fKind == BasicBlock::Node::kExpression_Kind);
expr = (*iter)->expression()->get();
- *outUpdated = true;
}
}
switch (expr->fKind) {
@@ -1011,6 +1027,9 @@
this->simplifyStatement(definitions, b, &iter, &undefinedVariables, &updated,
&needsRescan);
}
+ if (needsRescan) {
+ break;
+ }
this->addDefinitions(*iter, &definitions);
}
}
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 8099d45..d67f718 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -406,6 +406,7 @@
case Token::BITWISEANDEQ: // fall through
case Token::BITWISEXOREQ: // fall through
case Token::BITWISEOREQ: return GLSLCodeGenerator::kAssignment_Precedence;
+ case Token::COMMA: return GLSLCodeGenerator::kSequence_Precedence;
default: ABORT("unsupported binary operator");
}
}
diff --git a/src/sksl/SkSLGLSLCodeGenerator.h b/src/sksl/SkSLGLSLCodeGenerator.h
index 65be7dc..032b70e 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.h
+++ b/src/sksl/SkSLGLSLCodeGenerator.h
@@ -68,7 +68,7 @@
kTernary_Precedence = 15,
kAssignment_Precedence = 16,
kSequence_Precedence = 17,
- kTopLevel_Precedence = 18
+ kTopLevel_Precedence = kSequence_Precedence
};
GLSLCodeGenerator(const Context* context, const Program* program, ErrorReporter* errors,
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index e160a71..2e280d8 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -917,10 +917,15 @@
case Token::MINUS: // fall through
case Token::MINUSEQ: // fall through
case Token::SLASH: // fall through
- case Token::SLASHEQ:
+ case Token::SLASHEQ: // fall through
isLogical = false;
validMatrixOrVectorOp = true;
break;
+ case Token::COMMA:
+ *outLeftType = &left;
+ *outRightType = &right;
+ *outResultType = &right;
+ return true;
default:
isLogical = false;
validMatrixOrVectorOp = false;
diff --git a/src/sksl/SkSLParser.cpp b/src/sksl/SkSLParser.cpp
index 04e2517..5e8ec63 100644
--- a/src/sksl/SkSLParser.cpp
+++ b/src/sksl/SkSLParser.cpp
@@ -439,8 +439,8 @@
return nullptr;
}
-/* (LBRACKET expression? RBRACKET)* (EQ expression)? (COMMA IDENTIFER
- (LBRACKET expression? RBRACKET)* (EQ expression)?)* SEMICOLON */
+/* (LBRACKET expression? RBRACKET)* (EQ assignmentExpression)? (COMMA IDENTIFER
+ (LBRACKET expression? RBRACKET)* (EQ assignmentExpression)?)* SEMICOLON */
std::unique_ptr<ASTVarDeclarations> Parser::varDeclarationEnd(Modifiers mods,
std::unique_ptr<ASTType> type,
String name) {
@@ -462,7 +462,7 @@
}
std::unique_ptr<ASTExpression> value;
if (this->checkNext(Token::EQ)) {
- value = this->expression();
+ value = this->assignmentExpression();
if (!value) {
return nullptr;
}
@@ -490,7 +490,7 @@
}
}
if (this->checkNext(Token::EQ)) {
- value = this->expression();
+ value = this->assignmentExpression();
if (!value) {
return nullptr;
}
@@ -1222,7 +1222,24 @@
if (!depth.checkValid()) {
return nullptr;
}
- return this->assignmentExpression();
+ return this->commaExpression();
+}
+
+/* assignmentExpression (COMMA assignmentExpression)* */
+std::unique_ptr<ASTExpression> Parser::commaExpression() {
+ std::unique_ptr<ASTExpression> result = this->assignmentExpression();
+ if (!result) {
+ return nullptr;
+ }
+ Token t;
+ while (this->checkNext(Token::COMMA, &t)) {
+ std::unique_ptr<ASTExpression> right = this->commaExpression();
+ if (!right) {
+ return nullptr;
+ }
+ result.reset(new ASTBinaryExpression(std::move(result), t, std::move(right)));
+ }
+ return result;
}
/* ternaryExpression ((EQEQ | STAREQ | SLASHEQ | PERCENTEQ | PLUSEQ | MINUSEQ | SHLEQ | SHREQ |
@@ -1587,15 +1604,14 @@
std::vector<std::unique_ptr<ASTExpression>> parameters;
if (this->peek().fKind != Token::RPAREN) {
for (;;) {
- std::unique_ptr<ASTExpression> expr = this->expression();
+ std::unique_ptr<ASTExpression> expr = this->assignmentExpression();
if (!expr) {
return nullptr;
}
parameters.push_back(std::move(expr));
- if (this->peek().fKind != Token::COMMA) {
+ if (!this->checkNext(Token::COMMA)) {
break;
}
- this->nextToken();
}
}
this->expect(Token::RPAREN, "')' to complete function parameters");
diff --git a/src/sksl/SkSLParser.h b/src/sksl/SkSLParser.h
index 6d4a0da..2f55b34 100644
--- a/src/sksl/SkSLParser.h
+++ b/src/sksl/SkSLParser.h
@@ -169,6 +169,8 @@
std::unique_ptr<ASTExpression> expression();
+ std::unique_ptr<ASTExpression> commaExpression();
+
std::unique_ptr<ASTExpression> assignmentExpression();
std::unique_ptr<ASTExpression> ternaryExpression();
diff --git a/src/sksl/SkSLToken.h b/src/sksl/SkSLToken.h
index 07eb856..92193b9 100644
--- a/src/sksl/SkSLToken.h
+++ b/src/sksl/SkSLToken.h
@@ -174,6 +174,7 @@
case Token::BITWISEXOREQ: return String("^=");
case Token::PLUSPLUS: return String("++");
case Token::MINUSMINUS: return String("--");
+ case Token::COMMA: return String(",");
default:
ABORT("unsupported operator: %d\n", kind);
}
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index ff1d5f5..ba6bbbd 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -141,6 +141,8 @@
"z >>= 2;"
"z <<= 4;"
"z %= 5;"
+ "x = (vec2(sqrt(1)) , 6);"
+ "z = (vec2(sqrt(1)) , 6);"
"}",
*SkSL::ShaderCapsFactory::Default(),
"#version 400\n"
@@ -164,6 +166,8 @@
" z >>= 2;\n"
" z <<= 4;\n"
" z %= 5;\n"
+ " x = float((vec2(sqrt(1.0)) , 6));\n"
+ " z = (vec2(sqrt(1.0)) , 6);\n"
"}\n");
}
@@ -1334,4 +1338,26 @@
"}\n");
}
+DEF_TEST(SkSLComplexDelete, r) {
+ test(r,
+ "uniform mat4 colorXform;"
+ "uniform sampler2D sampler;"
+ "void main() {"
+ "vec4 tmpColor;"
+ "sk_FragColor = vec4(1.0) * (tmpColor = texture(sampler, vec2(1)) , "
+ "colorXform != mat4(1.0) ? vec4(clamp((mat4(colorXform) * vec4(tmpColor.xyz, 1.0)).xyz, "
+ "0.0, tmpColor.w), tmpColor.w) : tmpColor);"
+ "}",
+ *SkSL::ShaderCapsFactory::Default(),
+ "#version 400\n"
+ "out vec4 sk_FragColor;\n"
+ "uniform mat4 colorXform;\n"
+ "uniform sampler2D sampler;\n"
+ "void main() {\n"
+ " vec4 tmpColor;\n"
+ " sk_FragColor = (tmpColor = texture(sampler, vec2(1.0)) , colorXform != mat4(1.0) ? "
+ "vec4(clamp((colorXform * vec4(tmpColor.xyz, 1.0)).xyz, 0.0, tmpColor.w), tmpColor.w) : "
+ "tmpColor);\n"
+ "}\n");
+}
#endif