[spirv-writer] Add start of break and continue.

This CL adds the beginning of break and continue support. The
conditional versions are not supported, just the non-conditional.

Bug: tint:5
Change-Id: I84418cffd3e29dc011c4313bf9aa3da4833c009f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/20500
Reviewed-by: David Neto <dneto@google.com>

.gitignore

@@ -6,6 +6,7 @@
 build
 buildtools
 out
+testing
 third_party/cpplint
 third_party/binutils
 third_party/googletest

src/reader/spirv/function.cc

@@ -495,7 +495,8 @@ bool FunctionEmitter::RegisterMerges() {
     const auto block_id = block.id();
     auto* block_info = GetBlockInfo(block_id);
     if (!block_info) {
-      return Fail() << "internal error: block " << block_id << " missing; blocks should already "
+      return Fail() << "internal error: block " << block_id
+                    << " missing; blocks should already "
                        "have been registered";
     }
 
@@ -617,48 +618,47 @@ bool FunctionEmitter::VerifyHeaderContinueMergeOrder() {
     if (merge == 0) {
       continue;
     }
-      // This is a header.
+    // This is a header.
-      const auto header = block_id;
+    const auto header = block_id;
-      const auto* header_info = block_info;
+    const auto* header_info = block_info;
-      const auto header_pos = header_info->pos;
+    const auto header_pos = header_info->pos;
-      const auto merge_pos = GetBlockInfo(merge)->pos;
+    const auto merge_pos = GetBlockInfo(merge)->pos;
 
-      // Pos(H) < Pos(M(H))
+    // Pos(H) < Pos(M(H))
-      // Note: When recording merges we made sure H != M(H)
+    // Note: When recording merges we made sure H != M(H)
-      if (merge_pos <= header_pos) {
+    if (merge_pos <= header_pos) {
-        return Fail() << "Header " << header
+      return Fail() << "Header " << header
-                      << " does not strictly dominate its merge block "
+                    << " does not strictly dominate its merge block " << merge;
-                      << merge;
+      // TODO(dneto): Report a path from the entry block to the merge block
-        // TODO(dneto): Report a path from the entry block to the merge block
+      // without going through the header block.
-        // without going through the header block.
+    }
-      }
 
-      const auto ct = block_info->continue_for_header;
+    const auto ct = block_info->continue_for_header;
-      if (ct == 0) {
+    if (ct == 0) {
-        continue;
+      continue;
-      }
+    }
-      // Furthermore, this is a loop header.
+    // Furthermore, this is a loop header.
-      const auto* ct_info = GetBlockInfo(ct);
+    const auto* ct_info = GetBlockInfo(ct);
-      const auto ct_pos = ct_info->pos;
+    const auto ct_pos = ct_info->pos;
-      // Pos(H) <= Pos(CT(H)), with equality only for single-block loops.
+    // Pos(H) <= Pos(CT(H)), with equality only for single-block loops.
-      if (header_info->is_single_block_loop && ct_pos != header_pos) {
+    if (header_info->is_single_block_loop && ct_pos != header_pos) {
-        Fail() << "Internal error: Single block loop.  CT pos is not the "
+      Fail() << "Internal error: Single block loop.  CT pos is not the "
-                  "header pos. Should have already checked this";
+                "header pos. Should have already checked this";
-      }
+    }
-      if (!header_info->is_single_block_loop && (ct_pos <= header_pos)) {
+    if (!header_info->is_single_block_loop && (ct_pos <= header_pos)) {
-        Fail() << "Loop header " << header
+      Fail() << "Loop header " << header
-               << " does not dominate its continue target " << ct;
+             << " does not dominate its continue target " << ct;
-      }
+    }
-        // Pos(CT(H)) < Pos(M(H))
+    // Pos(CT(H)) < Pos(M(H))
-        // Note: When recording merges we made sure CT(H) != M(H)
+    // Note: When recording merges we made sure CT(H) != M(H)
-        if (merge_pos <= ct_pos) {
+    if (merge_pos <= ct_pos) {
-          return Fail() << "Merge block " << merge
+      return Fail() << "Merge block " << merge << " for loop headed at block "
-                        << " for loop headed at block " << header
+                    << header
-                        << " appears at or before the loop's continue "
+                    << " appears at or before the loop's continue "
-                           "construct headed by "
+                       "construct headed by "
-                           "block "
+                       "block "
-                        << ct;
+                    << ct;
-        }
+    }
   }
   return success();
 }

