[tint] Emit `unreachable` as `return` for non-void functions
The `PreventInfiniteLoops` transforms changes the control flow such
that some `unreachable` instructions become statically reachable. This
may upset some downstream compilers that want to ensure that non-void
functions always return.
There are various places that unreachable instructions show up, so
just always emit them as `return` statements in the textual backends
instead of only handling this in the infinite loop transform.
Fixed: 381541325
Change-Id: I92b5063361b179f0618e0f5130a29dfae98ea735
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/217894
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: James Price <jrprice@google.com>
diff --git a/src/tint/lang/glsl/writer/if_test.cc b/src/tint/lang/glsl/writer/if_test.cc
index aeae183..08ff85a 100644
--- a/src/tint/lang/glsl/writer/if_test.cc
+++ b/src/tint/lang/glsl/writer/if_test.cc
@@ -128,6 +128,32 @@
)");
}
+TEST_F(GlslWriterTest, IfBothBranchesReturn_NonVoidFunction) {
+ auto* func = b.Function("foo", ty.u32());
+ b.Append(func->Block(), [&] {
+ auto* if_ = b.If(true);
+ b.Append(if_->True(), [&] { b.Return(func, 0_u); });
+ b.Append(if_->False(), [&] { b.Return(func, 1_u); });
+ b.Unreachable();
+ });
+
+ ASSERT_TRUE(Generate()) << err_ << output_.glsl;
+ EXPECT_EQ(output_.glsl, GlslHeader() + R"(
+uint foo() {
+ if (true) {
+ return 0u;
+ } else {
+ return 1u;
+ }
+ /* unreachable */
+ return 0u;
+}
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void main() {
+}
+)");
+}
+
TEST_F(GlslWriterTest, IfWithSinglePhi) {
auto* func = b.Function("foo", ty.void_());
b.Append(func->Block(), [&] {
diff --git a/src/tint/lang/glsl/writer/printer/printer.cc b/src/tint/lang/glsl/writer/printer/printer.cc
index 0477b23..19938ca 100644
--- a/src/tint/lang/glsl/writer/printer/printer.cc
+++ b/src/tint/lang/glsl/writer/printer/printer.cc
@@ -1724,7 +1724,17 @@
}
/// Emit an unreachable instruction
- void EmitUnreachable() { Line() << "/* unreachable */"; }
+ void EmitUnreachable() {
+ Line() << "/* unreachable */";
+ if (!current_function_->ReturnType()->Is<core::type::Void>()) {
+ // If this is inside a non-void function, emit a return statement to avoid potential
+ // errors due to missing return statements.
+ auto out = Line();
+ out << "return ";
+ EmitZeroValue(out, current_function_->ReturnType());
+ out << ";";
+ }
+ }
};
} // namespace
diff --git a/src/tint/lang/hlsl/writer/if_test.cc b/src/tint/lang/hlsl/writer/if_test.cc
index 524a1c4..c45e6e6 100644
--- a/src/tint/lang/hlsl/writer/if_test.cc
+++ b/src/tint/lang/hlsl/writer/if_test.cc
@@ -124,6 +124,34 @@
)");
}
+TEST_F(HlslWriterTest, IfBothBranchesReturn_NonVoidFunction) {
+ auto* func = b.Function("foo", ty.u32());
+ b.Append(func->Block(), [&] {
+ auto* if_ = b.If(true);
+ b.Append(if_->True(), [&] { b.Return(func, 0_u); });
+ b.Append(if_->False(), [&] { b.Return(func, 1_u); });
+ b.Unreachable();
+ });
+
+ ASSERT_TRUE(Generate()) << err_ << output_.hlsl;
+ EXPECT_EQ(output_.hlsl, R"(
+uint foo() {
+ if (true) {
+ return 0u;
+ } else {
+ return 1u;
+ }
+ /* unreachable */
+ return 0u;
+}
+
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+}
+
+)");
+}
+
TEST_F(HlslWriterTest, IfWithSinglePhi) {
auto* func = b.ComputeFunction("foo");
b.Append(func->Block(), [&] {
diff --git a/src/tint/lang/hlsl/writer/printer/printer.cc b/src/tint/lang/hlsl/writer/printer/printer.cc
index 80b9f98..2bffce7 100644
--- a/src/tint/lang/hlsl/writer/printer/printer.cc
+++ b/src/tint/lang/hlsl/writer/printer/printer.cc
@@ -434,16 +434,13 @@
/// Emit an unreachable instruction
void EmitUnreachable() {
Line() << "/* unreachable */";
- if (current_block_ == current_function_->Block() &&
- !current_function_->ReturnType()->Is<core::type::Void>()) {
- // If this is the end of a non-void function, emit a return statement to avoid DXC
- // errors due to -Wreturn-type + -Werror (see crbug.com/378517038).
- // TODO(381541325): Remove this once these reachable-unreachables are replaced.
+ if (!current_function_->ReturnType()->Is<core::type::Void>()) {
+ // If this is inside a non-void function, emit a return statement to avoid DXC errors
+ // due to -Wreturn-type + -Werror (see crbug.com/378517038).
auto out = Line();
out << "return ";
EmitZeroValue(out, current_function_->ReturnType());
out << ";";
- return;
}
}
diff --git a/src/tint/lang/msl/writer/if_test.cc b/src/tint/lang/msl/writer/if_test.cc
index 8338165..a10591a 100644
--- a/src/tint/lang/msl/writer/if_test.cc
+++ b/src/tint/lang/msl/writer/if_test.cc
@@ -116,6 +116,29 @@
)");
}
+TEST_F(MslWriterTest, IfBothBranchesReturn_NonVoidFunction) {
+ auto* func = b.Function("foo", ty.u32());
+ b.Append(func->Block(), [&] {
+ auto* if_ = b.If(true);
+ b.Append(if_->True(), [&] { b.Return(func, 0_u); });
+ b.Append(if_->False(), [&] { b.Return(func, 1_u); });
+ b.Unreachable();
+ });
+
+ ASSERT_TRUE(Generate()) << err_ << output_.msl;
+ EXPECT_EQ(output_.msl, MetalHeader() + R"(
+uint foo() {
+ if (true) {
+ return 0u;
+ } else {
+ return 1u;
+ }
+ /* unreachable */
+ return 0u;
+}
+)");
+}
+
TEST_F(MslWriterTest, IfWithSinglePhi) {
auto* func = b.Function("foo", ty.void_());
b.Append(func->Block(), [&] {
diff --git a/src/tint/lang/msl/writer/printer/printer.cc b/src/tint/lang/msl/writer/printer/printer.cc
index f493452..ab1dd56 100644
--- a/src/tint/lang/msl/writer/printer/printer.cc
+++ b/src/tint/lang/msl/writer/printer/printer.cc
@@ -795,7 +795,17 @@
}
/// Emit an unreachable instruction
- void EmitUnreachable() { Line() << "/* unreachable */"; }
+ void EmitUnreachable() {
+ Line() << "/* unreachable */";
+ if (!current_function_->ReturnType()->Is<core::type::Void>()) {
+ // If this is inside a non-void function, emit a return statement to avoid potential
+ // errors due to missing return statements.
+ auto out = Line();
+ out << "return ";
+ EmitZeroValue(out, current_function_->ReturnType());
+ out << ";";
+ }
+ }
/// Emit a discard instruction
void EmitDiscard() { Line() << "discard_fragment();"; }
diff --git a/test/tint/loops/loop.wgsl.expected.glsl b/test/tint/loops/loop.wgsl.expected.glsl
index 01d1505..a729ae7 100644
--- a/test/tint/loops/loop.wgsl.expected.glsl
+++ b/test/tint/loops/loop.wgsl.expected.glsl
@@ -22,6 +22,7 @@
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/loop.wgsl.expected.ir.msl b/test/tint/loops/loop.wgsl.expected.ir.msl
index 8a55eb3..8abcec5 100644
--- a/test/tint/loops/loop.wgsl.expected.ir.msl
+++ b/test/tint/loops/loop.wgsl.expected.ir.msl
@@ -23,4 +23,5 @@
}
}
/* unreachable */
+ return 0;
}
diff --git a/test/tint/loops/loop_robustness.wgsl.expected.glsl b/test/tint/loops/loop_robustness.wgsl.expected.glsl
index 01d1505..a729ae7 100644
--- a/test/tint/loops/loop_robustness.wgsl.expected.glsl
+++ b/test/tint/loops/loop_robustness.wgsl.expected.glsl
@@ -22,6 +22,7 @@
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/loop_robustness.wgsl.expected.ir.msl b/test/tint/loops/loop_robustness.wgsl.expected.ir.msl
index 8a55eb3..8abcec5 100644
--- a/test/tint/loops/loop_robustness.wgsl.expected.ir.msl
+++ b/test/tint/loops/loop_robustness.wgsl.expected.ir.msl
@@ -23,4 +23,5 @@
}
}
/* unreachable */
+ return 0;
}
diff --git a/test/tint/loops/loop_with_continuing.wgsl.expected.glsl b/test/tint/loops/loop_with_continuing.wgsl.expected.glsl
index ba98f1c..cabda69 100644
--- a/test/tint/loops/loop_with_continuing.wgsl.expected.glsl
+++ b/test/tint/loops/loop_with_continuing.wgsl.expected.glsl
@@ -22,6 +22,7 @@
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/loop_with_continuing.wgsl.expected.ir.msl b/test/tint/loops/loop_with_continuing.wgsl.expected.ir.msl
index 9e61e13..653b197 100644
--- a/test/tint/loops/loop_with_continuing.wgsl.expected.ir.msl
+++ b/test/tint/loops/loop_with_continuing.wgsl.expected.ir.msl
@@ -23,4 +23,5 @@
}
}
/* unreachable */
+ return 0;
}
diff --git a/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.glsl b/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.glsl
index ba98f1c..cabda69 100644
--- a/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.glsl
+++ b/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.glsl
@@ -22,6 +22,7 @@
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.ir.msl b/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.ir.msl
index 9e61e13..653b197 100644
--- a/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.ir.msl
+++ b/test/tint/loops/loop_with_continuing_robustness.wgsl.expected.ir.msl
@@ -23,4 +23,5 @@
}
}
/* unreachable */
+ return 0;
}
diff --git a/test/tint/loops/nested_loops.wgsl.expected.glsl b/test/tint/loops/nested_loops.wgsl.expected.glsl
index a28aa87..6780ab0 100644
--- a/test/tint/loops/nested_loops.wgsl.expected.glsl
+++ b/test/tint/loops/nested_loops.wgsl.expected.glsl
@@ -33,9 +33,11 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/nested_loops.wgsl.expected.ir.dxc.hlsl b/test/tint/loops/nested_loops.wgsl.expected.ir.dxc.hlsl
index 408d692..052ea2c 100644
--- a/test/tint/loops/nested_loops.wgsl.expected.ir.dxc.hlsl
+++ b/test/tint/loops/nested_loops.wgsl.expected.ir.dxc.hlsl
@@ -32,6 +32,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops.wgsl.expected.ir.fxc.hlsl b/test/tint/loops/nested_loops.wgsl.expected.ir.fxc.hlsl
index 408d692..052ea2c 100644
--- a/test/tint/loops/nested_loops.wgsl.expected.ir.fxc.hlsl
+++ b/test/tint/loops/nested_loops.wgsl.expected.ir.fxc.hlsl
@@ -32,6 +32,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops.wgsl.expected.ir.msl b/test/tint/loops/nested_loops.wgsl.expected.ir.msl
index 5041162..f1447d7 100644
--- a/test/tint/loops/nested_loops.wgsl.expected.ir.msl
+++ b/test/tint/loops/nested_loops.wgsl.expected.ir.msl
@@ -34,7 +34,9 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
diff --git a/test/tint/loops/nested_loops_robustness.wgsl.expected.glsl b/test/tint/loops/nested_loops_robustness.wgsl.expected.glsl
index a28aa87..6780ab0 100644
--- a/test/tint/loops/nested_loops_robustness.wgsl.expected.glsl
+++ b/test/tint/loops/nested_loops_robustness.wgsl.expected.glsl
@@ -33,9 +33,11 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.dxc.hlsl b/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.dxc.hlsl
index 408d692..052ea2c 100644
--- a/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.dxc.hlsl
+++ b/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.dxc.hlsl
@@ -32,6 +32,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.fxc.hlsl b/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.fxc.hlsl
index 408d692..052ea2c 100644
--- a/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.fxc.hlsl
+++ b/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.fxc.hlsl
@@ -32,6 +32,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.msl b/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.msl
index 5041162..f1447d7 100644
--- a/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.msl
+++ b/test/tint/loops/nested_loops_robustness.wgsl.expected.ir.msl
@@ -34,7 +34,9 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
diff --git a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.glsl b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.glsl
index c94016d..9133f30 100644
--- a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.glsl
+++ b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.glsl
@@ -32,9 +32,11 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.dxc.hlsl b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.dxc.hlsl
index 75b60e6..65a61dd 100644
--- a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.dxc.hlsl
+++ b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.dxc.hlsl
@@ -31,6 +31,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.fxc.hlsl b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.fxc.hlsl
index 75b60e6..65a61dd 100644
--- a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.fxc.hlsl
+++ b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.fxc.hlsl
@@ -31,6 +31,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.msl b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.msl
index 2b4726c..d0e6af1 100644
--- a/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.msl
+++ b/test/tint/loops/nested_loops_with_continuing.wgsl.expected.ir.msl
@@ -33,7 +33,9 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
diff --git a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.glsl b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.glsl
index c94016d..9133f30 100644
--- a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.glsl
+++ b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.glsl
@@ -32,9 +32,11 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
void main() {
diff --git a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.dxc.hlsl b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.dxc.hlsl
index 75b60e6..65a61dd 100644
--- a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.dxc.hlsl
+++ b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.dxc.hlsl
@@ -31,6 +31,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.fxc.hlsl b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.fxc.hlsl
index 75b60e6..65a61dd 100644
--- a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.fxc.hlsl
+++ b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.fxc.hlsl
@@ -31,6 +31,7 @@
}
}
/* unreachable */
+ return int(0);
}
}
/* unreachable */
diff --git a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.msl b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.msl
index 2b4726c..d0e6af1 100644
--- a/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.msl
+++ b/test/tint/loops/nested_loops_with_continuing_robustness.wgsl.expected.ir.msl
@@ -33,7 +33,9 @@
}
}
/* unreachable */
+ return 0;
}
}
/* unreachable */
+ return 0;
}
