Fix simplification of switch statements with casts.
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>
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 327f8b8..68ff703 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -647,27 +647,43 @@
* 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 @@
// 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)) {
diff --git a/tests/SkSLFPTest.cpp b/tests/SkSLFPTest.cpp
index 2ee85b8..bb87e25 100644
--- a/tests/SkSLFPTest.cpp
+++ b/tests/SkSLFPTest.cpp
@@ -1081,6 +1081,40 @@
});
}
+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(),