Fuse variable declarations with neighboring init-expressions.

When the optimizer is on, we look for variable declarations that are
immediately followed by an initialization expression, and fuse them
into one statement. (e.g.: `int i; i = 1;` can become `int i = 1;`)

This benefits SkRP, since it no longer needs to zero the variable
at declaration time, or apply a write-mask when initializing it.

Change-Id: I7957e41ed3d4abf30b163b6f8451b24595278fe8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/686636
Auto-Submit: John Stiles <[email protected]>
Commit-Queue: John Stiles <[email protected]>
Reviewed-by: Arman Uguray <[email protected]>

src/sksl/ir/SkSLFunctionDefinition.cpp

@@ -14,16 +14,20 @@
 #include "src/sksl/SkSLCompiler.h"
 #include "src/sksl/SkSLContext.h"
 #include "src/sksl/SkSLErrorReporter.h"
+#include "src/sksl/SkSLOperator.h"
 #include "src/sksl/SkSLProgramSettings.h"
 #include "src/sksl/SkSLThreadContext.h"
 #include "src/sksl/dsl/DSLCore.h"
 #include "src/sksl/dsl/DSLExpression.h"
 #include "src/sksl/dsl/DSLStatement.h"
 #include "src/sksl/dsl/DSLType.h"
+#include "src/sksl/ir/SkSLBinaryExpression.h"
 #include "src/sksl/ir/SkSLBlock.h"
 #include "src/sksl/ir/SkSLExpression.h"
+#include "src/sksl/ir/SkSLExpressionStatement.h"
 #include "src/sksl/ir/SkSLField.h"
 #include "src/sksl/ir/SkSLFieldAccess.h"
+#include "src/sksl/ir/SkSLNop.h"
 #include "src/sksl/ir/SkSLReturnStatement.h"
 #include "src/sksl/ir/SkSLSymbol.h"
 #include "src/sksl/ir/SkSLSymbolTable.h"
@@ -98,6 +102,11 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
             }
         }
 
+        ~Finalizer() override {
+            SkASSERT(fBreakableLevel == 0);
+            SkASSERT(fContinuableLevel == std::forward_list<int>{0});
+        }
+
         void addLocalVariable(const Variable* var, Position pos) {
             if (var->type().isOrContainsUnsizedArray()) {
                 fContext.fErrors->error(pos, "unsized arrays are not permitted here");
@@ -117,24 +126,90 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
             }
         }
 
