[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;
 }