[ir] Testing updates.
This CL renames the `Build` method in the test helper to be
`CreateBuilder` to maek the intention clearer. A second,
`CreateEmptyBuilder` is provided to allow for easier testing of
expressions.
The `current_flow_block` and `builder` are moved to public so they can
be used/updated during testing.
Bug: tint:1718
Change-Id: I663d4c7a3c76e6bf5396ca05f54fe634d35d0d56
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/110781
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/tint/ir/builder_impl.cc b/src/tint/ir/builder_impl.cc
index f332052..fb6200c 100644
--- a/src/tint/ir/builder_impl.cc
+++ b/src/tint/ir/builder_impl.cc
@@ -77,20 +77,20 @@
} // namespace
-BuilderImpl::BuilderImpl(const Program* program) : builder_(program) {}
+BuilderImpl::BuilderImpl(const Program* program) : builder(program) {}
BuilderImpl::~BuilderImpl() = default;
void BuilderImpl::BranchTo(FlowNode* node) {
- TINT_ASSERT(IR, current_flow_block_);
- TINT_ASSERT(IR, !IsBranched(current_flow_block_));
+ TINT_ASSERT(IR, current_flow_block);
+ TINT_ASSERT(IR, !IsBranched(current_flow_block));
- builder_.Branch(current_flow_block_, node);
- current_flow_block_ = nullptr;
+ builder.Branch(current_flow_block, node);
+ current_flow_block = nullptr;
}
void BuilderImpl::BranchToIfNeeded(FlowNode* node) {
- if (!current_flow_block_ || IsBranched(current_flow_block_)) {
+ if (!current_flow_block || IsBranched(current_flow_block)) {
return;
}
BranchTo(node);
@@ -112,7 +112,7 @@
}
ResultType BuilderImpl::Build() {
- auto* sem = builder_.ir.program->Sem().Module();
+ auto* sem = builder.ir.program->Sem().Module();
for (auto* decl : sem->DependencyOrderedDeclarations()) {
bool ok = tint::Switch(
@@ -140,27 +140,27 @@
}
}
- return ResultType{std::move(builder_.ir)};
+ return ResultType{std::move(builder.ir)};
}
bool BuilderImpl::EmitFunction(const ast::Function* ast_func) {
// The flow stack should have been emptied when the previous function finshed building.
TINT_ASSERT(IR, flow_stack.IsEmpty());
- auto* ir_func = builder_.CreateFunction(ast_func);
+ auto* ir_func = builder.CreateFunction(ast_func);
current_function_ = ir_func;
- builder_.ir.functions.Push(ir_func);
+ builder.ir.functions.Push(ir_func);
ast_to_flow_[ast_func] = ir_func;
if (ast_func->IsEntryPoint()) {
- builder_.ir.entry_points.Push(ir_func);
+ builder.ir.entry_points.Push(ir_func);
}
{
FlowStackScope scope(this, ir_func);
- current_flow_block_ = ir_func->start_target;
+ current_flow_block = ir_func->start_target;
if (!EmitStatements(ast_func->body->statements)) {
return false;
}
@@ -171,7 +171,7 @@
}
TINT_ASSERT(IR, flow_stack.IsEmpty());
- current_flow_block_ = nullptr;
+ current_flow_block = nullptr;
current_function_ = nullptr;
return true;
@@ -185,7 +185,7 @@
// If the current flow block has a branch target then the rest of the statements in this
// block are dead code. Skip them.
- if (!current_flow_block_ || IsBranched(current_flow_block_)) {
+ if (!current_flow_block || IsBranched(current_flow_block)) {
break;
}
}
@@ -229,7 +229,7 @@
}
bool BuilderImpl::EmitIf(const ast::IfStatement* stmt) {
- auto* if_node = builder_.CreateIf(stmt);
+ auto* if_node = builder.CreateIf(stmt);
// TODO(dsinclair): Emit the condition expression into the current block
@@ -242,34 +242,34 @@
// TODO(dsinclair): set if condition register into if flow node
- current_flow_block_ = if_node->true_target;
+ current_flow_block = if_node->true_target;
if (!EmitStatement(stmt->body)) {
return false;
}
// If the true branch did not execute control flow, then go to the merge target
BranchToIfNeeded(if_node->merge_target);
- current_flow_block_ = if_node->false_target;
+ current_flow_block = if_node->false_target;
if (stmt->else_statement && !EmitStatement(stmt->else_statement)) {
return false;
}
// If the false branch did not execute control flow, then go to the merge target
BranchToIfNeeded(if_node->merge_target);
}
- current_flow_block_ = nullptr;
+ current_flow_block = nullptr;
// If both branches went somewhere, then they both returned, continued or broke. So,
// there is no need for the if merge-block and there is nothing to branch to the merge
// block anyway.
if (IsConnected(if_node->merge_target)) {
- current_flow_block_ = if_node->merge_target;
+ current_flow_block = if_node->merge_target;
}
return true;
}
bool BuilderImpl::EmitLoop(const ast::LoopStatement* stmt) {
- auto* loop_node = builder_.CreateLoop(stmt);
+ auto* loop_node = builder.CreateLoop(stmt);
BranchTo(loop_node);
@@ -278,7 +278,7 @@
{
FlowStackScope scope(this, loop_node);
- current_flow_block_ = loop_node->start_target;
+ current_flow_block = loop_node->start_target;
if (!EmitStatement(stmt->body)) {
return false;
}
@@ -286,7 +286,7 @@
// The current block didn't `break`, `return` or `continue`, go to the continuing block.
BranchToIfNeeded(loop_node->continuing_target);
- current_flow_block_ = loop_node->continuing_target;
+ current_flow_block = loop_node->continuing_target;
if (stmt->continuing) {
if (!EmitStatement(stmt->continuing)) {
return false;
@@ -299,17 +299,17 @@
// The loop merge can get disconnected if the loop returns directly, or the continuing target
// branches, eventually, to the merge, but nothing branched to the continuing target.
- current_flow_block_ = loop_node->merge_target;
+ current_flow_block = loop_node->merge_target;
if (!IsConnected(loop_node->merge_target)) {
- current_flow_block_ = nullptr;
+ current_flow_block = nullptr;
}
return true;
}
bool BuilderImpl::EmitWhile(const ast::WhileStatement* stmt) {
- auto* loop_node = builder_.CreateLoop(stmt);
+ auto* loop_node = builder.CreateLoop(stmt);
// Continue is always empty, just go back to the start
- builder_.Branch(loop_node->continuing_target, loop_node->start_target);
+ builder.Branch(loop_node->continuing_target, loop_node->start_target);
BranchTo(loop_node);
@@ -318,19 +318,19 @@
{
FlowStackScope scope(this, loop_node);
- current_flow_block_ = loop_node->start_target;
+ current_flow_block = loop_node->start_target;
// TODO(dsinclair): Emit the instructions for the condition
// Create an if (cond) {} else {break;} control flow
- auto* if_node = builder_.CreateIf(nullptr);
- builder_.Branch(if_node->true_target, if_node->merge_target);
- builder_.Branch(if_node->false_target, loop_node->merge_target);
+ auto* if_node = builder.CreateIf(nullptr);
+ builder.Branch(if_node->true_target, if_node->merge_target);
+ builder.Branch(if_node->false_target, loop_node->merge_target);
// TODO(dsinclair): set if condition register into if flow node
BranchTo(if_node);
- current_flow_block_ = if_node->merge_target;
+ current_flow_block = if_node->merge_target;
if (!EmitStatement(stmt->body)) {
return false;
}
@@ -339,13 +339,13 @@
}
// The while loop always has a path to the merge target as the break statement comes before
// anything inside the loop.
- current_flow_block_ = loop_node->merge_target;
+ current_flow_block = loop_node->merge_target;
return true;
}
bool BuilderImpl::EmitForLoop(const ast::ForLoopStatement* stmt) {
- auto* loop_node = builder_.CreateLoop(stmt);
- builder_.Branch(loop_node->continuing_target, loop_node->start_target);
+ auto* loop_node = builder.CreateLoop(stmt);
+ builder.Branch(loop_node->continuing_target, loop_node->start_target);
if (stmt->initializer) {
// Emit the for initializer before branching to the loop
@@ -361,19 +361,19 @@
{
FlowStackScope scope(this, loop_node);
- current_flow_block_ = loop_node->start_target;
+ current_flow_block = loop_node->start_target;
if (stmt->condition) {
// TODO(dsinclair): Emit the instructions for the condition
// Create an if (cond) {} else {break;} control flow
- auto* if_node = builder_.CreateIf(nullptr);
- builder_.Branch(if_node->true_target, if_node->merge_target);
- builder_.Branch(if_node->false_target, loop_node->merge_target);
+ auto* if_node = builder.CreateIf(nullptr);
+ builder.Branch(if_node->true_target, if_node->merge_target);
+ builder.Branch(if_node->false_target, loop_node->merge_target);
// TODO(dsinclair): set if condition register into if flow node
BranchTo(if_node);
- current_flow_block_ = if_node->merge_target;
+ current_flow_block = if_node->merge_target;
}
if (!EmitStatement(stmt->body)) {
@@ -383,7 +383,7 @@
BranchToIfNeeded(loop_node->continuing_target);
if (stmt->continuing) {
- current_flow_block_ = loop_node->continuing_target;
+ current_flow_block = loop_node->continuing_target;
if (!EmitStatement(stmt->continuing)) {
return false;
}
@@ -391,12 +391,12 @@
}
// The while loop always has a path to the merge target as the break statement comes before
// anything inside the loop.
- current_flow_block_ = loop_node->merge_target;
+ current_flow_block = loop_node->merge_target;
return true;
}
bool BuilderImpl::EmitSwitch(const ast::SwitchStatement* stmt) {
- auto* switch_node = builder_.CreateSwitch(stmt);
+ auto* switch_node = builder.CreateSwitch(stmt);
// TODO(dsinclair): Emit the condition expression into the current block
@@ -408,17 +408,17 @@
FlowStackScope scope(this, switch_node);
for (const auto* c : stmt->body) {
- current_flow_block_ = builder_.CreateCase(switch_node, c->selectors);
+ current_flow_block = builder.CreateCase(switch_node, c->selectors);
if (!EmitStatement(c->body)) {
return false;
}
BranchToIfNeeded(switch_node->merge_target);
}
}
- current_flow_block_ = nullptr;
+ current_flow_block = nullptr;
if (IsConnected(switch_node->merge_target)) {
- current_flow_block_ = switch_node->merge_target;
+ current_flow_block = switch_node->merge_target;
}
return true;
@@ -461,7 +461,7 @@
}
bool BuilderImpl::EmitBreakIf(const ast::BreakIfStatement* stmt) {
- auto* if_node = builder_.CreateIf(stmt);
+ auto* if_node = builder.CreateIf(stmt);
// TODO(dsinclair): Emit the condition expression into the current block
@@ -477,13 +477,13 @@
// TODO(dsinclair): set if condition register into if flow node
- current_flow_block_ = if_node->true_target;
+ current_flow_block = if_node->true_target;
BranchTo(loop->merge_target);
- current_flow_block_ = if_node->false_target;
+ current_flow_block = if_node->false_target;
BranchTo(if_node->merge_target);
- current_flow_block_ = if_node->merge_target;
+ current_flow_block = if_node->merge_target;
// The `break-if` has to be the last item in the continuing block. The false branch of the
// `break-if` will always take us back to the start of the loop.
diff --git a/src/tint/ir/builder_impl.h b/src/tint/ir/builder_impl.h
index 186de64..c917413 100644
--- a/src/tint/ir/builder_impl.h
+++ b/src/tint/ir/builder_impl.h
@@ -178,6 +178,12 @@
/// The stack of flow control blocks.
utils::Vector<FlowNode*, 8> flow_stack;
+ /// The IR builder being used by the impl.
+ Builder builder;
+
+ /// The current flow block for expressions
+ Block* current_flow_block = nullptr;
+
private:
enum class ControlFlags { kNone, kExcludeSwitch };
@@ -186,11 +192,8 @@
FlowNode* FindEnclosingControl(ControlFlags flags);
- Builder builder_;
-
diag::List diagnostics_;
- Block* current_flow_block_ = nullptr;
Function* current_function_ = nullptr;
/// Map from ast nodes to flow nodes, used to retrieve the flow node for a given AST node.
diff --git a/src/tint/ir/builder_impl_test.cc b/src/tint/ir/builder_impl_test.cc
index 14f01ba..510d1e0 100644
--- a/src/tint/ir/builder_impl_test.cc
+++ b/src/tint/ir/builder_impl_test.cc
@@ -28,7 +28,7 @@
// func -> start -> end
Func("f", utils::Empty, ty.void_(), utils::Empty);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -50,7 +50,7 @@
TEST_F(IRBuilderImplTest, EntryPoint) {
Func("f", utils::Empty, ty.void_(), utils::Empty,
utils::Vector{Stage(ast::PipelineStage::kFragment)});
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -70,7 +70,7 @@
//
auto* ast_if = If(true, Block(), Else(Block()));
WrapInFunction(ast_if);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -113,7 +113,7 @@
//
auto* ast_if = If(true, Block(Return()));
WrapInFunction(ast_if);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -154,7 +154,7 @@
//
auto* ast_if = If(true, Block(), Else(Block(Return())));
WrapInFunction(ast_if);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -195,7 +195,7 @@
//
auto* ast_if = If(true, Block(Return()), Else(Block(Return())));
WrapInFunction(ast_if);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -246,7 +246,7 @@
auto* ast_loop = Loop(Block(Break()));
auto* ast_if = If(true, Block(ast_loop));
WrapInFunction(ast_if);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -289,7 +289,7 @@
//
auto* ast_loop = Loop(Block(Break()));
WrapInFunction(ast_loop);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -333,7 +333,7 @@
auto* ast_if = If(true, Block(Break()));
auto* ast_loop = Loop(Block(ast_if, Continue()));
WrapInFunction(ast_loop);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -392,7 +392,7 @@
auto* ast_break_if = BreakIf(true);
auto* ast_loop = Loop(Block(), Block(ast_break_if));
WrapInFunction(ast_loop);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -452,7 +452,7 @@
auto* ast_if = If(true, Block(Return()));
auto* ast_loop = Loop(Block(ast_if, Continue()));
WrapInFunction(ast_loop);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -520,7 +520,7 @@
// block.
auto* ast_loop = Loop(Block(Return(), Continue()));
WrapInFunction(ast_loop, If(true, Block(Return())));
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -578,7 +578,7 @@
auto* ast_loop = Loop(Block(Return()), Block(ast_break_if));
auto* ast_if = If(true, Block(Return()));
WrapInFunction(Block(ast_loop, ast_if));
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -635,7 +635,7 @@
auto* ast_if = If(true, Block(Break()), Else(Block(Break())));
auto* ast_loop = Loop(Block(ast_if, Continue()));
WrapInFunction(ast_loop);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -744,7 +744,7 @@
auto* ast_loop_a = Loop(Block(ast_loop_b, ast_if_d));
WrapInFunction(ast_loop_a);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -896,7 +896,7 @@
//
auto* ast_while = While(false, Block());
WrapInFunction(ast_while);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -956,7 +956,7 @@
//
auto* ast_while = While(true, Block(Return()));
WrapInFunction(ast_while);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -1015,7 +1015,7 @@
//
auto* ast_for = For(Decl(Var("i", ty.i32())), LessThan("i", 10_a), Increment("i"), Block());
WrapInFunction(ast_for);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -1067,7 +1067,7 @@
//
auto* ast_for = For(nullptr, nullptr, nullptr, Block(Break()));
WrapInFunction(ast_for);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -1111,7 +1111,7 @@
Case(utils::Vector{CaseSelector(1_i)}, Block()), DefaultCase(Block())});
WrapInFunction(ast_switch);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -1159,7 +1159,7 @@
auto* ast_switch = Switch(1_i, utils::Vector{DefaultCase(Block())});
WrapInFunction(ast_switch);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -1211,7 +1211,7 @@
DefaultCase(Block())});
WrapInFunction(ast_switch);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@@ -1275,7 +1275,7 @@
auto* ast_if = If(true, Block(Return()));
WrapInFunction(ast_switch, ast_if);
- auto& b = Build();
+ auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
diff --git a/src/tint/ir/test_helper.h b/src/tint/ir/test_helper.h
index fdc4787..f2bdeb5 100644
--- a/src/tint/ir/test_helper.h
+++ b/src/tint/ir/test_helper.h
@@ -36,7 +36,7 @@
/// @note The builder is only created once. Multiple calls to Build() will
/// return the same builder without rebuilding.
/// @return the builder
- BuilderImpl& Build() {
+ BuilderImpl& CreateBuilder() {
if (gen_) {
return *gen_;
}
@@ -47,6 +47,16 @@
return *gen_;
}
+ /// Creates a BuilderImpl without an originating program. This is used for testing the
+ /// expressions which don't require the full builder implementation. The current flow block
+ /// is initialized with an empty block.
+ /// @returns the BuilderImpl for testing.
+ BuilderImpl& CreateEmptyBuilder() {
+ gen_ = std::make_unique<BuilderImpl>(nullptr);
+ gen_->current_flow_block = gen_->builder.CreateBlock();
+ return *gen_;
+ }
+
/// The program built with a call to Build()
std::unique_ptr<Program> program;
