[spirv-writer] Fix implicit blocks with results
When an if instruction produces result values, we shouldn't emit
implicit blocks using OpUnreachable as they should actually branching
to the merge block.
Change-Id: I814946aeb924aa4630323cde30725b0d791ded12
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/161141
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/tint/lang/spirv/writer/if_test.cc b/src/tint/lang/spirv/writer/if_test.cc
index a91ab22..5dd00ed 100644
--- a/src/tint/lang/spirv/writer/if_test.cc
+++ b/src/tint/lang/spirv/writer/if_test.cc
@@ -241,6 +241,34 @@
)");
}
+TEST_F(SpirvWriterTest, If_Phi_SingleValue_ImplicitFalse) {
+ auto* func = b.Function("foo", ty.i32());
+ b.Append(func->Block(), [&] {
+ auto* i = b.If(true);
+ i->SetResults(b.InstructionResult(ty.i32()));
+ b.Append(i->True(), [&] { //
+ b.ExitIf(i, 10_i);
+ });
+ b.Return(func, i);
+ });
+
+ ASSERT_TRUE(Generate()) << Error() << output_;
+ EXPECT_INST("%12 = OpUndef %int");
+ EXPECT_INST(R"(
+ %4 = OpLabel
+ OpSelectionMerge %5 None
+ OpBranchConditional %true %6 %7
+ %6 = OpLabel
+ OpBranch %5
+ %7 = OpLabel
+ OpBranch %5
+ %5 = OpLabel
+ %10 = OpPhi %int %int_10 %6 %12 %7
+ OpReturnValue %10
+ OpFunctionEnd
+)");
+}
+
TEST_F(SpirvWriterTest, If_Phi_MultipleValue_0) {
auto* func = b.Function("foo", ty.i32());
b.Append(func->Block(), [&] {
diff --git a/src/tint/lang/spirv/writer/loop_test.cc b/src/tint/lang/spirv/writer/loop_test.cc
index 332c52a..c262b2c 100644
--- a/src/tint/lang/spirv/writer/loop_test.cc
+++ b/src/tint/lang/spirv/writer/loop_test.cc
@@ -572,5 +572,54 @@
)");
}
+TEST_F(SpirvWriterTest, Loop_Phi_NestedIfWithResultAndImplicitFalse_InContinuing) {
+ auto* func = b.Function("foo", ty.void_());
+
+ b.Append(func->Block(), [&] {
+ auto* loop = b.Loop();
+
+ b.Append(loop->Body(), [&] { //
+ b.Continue(loop);
+ });
+
+ b.Append(loop->Continuing(), [&] {
+ auto* if_ = b.If(true);
+ auto* cond = b.InstructionResult(ty.bool_());
+ if_->SetResults(Vector{cond});
+ b.Append(if_->True(), [&] { //
+ b.ExitIf(if_, true);
+ });
+ b.BreakIf(loop, cond);
+ });
+
+ b.Return(func);
+ });
+
+ ASSERT_TRUE(Generate()) << Error() << output_;
+ EXPECT_INST("%15 = OpUndef %bool");
+ EXPECT_INST(R"(
+ %4 = OpLabel
+ OpBranch %7
+ %7 = OpLabel
+ OpLoopMerge %8 %6 None
+ OpBranch %5
+ %5 = OpLabel
+ OpBranch %6
+ %6 = OpLabel
+ OpSelectionMerge %9 None
+ OpBranchConditional %true %10 %11
+ %10 = OpLabel
+ OpBranch %9
+ %11 = OpLabel
+ OpBranch %9
+ %9 = OpLabel
+ %14 = OpPhi %bool %true %10 %15 %11
+ OpBranchConditional %14 %8 %7
+ %8 = OpLabel
+ OpReturn
+ OpFunctionEnd
+)");
+}
+
} // namespace
} // namespace tint::spirv::writer
diff --git a/src/tint/lang/spirv/writer/printer/printer.cc b/src/tint/lang/spirv/writer/printer/printer.cc
index 78bca04..942c326 100644
--- a/src/tint/lang/spirv/writer/printer/printer.cc
+++ b/src/tint/lang/spirv/writer/printer/printer.cc
@@ -837,7 +837,11 @@
// If there are no instructions in the block, it's a dead end, so we shouldn't be able to
// get here to begin with.
if (block->IsEmpty()) {
- current_function_.push_inst(spv::Op::OpUnreachable, {});
+ if (!block->Parent()->Results().IsEmpty()) {
+ current_function_.push_inst(spv::Op::OpBranch, {GetMergeLabel(block->Parent())});
+ } else {
+ current_function_.push_inst(spv::Op::OpUnreachable, {});
+ }
return;
}
@@ -2163,6 +2167,13 @@
}
branches.Sort(); // Sort the branches by label to ensure deterministic output
+ // Also add phi nodes from implicit exit blocks.
+ inst->ForeachBlock([&](core::ir::Block* block) {
+ if (block->IsEmpty()) {
+ branches.Push(Branch{Label(block), nullptr});
+ }
+ });
+
OperandList ops{Type(ty), Value(result)};
for (auto& branch : branches) {
if (branch.value == nullptr) {