src/writer/spirv/builder.cc

@@ -84,6 +84,19 @@ uint32_t pipeline_stage_to_execution_model(ast::PipelineStage stage) {
   return model;
 }
 
+// A terminator is anything which will case a SPIR-V terminator to be emitted.
+// This means things like breaks, fallthroughs and continues which all emit an
+// OpBranch or return for the OpReturn emission.
+bool LastIsTerminator(const ast::StatementList& stmts) {
+  if (stmts.empty()) {
+    return false;
+  }
+
+  auto* last = stmts.back().get();
+  return last->IsBreak() || last->IsContinue() || last->IsReturn() ||
+         last->IsKill() || last->IsFallthrough();
+}
+
 }  // namespace
 
 Builder::Builder(ast::Module* mod) : mod_(mod), scope_stack_({}) {}
@@ -178,6 +191,24 @@ bool Builder::GenerateAssignStatement(ast::AssignmentStatement* assign) {
   return true;
 }
 
+bool Builder::GenerateBreakStatement(ast::BreakStatement*) {
+  if (merge_stack_.empty()) {
+    error_ = "Attempted to break with a merge block";
+    return false;
+  }
+  push_function_inst(spv::Op::OpBranch, {Operand::Int(merge_stack_.back())});
+  return true;
+}
+
+bool Builder::GenerateContinueStatement(ast::ContinueStatement*) {
+  if (continue_stack_.empty()) {
+    error_ = "Attempted to continue with a continue block";
+    return false;
+  }
+  push_function_inst(spv::Op::OpBranch, {Operand::Int(continue_stack_.back())});
+  return true;
+}
+
 bool Builder::GenerateEntryPoint(ast::EntryPoint* ep) {
   auto name = ep->name();
   if (name.empty()) {
@@ -988,12 +1019,19 @@ bool Builder::GenerateLoopStatement(ast::LoopStatement* stmt) {
       {Operand::Int(merge_block_id), Operand::Int(continue_block_id),
        Operand::Int(SpvLoopControlMaskNone)});
 
+  continue_stack_.push_back(continue_block_id);
+  merge_stack_.push_back(merge_block_id);
+
   push_function_inst(spv::Op::OpBranch, {Operand::Int(body_block_id)});
   push_function_inst(spv::Op::OpLabel, {body_block});
   if (!GenerateStatementList(stmt->body())) {
     return false;
   }
-  push_function_inst(spv::Op::OpBranch, {Operand::Int(continue_block_id)});
+
+  // We only branch if the last element of the body didn't already branch.
+  if (!LastIsTerminator(stmt->body())) {
+    push_function_inst(spv::Op::OpBranch, {Operand::Int(continue_block_id)});
+  }
 
   push_function_inst(spv::Op::OpLabel, {continue_block});
   if (!GenerateStatementList(stmt->continuing())) {
@@ -1001,6 +1039,9 @@ bool Builder::GenerateLoopStatement(ast::LoopStatement* stmt) {
   }
   push_function_inst(spv::Op::OpBranch, {Operand::Int(loop_header_id)});
 
+  merge_stack_.pop_back();
+  continue_stack_.pop_back();
+
   push_function_inst(spv::Op::OpLabel, {merge_block});
 
   return true;
@@ -1019,6 +1060,12 @@ bool Builder::GenerateStatement(ast::Statement* stmt) {
   if (stmt->IsAssign()) {
     return GenerateAssignStatement(stmt->AsAssign());
   }
+  if (stmt->IsBreak()) {
+    return GenerateBreakStatement(stmt->AsBreak());
+  }
+  if (stmt->IsContinue()) {
+    return GenerateContinueStatement(stmt->AsContinue());
+  }
   if (stmt->IsIf()) {
     return GenerateIfStatement(stmt->AsIf());
   }

src/writer/spirv/builder.h

@@ -149,6 +149,14 @@ class Builder {
   /// @param assign the statement to generate
   /// @returns true if the statement was successfully generated
   bool GenerateAssignStatement(ast::AssignmentStatement* assign);
+  /// Generates a break statement
+  /// @param stmt the statement to generate
+  /// @returns true if the statement was successfully generated
+  bool GenerateBreakStatement(ast::BreakStatement* stmt);
+  /// Generates a continue statement
+  /// @param stmt the statement to generate
+  /// @returns true if the statement was successfully generated
+  bool GenerateContinueStatement(ast::ContinueStatement* stmt);
   /// Generates an entry point instruction
   /// @param ep the entry point
   /// @returns true if the instruction was generated, false otherwise
@@ -308,6 +316,8 @@ class Builder {
   std::unordered_map<std::string, uint32_t> const_to_id_;
   ScopeStack<uint32_t> scope_stack_;
   std::unordered_map<uint32_t, ast::Variable*> spirv_id_to_variable_;
+  std::vector<uint32_t> merge_stack_;
+  std::vector<uint32_t> continue_stack_;
 };
 
 }  // namespace spirv

src/writer/spirv/builder_accessor_expression_test.cc

@@ -278,8 +278,8 @@ TEST_F(BuilderTest, MemberAccessor_Nested_WithAlias) {
   ast::type::AliasType alias("Inner", &inner_struct);
 
   ast::StructMemberList outer_members;
-  outer_members.push_back(std::make_unique<ast::StructMember>(
+  outer_members.push_back(
-      "inner", &alias, std::move(decos)));
+      std::make_unique<ast::StructMember>("inner", &alias, std::move(decos)));
 
   ast::type::StructType s_type(std::make_unique<ast::Struct>(
       ast::StructDecoration::kNone, std::move(outer_members)));

src/writer/spirv/builder_loop_test.cc

@@ -16,6 +16,8 @@
 
 #include "gtest/gtest.h"
 #include "src/ast/assignment_statement.h"
+#include "src/ast/break_statement.h"
+#include "src/ast/continue_statement.h"
 #include "src/ast/identifier_expression.h"
 #include "src/ast/int_literal.h"
 #include "src/ast/loop_statement.h"
@@ -166,9 +168,73 @@ OpBranch %4
 )");
 }
 
-TEST_F(BuilderTest, DISABLED_Loop_WithContinuing) {}
+TEST_F(BuilderTest, Loop_WithContinue) {
+  // loop {
+  //   continue;
+  // }
+  ast::StatementList body;
+  body.push_back(std::make_unique<ast::ContinueStatement>());
 
-TEST_F(BuilderTest, DISABLED_Loop_WithBreak) {}
+  ast::StatementList continuing;
+  ast::LoopStatement expr(std::move(body), std::move(continuing));
+
+  Context ctx;
+  ast::Module mod;
+  TypeDeterminer td(&ctx, &mod);
+  ASSERT_TRUE(td.DetermineResultType(&expr)) << td.error();
+
+  Builder b(&mod);
+  b.push_function(Function{});
+
+  EXPECT_TRUE(b.GenerateLoopStatement(&expr)) << b.error();
+  EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
+            R"(OpBranch %1
+%1 = OpLabel
+OpLoopMerge %2 %3 None
+OpBranch %4
+%4 = OpLabel
+OpBranch %3
+%3 = OpLabel
+OpBranch %1
+%2 = OpLabel
+)");
+}
+
+TEST_F(BuilderTest, DISABLED_Loop_WithConditionalContinue) {}
+
+TEST_F(BuilderTest, Loop_WithBreak) {
+  // loop {
+  //   break;
+  // }
+  ast::StatementList body;
+  body.push_back(std::make_unique<ast::BreakStatement>());
+
+  ast::StatementList continuing;
+  ast::LoopStatement expr(std::move(body), std::move(continuing));
+
+  Context ctx;
+  ast::Module mod;
+  TypeDeterminer td(&ctx, &mod);
+  ASSERT_TRUE(td.DetermineResultType(&expr)) << td.error();
+
+  Builder b(&mod);
+  b.push_function(Function{});
+
+  EXPECT_TRUE(b.GenerateLoopStatement(&expr)) << b.error();
+  EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
+            R"(OpBranch %1
+%1 = OpLabel
+OpLoopMerge %2 %3 None
+OpBranch %4
+%4 = OpLabel
+OpBranch %2
+%3 = OpLabel
+OpBranch %1
+%2 = OpLabel
+)");
+}
+
+TEST_F(BuilderTest, DISABLED_Loop_WithConditionalBreak) {}
 
 }  // namespace
 }  // namespace spirv