[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>
This commit is contained in:
dan sinclair 2022-11-22 13:15:08 +00:00 committed by Dawn LUCI CQ
parent 3769b4b696
commit e02377c2b3
4 changed files with 89 additions and 76 deletions

View File

@ -77,20 +77,20 @@ bool IsConnected(const FlowNode* b) {
} // 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 @@ FlowNode* BuilderImpl::FindEnclosingControl(ControlFlags flags) {
}
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 @@ ResultType BuilderImpl::Build() {
}
}
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 @@ bool BuilderImpl::EmitFunction(const ast::Function* ast_func) {
}
TINT_ASSERT(IR, flow_stack.IsEmpty());
current_flow_block_ = nullptr;
current_flow_block = nullptr;
current_function_ = nullptr;
return true;
@ -185,7 +185,7 @@ bool BuilderImpl::EmitStatements(utils::VectorRef<const ast::Statement*> stmts)
// 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::EmitBlock(const ast::BlockStatement* block) {
}
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 @@ bool BuilderImpl::EmitIf(const ast::IfStatement* stmt) {
// 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 @@ bool BuilderImpl::EmitLoop(const ast::LoopStatement* stmt) {
{
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 @@ bool BuilderImpl::EmitLoop(const ast::LoopStatement* stmt) {
// 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 @@ bool BuilderImpl::EmitLoop(const ast::LoopStatement* stmt) {
// 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 @@ bool BuilderImpl::EmitWhile(const ast::WhileStatement* stmt) {
{
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 @@ bool BuilderImpl::EmitWhile(const ast::WhileStatement* stmt) {
}
// 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 @@ bool BuilderImpl::EmitForLoop(const ast::ForLoopStatement* stmt) {
{
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 @@ bool BuilderImpl::EmitForLoop(const ast::ForLoopStatement* stmt) {
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 @@ bool BuilderImpl::EmitForLoop(const ast::ForLoopStatement* stmt) {
}
// 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 @@ bool BuilderImpl::EmitSwitch(const ast::SwitchStatement* stmt) {
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::EmitContinue(const ast::ContinueStatement*) {
}
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 @@ bool BuilderImpl::EmitBreakIf(const ast::BreakIfStatement* stmt) {
// 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.

View File

@ -178,6 +178,12 @@ class BuilderImpl {
/// 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 @@ class BuilderImpl {
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.

View File

@ -28,7 +28,7 @@ TEST_F(IRBuilderImplTest, Func) {
// 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, Func) {
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 @@ TEST_F(IRBuilderImplTest, IfStatement) {
//
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 @@ TEST_F(IRBuilderImplTest, IfStatement_TrueReturns) {
//
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 @@ TEST_F(IRBuilderImplTest, IfStatement_FalseReturns) {
//
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 @@ TEST_F(IRBuilderImplTest, IfStatement_BothReturn) {
//
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 @@ TEST_F(IRBuilderImplTest, IfStatement_JumpChainToMerge) {
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 @@ TEST_F(IRBuilderImplTest, Loop_WithBreak) {
//
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 @@ TEST_F(IRBuilderImplTest, Loop_WithContinue) {
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 @@ TEST_F(IRBuilderImplTest, Loop_WithContinuing_BreakIf) {
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 @@ TEST_F(IRBuilderImplTest, Loop_WithReturn) {
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 @@ TEST_F(IRBuilderImplTest, Loop_WithOnlyReturn) {
// 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 @@ TEST_F(IRBuilderImplTest, Loop_WithOnlyReturn_ContinuingBreakIf) {
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 @@ TEST_F(IRBuilderImplTest, Loop_WithIf_BothBranchesBreak) {
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 @@ TEST_F(IRBuilderImplTest, Loop_Nested) {
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 @@ TEST_F(IRBuilderImplTest, While) {
//
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 @@ TEST_F(IRBuilderImplTest, While_Return) {
//
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 @@ TEST_F(IRBuilderImplTest, DISABLED_For) {
//
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 @@ TEST_F(IRBuilderImplTest, For_NoInitCondOrContinuing) {
//
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 @@ TEST_F(IRBuilderImplTest, Switch) {
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 @@ TEST_F(IRBuilderImplTest, Switch_OnlyDefault) {
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 @@ TEST_F(IRBuilderImplTest, Switch_WithBreak) {
DefaultCase(Block())});
WrapInFunction(ast_switch);
auto& b = Build();
auto& b = CreateBuilder();
auto r = b.Build();
ASSERT_TRUE(r) << b.error();
@ -1275,7 +1275,7 @@ TEST_F(IRBuilderImplTest, Switch_AllReturn) {
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();

View File

@ -36,7 +36,7 @@ class TestHelperBase : public BASE, public ProgramBuilder {
/// @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 @@ class TestHelperBase : public BASE, public ProgramBuilder {
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;