[spirv-reader] Refactor emission for OpBranch

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

src/reader/spirv/function.cc

@@ -399,7 +399,9 @@ ast::Statement* FunctionEmitter::AddStatement(
     std::unique_ptr<ast::Statement> statement) {
   assert(!statements_stack_.empty());
   auto* result = statement.get();
-  statements_stack_.back().statements.emplace_back(std::move(statement));
+  if (result != nullptr) {
+    statements_stack_.back().statements.emplace_back(std::move(statement));
+  }
   return result;
 }
 
@@ -1789,37 +1791,9 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) {
       }
       return true;
     case SpvOpBranch: {
-      const auto dest = terminator.GetSingleWordInOperand(0);
+      const auto dest_id = terminator.GetSingleWordInOperand(0);
-      const auto kind = block_info.succ_edge.find(dest)->second;
+      AddStatement(MakeBranch(block_info, *GetBlockInfo(dest_id)));
-      switch (kind) {
+      return true;
-        case EdgeKind::kBack:
-          // Nothing to do. The loop backedge is implicit.
-          return true;
-        case EdgeKind::kSwitchBreak:
-        case EdgeKind::kLoopBreak:
-          AddStatement(std::make_unique<ast::BreakStatement>());
-          return true;
-        case EdgeKind::kLoopContinue:
-          // An unconditional continue to the next block is redundant and ugly.
-          // Skip it in that case.
-          if (GetBlockInfo(dest)->pos == 1 + block_info.pos) {
-            return true;
-          }
-          // Otherwise, emit a regular continue statement.
-          AddStatement(std::make_unique<ast::ContinueStatement>());
-          return true;
-        case EdgeKind::kIfBreak:
-          // For an unconditional branch, the break out to an if-selection
-          // merge block is implicit.
-          return true;
-        case EdgeKind::kCaseFallThrough:
-          AddStatement(std::make_unique<ast::FallthroughStatement>());
-          return true;
-        case EdgeKind::kForward:
-          // Unconditional forward branch is implicit.
-          return true;
-      }
-      break;
     }
     default:
       break;
@@ -1828,6 +1802,38 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) {
   return success();
 }
 
+std::unique_ptr<ast::Statement> FunctionEmitter::MakeBranch(
+    const BlockInfo& src_info,
+    const BlockInfo& dest_info) const {
+  auto kind = src_info.succ_edge.find(dest_info.id)->second;
+  switch (kind) {
+    case EdgeKind::kBack:
+      // Nothing to do. The loop backedge is implicit.
+      break;
+    case EdgeKind::kSwitchBreak:
+    case EdgeKind::kLoopBreak:
+      return std::make_unique<ast::BreakStatement>();
+    case EdgeKind::kLoopContinue:
+      // An unconditional continue to the next block is redundant and ugly.
+      // Skip it in that case.
+      if (dest_info.pos == 1 + src_info.pos) {
+        break;
+      }
+      // Otherwise, emit a regular continue statement.
+      return std::make_unique<ast::ContinueStatement>();
+    case EdgeKind::kIfBreak:
+      // For an unconditional branch, the break out to an if-selection
+      // merge block is implicit.
+      break;
+    case EdgeKind::kCaseFallThrough:
+      return std::make_unique<ast::FallthroughStatement>();
+    case EdgeKind::kForward:
+      // Unconditional forward branch is implicit.
+      break;
+  }
+  return {nullptr};
+}
+
 bool FunctionEmitter::EmitStatementsInBasicBlock(const BlockInfo& block_info,
                                                  bool* already_emitted) {
   if (*already_emitted) {

src/reader/spirv/function.h

@@ -329,6 +329,14 @@ class FunctionEmitter {
   /// @returns false if emission failed
   bool EmitNormalTerminator(const BlockInfo& block_info);
 
+  /// Returns a new statement to represent the given branch representing a
+  /// "normal" terminator, as in the sense of EmitNormalTerminator.  If no
+  /// WGSL statement is required, the statement will nullptr.
+  /// @param src_info the source block
+  /// @param dest_info the destination block
+  std::unique_ptr<ast::Statement> MakeBranch(const BlockInfo& src_info,
+                                             const BlockInfo& dest_info) const;
+
   /// Emits a normal instruction: not a terminator, label, or variable
   /// declaration.
   /// @param inst the instruction
@@ -403,6 +411,7 @@ class FunctionEmitter {
   BlockInfo* HeaderIfBreakable(const Construct* c);
 
   /// Appends a new statement to the top of the statement stack.
+  /// Does nothing if the statement is null.
   /// @param statement the new statement
   /// @returns a pointer to the statement.
   ast::Statement* AddStatement(std::unique_ptr<ast::Statement> statement);