Improve constructor handling in Analysis::IsTrivialExpression.

This code was originally written before our constructor IR types were
split apart, so its treatment of constructors was pretty simplistic. The
new code handles each type of constructor separately, and makes better
decisions as a result.

Compound constructors are now only considered to be trivial when they
are compile-time constants; this causes vector<->matrix conversion
constructors to be detected as non-trivial.

Change-Id: I534fcf69f5d5f43ac705d68ae00f97fce8496c2a
Bug: skia:13378
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546555
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>

resources/sksl/inliner/TrivialArgumentsInlineDirectly.sksl

@@ -62,11 +62,11 @@ half4 main(float2 coords) {
     var = func4(half4(s.h));
     var = func4(s.ah4[0].xxxy);
     var = func4(colorGreen);
-    var = func4(half4(testMatrix2x2));  // matrix -> vector cast
-    mat = func2x2(half2x2(colorGreen)); // vector -> matrix cast
 
     // These expressions are considered "non-trivial" and will be placed in a temporary variable
     // when inlining occurs.
+    var = func4(half4(testMatrix2x2));  // matrix -> vector cast
+    mat = func2x2(half2x2(colorGreen)); // vector -> matrix cast
     var = func1(-s.h);
     var = funcb(!b);
 //  var = func2(as[i].h4.yw);  // indexing by non-constant expressions disallowed in ES2

src/sksl/SkSLAnalysis.h

@@ -125,7 +125,7 @@ bool UpdateVariableRefKind(Expression* expr, VariableRefKind kind, ErrorReporter
  *
  * Trivial-ness is stackable. Somewhat large expressions can occasionally make the cut:
  * - half4(myColor.a)
- * - myStruct.myArrayField[7].xyz
+ * - myStruct.myArrayField[7].xzy
  */
 bool IsTrivialExpression(const Expression& expr);
 

src/sksl/analysis/SkSLIsTrivialExpression.cpp

@@ -19,20 +19,50 @@
 namespace SkSL {
 
 bool Analysis::IsTrivialExpression(const Expression& expr) {
-    return expr.is<Literal>() ||
+    switch (expr.kind()) {
-           expr.is<VariableReference>() ||
+        case Expression::Kind::kLiteral:
-           (expr.is<Swizzle>() &&
+        case Expression::Kind::kVariableReference:
-            IsTrivialExpression(*expr.as<Swizzle>().base())) ||
+            return true;
-           (expr.is<FieldAccess>() &&
+
-            IsTrivialExpression(*expr.as<FieldAccess>().base())) ||
+        case Expression::Kind::kSwizzle:
-           (expr.isAnyConstructor() &&
+            // All swizzles are considered to be trivial.
-            expr.asAnyConstructor().argumentSpan().size() == 1 &&
+            return IsTrivialExpression(*expr.as<Swizzle>().base());
-            IsTrivialExpression(*expr.asAnyConstructor().argumentSpan().front())) ||
+
-           (expr.isAnyConstructor() &&
+        case Expression::Kind::kFieldAccess:
-            expr.isConstantOrUniform()) ||
+            // Accessing a field is trivial.
-           (expr.is<IndexExpression>() &&
+            return IsTrivialExpression(*expr.as<FieldAccess>().base());
-            expr.as<IndexExpression>().index()->isIntLiteral() &&
+
-            IsTrivialExpression(*expr.as<IndexExpression>().base()));
+        case Expression::Kind::kIndex: {
+            // Accessing a constant array index is trivial.
+            const IndexExpression& inner = expr.as<IndexExpression>();
+            return inner.index()->isIntLiteral() && IsTrivialExpression(*inner.base());
+        }
+        case Expression::Kind::kConstructorArray:
+        case Expression::Kind::kConstructorStruct:
+            // Only consider small arrays/structs of compile-time-constants to be trivial.
+            return expr.type().slotCount() <= 4 && expr.isCompileTimeConstant();
+
+        case Expression::Kind::kConstructorArrayCast:
+        case Expression::Kind::kConstructorMatrixResize:
+            // These operations require function calls in Metal, so they're never trivial.
+            return false;
+
+        case Expression::Kind::kConstructorCompound:
+            // Only compile-time-constant compound constructors are considered to be trivial.
+            return expr.isCompileTimeConstant();
+
+        case Expression::Kind::kConstructorCompoundCast:
+        case Expression::Kind::kConstructorScalarCast:
+        case Expression::Kind::kConstructorSplat:
+        case Expression::Kind::kConstructorDiagonalMatrix: {
+            // Single-argument constructors are trivial when their inner expression is trivial.
+            SkASSERT(expr.asAnyConstructor().argumentSpan().size() == 1);
+            const Expression& inner = *expr.asAnyConstructor().argumentSpan().front();
+            return IsTrivialExpression(inner);
+        }
+        default:
+            return false;
+    }
 }
 
 }  // namespace SkSL

tests/sksl/inliner/TrivialArgumentsInlineDirectly.glsl

@@ -30,15 +30,17 @@ vec4 main() {
     var = vec4(s.h) * vec4(s.h);
     var = s.ah4[0].xxxy * s.ah4[0].xxxy;
     var = colorGreen * colorGreen;
-    var = vec4(testMatrix2x2) * vec4(testMatrix2x2);
+    vec4 _0_h4 = vec4(testMatrix2x2);
-    mat = mat2(colorGreen) * mat2(colorGreen)[0].x;
+    var = _0_h4 * _0_h4;
-    float _0_h = -s.h;
+    mat2 _1_m = mat2(colorGreen);
-    var = vec4(_0_h) * vec4(_0_h);
+    mat = _1_m * _1_m[0].x;
-    bool _1_b = !b;
+    float _2_h = -s.h;
-    var = vec4(float(_1_b), float(_1_b), float(_1_b), float(!_1_b));
+    var = vec4(_2_h) * vec4(_2_h);
-    vec3 _2_h3 = s.h4.yyy + s.h4.zzz;
+    bool _3_b = !b;
-    var = _2_h3.xyzx * _2_h3.xyzx;
+    var = vec4(float(_3_b), float(_3_b), float(_3_b), float(!_3_b));
-    vec4 _3_h4 = vec4(s.h4.y, 0.0, 0.0, 1.0);
+    vec3 _4_h3 = s.h4.yyy + s.h4.zzz;
-    var = _3_h4 * _3_h4;
+    var = _4_h3.xyzx * _4_h3.xyzx;
+    vec4 _5_h4 = vec4(s.h4.y, 0.0, 0.0, 1.0);
+    var = _5_h4 * _5_h4;
     return colorGreen;
 }