HLSL: workaround FXC error "continue cannot be used in a switch"
Added a new transform::RemoveContinueInSwitch that replaces continue
statements in switch cases with setting a bool variable, and checking if
the variable is set after the switch to continue.
Bug: tint:1080
Change-Id: I3c0a6c790e1bb612fac3f927a4bd5beb2d0d4ed1
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/84960
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn
index a2541bd..0d52704 100644
--- a/src/tint/BUILD.gn
+++ b/src/tint/BUILD.gn
@@ -471,6 +471,8 @@
"transform/promote_initializers_to_const_var.h",
"transform/promote_side_effects_to_decl.cc",
"transform/promote_side_effects_to_decl.h",
+ "transform/remove_continue_in_switch.cc",
+ "transform/remove_continue_in_switch.h",
"transform/remove_phonies.cc",
"transform/remove_phonies.h",
"transform/remove_unreachable_statements.cc",
diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt
index 0a3ce1c..30e2322 100644
--- a/src/tint/CMakeLists.txt
+++ b/src/tint/CMakeLists.txt
@@ -351,6 +351,8 @@
transform/promote_side_effects_to_decl.h
transform/remove_phonies.cc
transform/remove_phonies.h
+ transform/remove_continue_in_switch.cc
+ transform/remove_continue_in_switch.h
transform/remove_unreachable_statements.cc
transform/remove_unreachable_statements.h
transform/renamer.cc
@@ -1027,6 +1029,7 @@
transform/num_workgroups_from_uniform_test.cc
transform/promote_initializers_to_const_var_test.cc
transform/promote_side_effects_to_decl_test.cc
+ transform/remove_continue_in_switch_test.cc
transform/remove_phonies_test.cc
transform/remove_unreachable_statements_test.cc
transform/renamer_test.cc
diff --git a/src/tint/transform/remove_continue_in_switch.cc b/src/tint/transform/remove_continue_in_switch.cc
new file mode 100644
index 0000000..9ee05c0
--- /dev/null
+++ b/src/tint/transform/remove_continue_in_switch.cc
@@ -0,0 +1,145 @@
+// Copyright 2022 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/tint/transform/remove_continue_in_switch.h"
+
+#include <string>
+#include <unordered_map>
+#include <utility>
+
+#include "src/tint/ast/continue_statement.h"
+#include "src/tint/ast/switch_statement.h"
+#include "src/tint/program_builder.h"
+#include "src/tint/sem/block_statement.h"
+#include "src/tint/sem/for_loop_statement.h"
+#include "src/tint/sem/loop_statement.h"
+#include "src/tint/sem/switch_statement.h"
+#include "src/tint/transform/utils/get_insertion_point.h"
+#include "src/tint/utils/map.h"
+
+TINT_INSTANTIATE_TYPEINFO(tint::transform::RemoveContinueInSwitch);
+
+namespace tint::transform {
+namespace {
+
+class State {
+ private:
+ CloneContext& ctx;
+ ProgramBuilder& b;
+ const sem::Info& sem;
+
+ // Map of switch statement to 'tint_continue' variable.
+ std::unordered_map<const ast::SwitchStatement*, Symbol>
+ switch_to_cont_var_name;
+
+ // If `cont` is within a switch statement within a loop, returns a pointer to
+ // that switch statement.
+ static const ast::SwitchStatement* GetParentSwitchInLoop(
+ const sem::Info& sem,
+ const ast::ContinueStatement* cont) {
+ // Find whether first parent is a switch or a loop
+ auto* sem_stmt = sem.Get(cont);
+ auto* sem_parent =
+ sem_stmt->FindFirstParent<sem::SwitchStatement, sem::LoopBlockStatement,
+ sem::ForLoopStatement>();
+ if (!sem_parent) {
+ return nullptr;
+ }
+ return sem_parent->Declaration()->As<ast::SwitchStatement>();
+ }
+
+ public:
+ /// Constructor
+ /// @param ctx_in the context
+ explicit State(CloneContext& ctx_in)
+ : ctx(ctx_in), b(*ctx_in.dst), sem(ctx_in.src->Sem()) {}
+
+ /// Returns true if this transform should be run for the given program
+ static bool ShouldRun(const Program* program) {
+ for (auto* node : program->ASTNodes().Objects()) {
+ auto* stmt = node->As<ast::ContinueStatement>();
+ if (!stmt) {
+ continue;
+ }
+ if (GetParentSwitchInLoop(program->Sem(), stmt)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /// Runs the transform
+ void Run() {
+ for (auto* node : ctx.src->ASTNodes().Objects()) {
+ auto* cont = node->As<ast::ContinueStatement>();
+ if (!cont) {
+ continue;
+ }
+
+ // If first parent is not a switch within a loop, skip
+ auto* switch_stmt = GetParentSwitchInLoop(sem, cont);
+ if (!switch_stmt) {
+ continue;
+ }
+
+ auto cont_var_name =
+ tint::utils::GetOrCreate(switch_to_cont_var_name, switch_stmt, [&]() {
+ // Create and insert 'var tint_continue : bool = false;' before the
+ // switch.
+ auto var_name = b.Symbols().New("tint_continue");
+ auto* decl = b.Decl(b.Var(var_name, b.ty.bool_(), b.Expr(false)));
+ auto ip = utils::GetInsertionPoint(ctx, switch_stmt);
+ ctx.InsertBefore(ip.first->Declaration()->statements, ip.second,
+ decl);
+
+ // Create and insert 'if (tint_continue) { continue; }' after
+ // switch.
+ auto* if_stmt = b.If(b.Expr(var_name), b.Block(b.Continue()));
+ ctx.InsertAfter(ip.first->Declaration()->statements, ip.second,
+ if_stmt);
+
+ // Return the new var name
+ return var_name;
+ });
+
+ // Replace 'continue;' with '{ tint_continue = true; break; }'
+ auto* new_stmt = b.Block( //
+ b.Assign(b.Expr(cont_var_name), true), //
+ b.Break());
+
+ ctx.Replace(cont, new_stmt);
+ }
+
+ ctx.Clone();
+ }
+};
+
+} // namespace
+
+RemoveContinueInSwitch::RemoveContinueInSwitch() = default;
+RemoveContinueInSwitch::~RemoveContinueInSwitch() = default;
+
+bool RemoveContinueInSwitch::ShouldRun(const Program* program,
+ const DataMap& /*data*/) const {
+ return State::ShouldRun(program);
+}
+
+void RemoveContinueInSwitch::Run(CloneContext& ctx,
+ const DataMap&,
+ DataMap&) const {
+ State state(ctx);
+ state.Run();
+}
+
+} // namespace tint::transform
diff --git a/src/tint/transform/remove_continue_in_switch.h b/src/tint/transform/remove_continue_in_switch.h
new file mode 100644
index 0000000..f875660
--- /dev/null
+++ b/src/tint/transform/remove_continue_in_switch.h
@@ -0,0 +1,55 @@
+// Copyright 2022 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SRC_TINT_TRANSFORM_REMOVE_CONTINUE_IN_SWITCH_H_
+#define SRC_TINT_TRANSFORM_REMOVE_CONTINUE_IN_SWITCH_H_
+
+#include "src/tint/transform/transform.h"
+
+namespace tint::transform {
+
+/// This transform replaces continue statements in switch cases with setting a
+/// bool variable, and checking if the variable is set after the switch to
+/// continue. It is necessary to work around FXC "error X3708: continue cannot
+/// be used in a switch". See crbug.com/tint/1080.
+class RemoveContinueInSwitch
+ : public Castable<RemoveContinueInSwitch, Transform> {
+ public:
+ /// Constructor
+ RemoveContinueInSwitch();
+
+ /// Destructor
+ ~RemoveContinueInSwitch() override;
+
+ protected:
+ /// @param program the program to inspect
+ /// @param data optional extra transform-specific input data
+ /// @returns true if this transform should be run for the given program
+ bool ShouldRun(const Program* program,
+ const DataMap& data = {}) const override;
+
+ /// Runs the transform using the CloneContext built for transforming a
+ /// program. Run() is responsible for calling Clone() on the CloneContext.
+ /// @param ctx the CloneContext primed with the input program and
+ /// ProgramBuilder
+ /// @param inputs optional extra transform-specific input data
+ /// @param outputs optional extra transform-specific output data
+ void Run(CloneContext& ctx,
+ const DataMap& inputs,
+ DataMap& outputs) const override;
+};
+
+} // namespace tint::transform
+
+#endif // SRC_TINT_TRANSFORM_REMOVE_CONTINUE_IN_SWITCH_H_
diff --git a/src/tint/transform/remove_continue_in_switch_test.cc b/src/tint/transform/remove_continue_in_switch_test.cc
new file mode 100644
index 0000000..cd485b0
--- /dev/null
+++ b/src/tint/transform/remove_continue_in_switch_test.cc
@@ -0,0 +1,565 @@
+// Copyright 2022 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/tint/transform/remove_continue_in_switch.h"
+#include "src/tint/transform/test_helper.h"
+
+namespace tint {
+namespace transform {
+namespace {
+
+using RemoveContinueInSwitchTest = TransformTest;
+
+TEST_F(RemoveContinueInSwitchTest, ShouldRun_True) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ loop {
+ switch(i) {
+ case 0: {
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ break;
+ }
+}
+)";
+
+ EXPECT_TRUE(ShouldRun<RemoveContinueInSwitch>(src));
+}
+
+TEST_F(RemoveContinueInSwitchTest, ShouldRunEmptyModule_False) {
+ auto* src = "";
+
+ EXPECT_FALSE(ShouldRun<RemoveContinueInSwitch>(src));
+}
+
+TEST_F(RemoveContinueInSwitchTest, ShouldRunContinueNotInSwitch_False) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ loop {
+ switch(i) {
+ case 0: {
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+
+ if (true) {
+ continue;
+ }
+ break;
+ }
+}
+)";
+
+ EXPECT_FALSE(ShouldRun<RemoveContinueInSwitch>(src));
+}
+
+TEST_F(RemoveContinueInSwitchTest, ShouldRunContinueInLoopInSwitch_False) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ switch(i) {
+ case 0: {
+ loop {
+ if (true) {
+ continue;
+ }
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+}
+)";
+
+ EXPECT_FALSE(ShouldRun<RemoveContinueInSwitch>(src));
+}
+
+TEST_F(RemoveContinueInSwitchTest, EmptyModule) {
+ auto* src = "";
+ auto* expect = src;
+
+ DataMap data;
+ auto got = Run<RemoveContinueInSwitch>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(RemoveContinueInSwitchTest, SingleContinue) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ switch(i) {
+ case 0: {
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker2 = 0;
+ break;
+
+ continuing {
+ let marker3 = 0;
+ }
+ }
+}
+)";
+
+ auto* expect = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ var tint_continue : bool = false;
+ switch(i) {
+ case 0: {
+ {
+ tint_continue = true;
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue) {
+ continue;
+ }
+ let marker2 = 0;
+ break;
+
+ continuing {
+ let marker3 = 0;
+ }
+ }
+}
+)";
+
+ DataMap data;
+ auto got = Run<RemoveContinueInSwitch>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(RemoveContinueInSwitchTest, MultipleContinues) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ switch(i) {
+ case 0: {
+ continue;
+ break;
+ }
+ case 1: {
+ continue;
+ break;
+ }
+ case 2: {
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker2 = 0;
+ break;
+
+ continuing {
+ let marker3 = 0;
+ }
+ }
+}
+)";
+
+ auto* expect = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ var tint_continue : bool = false;
+ switch(i) {
+ case 0: {
+ {
+ tint_continue = true;
+ break;
+ }
+ break;
+ }
+ case 1: {
+ {
+ tint_continue = true;
+ break;
+ }
+ break;
+ }
+ case 2: {
+ {
+ tint_continue = true;
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue) {
+ continue;
+ }
+ let marker2 = 0;
+ break;
+
+ continuing {
+ let marker3 = 0;
+ }
+ }
+}
+)";
+
+ DataMap data;
+ auto got = Run<RemoveContinueInSwitch>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(RemoveContinueInSwitchTest, MultipleSwitch) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ switch(i) {
+ case 0: {
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker2 = 0;
+
+ let marker3 = 0;
+ switch(i) {
+ case 0: {
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker4 = 0;
+
+ break;
+ }
+}
+)";
+
+ auto* expect = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ var tint_continue : bool = false;
+ switch(i) {
+ case 0: {
+ {
+ tint_continue = true;
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue) {
+ continue;
+ }
+ let marker2 = 0;
+ let marker3 = 0;
+ var tint_continue_1 : bool = false;
+ switch(i) {
+ case 0: {
+ {
+ tint_continue_1 = true;
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue_1) {
+ continue;
+ }
+ let marker4 = 0;
+ break;
+ }
+}
+)";
+
+ DataMap data;
+ auto got = Run<RemoveContinueInSwitch>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(RemoveContinueInSwitchTest, NestedLoopSwitch) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ switch(i) {
+ case 0: {
+ var j = 0;
+ loop {
+ let marker3 = 0;
+ switch(j) {
+ case 0: {
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker4 = 0;
+ break;
+ }
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker2 = 0;
+ break;
+ }
+}
+)";
+
+ auto* expect = R"(
+fn f() {
+ var i = 0;
+ loop {
+ let marker1 = 0;
+ var tint_continue_1 : bool = false;
+ switch(i) {
+ case 0: {
+ var j = 0;
+ loop {
+ let marker3 = 0;
+ var tint_continue : bool = false;
+ switch(j) {
+ case 0: {
+ {
+ tint_continue = true;
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue) {
+ continue;
+ }
+ let marker4 = 0;
+ break;
+ }
+ {
+ tint_continue_1 = true;
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue_1) {
+ continue;
+ }
+ let marker2 = 0;
+ break;
+ }
+}
+)";
+
+ DataMap data;
+ auto got = Run<RemoveContinueInSwitch>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(RemoveContinueInSwitchTest, ExtraScopes) {
+ auto* src = R"(
+fn f() {
+ var i = 0;
+ var a = true;
+ var b = true;
+ var c = true;
+ var d = true;
+ loop {
+ if (a) {
+ if (b) {
+ let marker1 = 0;
+ switch(i) {
+ case 0: {
+ if (c) {
+ if (d) {
+ continue;
+ }
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker2 = 0;
+ break;
+ }
+ }
+ }
+}
+)";
+
+ auto* expect = R"(
+fn f() {
+ var i = 0;
+ var a = true;
+ var b = true;
+ var c = true;
+ var d = true;
+ loop {
+ if (a) {
+ if (b) {
+ let marker1 = 0;
+ var tint_continue : bool = false;
+ switch(i) {
+ case 0: {
+ if (c) {
+ if (d) {
+ {
+ tint_continue = true;
+ break;
+ }
+ }
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue) {
+ continue;
+ }
+ let marker2 = 0;
+ break;
+ }
+ }
+ }
+}
+)";
+
+ DataMap data;
+ auto got = Run<RemoveContinueInSwitch>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(RemoveContinueInSwitchTest, ForLoop) {
+ auto* src = R"(
+fn f() {
+ for (var i = 0; i < 4; i = i + 1) {
+ let marker1 = 0;
+ switch(i) {
+ case 0: {
+ continue;
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ let marker2 = 0;
+ break;
+ }
+}
+)";
+
+ auto* expect = R"(
+fn f() {
+ for(var i = 0; (i < 4); i = (i + 1)) {
+ let marker1 = 0;
+ var tint_continue : bool = false;
+ switch(i) {
+ case 0: {
+ {
+ tint_continue = true;
+ break;
+ }
+ break;
+ }
+ default: {
+ break;
+ }
+ }
+ if (tint_continue) {
+ continue;
+ }
+ let marker2 = 0;
+ break;
+ }
+}
+)";
+
+ DataMap data;
+ auto got = Run<RemoveContinueInSwitch>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+} // namespace
+} // namespace transform
+} // namespace tint
diff --git a/src/tint/writer/hlsl/generator_impl.cc b/src/tint/writer/hlsl/generator_impl.cc
index e384156..9ed948d 100644
--- a/src/tint/writer/hlsl/generator_impl.cc
+++ b/src/tint/writer/hlsl/generator_impl.cc
@@ -59,6 +59,7 @@
#include "src/tint/transform/num_workgroups_from_uniform.h"
#include "src/tint/transform/promote_initializers_to_const_var.h"
#include "src/tint/transform/promote_side_effects_to_decl.h"
+#include "src/tint/transform/remove_continue_in_switch.h"
#include "src/tint/transform/remove_phonies.h"
#include "src/tint/transform/simplify_pointers.h"
#include "src/tint/transform/unshadow.h"
@@ -210,6 +211,8 @@
manager.Add<transform::CalculateArrayLength>();
manager.Add<transform::PromoteInitializersToConstVar>();
+ manager.Add<transform::RemoveContinueInSwitch>();
+
manager.Add<transform::AddEmptyEntryPoint>();
data.Add<transform::CanonicalizeEntryPointIO::Config>(
