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

Fix simplification of switch statements with casts.

Browse Source 
The original code assumed that the switch value and case value would
both be IntLiterals, and asserted if this were not true. However, in
sufficiently complicated cases, the switch value can sometimes be a
Constructor containing a scalar.

It's possible that there's a fix to constant propagation which would
flatten out this Constructor to just a scalar, but that has eluded me
for today, and this approach does seem to solve the issue. (Also, it's
surprising that the optimizer does not notice that it can simplify
`half4(1.0, 2.0, 3.0, 4.0).yyyy`.)

Change-Id: Id490b15c724dd174c448665a19242cc80cdce213
Bug: skia:10623
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-08-19 09:56:49 -04:00 committed by Skia Commit-Bot
parent 18305c3d36
commit 9d944236e8
2 changed files with 65 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
src/sksl/SkSLCompiler.cpp
View File

@ -647,27 +647,43 @@ static bool try_replace_expression(BasicBlock* b,
* Returns true if the expression is a constant numeric literal with the specified value, or a
* constant vector with all elements equal to the specified value.
*/
static bool is_constant(const Expression& expr, double value) {
template <typename T = double>
static bool is_constant(const Expression& expr, T value) {
switch (expr.fKind) {
case Expression::kIntLiteral_Kind:
return expr.as<IntLiteral>().fValue == value;
case Expression::kFloatLiteral_Kind:
return expr.as<FloatLiteral>().fValue == value;
case Expression::kConstructor_Kind: {
const Constructor& c = expr.as<Constructor>();
bool isFloat = c.fType.columns() > 1 ? c.fType.componentType().isFloat()
: c.fType.isFloat();
if (c.fType.kind() == Type::kVector_Kind && c.isCompileTimeConstant()) {
for (int i = 0; i < c.fType.columns(); ++i) {
if (isFloat) {
if (c.getFVecComponent(i) != value) {
return false;
const Constructor& constructor = expr.as<Constructor>();
if (constructor.isCompileTimeConstant()) {
bool isFloat = constructor.fType.columns() > 1
? constructor.fType.componentType().isFloat()
: constructor.fType.isFloat();
switch (constructor.fType.kind()) {
case Type::kVector_Kind:
for (int i = 0; i < constructor.fType.columns(); ++i) {
if (isFloat) {
if (constructor.getFVecComponent(i) != value) {
return false;
}
} else {
if (constructor.getIVecComponent(i) != value) {
return false;
}
}
}
} else if (c.getIVecComponent(i) != value) {
return true;
case Type::kScalar_Kind:
SkASSERT(constructor.fArguments.size() == 1);
return is_constant<T>(*constructor.fArguments[0], value);
default:
return false;
}
}
return true;
}
return false;
}
@ -1315,17 +1331,16 @@ void Compiler::simplifyStatement(DefinitionMap& definitions,
// switch is constant, replace it with the case that matches
bool found = false;
SwitchCase* defaultCase = nullptr;
for (const auto& c : s.fCases) {
for (const std::unique_ptr<SwitchCase>& c : s.fCases) {
if (!c->fValue) {
defaultCase = c.get();
continue;
}
SkASSERT(c->fValue->fKind == s.fValue->fKind);
found = c->fValue->compareConstant(*fContext, *s.fValue);
if (found) {
if (is_constant<int64_t>(*s.fValue, c->fValue->getConstantInt())) {
std::unique_ptr<Statement> newBlock = block_for_case(&s, c.get());
if (newBlock) {
(*iter)->setStatement(std::move(newBlock));
found = true;
break;
} else {
if (s.fIsStatic && !(fFlags & kPermitInvalidStaticTests_Flag)) {

34
tests/SkSLFPTest.cpp
View File

@ -1081,6 +1081,40 @@ R"SkSL(%s = %s(%s);
});
}
DEF_TEST(SkSLFPSwitchWithCastCanBeInlined, r) {
test(r,
*SkSL::ShaderCapsFactory::Default(),
R"__SkSL__(
half4 switchy(half4 c) {
half4 result;
switch (int(c.x)) {
case 1: result = c.yyyy; break;
default: result = c.zzzz; break;
}
return result;
}
void main() {
sk_OutColor = switchy(half4(1, 2, 3, 4));
}
)__SkSL__",
/*expectedH=*/{},
/*expectedCPP=*/{R"__Cpp__(
fragBuilder->codeAppendf(
R"SkSL(half4 _inlineResulthalf4switchyhalf40;
{
half4 result;
{
result = half4(1.0, 2.0, 3.0, 4.0).yyyy;
}
_inlineResulthalf4switchyhalf40 = result;
}
%s = _inlineResulthalf4switchyhalf40;
)SkSL"
, args.fOutputColor);
)__Cpp__"});
}
DEF_TEST(SkSLFPForLoopWithoutReturnInsideCanBeInlined, r) {
test(r,
*SkSL::ShaderCapsFactory::Default(),