# Enter a description of the change.

Reland "Migrate if-statement simplifyStatement logic to IfStatement::Make."

This reverts commit 7e685f0377.

Reason for revert: fixed SkSLBench perf test

Original change's description:
> Revert "Migrate if-statement simplifyStatement logic to IfStatement::Make."
>
> This reverts commit e4da7b672f.
>
> Reason for revert: breaks SkSLBench perf test
>
> Original change's description:
> > Migrate if-statement simplifyStatement logic to IfStatement::Make.
> >
> > This performs essentially the same simplifications as before, just at
> > a different phase of compilation.
> >
> > Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
> > Bug: skia:11343, skia:11319
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I0051188ffe69426904066eb60a932435efdc2af8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11343
> Bug: skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379062
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

Bug: skia:11343
Bug: skia:11319
Change-Id: I74cc3295004133e9fdcf16e388106eb83603f526
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379063
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>

resources/sksl/shared/DeadStripFunctions.sksl

@@ -1,4 +1,4 @@
-/*#pragma settings NoInline*/
+/*#pragma settings NoInline NoControlFlowAnalysis*/
 
 uniform half4 colorGreen, colorRed;
 
@@ -11,10 +11,10 @@ half4 live_fn(half4 a, half4 b) {
 }
 
 half4 main() {
-    bool TRUE = true, FALSE = false;
+    const bool TRUE = true, FALSE = false;
     half4 a, b;
 
-    if (FALSE) {
+    @if (FALSE) {
         // Dead stripping a user function.
         half4 unused = dead_fn(half4(0.5), half4(2));
     } else {
@@ -22,7 +22,7 @@ half4 main() {
         a = live_fn(half4(3), half4(-5));
     }
 
-    if (TRUE) {
+    @if (TRUE) {
         // A live built-in function.
         b = unpremul(half4(1));
     } else {

resources/sksl/shared/StaticIf.sksl

@@ -1,9 +1,11 @@
+/*#pragma settings NoControlFlowAnalysis*/
+
 uniform half4 colorRed, colorGreen;
 
 half4 main() {
     half4 result = colorRed;
-    float x = 5;
-    float y = 10;
+    const float x = 5;
+    const float y = 10;
     @if (x < y) {
         result = colorGreen;
     }

src/sksl/SkSLCompiler.cpp

@@ -1176,6 +1176,8 @@ void Compiler::simplifyStatement(DefinitionMap& definitions,
             break;
         }
         case Statement::Kind::kIf: {
+            // TODO(skia:11319): this optimization logic is redundant with the optimization code
+            // found in IfStatement.cpp.
             IfStatement& i = stmt->as<IfStatement>();
             if (i.test()->kind() == Expression::Kind::kBoolLiteral) {
                 // constant if, collapse down to a single branch

src/sksl/ir/SkSLIfStatement.cpp

@@ -5,8 +5,11 @@
  * found in the LICENSE file.
  */
 
+#include "src/sksl/SkSLConstantFolder.h"
 #include "src/sksl/SkSLContext.h"
+#include "src/sksl/SkSLProgramSettings.h"
 #include "src/sksl/ir/SkSLBoolLiteral.h"
+#include "src/sksl/ir/SkSLExpressionStatement.h"
 #include "src/sksl/ir/SkSLIfStatement.h"
 #include "src/sksl/ir/SkSLNop.h"
 #include "src/sksl/ir/SkSLType.h"
@@ -43,25 +46,53 @@ std::unique_ptr<Statement> IfStatement::Convert(const Context& context, int offs
                              std::move(ifTrue), std::move(ifFalse));
 }
 
+static std::unique_ptr<Statement> replace_empty_with_nop(std::unique_ptr<Statement> stmt,
+                                                         bool isEmpty) {
+    return (stmt && (!isEmpty || stmt->is<Nop>())) ? std::move(stmt)
+                                                   : std::make_unique<Nop>();
+}
+
 std::unique_ptr<Statement> IfStatement::Make(const Context& context, int offset, bool isStatic,
                                              std::unique_ptr<Expression> test,
                                              std::unique_ptr<Statement> ifTrue,
                                              std::unique_ptr<Statement> ifFalse) {
     SkASSERT(test->type() == *context.fTypes.fBool);
 
-    if (test->is<BoolLiteral>()) {
-        // Static Boolean values can fold down to a single branch.
-        if (test->as<BoolLiteral>().value()) {
-            return ifTrue;
+    const bool optimize = context.fConfig->fSettings.fOptimize;
+    bool trueIsEmpty = false;
+    bool falseIsEmpty = false;
+
+    if (optimize) {
+        // If both sides are empty, the if statement can be reduced to its test expression.
+        trueIsEmpty = ifTrue->isEmpty();
+        falseIsEmpty = !ifFalse || ifFalse->isEmpty();
+        if (trueIsEmpty && falseIsEmpty) {
+            return ExpressionStatement::Make(context, std::move(test));
         }
-        if (ifFalse) {
-            return ifFalse;
-        }
-        // False, but no else-clause. Not an error, so don't return null!
-        return std::make_unique<Nop>();
     }
-    return std::make_unique<IfStatement>(offset, isStatic, std::move(test), std::move(ifTrue),
-                                         std::move(ifFalse));
+
+    if (isStatic || optimize) {
+        // Static Boolean values can fold down to a single branch.
+        const Expression* testValue = ConstantFolder::GetConstantValueForVariable(*test);
+        if (testValue->is<BoolLiteral>()) {
+            if (testValue->as<BoolLiteral>().value()) {
+                return replace_empty_with_nop(std::move(ifTrue), trueIsEmpty);
+            } else {
+                return replace_empty_with_nop(std::move(ifFalse), falseIsEmpty);
+            }
+        }
+    }
+
+    if (optimize) {
+        // Replace an empty if-true branches with Nop; eliminate empty if-false branches entirely.
+        ifTrue = replace_empty_with_nop(std::move(ifTrue), trueIsEmpty);
+        if (falseIsEmpty) {
+            ifFalse = nullptr;
+        }
+    }
+
+    return std::make_unique<IfStatement>(offset, isStatic, std::move(test),
+                                         std::move(ifTrue), std::move(ifFalse));
 }
 
 }  // namespace SkSL

tests/sksl/inliner/IfElseBodyMustBeInlinedIntoAScope.glsl

@@ -3,8 +3,7 @@ out vec4 sk_FragColor;
 uniform vec4 color;
 void main() {
     vec4 c = color;
-    if (c.x >= 0.5) {
-    } else {
+    if (c.x >= 0.5) ; else {
         c = color + vec4(0.125);
     }
     sk_FragColor = c;

tests/sksl/shared/DeadStripFunctions.asm.frag

@@ -12,6 +12,8 @@ OpName %_entrypoint "_entrypoint"
 OpName %unpremul "unpremul"
 OpName %live_fn "live_fn"
 OpName %main "main"
+OpName %TRUE "TRUE"
+OpName %FALSE "FALSE"
 OpName %a "a"
 OpName %b "b"
 OpDecorate %sk_FragColor RelaxedPrecision
@@ -32,11 +34,11 @@ OpDecorate %37 RelaxedPrecision
 OpDecorate %44 RelaxedPrecision
 OpDecorate %45 RelaxedPrecision
 OpDecorate %46 RelaxedPrecision
-OpDecorate %62 RelaxedPrecision
-OpDecorate %70 RelaxedPrecision
-OpDecorate %82 RelaxedPrecision
-OpDecorate %85 RelaxedPrecision
+OpDecorate %66 RelaxedPrecision
+OpDecorate %74 RelaxedPrecision
 OpDecorate %86 RelaxedPrecision
+OpDecorate %89 RelaxedPrecision
+OpDecorate %90 RelaxedPrecision
 %float = OpTypeFloat 32
 %v4float = OpTypeVector %float 4
 %_ptr_Output_v4float = OpTypePointer Output %v4float
@@ -56,14 +58,16 @@ OpDecorate %86 RelaxedPrecision
 %float_1 = OpConstant %float 1
 %40 = OpTypeFunction %v4float %_ptr_Function_v4float %_ptr_Function_v4float
 %47 = OpTypeFunction %v4float
-%float_3 = OpConstant %float 3
-%52 = OpConstantComposite %v4float %float_3 %float_3 %float_3 %float_3
-%float_n5 = OpConstant %float -5
-%55 = OpConstantComposite %v4float %float_n5 %float_n5 %float_n5 %float_n5
-%58 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
+%_ptr_Function_bool = OpTypePointer Function %bool
+%true = OpConstantTrue %bool
 %false = OpConstantFalse %bool
+%float_3 = OpConstant %float 3
+%57 = OpConstantComposite %v4float %float_3 %float_3 %float_3 %float_3
+%float_n5 = OpConstant %float -5
+%60 = OpConstantComposite %v4float %float_n5 %float_n5 %float_n5 %float_n5
+%63 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
 %float_0 = OpConstant %float 0
-%64 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+%68 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
 %v4bool = OpTypeVector %bool 4
 %_ptr_Uniform_v4float = OpTypePointer Uniform %v4float
 %int = OpTypeInt 32 1
@@ -104,44 +108,48 @@ OpReturnValue %46
 OpFunctionEnd
 %main = OpFunction %v4float None %47
 %48 = OpLabel
+%TRUE = OpVariable %_ptr_Function_bool Function
+%FALSE = OpVariable %_ptr_Function_bool Function
 %a = OpVariable %_ptr_Function_v4float Function
 %b = OpVariable %_ptr_Function_v4float Function
-%53 = OpVariable %_ptr_Function_v4float Function
-%56 = OpVariable %_ptr_Function_v4float Function
-%59 = OpVariable %_ptr_Function_v4float Function
-%74 = OpVariable %_ptr_Function_v4float Function
-OpStore %53 %52
-OpStore %56 %55
-%57 = OpFunctionCall %v4float %live_fn %53 %56
-OpStore %a %57
-OpStore %59 %58
-%60 = OpFunctionCall %v4float %unpremul %59
-OpStore %b %60
-%62 = OpLoad %v4float %a
-%65 = OpFOrdNotEqual %v4bool %62 %64
-%67 = OpAny %bool %65
-OpSelectionMerge %69 None
-OpBranchConditional %67 %68 %69
-%68 = OpLabel
-%70 = OpLoad %v4float %b
-%71 = OpFOrdNotEqual %v4bool %70 %64
-%72 = OpAny %bool %71
-OpBranch %69
-%69 = OpLabel
-%73 = OpPhi %bool %false %48 %72 %68
-OpSelectionMerge %77 None
-OpBranchConditional %73 %75 %76
-%75 = OpLabel
-%78 = OpAccessChain %_ptr_Uniform_v4float %12 %int_0
-%82 = OpLoad %v4float %78
-OpStore %74 %82
-OpBranch %77
-%76 = OpLabel
-%83 = OpAccessChain %_ptr_Uniform_v4float %12 %int_1
-%85 = OpLoad %v4float %83
-OpStore %74 %85
-OpBranch %77
-%77 = OpLabel
-%86 = OpLoad %v4float %74
-OpReturnValue %86
+%58 = OpVariable %_ptr_Function_v4float Function
+%61 = OpVariable %_ptr_Function_v4float Function
+%64 = OpVariable %_ptr_Function_v4float Function
+%78 = OpVariable %_ptr_Function_v4float Function
+OpStore %TRUE %true
+OpStore %FALSE %false
+OpStore %58 %57
+OpStore %61 %60
+%62 = OpFunctionCall %v4float %live_fn %58 %61
+OpStore %a %62
+OpStore %64 %63
+%65 = OpFunctionCall %v4float %unpremul %64
+OpStore %b %65
+%66 = OpLoad %v4float %a
+%69 = OpFOrdNotEqual %v4bool %66 %68
+%71 = OpAny %bool %69
+OpSelectionMerge %73 None
+OpBranchConditional %71 %72 %73
+%72 = OpLabel
+%74 = OpLoad %v4float %b
+%75 = OpFOrdNotEqual %v4bool %74 %68
+%76 = OpAny %bool %75
+OpBranch %73
+%73 = OpLabel
+%77 = OpPhi %bool %false %48 %76 %72
+OpSelectionMerge %81 None
+OpBranchConditional %77 %79 %80
+%79 = OpLabel
+%82 = OpAccessChain %_ptr_Uniform_v4float %12 %int_0
+%86 = OpLoad %v4float %82
+OpStore %78 %86
+OpBranch %81
+%80 = OpLabel
+%87 = OpAccessChain %_ptr_Uniform_v4float %12 %int_1
+%89 = OpLoad %v4float %87
+OpStore %78 %89
+OpBranch %81
+%81 = OpLabel
+%90 = OpLoad %v4float %78
+OpReturnValue %90
 OpFunctionEnd

tests/sksl/shared/DeadStripFunctions.glsl

@@ -9,6 +9,9 @@ vec4 live_fn(vec4 a, vec4 b) {
     return a + b;
 }
 vec4 main() {
+    const bool TRUE = true;
+    const bool FALSE = false;
+
     vec4 a;
     vec4 b;
 

tests/sksl/shared/DeadStripFunctions.metal

@@ -21,6 +21,9 @@ float4 live_fn(float4 a, float4 b) {
 fragment Outputs fragmentMain(Inputs _in [[stage_in]], constant Uniforms& _uniforms [[buffer(0)]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
     Outputs _out;
     (void)_out;
+    const bool TRUE = true;
+    const bool FALSE = false;
+
     float4 a;
     float4 b;
 

tests/sksl/shared/StaticIf.asm.frag

@@ -11,6 +11,8 @@ OpMemberName %_UniformBuffer 1 "colorGreen"
 OpName %_entrypoint "_entrypoint"
 OpName %main "main"
 OpName %result "result"
+OpName %x "x"
+OpName %y "y"
 OpDecorate %sk_FragColor RelaxedPrecision
 OpDecorate %sk_FragColor Location 0
 OpDecorate %sk_FragColor Index 0
@@ -24,8 +26,8 @@ OpDecorate %_UniformBuffer Block
 OpDecorate %10 Binding 0
 OpDecorate %10 DescriptorSet 0
 OpDecorate %26 RelaxedPrecision
-OpDecorate %29 RelaxedPrecision
-OpDecorate %30 RelaxedPrecision
+OpDecorate %34 RelaxedPrecision
+OpDecorate %35 RelaxedPrecision
 %float = OpTypeFloat 32
 %v4float = OpTypeVector %float 4
 %_ptr_Output_v4float = OpTypePointer Output %v4float
@@ -43,6 +45,9 @@ OpDecorate %30 RelaxedPrecision
 %_ptr_Uniform_v4float = OpTypePointer Uniform %v4float
 %int = OpTypeInt 32 1
 %int_0 = OpConstant %int 0
+%_ptr_Function_float = OpTypePointer Function %float
+%float_5 = OpConstant %float 5
+%float_10 = OpConstant %float 10
 %int_1 = OpConstant %int 1
 %_entrypoint = OpFunction %void None %15
 %16 = OpLabel
@@ -53,12 +58,16 @@ OpFunctionEnd
 %main = OpFunction %v4float None %18
 %19 = OpLabel
 %result = OpVariable %_ptr_Function_v4float Function
+%x = OpVariable %_ptr_Function_float Function
+%y = OpVariable %_ptr_Function_float Function
 %22 = OpAccessChain %_ptr_Uniform_v4float %10 %int_0
 %26 = OpLoad %v4float %22
 OpStore %result %26
-%27 = OpAccessChain %_ptr_Uniform_v4float %10 %int_1
-%29 = OpLoad %v4float %27
-OpStore %result %29
-%30 = OpLoad %v4float %result
-OpReturnValue %30
+OpStore %x %float_5
+OpStore %y %float_10
+%32 = OpAccessChain %_ptr_Uniform_v4float %10 %int_1
+%34 = OpLoad %v4float %32
+OpStore %result %34
+%35 = OpLoad %v4float %result
+OpReturnValue %35
 OpFunctionEnd

tests/sksl/shared/StaticIf.glsl

@@ -4,6 +4,8 @@ uniform vec4 colorRed;
 uniform vec4 colorGreen;
 vec4 main() {
     vec4 result = colorRed;
+    const float x = 5.0;
+    const float y = 10.0;
     {
         result = colorGreen;
     }

tests/sksl/shared/StaticIf.metal

@@ -16,6 +16,8 @@ fragment Outputs fragmentMain(Inputs _in [[stage_in]], constant Uniforms& _unifo
     Outputs _out;
     (void)_out;
     float4 result = _uniforms.colorRed;
+    const float x = 5.0;
+    const float y = 10.0;
     {
         result = _uniforms.colorGreen;
     }