[spirv-reader] Avoid emitting empty elses

Produce less noise in ASTs for a common case.

Also test that an empty continuing construct doesn't show up in the
AST.  That's currently handled by the AST code. We want to keep this
behaviour even if the AST implementation changes. Right now
code change is needed when emitting the start of a continuing
construct.

Bug: tint:3
Change-Id: I96a12087e305c64647561f65d87acda907ae9c42
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/22844
Reviewed-by: dan sinclair <dsinclair@google.com>

src/reader/spirv/function.cc

@@ -1790,12 +1790,15 @@ bool FunctionEmitter::EmitIfStart(const BlockInfo& block_info) {
   auto push_else = [this, if_stmt, else_end, construct]() {
     // Push the else clause onto the stack first.
     PushNewStatementBlock(construct, else_end, [if_stmt](StatementBlock* s) {
+      // Only set the else-clause if there are statements to fill it.
+      if (!s->statements.empty()) {
         // The "else" consists of the statement list from the top of statments
         // stack, without an elseif condition.
         ast::ElseStatementList else_stmts;
         else_stmts.emplace_back(std::make_unique<ast::ElseStatement>(
             nullptr, std::move(s->statements)));
         if_stmt->set_else_statements(std::move(else_stmts));
+      }
     });
   };
 

src/reader/spirv/function_cfg_test.cc

@@ -7406,10 +7406,6 @@ TEST_F(SpvParserTest, EmitBody_If_Empty) {
   {
   }
 }
-Else{
-  {
-  }
-}
 Return{}
 )"));
 }
@@ -7452,10 +7448,6 @@ If{
     }
   }
 }
-Else{
-  {
-  }
-}
 Assignment{
   Identifier{var}
   ScalarConstructor{999}
@@ -7684,10 +7676,6 @@ If{
     }
   }
 }
-Else{
-  {
-  }
-}
 Assignment{
   Identifier{var}
   ScalarConstructor{3}
@@ -7828,10 +7816,6 @@ If{
         }
       }
     }
-    Else{
-      {
-      }
-    }
     Assignment{
       Identifier{var}
       ScalarConstructor{3}
@@ -8252,10 +8236,6 @@ Loop{
         }
       }
     }
-    Else{
-      {
-      }
-    }
     Assignment{
       Identifier{var}
       ScalarConstructor{5}
@@ -8504,10 +8484,6 @@ TEST_F(SpvParserTest, EmitBody_Loop_NestedIfContinue) {
       Continue{}
     }
   }
-  Else{
-    {
-    }
-  }
   Assignment{
     Identifier{var}
     ScalarConstructor{2}
@@ -8849,10 +8825,6 @@ TEST_F(SpvParserTest, EmitBody_Return_InsideIf) {
     Return{}
   }
 }
-Else{
-  {
-  }
-}
 Return{}
 )")) << ToString(fe.ast_body());
 }
@@ -8960,10 +8932,6 @@ TEST_F(SpvParserTest, EmitBody_ReturnValue_InsideIf) {
     }
   }
 }
-Else{
-  {
-  }
-}
 Return{
   {
     ScalarConstructor{3}
@@ -9067,10 +9035,6 @@ TEST_F(SpvParserTest, EmitBody_Kill_InsideIf) {
     Kill{}
   }
 }
-Else{
-  {
-  }
-}
 Kill{}
 )")) << ToString(fe.ast_body());
 }
@@ -9153,10 +9117,6 @@ TEST_F(SpvParserTest, EmitBody_Unreachable_InsideIf) {
     Return{}
   }
 }
-Else{
-  {
-  }
-}
 Return{}
 )")) << ToString(fe.ast_body());
 }
@@ -9507,10 +9467,6 @@ TEST_F(SpvParserTest, EmitBody_Branch_LoopContinue_BeforeLast) {
       Continue{}
     }
   }
-  Else{
-    {
-    }
-  }
   Assignment{
     Identifier{var}
     ScalarConstructor{2}
@@ -9559,10 +9515,6 @@ TEST_F(SpvParserTest, EmitBody_Branch_IfBreak_FromThen) {
     }
   }
 }
-Else{
-  {
-  }
-}
 Assignment{
   Identifier{var}
   ScalarConstructor{2}
@@ -10188,10 +10140,6 @@ Loop{
       }
     }
   }
-  Else{
-    {
-    }
-  }
   Assignment{
     Identifier{var}
     ScalarConstructor{3}
@@ -10286,10 +10234,6 @@ Loop{
       }
     }
   }
-  Else{
-    {
-    }
-  }
   Assignment{
     Identifier{var}
     ScalarConstructor{3}
@@ -10648,10 +10592,6 @@ Loop{
       Continue{}
     }
   }
-  Else{
-    {
-    }
-  }
   Assignment{
     Identifier{var}
     ScalarConstructor{4}
@@ -10671,6 +10611,86 @@ Return{}
 )")) << ToString(fe.ast_body());
 }
 
+TEST_F(
+    SpvParserTest,
+    EmitBody_BranchConditional_Continue_Continue_AfterHeader_Conditional_EmptyContinuing) {
+  // Like the previous tests, but with an empty continuing clause.
+  auto* p = parser(test::Assemble(CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpStore %var %uint_0
+     OpBranch %20
+
+     %20 = OpLabel
+     OpStore %var %uint_1
+     OpLoopMerge %99 %80 None
+     OpBranch %30
+
+     %30 = OpLabel
+     OpStore %var %uint_2
+     OpSelectionMerge %50 None
+     OpBranchConditional %cond2 %40 %50
+
+     %40 = OpLabel
+     OpStore %var %uint_3
+     OpBranchConditional %cond3 %80 %80 ; to continue
+
+     %50 = OpLabel ; merge for selection
+     OpStore %var %uint_4
+     OpBranch %80
+
+     %80 = OpLabel ; continue target
+     ; no statements here.
+     OpBranch %20
+
+     %99 = OpLabel
+     OpStore %var %uint_6
+     OpReturn
+
+     OpFunctionEnd
+  )"));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_TRUE(fe.EmitBody()) << p->error();
+  EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Assignment{
+  Identifier{var}
+  ScalarConstructor{0}
+}
+Loop{
+  Assignment{
+    Identifier{var}
+    ScalarConstructor{1}
+  }
+  Assignment{
+    Identifier{var}
+    ScalarConstructor{2}
+  }
+  If{
+    (
+      ScalarConstructor{true}
+    )
+    {
+      Assignment{
+        Identifier{var}
+        ScalarConstructor{3}
+      }
+      Continue{}
+    }
+  }
+  Assignment{
+    Identifier{var}
+    ScalarConstructor{4}
+  }
+}
+Assignment{
+  Identifier{var}
+  ScalarConstructor{6}
+}
+Return{}
+)")) << ToString(fe.ast_body());
+}
+
 TEST_F(SpvParserTest, EmitBody_BranchConditional_Continue_IfBreak_OnTrue) {
   auto* p = parser(test::Assemble(CommonTypes() + R"(
      %100 = OpFunction %void None %voidfn
@@ -10747,10 +10767,6 @@ Loop{
       }
     }
   }
-  Else{
-    {
-    }
-  }
   Assignment{
     Identifier{var}
     ScalarConstructor{4}
@@ -10842,10 +10858,6 @@ Loop{
       }
     }
   }
-  Else{
-    {
-    }
-  }
   Assignment{
     Identifier{var}
     ScalarConstructor{4}
@@ -11061,10 +11073,6 @@ If{
   {
   }
 }
-Else{
-  {
-  }
-}
 Assignment{
   Identifier{var}
   ScalarConstructor{5}