Add const modifier to variables during inlineStatement.

During the inlining pass, we know the ProgramUsage for every variable. When the inliner is about to emit a var-declaration like `float PI = 3.14;`, we can check the ProgramUsage to see if we ever assign to PI elsewhere in the code. If we don't, we can emit `const float PI = 3.14;` instead. This small change gives the constant folder many, many more opportunities to do its job. (See the attached bug for more information.) Change-Id: I2481bd7fbcf360a56b746800d29816c2ba7f2e5b Bug: skia:13544 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/559401 Reviewed-by: Brian Osman <[email protected]> Auto-Submit: John Stiles <[email protected]> Commit-Queue: John Stiles <[email protected]>
armansito · Jul 18, 2022 · 78b7d53 · 78b7d53
1 parent 80723ae
commit 78b7d53
Show file tree

Hide file tree

Showing 27 changed files with 312 additions and 542 deletions.
diff --git a/gn/sksl.gni b/gn/sksl.gni
@@ -204,6 +204,7 @@ skia_sksl_sources = [
   "$_src/sksl/spirv.h",
   "$_src/sksl/tracing/SkVMDebugTrace.cpp",
   "$_src/sksl/tracing/SkVMDebugTrace.h",
+  "$_src/sksl/transform/SkSLAddConstToVarModifiers.cpp",
   "$_src/sksl/transform/SkSLBuiltinVariableScanner.cpp",
   "$_src/sksl/transform/SkSLEliminateDeadFunctions.cpp",
   "$_src/sksl/transform/SkSLEliminateDeadGlobalVariables.cpp",

diff --git a/public.bzl b/public.bzl
@@ -1625,6 +1625,7 @@ BASE_SRCS_ALL = [
     "src/sksl/tracing/SkVMDebugTrace.h",
     "src/sksl/tracing/SkVMDebugTracePlayer.cpp",
     "src/sksl/tracing/SkVMDebugTracePlayer.h",
+    "src/sksl/transform/SkSLAddConstToVarModifiers.cpp",
     "src/sksl/transform/SkSLBuiltinVariableScanner.cpp",
     "src/sksl/transform/SkSLEliminateDeadFunctions.cpp",
     "src/sksl/transform/SkSLEliminateDeadGlobalVariables.cpp",

diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
@@ -56,6 +56,7 @@
 #include "src/sksl/ir/SkSLVarDeclarations.h"
 #include "src/sksl/ir/SkSLVariable.h"
 #include "src/sksl/ir/SkSLVariableReference.h"
+#include "src/sksl/transform/SkSLTransform.h"
 
 #include <algorithm>
 #include <climits>
@@ -433,11 +434,12 @@ std::unique_ptr<Statement> Inliner::inlineStatement(Position pos,
                                                     std::unique_ptr<Expression>* resultExpr,
                                                     ReturnComplexity returnComplexity,
                                                     const Statement& statement,
+                                                    const ProgramUsage& usage,
                                                     bool isBuiltinCode) {
     auto stmt = [&](const std::unique_ptr<Statement>& s) -> std::unique_ptr<Statement> {
         if (s) {
             return this->inlineStatement(pos, varMap, symbolTableForStatement, resultExpr,
-                                         returnComplexity, *s, isBuiltinCode);
+                                         returnComplexity, *s, usage, isBuiltinCode);
         }
         return nullptr;
     };
@@ -455,6 +457,10 @@ std::unique_ptr<Statement> Inliner::inlineStatement(Position pos,
         }
         return nullptr;
     };
+    auto variableModifiers = [&](const Variable& variable,
+                                 const Expression* initialValue) -> const Modifiers* {
+        return Transform::AddConstToVarModifiers(*fContext, variable, initialValue, &usage);
+    };
 
     ++fInlinedStatementCounter;
 
@@ -563,13 +569,13 @@ std::unique_ptr<Statement> Inliner::inlineStatement(Position pos,
             const std::string* name = symbolTableForStatement->takeOwnershipOfString(
                     fContext->fMangler->uniqueName(variable.name(), symbolTableForStatement));
             auto clonedVar = std::make_unique<Variable>(
-                                                     pos,
-                                                     variable.modifiersPosition(),
-                                                     &variable.modifiers(),
-                                                     name->c_str(),
-                                                     variable.type().clone(symbolTableForStatement),
-                                                     isBuiltinCode,
-                                                     variable.storage());
+                    pos,
+                    variable.modifiersPosition(),
+                    variableModifiers(variable, initialValue.get()),
+                    name->c_str(),
+                    variable.type().clone(symbolTableForStatement),
+                    isBuiltinCode,
+                    variable.storage());
             varMap->set(&variable, VariableReference::Make(pos, clonedVar.get()));
             auto result = VarDeclaration::Make(*fContext,
                                                clonedVar.get(),
@@ -664,7 +670,7 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
     for (const std::unique_ptr<Statement>& stmt : body.children()) {
         inlineStatements.push_back(this->inlineStatement(pos, &varMap, symbolTable.get(),
                                                          &resultExpr, returnComplexity, *stmt,
-                                                         caller->isBuiltin()));
+                                                         usage, caller->isBuiltin()));
     }
 
     SkASSERT(inlineStatements.count() <= expectedStmtCount);

