AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Ensure that inlined if/else blocks are properly scoped.

Browse Source 
The bulk of the diff is just reordering existing code; the logical
changes are very small.

Change-Id: I3b918e64f5229da43d43f0922e8b59a007a6ad3e
Bug: skia:10687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314882
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This commit is contained in:
John Stiles 2020-09-02 15:01:47 -04:00 committed by Skia Commit-Bot
parent da6320ca11
commit ad2319f7c4
2 changed files with 51 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
src/sksl/SkSLIRGenerator.cpp
View File

@ -498,42 +498,6 @@ std::unique_ptr<ModifiersDeclaration> IRGenerator::convertModifiersDeclaration(c
return std::make_unique<ModifiersDeclaration>(modifiers);
}
std::unique_ptr<Statement> IRGenerator::convertIf(const ASTNode& n) {
SkASSERT(n.fKind == ASTNode::Kind::kIf);
auto iter = n.begin();
std::unique_ptr<Expression> test = this->coerce(this->convertExpression(*(iter++)),
*fContext.fBool_Type);
if (!test) {
return nullptr;
}
std::unique_ptr<Statement> ifTrue = this->convertStatement(*(iter++));
if (!ifTrue) {
return nullptr;
}
std::unique_ptr<Statement> ifFalse;
if (iter != n.end()) {
ifFalse = this->convertStatement(*(iter++));
if (!ifFalse) {
return nullptr;
}
}
if (test->fKind == Expression::kBoolLiteral_Kind) {
// static boolean value, fold down to a single branch
if (test->as<BoolLiteral>().fValue) {
return ifTrue;
} else if (ifFalse) {
return ifFalse;
} else {
// False & no else clause. Not an error, so don't return null!
std::vector<std::unique_ptr<Statement>> empty;
return std::unique_ptr<Statement>(new Block(n.fOffset, std::move(empty),
fSymbolTable));
}
}
return std::unique_ptr<Statement>(new IfStatement(n.fOffset, n.getBool(), std::move(test),
std::move(ifTrue), std::move(ifFalse)));
}
static void ensure_scoped_blocks(Statement* stmt) {
// No changes necessary if this statement isn't actually a block.
if (stmt->fKind != Statement::kBlock_Kind) {
@ -570,6 +534,43 @@ static void ensure_scoped_blocks(Statement* stmt) {
}
}
std::unique_ptr<Statement> IRGenerator::convertIf(const ASTNode& n) {
SkASSERT(n.fKind == ASTNode::Kind::kIf);
auto iter = n.begin();
std::unique_ptr<Expression> test = this->coerce(this->convertExpression(*(iter++)),
*fContext.fBool_Type);
if (!test) {
return nullptr;
}
std::unique_ptr<Statement> ifTrue = this->convertStatement(*(iter++));
if (!ifTrue) {
return nullptr;
}
ensure_scoped_blocks(ifTrue.get());
std::unique_ptr<Statement> ifFalse;
if (iter != n.end()) {
ifFalse = this->convertStatement(*(iter++));
if (!ifFalse) {
return nullptr;
}
ensure_scoped_blocks(ifFalse.get());
}
if (test->fKind == Expression::kBoolLiteral_Kind) {
// static boolean value, fold down to a single branch
if (test->as<BoolLiteral>().fValue) {
return ifTrue;
} else if (ifFalse) {
return ifFalse;
} else {
// False & no else clause. Not an error, so don't return null!
std::vector<std::unique_ptr<Statement>> empty;
return std::make_unique<Block>(n.fOffset, std::move(empty), fSymbolTable);
}
}
return std::make_unique<IfStatement>(n.fOffset, n.getBool(),
std::move(test), std::move(ifTrue), std::move(ifFalse));
}
std::unique_ptr<Statement> IRGenerator::convertFor(const ASTNode& f) {
SkASSERT(f.fKind == ASTNode::Kind::kFor);
AutoLoopLevel level(this);

30
tests/SkSLFPTest.cpp
View File

@ -1432,8 +1432,6 @@ if (_0_ifTest) %s = half4(1.0); else %s = half4(0.5);
}
DEF_TEST(SkSLFPInlinedIfBodyMustBeInAScope, r) {
// NOTE: this test exposes a bug with the inliner. The inlined function body is not wrapped in a
// scope, so the inlined code is emitted outside of the if-body.
test(r,
*SkSL::ShaderCapsFactory::Default(),
R"__SkSL__(
@ -1452,13 +1450,14 @@ DEF_TEST(SkSLFPInlinedIfBodyMustBeInAScope, r) {
/*expectedCPP=*/{
R"__Cpp__(fragBuilder->codeAppendf(
R"SkSL(half4 c = %s;
if (c.x >= 0.5) half4 _0_ifBody;
{
_0_ifBody = %s + half4(0.125);
if (c.x >= 0.5) {
half4 _0_ifBody;
{
_0_ifBody = %s + half4(0.125);
}
c = _0_ifBody;
}
c = _0_ifBody;
%s = c;
)SkSL"
, args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar));
@ -1466,8 +1465,6 @@ c = _0_ifBody;
}
DEF_TEST(SkSLFPInlinedElseBodyMustBeInAScope, r) {
// NOTE: this test exposes a bug with the inliner. The inlined function body is not wrapped in a
// scope, so the inlined code is emitted outside of the else-body.
test(r,
*SkSL::ShaderCapsFactory::Default(),
R"__SkSL__(
@ -1489,13 +1486,14 @@ DEF_TEST(SkSLFPInlinedElseBodyMustBeInAScope, r) {
R"__Cpp__(fragBuilder->codeAppendf(
R"SkSL(half4 c = %s;
if (c.x >= 0.5) {
} else half4 _0_elseBody;
{
_0_elseBody = %s + half4(0.125);
} else {
half4 _0_elseBody;
{
_0_elseBody = %s + half4(0.125);
}
c = _0_elseBody;
}
c = _0_elseBody;
%s = c;
)SkSL"
, args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar));