[ir] Fix MergeReturn for trailing returns
Always create a new if-continue-execution instruction when the
subsequent instruction is a return that has a value. Otherwise, the
return gets folded into the existing if-continue-execution block
without being guarded, and then overwrites the return value.
Bug: tint:1906
Change-Id: I638c430a342f8e148a6ab8e449357fea3ccfd63e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/145580
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/lang/spirv/writer/raise/merge_return.cc b/src/tint/lang/spirv/writer/raise/merge_return.cc
index 22a0665..15a42bb 100644
--- a/src/tint/lang/spirv/writer/raise/merge_return.cc
+++ b/src/tint/lang/spirv/writer/raise/merge_return.cc
@@ -154,7 +154,7 @@
if (holds_return_.Contains(ctrl)) {
// Control instruction transitively holds a return.
ctrl->ForeachBlock([&](ir::Block* ctrl_block) { ProcessBlock(ctrl_block); });
- if (next && next != fn_return &&
+ if (next && (next != fn_return || fn_return->Value()) &&
!tint::IsAnyOf<ir::Exit, ir::Unreachable>(next)) {
inner_if = CreateIfContinueExecution(ctrl);
}
diff --git a/src/tint/lang/spirv/writer/raise/merge_return_test.cc b/src/tint/lang/spirv/writer/raise/merge_return_test.cc
index e8f8023..956cd56 100644
--- a/src/tint/lang/spirv/writer/raise/merge_return_test.cc
+++ b/src/tint/lang/spirv/writer/raise/merge_return_test.cc
@@ -1060,6 +1060,113 @@
EXPECT_EQ(expect, str());
}
+TEST_F(SpirvWriter_MergeReturnTest, IfElse_Consecutive) {
+ auto* value = b.FunctionParam(ty.i32());
+ auto* func = b.Function("foo", ty.i32());
+ func->SetParams({value});
+
+ b.Append(func->Block(), [&] {
+ {
+ auto* ifelse = b.If(b.Equal(ty.bool_(), value, 1_i));
+ b.Append(ifelse->True(), [&] { b.Return(func, 101_i); });
+ }
+ {
+ auto* ifelse = b.If(b.Equal(ty.bool_(), value, 2_i));
+ b.Append(ifelse->True(), [&] { b.Return(func, 202_i); });
+ }
+ {
+ auto* ifelse = b.If(b.Equal(ty.bool_(), value, 3_i));
+ b.Append(ifelse->True(), [&] { b.Return(func, 303_i); });
+ }
+ b.Return(func, 404_i);
+ });
+
+ auto* src = R"(
+%foo = func(%2:i32):i32 -> %b1 {
+ %b1 = block {
+ %3:bool = eq %2, 1i
+ if %3 [t: %b2] { # if_1
+ %b2 = block { # true
+ ret 101i
+ }
+ }
+ %4:bool = eq %2, 2i
+ if %4 [t: %b3] { # if_2
+ %b3 = block { # true
+ ret 202i
+ }
+ }
+ %5:bool = eq %2, 3i
+ if %5 [t: %b4] { # if_3
+ %b4 = block { # true
+ ret 303i
+ }
+ }
+ ret 404i
+ }
+}
+)";
+ EXPECT_EQ(src, str());
+
+ auto* expect = R"(
+%foo = func(%2:i32):i32 -> %b1 {
+ %b1 = block {
+ %return_value:ptr<function, i32, read_write> = var
+ %continue_execution:ptr<function, bool, read_write> = var, true
+ %5:bool = eq %2, 1i
+ if %5 [t: %b2] { # if_1
+ %b2 = block { # true
+ store %continue_execution, false
+ store %return_value, 101i
+ exit_if # if_1
+ }
+ }
+ %6:bool = load %continue_execution
+ if %6 [t: %b3] { # if_2
+ %b3 = block { # true
+ %7:bool = eq %2, 2i
+ if %7 [t: %b4] { # if_3
+ %b4 = block { # true
+ store %continue_execution, false
+ store %return_value, 202i
+ exit_if # if_3
+ }
+ }
+ %8:bool = load %continue_execution
+ if %8 [t: %b5] { # if_4
+ %b5 = block { # true
+ %9:bool = eq %2, 3i
+ if %9 [t: %b6] { # if_5
+ %b6 = block { # true
+ store %continue_execution, false
+ store %return_value, 303i
+ exit_if # if_5
+ }
+ }
+ %10:bool = load %continue_execution
+ if %10 [t: %b7] { # if_6
+ %b7 = block { # true
+ store %return_value, 404i
+ exit_if # if_6
+ }
+ }
+ exit_if # if_4
+ }
+ }
+ exit_if # if_2
+ }
+ }
+ %11:i32 = load %return_value
+ ret %11
+ }
+}
+)";
+
+ Run(MergeReturn);
+
+ EXPECT_EQ(expect, str());
+}
+
TEST_F(SpirvWriter_MergeReturnTest, Loop_UnconditionalReturnInBody) {
auto* func = b.Function("foo", ty.i32());
