[IR] Fix setting of if merge target
When emitting the if true/false blocks we attempt to set the merge
target if the start block hasn't branched. This isn't right, as if the
block branched to other control flow, then that isn't the end of the
flow chain for that branch. We have to look at the current branch
target, and, if it exists, branch that to the if merge block.
Bug: tint:1718
Change-Id: Ifcafc4dd12c805efbee9d1dbcbc42c6add8f06a9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/107861
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/ir/builder_impl.cc b/src/tint/ir/builder_impl.cc
index 5a35b12..6bbcf6d 100644
--- a/src/tint/ir/builder_impl.cc
+++ b/src/tint/ir/builder_impl.cc
@@ -254,23 +254,18 @@
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;
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;
- // If the true branch did not execute control flow, then go to the merge target
- if (!IsBranched(if_node->true_target)) {
- builder_.Branch(if_node->true_target, if_node->merge_target);
- }
- // If the false branch did not execute control flow, then go to the merge target
- if (!IsBranched(if_node->false_target)) {
- builder_.Branch(if_node->false_target, if_node->merge_target);
- }
-
// 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.
@@ -454,10 +449,6 @@
// TODO(dsinclair): Emit the return value ....
BranchTo(current_function_->end_target);
-
- // TODO(dsinclair): The BranchTo will reset the current block. What happens with dead code
- // after the return?
-
return true;
}
@@ -474,8 +465,6 @@
return false;
}
- // TODO(dsinclair): The BranchTo will reset the current block. What happens with dead code
- // after the break?
return true;
}
@@ -489,9 +478,6 @@
TINT_UNREACHABLE(IR, diagnostics_);
}
- // TODO(dsinclair): The BranchTo will reset the current block. What happens with dead code
- // after the continue?
-
return true;
}
diff --git a/src/tint/ir/builder_impl_test.cc b/src/tint/ir/builder_impl_test.cc
index 1e2325e..1ee66e5 100644
--- a/src/tint/ir/builder_impl_test.cc
+++ b/src/tint/ir/builder_impl_test.cc
@@ -225,6 +225,63 @@
EXPECT_EQ(flow->false_target->branch_target, func->end_target);
}
+TEST_F(IRBuilderImplTest, IfStatement_JumpChainToMerge) {
+ // if (true) {
+ // loop {
+ // break;
+ // }
+ // }
+ //
+ // func -> start -> if true
+ // -> if false
+ //
+ // [if true] -> loop
+ // [if false] -> if merge
+ // [if merge] -> func end
+ // [loop] -> loop start
+ // [loop start] -> loop merge
+ // [loop continuing] -> loop start
+ // [loop merge] -> if merge
+ //
+ auto* ast_loop = Loop(Block(Break()));
+ auto* ast_if = If(true, Block(ast_loop));
+ WrapInFunction(ast_if);
+ auto& b = Build();
+
+ auto r = b.Build();
+ ASSERT_TRUE(r) << b.error();
+ auto m = r.Move();
+
+ auto* ir_if = b.FlowNodeForAstNode(ast_if);
+ ASSERT_NE(ir_if, nullptr);
+ EXPECT_TRUE(ir_if->Is<ir::If>());
+
+ auto* if_flow = ir_if->As<ir::If>();
+ ASSERT_NE(if_flow->true_target, nullptr);
+ ASSERT_NE(if_flow->false_target, nullptr);
+ ASSERT_NE(if_flow->merge_target, nullptr);
+
+ auto* ir_loop = b.FlowNodeForAstNode(ast_loop);
+ ASSERT_NE(ir_loop, nullptr);
+ EXPECT_TRUE(ir_loop->Is<ir::Loop>());
+
+ auto* loop_flow = ir_loop->As<ir::Loop>();
+ ASSERT_NE(loop_flow->start_target, nullptr);
+ ASSERT_NE(loop_flow->continuing_target, nullptr);
+ ASSERT_NE(loop_flow->merge_target, nullptr);
+
+ ASSERT_EQ(1u, m.functions.Length());
+ auto* func = m.functions[0];
+
+ EXPECT_EQ(func->start_target->branch_target, if_flow);
+ EXPECT_EQ(if_flow->true_target->branch_target, loop_flow);
+ EXPECT_EQ(loop_flow->start_target->branch_target, loop_flow->merge_target);
+ EXPECT_EQ(loop_flow->merge_target->branch_target, if_flow->merge_target);
+ EXPECT_EQ(loop_flow->continuing_target->branch_target, loop_flow->start_target);
+ EXPECT_EQ(if_flow->false_target->branch_target, if_flow->merge_target);
+ EXPECT_EQ(if_flow->merge_target->branch_target, func->end_target);
+}
+
TEST_F(IRBuilderImplTest, Loop_WithBreak) {
// func -> start -> loop -> loop start -> loop merge -> func end
//
