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

Disallow inlining ternary true/false branches.

Browse Source 
This prevents unnecessary work from being done, and also prevents us
from executing side-effects from the wrong side.

Change-Id: I4dbf3974388807f15e9eadb2abf1b1243d047ce2
Bug: skia:10688
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314797
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-09-02 11:27:23 -04:00 committed by Skia Commit-Bot
parent 648a81e5e8
commit 8fa3b4e8cd
2 changed files with 31 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/sksl/SkSLIRGenerator.cpp
View File

@ -1879,14 +1879,19 @@ std::unique_ptr<Expression> IRGenerator::convertTernaryExpression(const ASTNode&
if (!test) {
return nullptr;
}
std::unique_ptr<Expression> ifTrue = this->convertExpression(*(iter++));
std::unique_ptr<Expression> ifTrue;
std::unique_ptr<Expression> ifFalse;
{
AutoDisableInline disableInline(this);
ifTrue = this->convertExpression(*(iter++));
if (!ifTrue) {
return nullptr;
}
std::unique_ptr<Expression> ifFalse = this->convertExpression(*(iter++));
ifFalse = this->convertExpression(*(iter++));
if (!ifFalse) {
return nullptr;
}
}
const Type* trueType;
const Type* falseType;
const Type* resultType;
@ -1918,10 +1923,10 @@ std::unique_ptr<Expression> IRGenerator::convertTernaryExpression(const ASTNode&
return ifFalse;
}
}
return std::unique_ptr<Expression>(new TernaryExpression(node.fOffset,
return std::make_unique<TernaryExpression>(node.fOffset,
std::move(test),
std::move(ifTrue),
std::move(ifFalse)));
std::move(ifFalse));
}
void IRGenerator::copyIntrinsicIfNeeded(const FunctionDeclaration& function) {

33
tests/SkSLFPTest.cpp
View File

@ -1088,9 +1088,6 @@ R"SkSL(%s = %s(%s);
}
DEF_TEST(SkSLFPTernaryExpressionsShouldNotInlineResults, r) {
// NOTE: this test exposes a bug with the inliner. We don't want to inline both sides of a
// ternary, since only one side needs to be evaluated, and side effects from the wrong side
// must not occur.
test(r,
*SkSL::ShaderCapsFactory::Default(),
R"__SkSL__(
@ -1114,30 +1111,30 @@ DEF_TEST(SkSLFPTernaryExpressionsShouldNotInlineResults, r) {
)__SkSL__",
/*expectedH=*/{},
/*expectedCPP=*/{
R"__Cpp__(fragBuilder->codeAppendf(
R"__Cpp__(SkString trueSide_name;
const GrShaderVar trueSide_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
fragBuilder->emitFunction(kHalf4_GrSLType, "trueSide", 1, trueSide_args,
R"SkSL(count += 1.0;
return half4(sin(v.x), sin(v.y), sin(v.z), sin(v.w));
)SkSL", &trueSide_name);
SkString falseSide_name;
const GrShaderVar falseSide_args[] = { GrShaderVar("v", kHalf4_GrSLType)};
fragBuilder->emitFunction(kHalf4_GrSLType, "falseSide", 1, falseSide_args,
R"SkSL(count += 1.0;
return half4(cos(v.y), cos(v.z), cos(v.w), cos(v.z));
)SkSL", &falseSide_name);
fragBuilder->codeAppendf(
R"SkSL(half count = %f;
bool _0_test;
{
_0_test = %s.x <= 0.5;
}
half4 _1_trueSide;
{
count += 1.0;
_1_trueSide = half4(sin(%s.x), sin(%s.y), sin(%s.z), sin(%s.w));
}
half4 _2_falseSide;
{
count += 1.0;
_2_falseSide = half4(cos(%s.y), cos(%s.z), cos(%s.w), cos(%s.z));
}
%s = _0_test ? _1_trueSide : _2_falseSide;
%s = _0_test ? %s(%s) : %s(%s);
%s *= count;
)SkSL"
, count, args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fUniformHandler->getUniformCStr(colorVar), args.fOutputColor, args.fOutputColor);
, count, args.fUniformHandler->getUniformCStr(colorVar), args.fOutputColor, trueSide_name.c_str(), args.fUniformHandler->getUniformCStr(colorVar), falseSide_name.c_str(), args.fUniformHandler->getUniformCStr(colorVar), args.fOutputColor);
)__Cpp__"});
}