Fixes for bugs around unreachable code
Remove the ICE check for expression behaviors always being either `{Next}` or `{Next, Discard}`. Unreachable code may be result in something else.
Add the RemoveUnreachableStatements transform to the SPIR-V writer sanitizer transform list. The writer cannot correctly handle unreachable statements.
Bug: tint:1369
Bug: chromium:1285622
Change-Id: I9fa54c6d2096b1ee633dd551b628c7dd3ba64fb5
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/76300
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc
index e346ed3..d732f0c 100644
--- a/src/resolver/resolver.cc
+++ b/src/resolver/resolver.cc
@@ -1149,18 +1149,6 @@
return nullptr;
}
- // https://www.w3.org/TR/WGSL/#behaviors-rules
- // an expression behavior is always either {Next} or {Next, Discard}
- if (sem_expr->Behaviors() != sem::Behavior::kNext &&
- sem_expr->Behaviors() != sem::Behaviors{sem::Behavior::kNext, // NOLINT
- sem::Behavior::kDiscard} &&
- !IsCallStatement(expr)) {
- TINT_ICE(Resolver, diagnostics_)
- << expr->TypeInfo().name
- << " behaviors are: " << sem_expr->Behaviors();
- return nullptr;
- }
-
builder_->Sem().Add(expr, sem_expr);
if (expr == root) {
return sem_expr;
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index f7f354f..f4b93ac 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -47,6 +47,7 @@
#include "src/transform/fold_constants.h"
#include "src/transform/for_loop_to_loop.h"
#include "src/transform/manager.h"
+#include "src/transform/remove_unreachable_statements.h"
#include "src/transform/simplify_pointers.h"
#include "src/transform/unshadow.h"
#include "src/transform/var_for_dynamic_index.h"
@@ -260,6 +261,7 @@
if (!disable_workgroup_init) {
manager.Add<transform::ZeroInitWorkgroupMemory>();
}
+ manager.Add<transform::RemoveUnreachableStatements>();
manager.Add<transform::SimplifyPointers>(); // Required for arrayLength()
manager.Add<transform::FoldConstants>();
manager.Add<transform::ExternalTextureTransform>();
diff --git a/test/bug/tint/1369.wgsl b/test/bug/tint/1369.wgsl
new file mode 100644
index 0000000..434bb2b
--- /dev/null
+++ b/test/bug/tint/1369.wgsl
@@ -0,0 +1,10 @@
+fn call_discard() -> bool {
+ discard;
+ return true;
+}
+
+[[stage(fragment)]]
+fn f() {
+ var v = call_discard();
+ var also_unreachable : bool;
+}
diff --git a/test/bug/tint/1369.wgsl.expected.hlsl b/test/bug/tint/1369.wgsl.expected.hlsl
new file mode 100644
index 0000000..dff55b3
--- /dev/null
+++ b/test/bug/tint/1369.wgsl.expected.hlsl
@@ -0,0 +1,22 @@
+bug/tint/1369.wgsl:3:3 warning: code is unreachable
+ return true;
+ ^^^^^^
+
+bug/tint/1369.wgsl:9:9 warning: code is unreachable
+ var also_unreachable : bool;
+ ^^^^^^^^^^^^^^^^
+
+bool call_discard() {
+ if (true) {
+ discard;
+ return true;
+ }
+ bool unused;
+ return unused;
+}
+
+void f() {
+ bool v = call_discard();
+ bool also_unreachable = false;
+ return;
+}
diff --git a/test/bug/tint/1369.wgsl.expected.msl b/test/bug/tint/1369.wgsl.expected.msl
new file mode 100644
index 0000000..8d1a7d7
--- /dev/null
+++ b/test/bug/tint/1369.wgsl.expected.msl
@@ -0,0 +1,22 @@
+bug/tint/1369.wgsl:3:3 warning: code is unreachable
+ return true;
+ ^^^^^^
+
+bug/tint/1369.wgsl:9:9 warning: code is unreachable
+ var also_unreachable : bool;
+ ^^^^^^^^^^^^^^^^
+
+#include <metal_stdlib>
+
+using namespace metal;
+bool call_discard() {
+ discard_fragment();
+ return true;
+}
+
+fragment void f() {
+ bool v = call_discard();
+ bool also_unreachable = false;
+ return;
+}
+
diff --git a/test/bug/tint/1369.wgsl.expected.spvasm b/test/bug/tint/1369.wgsl.expected.spvasm
new file mode 100644
index 0000000..a78ae04
--- /dev/null
+++ b/test/bug/tint/1369.wgsl.expected.spvasm
@@ -0,0 +1,37 @@
+bug/tint/1369.wgsl:3:3 warning: code is unreachable
+ return true;
+ ^^^^^^
+
+bug/tint/1369.wgsl:9:9 warning: code is unreachable
+ var also_unreachable : bool;
+ ^^^^^^^^^^^^^^^^
+
+; SPIR-V
+; Version: 1.3
+; Generator: Google Tint Compiler; 0
+; Bound: 13
+; Schema: 0
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %f "f"
+ OpExecutionMode %f OriginUpperLeft
+ OpName %call_discard "call_discard"
+ OpName %f "f"
+ OpName %v "v"
+ %bool = OpTypeBool
+ %1 = OpTypeFunction %bool
+ %void = OpTypeVoid
+ %5 = OpTypeFunction %void
+%_ptr_Function_bool = OpTypePointer Function %bool
+ %12 = OpConstantNull %bool
+%call_discard = OpFunction %bool None %1
+ %4 = OpLabel
+ OpKill
+ OpFunctionEnd
+ %f = OpFunction %void None %5
+ %8 = OpLabel
+ %v = OpVariable %_ptr_Function_bool Function %12
+ %9 = OpFunctionCall %bool %call_discard
+ OpStore %v %9
+ OpReturn
+ OpFunctionEnd
diff --git a/test/bug/tint/1369.wgsl.expected.wgsl b/test/bug/tint/1369.wgsl.expected.wgsl
new file mode 100644
index 0000000..eb44227
--- /dev/null
+++ b/test/bug/tint/1369.wgsl.expected.wgsl
@@ -0,0 +1,18 @@
+bug/tint/1369.wgsl:3:3 warning: code is unreachable
+ return true;
+ ^^^^^^
+
+bug/tint/1369.wgsl:9:9 warning: code is unreachable
+ var also_unreachable : bool;
+ ^^^^^^^^^^^^^^^^
+
+fn call_discard() -> bool {
+ discard;
+ return true;
+}
+
+[[stage(fragment)]]
+fn f() {
+ var v = call_discard();
+ var also_unreachable : bool;
+}
