[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>
diff --git a/.gitignore b/.gitignore
index 6d3242f..139e452 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,6 +6,7 @@
build
buildtools
out
+testing
third_party/cpplint
third_party/binutils
third_party/googletest
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 86fc03a..1f24a10 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -495,7 +495,8 @@
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 @@
if (merge == 0) {
continue;
}
- // This is a header.
- const auto header = block_id;
- const auto* header_info = block_info;
- const auto header_pos = header_info->pos;
- const auto merge_pos = GetBlockInfo(merge)->pos;
+ // This is a header.
+ const auto header = block_id;
+ const auto* header_info = block_info;
+ const auto header_pos = header_info->pos;
+ const auto merge_pos = GetBlockInfo(merge)->pos;
- // Pos(H) < Pos(M(H))
- // Note: When recording merges we made sure H != M(H)
- if (merge_pos <= header_pos) {
- return Fail() << "Header " << header
- << " does not strictly dominate its merge block "
- << merge;
- // TODO(dneto): Report a path from the entry block to the merge block
- // without going through the header block.
- }
+ // Pos(H) < Pos(M(H))
+ // Note: When recording merges we made sure H != M(H)
+ if (merge_pos <= header_pos) {
+ return Fail() << "Header " << header
+ << " does not strictly dominate its merge block " << merge;
+ // TODO(dneto): Report a path from the entry block to the merge block
+ // without going through the header block.
+ }
- const auto ct = block_info->continue_for_header;
- if (ct == 0) {
- continue;
- }
- // Furthermore, this is a loop header.
- const auto* ct_info = GetBlockInfo(ct);
- const auto ct_pos = ct_info->pos;
- // Pos(H) <= Pos(CT(H)), with equality only for single-block loops.
- if (header_info->is_single_block_loop && ct_pos != header_pos) {
- Fail() << "Internal error: Single block loop. CT pos is not the "
- "header pos. Should have already checked this";
- }
- if (!header_info->is_single_block_loop && (ct_pos <= header_pos)) {
- Fail() << "Loop header " << header
- << " does not dominate its continue target " << ct;
- }
- // Pos(CT(H)) < Pos(M(H))
- // Note: When recording merges we made sure CT(H) != M(H)
- if (merge_pos <= ct_pos) {
- return Fail() << "Merge block " << merge
- << " for loop headed at block " << header
- << " appears at or before the loop's continue "
- "construct headed by "
- "block "
- << ct;
- }
+ const auto ct = block_info->continue_for_header;
+ if (ct == 0) {
+ continue;
+ }
+ // Furthermore, this is a loop header.
+ const auto* ct_info = GetBlockInfo(ct);
+ const auto ct_pos = ct_info->pos;
+ // Pos(H) <= Pos(CT(H)), with equality only for single-block loops.
+ if (header_info->is_single_block_loop && ct_pos != header_pos) {
+ Fail() << "Internal error: Single block loop. CT pos is not the "
+ "header pos. Should have already checked this";
+ }
+ if (!header_info->is_single_block_loop && (ct_pos <= header_pos)) {
+ Fail() << "Loop header " << header
+ << " does not dominate its continue target " << ct;
+ }
+ // Pos(CT(H)) < Pos(M(H))
+ // Note: When recording merges we made sure CT(H) != M(H)
+ if (merge_pos <= ct_pos) {
+ return Fail() << "Merge block " << merge << " for loop headed at block "
+ << header
+ << " appears at or before the loop's continue "
+ "construct headed by "
+ "block "
+ << ct;
+ }
}
return success();
}
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index a3a7e89..2a32c8e 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -84,6 +84,19 @@
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 @@
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 @@
{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 @@
}
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 @@
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());
}
diff --git a/src/writer/spirv/builder.h b/src/writer/spirv/builder.h
index a425cb1..32af780 100644
--- a/src/writer/spirv/builder.h
+++ b/src/writer/spirv/builder.h
@@ -149,6 +149,14 @@
/// @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 @@
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
diff --git a/src/writer/spirv/builder_accessor_expression_test.cc b/src/writer/spirv/builder_accessor_expression_test.cc
index 99f1f5c..f09f0cf 100644
--- a/src/writer/spirv/builder_accessor_expression_test.cc
+++ b/src/writer/spirv/builder_accessor_expression_test.cc
@@ -278,8 +278,8 @@
ast::type::AliasType alias("Inner", &inner_struct);
ast::StructMemberList outer_members;
- outer_members.push_back(std::make_unique<ast::StructMember>(
- "inner", &alias, std::move(decos)));
+ outer_members.push_back(
+ 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)));
diff --git a/src/writer/spirv/builder_loop_test.cc b/src/writer/spirv/builder_loop_test.cc
index ec258d9..6333fad 100644
--- a/src/writer/spirv/builder_loop_test.cc
+++ b/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 @@
)");
}
-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