-        ~Finalizer() override {
-            SkASSERT(fBreakableLevel == 0);
-            SkASSERT(fContinuableLevel == std::forward_list<int>{0});
+        void fuseVariableDeclarationsWithInitialization(std::unique_ptr<Statement>& stmt) {
+            switch (stmt->kind()) {
+                case Statement::Kind::kNop:
+                case Statement::Kind::kBlock:
+                    // Blocks and no-ops are inert; it is safe to fuse a variable declaration with
+                    // its initialization across a nop or an open-brace, so we don't null out
+                    // `fUninitializedVarDecl` here.
+                    break;
+
+                case Statement::Kind::kVarDeclaration:
+                    // Look for variable declarations without an initializer.
+                    if (VarDeclaration& decl = stmt->as<VarDeclaration>(); !decl.value()) {
+                        fUninitializedVarDecl = &decl;
+                        break;
+                    }
+                    [[fallthrough]];
+
+                default:
+                    // We found an intervening statement; it's not safe to fuse a declaration
+                    // with an initializer if we encounter any other code.
+                    fUninitializedVarDecl = nullptr;
+                    break;
+
+                case Statement::Kind::kExpression: {
+                    // We found an expression-statement. If there was a variable declaration
+                    // immediately above it, it might be possible to fuse them.
+                    if (fUninitializedVarDecl) {
+                        VarDeclaration* vardecl = fUninitializedVarDecl;
+                        fUninitializedVarDecl = nullptr;
+
+                        std::unique_ptr<Expression>& nextExpr = stmt->as<ExpressionStatement>()
+                                                                     .expression();
+                        // This statement must be a binary-expression...
+                        if (!nextExpr->is<BinaryExpression>()) {
+                            break;
+                        }
+                        // ... performing simple `var = expr` assignment...
+                        BinaryExpression& binaryExpr = nextExpr->as<BinaryExpression>();
+                        if (binaryExpr.getOperator().kind() != OperatorKind::EQ) {
+                            break;
+                        }
+                        // ... directly into the variable (not a field/swizzle)...
+                        Expression& leftExpr = *binaryExpr.left();
+                        if (!leftExpr.is<VariableReference>()) {
+                            break;
+                        }
+                        // ... and it must be the same variable as our vardecl.
+                        VariableReference& varRef = leftExpr.as<VariableReference>();
+                        if (varRef.variable() != vardecl->var()) {
+                            break;
+                        }
+                        // We found a match! Move the init-expression directly onto the vardecl, and
+                        // turn the assignment into a no-op.
+                        vardecl->value() = std::move(binaryExpr.right());
+
+                        // Turn the expression-statement into a no-op.
+                        stmt = Nop::Make();
+                    }
+                    break;
+                }
+            }
         }
 
         bool functionReturnsValue() const {
             return !fFunction.returnType().isVoid();
         }
 
-        bool visitExpression(Expression& expr) override {
+        bool visitExpressionPtr(std::unique_ptr<Expression>& expr) override {
             // We don't need to scan expressions.
             return false;
         }
 
-        bool visitStatement(Statement& stmt) override {
-            switch (stmt.kind()) {
+        bool visitStatementPtr(std::unique_ptr<Statement>& stmt) override {
+            // When the optimizer is on, we look for variable declarations that are immediately
+            // followed by an initialization expression, and fuse them into one statement.
+            // (e.g.: `int i; i = 1;` can become `int i = 1;`)
+            if (fContext.fConfig->fSettings.fOptimize) {
+                this->fuseVariableDeclarationsWithInitialization(stmt);
+            }
+
+            // Perform error checking.
+            switch (stmt->kind()) {
                 case Statement::Kind::kVarDeclaration:
-                    this->addLocalVariable(stmt.as<VarDeclaration>().var(), stmt.fPosition);
+                    this->addLocalVariable(stmt->as<VarDeclaration>().var(), stmt->fPosition);
                     break;
 
                 case Statement::Kind::kReturn: {
@@ -143,12 +218,12 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
                     // issue, we can add normalization before each return statement.
                     if (ProgramConfig::IsVertex(fContext.fConfig->fKind) && fFunction.isMain()) {
                         fContext.fErrors->error(
-                                stmt.fPosition,
+                                stmt->fPosition,
                                 "early returns from vertex programs are not supported");
                     }
 
                     // Verify that the return statement matches the function's return type.
-                    ReturnStatement& returnStmt = stmt.as<ReturnStatement>();
+                    ReturnStatement& returnStmt = stmt->as<ReturnStatement>();
                     if (returnStmt.expression()) {
                         if (this->functionReturnsValue()) {
                             // Coerce return expression to the function's return type.
@@ -174,42 +249,44 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
                 case Statement::Kind::kFor: {
                     ++fBreakableLevel;
                     ++fContinuableLevel.front();
-                    bool result = INHERITED::visitStatement(stmt);
+                    bool result = INHERITED::visitStatementPtr(stmt);
                     --fContinuableLevel.front();
                     --fBreakableLevel;
                     return result;
                 }
                 case Statement::Kind::kSwitch: {
                     ++fBreakableLevel;
                     fContinuableLevel.push_front(0);
-                    bool result = INHERITED::visitStatement(stmt);
+                    bool result = INHERITED::visitStatementPtr(stmt);
                     fContinuableLevel.pop_front();
                     --fBreakableLevel;
                     return result;
                 }
                 case Statement::Kind::kBreak:
                     if (fBreakableLevel == 0) {
-                        fContext.fErrors->error(stmt.fPosition,
+                        fContext.fErrors->error(stmt->fPosition,
                                                 "break statement must be inside a loop or switch");
                     }
                     break;
+
                 case Statement::Kind::kContinue:
                     if (fContinuableLevel.front() == 0) {
                         if (std::any_of(fContinuableLevel.begin(),
                                         fContinuableLevel.end(),
                                         [](int level) { return level > 0; })) {
-                            fContext.fErrors->error(stmt.fPosition,
+                            fContext.fErrors->error(stmt->fPosition,
                                                    "continue statement cannot be used in a switch");
                         } else {
-                            fContext.fErrors->error(stmt.fPosition,
+                            fContext.fErrors->error(stmt->fPosition,
                                                     "continue statement must be inside a loop");
                         }
                     }
                     break;
+
                 default:
                     break;
             }
-            return INHERITED::visitStatement(stmt);
+            return INHERITED::visitStatementPtr(stmt);
         }
 
     private:
@@ -222,11 +299,14 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
         // how deeply nested we are in continuable constructs (for, do).
         // We keep a stack (via a forward_list) in order to disallow continue inside of switch.
         std::forward_list<int> fContinuableLevel{0};
+        // We track uninitialized variable declarations, and if they are immediately assigned-to,
+        // we can move the assignment directly into the decl.
+        VarDeclaration* fUninitializedVarDecl = nullptr;
 
         using INHERITED = ProgramWriter;
     };
 
-    Finalizer(context, function, pos).visitStatement(*body);
+    Finalizer(context, function, pos).visitStatementPtr(body);
     if (function.isMain() && ProgramConfig::IsVertex(context.fConfig->fKind)) {
         append_rtadjust_fixup_to_vertex_main(context, function, body->as<Block>());
     }

tests/sksl/folding/MatrixVectorNoOpFolding.glsl

@@ -52,8 +52,7 @@ bool test_Xno_Xop_Xvec2_XX_Xmat2_Xb() {
     const vec2 i = vec2(1.0);
     const vec2 z = vec2(0.0);
     vec2 v;
-    vec2 vv;
-    vv = vec2(0.0);
+    vec2 vv = vec2(0.0);
     vv = vec2(0.0);
     if (vv != z) return false;
     v = i * testMatrix2x2;
@@ -70,8 +69,7 @@ bool test_Xno_Xop_Xvec3_XX_Xmat3_Xb() {
     const vec3 i = vec3(1.0);
     const vec3 z = vec3(0.0);
     vec3 v;
-    vec3 vv;
-    vv = vec3(0.0);
+    vec3 vv = vec3(0.0);
     vv = vec3(0.0);
     if (vv != z) return false;
     v = i * testMatrix3x3;
@@ -89,8 +87,7 @@ bool test_Xno_Xop_Xvec4_XX_Xmat4_Xb() {
     const vec4 z = vec4(0.0);
     mat4 testMatrix4x4 = mat4(testMatrix2x2[0], testMatrix2x2[1], testMatrix2x2[0], testMatrix2x2[1], testMatrix2x2[0], testMatrix2x2[1], testMatrix2x2[0], testMatrix2x2[1]);
     vec4 v;
-    vec4 vv;
-    vv = vec4(0.0);
+    vec4 vv = vec4(0.0);
     vv = vec4(0.0);
     if (vv != z) return false;
     v = i * testMatrix4x4;

tests/sksl/folding/MatrixVectorNoOpFolding.minified.sksl

@@ -1 +1 @@
-uniform float2x2 testMatrix2x2;uniform float3x3 testMatrix3x3;uniform float4 testInputs;uniform half4 colorRed;uniform half4 colorGreen;uniform half unknownInput;bool a(){float2 e;float2 f;e=testInputs.xy;e=testInputs.xy;if(e!=testInputs.xy)return false;if(e!=testInputs.xy)return false;e=-testInputs.xy;e=-testInputs.xy;if(e!=-testInputs.xy)return false;f=float2(0.);f=float2(0.);return f==float2(0.);}bool b(){float3 f;float3 g;f=testInputs.xyz;f=testInputs.xyz;if(f!=testInputs.xyz)return false;if(f!=testInputs.xyz)return false;f=-testInputs.xyz;f=-testInputs.xyz;if(f!=-testInputs.xyz)return false;g=float3(0.);g=float3(0.);return g==float3(0.);}bool c(){float4 g;float4 h;g=testInputs;g=testInputs;if(g!=testInputs)return false;if(g!=testInputs)return false;g=-testInputs;g=-testInputs;if(g!=-testInputs)return false;h=float4(0.);h=float4(0.);return h==float4(0.);}bool d(){float2 h;float2 j;j=float2(0.);j=float2(0.);if(j!=float2(0.))return false;h=float2(1.)*testMatrix2x2;if(h!=float2(3.,7.))return false;h=testMatrix2x2*float2(1.);if(h!=float2(4.,6.))return false;h=float2(-1.)*testMatrix2x2;if(h!=float2(-3.,-7.))return false;h=testMatrix2x2*float2(-1.);return h==float2(-4.,-6.);}bool e(){float3 j;float3 k;k=float3(0.);k=float3(0.);if(k!=float3(0.))return false;j=float3(1.)*testMatrix3x3;if(j!=float3(6.,15.,24.))return false;j=testMatrix3x3*float3(1.);if(j!=float3(12.,15.,18.))return false;j=float3(-1.)*testMatrix3x3;if(j!=float3(-6.,-15.,-24.))return false;j=testMatrix3x3*float3(-1.);return j==float3(-12.,-15.,-18.);}bool f(){float4x4 k=float4x4(testMatrix2x2[0],testMatrix2x2[1],testMatrix2x2[0],testMatrix2x2[1],testMatrix2x2[0],testMatrix2x2[1],testMatrix2x2[0],testMatrix2x2[1]);float4 l;float4 m;m=float4(0.);m=float4(0.);if(m!=float4(0.))return false;l=float4(1.)*k;if(l!=float4(10.))return false;l=k*float4(1.);if(l!=float4(4.,8.,12.,16.))return false;l=float4(-1.)*k;if(l!=float4(-10.))return false;l=k*float4(-1.);return l==float4(-4.,-8.,-12.,-16.);}half4 main(float2 g){return((((a()&&b())&&c())&&d())&&e())&&f()?colorGreen:colorRed;}
+uniform float2x2 testMatrix2x2;uniform float3x3 testMatrix3x3;uniform float4 testInputs;uniform half4 colorRed;uniform half4 colorGreen;uniform half unknownInput;bool a(){float2 e;float2 f;e=testInputs.xy;e=testInputs.xy;if(e!=testInputs.xy)return false;if(e!=testInputs.xy)return false;e=-testInputs.xy;e=-testInputs.xy;if(e!=-testInputs.xy)return false;f=float2(0.);f=float2(0.);return f==float2(0.);}bool b(){float3 f;float3 g;f=testInputs.xyz;f=testInputs.xyz;if(f!=testInputs.xyz)return false;if(f!=testInputs.xyz)return false;f=-testInputs.xyz;f=-testInputs.xyz;if(f!=-testInputs.xyz)return false;g=float3(0.);g=float3(0.);return g==float3(0.);}bool c(){float4 g;float4 h;g=testInputs;g=testInputs;if(g!=testInputs)return false;if(g!=testInputs)return false;g=-testInputs;g=-testInputs;if(g!=-testInputs)return false;h=float4(0.);h=float4(0.);return h==float4(0.);}bool d(){float2 h;float2 j=float2(0.);j=float2(0.);if(j!=float2(0.))return false;h=float2(1.)*testMatrix2x2;if(h!=float2(3.,7.))return false;h=testMatrix2x2*float2(1.);if(h!=float2(4.,6.))return false;h=float2(-1.)*testMatrix2x2;if(h!=float2(-3.,-7.))return false;h=testMatrix2x2*float2(-1.);return h==float2(-4.,-6.);}bool e(){float3 j;float3 k=float3(0.);k=float3(0.);if(k!=float3(0.))return false;j=float3(1.)*testMatrix3x3;if(j!=float3(6.,15.,24.))return false;j=testMatrix3x3*float3(1.);if(j!=float3(12.,15.,18.))return false;j=float3(-1.)*testMatrix3x3;if(j!=float3(-6.,-15.,-24.))return false;j=testMatrix3x3*float3(-1.);return j==float3(-12.,-15.,-18.);}bool f(){float4x4 k=float4x4(testMatrix2x2[0],testMatrix2x2[1],testMatrix2x2[0],testMatrix2x2[1],testMatrix2x2[0],testMatrix2x2[1],testMatrix2x2[0],testMatrix2x2[1]);float4 l;float4 m=float4(0.);m=float4(0.);if(m!=float4(0.))return false;l=float4(1.)*k;if(l!=float4(10.))return false;l=k*float4(1.);if(l!=float4(4.,8.,12.,16.))return false;l=float4(-1.)*k;if(l!=float4(-10.))return false;l=k*float4(-1.);return l==float4(-4.,-8.,-12.,-16.);}half4 main(float2 g){return((((a()&&b())&&c())&&d())&&e())&&f()?colorGreen:colorRed;}

tests/sksl/folding/MatrixVectorNoOpFolding.skrp

@@ -201,16 +201,14 @@ label                          label 0x00000004
 load_condition_mask            CondMask = $71
 copy_constant                  $45 = 0
 merge_condition_mask           CondMask = $55 & $56
-branch_if_no_lanes_active      branch_if_no_lanes_active +78 (label 3 at #282)
+branch_if_no_lanes_active      branch_if_no_lanes_active +76 (label 3 at #280)
 store_return_mask              $46 = RetMask
 splat_2_constants              n = 0xBF800000 (-1.0)
 splat_2_constants              i = 0x3F800000 (1.0)
 splat_4_constants              z, v₃ = 0
 splat_2_constants              vv₃ = 0
 splat_2_constants              $47..48 = 0
 copy_2_slots_masked            vv₃ = Mask($47..48)
-splat_2_constants              $47..48 = 0
-copy_2_slots_masked            vv₃ = Mask($47..48)
 store_condition_mask           $47 = CondMask
 copy_2_slots_unmasked          $48..49 = vv₃
 splat_2_constants              $50..51 = 0
@@ -283,7 +281,7 @@ label                          label 0x00000003
 load_condition_mask            CondMask = $55
 copy_constant                  $27 = 0
 merge_condition_mask           CondMask = $44 & $45
-branch_if_no_lanes_active      branch_if_no_lanes_active +96 (label 2 at #382)
+branch_if_no_lanes_active      branch_if_no_lanes_active +94 (label 2 at #378)
 store_return_mask              $28 = RetMask
 splat_3_constants              n₁ = 0xBF800000 (-1.0)
 splat_3_constants              i₁ = 0x3F800000 (1.0)
@@ -292,8 +290,6 @@ splat_4_constants              v₄(1..2), vv₄(0..1) = 0
 copy_constant                  vv₄(2) = 0
 splat_3_constants              $29..31 = 0
 copy_3_slots_masked            vv₄ = Mask($29..31)
-splat_3_constants              $29..31 = 0
-copy_3_slots_masked            vv₄ = Mask($29..31)
 store_condition_mask           $29 = CondMask
 copy_3_slots_unmasked          $30..32 = vv₄
 splat_3_constants              $33..35 = 0
@@ -383,7 +379,7 @@ label                          label 0x00000002
 load_condition_mask            CondMask = $44
 copy_constant                  $0 = 0
 merge_condition_mask           CondMask = $26 & $27
-branch_if_no_lanes_active      branch_if_no_lanes_active +102 (label 1 at #488)
+branch_if_no_lanes_active      branch_if_no_lanes_active +100 (label 1 at #482)
 store_return_mask              $1 = RetMask
 splat_4_constants              n₂ = 0xBF800000 (-1.0)
 splat_4_constants              i₂ = 0x3F800000 (1.0)
@@ -396,8 +392,6 @@ splat_4_constants              v₅ = 0
 splat_4_constants              vv₅ = 0
 splat_4_constants              $2..5 = 0
 copy_4_slots_masked            vv₅ = Mask($2..5)
-splat_4_constants              $2..5 = 0
-copy_4_slots_masked            vv₅ = Mask($2..5)
 store_condition_mask           $2 = CondMask
 copy_4_slots_unmasked          $3..6 = vv₅
 splat_4_constants              $7..10 = 0

tests/sksl/folding/TernaryFolding.glsl

@@ -6,8 +6,7 @@ bool do_side_effect_bb(out bool x) {
     return false;
 }
 vec4 main() {
-    bool ok;
-    ok = true;
+    bool ok = true;
     vec4 green = colorGreen;
     vec4 red = colorRed;
     bool param = false;

tests/sksl/folding/TernaryFolding.minified.sksl

@@ -1 +1 @@
-uniform half4 colorRed;uniform half4 colorGreen;bool a(out bool b){b=true;return false;}half4 main(float2 b){bool c;c=true;half4 d=colorGreen;half4 e=colorRed;bool f=false;bool g=(a(f),true);return(c&&f)&&g?d:e;}
+uniform half4 colorRed;uniform half4 colorGreen;bool a(out bool b){b=true;return false;}half4 main(float2 b){bool c=true;half4 d=colorGreen;half4 e=colorRed;bool f=false;bool g=(a(f),true);return(c&&f)&&g?d:e;}

tests/sksl/shared/Assignment.glsl

@@ -16,12 +16,9 @@ void keepAlive_vf(inout float f) {
 void keepAlive_vi(inout int i) {
 }
 vec4 main() {
-    int i;
-    i = 0;
-    ivec4 i4;
-    i4 = ivec4(1, 2, 3, 4);
-    mat3 f3x3;
-    f3x3 = mat3(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
+    int i = 0;
+    ivec4 i4 = ivec4(1, 2, 3, 4);
+    mat3 f3x3 = mat3(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0);
     vec4 x;
     x.w = 0.0;
     x.yx = vec2(0.0);

tests/sksl/shared/Assignment.metal

@@ -90,12 +90,9 @@ fragment Outputs fragmentMain(Inputs _in [[stage_in]], constant Uniforms& _unifo
     (void)_globals;
     Outputs _out;
     (void)_out;
-    int i;
-    i = 0;
-    int4 i4;
-    i4 = int4(1, 2, 3, 4);
-    float3x3 f3x3;
-    f3x3 = float3x3(float3(1.0, 2.0, 3.0), float3(4.0, 5.0, 6.0), float3(7.0, 8.0, 9.0));
+    int i = 0;
+    int4 i4 = int4(1, 2, 3, 4);
+    float3x3 f3x3 = float3x3(float3(1.0, 2.0, 3.0), float3(4.0, 5.0, 6.0), float3(7.0, 8.0, 9.0));
     half4 x;
     x.w = 0.0h;
     x.yx = half2(0.0h);