diff --git a/src/sksl/SkSLInliner.h b/src/sksl/SkSLInliner.h
@@ -73,6 +73,7 @@ class Inliner {
                                                std::unique_ptr<Expression>* resultExpr,
                                                ReturnComplexity returnComplexity,
                                                const Statement& statement,
+                                               const ProgramUsage& usage,
                                                bool isBuiltinCode);
 
     /**

diff --git a/src/sksl/transform/BUILD.bazel b/src/sksl/transform/BUILD.bazel
@@ -5,6 +5,7 @@ licenses(["notice"])
 exports_files_legacy()
 
 TRANSFORM_FILES = [
+    "SkSLAddConstToVarModifiers.cpp",
     "SkSLBuiltinVariableScanner.cpp",
     "SkSLEliminateDeadFunctions.cpp",
     "SkSLEliminateDeadGlobalVariables.cpp",

diff --git a/src/sksl/transform/SkSLAddConstToVarModifiers.cpp b/src/sksl/transform/SkSLAddConstToVarModifiers.cpp
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2022 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "include/private/SkSLModifiers.h"
+#include "src/sksl/SkSLContext.h"
+#include "src/sksl/SkSLModifiersPool.h"
+#include "src/sksl/ir/SkSLExpression.h"
+#include "src/sksl/ir/SkSLProgram.h"
+#include "src/sksl/ir/SkSLVariable.h"
+#include "src/sksl/transform/SkSLTransform.h"
+
+namespace SkSL {
+
+const Modifiers* Transform::AddConstToVarModifiers(const Context& context,
+                                                   const Variable& var,
+                                                   const Expression* initialValue,
+                                                   const ProgramUsage* usage) {
+    // If the variable is already marked as `const`, keep our existing modifiers.
+    const Modifiers* modifiers = &var.modifiers();
+    if (modifiers->fFlags & Modifiers::kConst_Flag) {
+        return modifiers;
+    }
+    // If the variable doesn't have a compile-time-constant initial value, we can't `const` it.
+    if (!initialValue || !initialValue->isCompileTimeConstant()) {
+        return modifiers;
+    }
+    // This only works for variables that are written-to a single time.
+    ProgramUsage::VariableCounts counts = usage->get(var);
+    if (counts.fWrite != 1) {
+        return modifiers;
+    }
+    // Add `const` to our variable's modifiers, making it eligible for constant-folding.
+    Modifiers constModifiers = *modifiers;
+    constModifiers.fFlags |= Modifiers::kConst_Flag;
+    return context.fModifiersPool->add(constModifiers);
+}
+
+}  // namespace SkSL
diff --git a/src/sksl/transform/SkSLTransform.h b/src/sksl/transform/SkSLTransform.h
@@ -15,15 +15,28 @@
 namespace SkSL {
 
 class Context;
+class Expression;
 struct LoadedModule;
+struct Modifiers;
 struct Program;
 class ProgramElement;
 class ProgramUsage;
 class Statement;
+class Variable;
 enum class ProgramKind : int8_t;
 
 namespace Transform {
 
+/**
+ * Checks to see if it would be safe to add `const` to the modifiers of a variable. If so, returns
+ * the modifiers with `const` applied; if not, returns the existing modifiers as-is. Adding `const`
+ * allows the inliner to fold away more values and generate tighter code.
+ */
+const Modifiers* AddConstToVarModifiers(const Context& context,
+                                        const Variable& var,
+                                        const Expression* initialValue,
+                                        const ProgramUsage* usage);
+
 /**
  * Scans the finished program for built-in variables like `sk_FragColor` and adds them to the
  * program's shared elements.

diff --git a/tests/sksl/folding/ArrayFolding.glsl b/tests/sksl/folding/ArrayFolding.glsl
@@ -9,9 +9,9 @@ int side_effecting_ii(int value) {
 }
 vec4 main() {
     int _7_two = 2;
-    int _8_flatten0 = 1;
+    const int _8_flatten0 = 1;
     int _9_flatten1 = _7_two;
-    int _10_flatten2 = 3;
+    const int _10_flatten2 = 3;
     int _11_noFlatten0 = int[3](--_7_two, side_effecting_ii(2), 3)[0];
     int _12_noFlatten1 = int[3](side_effecting_ii(1), 2, 3)[1];
     int _13_noFlatten2 = int[3](1, ++_7_two, 3)[2];

diff --git a/tests/sksl/folding/BoolFolding.glsl b/tests/sksl/folding/BoolFolding.glsl
@@ -3,27 +3,5 @@ out vec4 sk_FragColor;
 uniform vec4 colorRed;
 uniform vec4 colorGreen;
 vec4 main() {
-    bool _0_a = true;
-    bool _1_b = false;
-    bool _2_c = true;
-    bool _3_d = false;
-    bool _4_e = true;
-    bool _5_f = false;
-    bool _6_g = true;
-    bool _7_h = false;
-    bool _8_i = true;
-    bool _9_j = false;
-    bool _12_k = true;
-    bool _13_l = false;
-    bool _14_m = true;
-    bool _15_n = false;
-    bool _16_o = true;
-    bool _17_p = false;
-    bool _18_q = true;
-    bool _19_r = false;
-    bool _20_s = true;
-    bool _21_t = false;
-    bool _22_u = true;
-    bool _23_v = false;
-    return ((((((((((((((((((((_0_a && !_1_b) && _2_c) && !_3_d) && _4_e) && !_5_f) && _6_g) && !_7_h) && _8_i) && !_9_j) && _12_k) && !_13_l) && _14_m) && !_15_n) && _16_o) && !_17_p) && _18_q) && !_19_r) && _20_s) && !_21_t) && _22_u) && !_23_v ? colorGreen : colorRed;
+    return true ? colorGreen : colorRed;
 }
diff --git a/tests/sksl/folding/CastFolding.glsl b/tests/sksl/folding/CastFolding.glsl
@@ -3,6 +3,6 @@ out vec4 sk_FragColor;
 uniform vec4 colorRed;
 uniform vec4 colorGreen;
 vec4 main() {
-    bool _4_ok = true;
+    const bool _4_ok = true;
     return _4_ok ? colorGreen : colorRed;
 }
diff --git a/tests/sksl/folding/MatrixFoldingES2.glsl b/tests/sksl/folding/MatrixFoldingES2.glsl
@@ -41,10 +41,6 @@ vec4 main() {
     _0_ok = _0_ok && mat3(unknownInput) == mat3(mat2(1.0));
     _0_ok = _0_ok && mat3(9.0, 0.0, 0.0, 0.0, 9.0, 0.0, 0.0, 0.0, unknownInput) == mat3(mat2(9.0));
     _0_ok = _0_ok && vec4(testMatrix2x2) == vec4(1.0, 2.0, 3.0, 4.0);
-    {
-        float _3_five = 5.0;
-        _0_ok = _0_ok && mat3(1.0, 2.0, 3.0, 4.0, _3_five, 6.0, 7.0, 8.0, 9.0)[1] == vec3(4.0, _3_five, 6.0);
-    }
     {
         _0_ok = _0_ok && mat4(mat3(testMatrix2x2))[0] == vec4(1.0, 2.0, 0.0, 0.0);
         _0_ok = _0_ok && mat4(mat3(testMatrix2x2))[1] == vec4(3.0, 4.0, 0.0, 0.0);

diff --git a/tests/sksl/folding/MatrixFoldingES3.glsl b/tests/sksl/folding/MatrixFoldingES3.glsl
@@ -31,6 +31,6 @@ bool test_matrix_op_vector_half_b() {
     return ok;
 }
 vec4 main() {
-    bool _0_ok = true;
+    const bool _0_ok = true;
     return ((((((_0_ok && test_eq_half_b()) && test_matrix_op_matrix_float_b()) && test_matrix_op_matrix_half_b()) && test_vector_op_matrix_float_b()) && test_vector_op_matrix_half_b()) && test_matrix_op_vector_float_b()) && test_matrix_op_vector_half_b() ? colorGreen : colorRed;
 }
diff --git a/tests/sksl/folding/Negation.glsl b/tests/sksl/folding/Negation.glsl
@@ -14,10 +14,6 @@ bool test_mat_b() {
     return ok;
 }
 vec4 main() {
-    const float _0_one = 1.0;
-    float _1_two = 2.0;
     bool _4_ok = true;
-    _4_ok = _4_ok && -vec4(_1_two) == vec4(-_1_two, vec3(-_1_two));
-    _4_ok = _4_ok && vec2(1.0, -2.0) == -vec2(_0_one - _1_two, _1_two);
     return (_4_ok && test_ivec_b()) && test_mat_b() ? colorGreen : colorRed;
 }
diff --git a/tests/sksl/folding/StructFieldFolding.glsl b/tests/sksl/folding/StructFieldFolding.glsl
@@ -8,9 +8,7 @@ struct S {
     int c;
 };
 vec4 main() {
-    int _6_two = 2;
-    int _7_flatten0 = 1;
+    const int _6_two = 2;
     int _8_flatten1 = _6_two;
-    int _9_flatten2 = 3;
-    return (_7_flatten0 == 1 && _8_flatten1 == 2) && _9_flatten2 == 3 ? colorGreen : colorRed;
+    return _8_flatten1 == 2 ? colorGreen : colorRed;
 }
diff --git a/tests/sksl/inliner/InlinerAvoidsVariableNameOverlap.glsl b/tests/sksl/inliner/InlinerAvoidsVariableNameOverlap.glsl
@@ -1,7 +1,6 @@
 
 out vec4 sk_FragColor;
 vec4 main() {
-    vec2 _1_reusedName = vec2(1.0, 2.0);
-    vec2 _2_reusedName = _1_reusedName - 1.0;
+    const vec2 _2_reusedName = vec2(0.0, 1.0);
     return _2_reusedName.xyxy;
 }