diff --git a/src/BUILD.gn b/src/BUILD.gn index c36f031317..3084e1fc62 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -465,8 +465,8 @@ libtint_source_set("libtint_core_all_src") { "transform/num_workgroups_from_uniform.h", "transform/pad_array_elements.cc", "transform/pad_array_elements.h", - "transform/promote_initializers_to_const_var.cc", - "transform/promote_initializers_to_const_var.h", + "transform/promote_side_effects_to_decl.cc", + "transform/promote_side_effects_to_decl.h", "transform/remove_phonies.cc", "transform/remove_phonies.h", "transform/remove_unreachable_statements.cc", @@ -483,8 +483,6 @@ libtint_source_set("libtint_core_all_src") { "transform/transform.h", "transform/unshadow.cc", "transform/unshadow.h", - "transform/var_for_dynamic_index.cc", - "transform/var_for_dynamic_index.h", "transform/vectorize_scalar_matrix_constructors.cc", "transform/vectorize_scalar_matrix_constructors.h", "transform/vertex_pulling.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ddcb686023..db5071f186 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -335,8 +335,8 @@ set(TINT_LIB_SRCS transform/num_workgroups_from_uniform.h transform/pad_array_elements.cc transform/pad_array_elements.h - transform/promote_initializers_to_const_var.cc - transform/promote_initializers_to_const_var.h + transform/promote_side_effects_to_decl.cc + transform/promote_side_effects_to_decl.h transform/remove_phonies.cc transform/remove_phonies.h transform/remove_unreachable_statements.cc @@ -355,8 +355,6 @@ set(TINT_LIB_SRCS transform/unshadow.h transform/vectorize_scalar_matrix_constructors.cc transform/vectorize_scalar_matrix_constructors.h - transform/var_for_dynamic_index.cc - transform/var_for_dynamic_index.h transform/vertex_pulling.cc transform/vertex_pulling.h transform/wrap_arrays_in_structs.cc @@ -991,7 +989,7 @@ if(TINT_BUILD_TESTS) transform/multiplanar_external_texture_test.cc transform/num_workgroups_from_uniform_test.cc transform/pad_array_elements_test.cc - transform/promote_initializers_to_const_var_test.cc + transform/promote_side_effects_to_decl_test.cc transform/remove_phonies_test.cc transform/remove_unreachable_statements_test.cc transform/renamer_test.cc @@ -1000,7 +998,6 @@ if(TINT_BUILD_TESTS) transform/single_entry_point_test.cc transform/test_helper.h transform/unshadow_test.cc - transform/var_for_dynamic_index_test.cc transform/vectorize_scalar_matrix_constructors_test.cc transform/vertex_pulling_test.cc transform/wrap_arrays_in_structs_test.cc diff --git a/src/transform/glsl.cc b/src/transform/glsl.cc index 38e92d4a71..e2419cd34c 100644 --- a/src/transform/glsl.cc +++ b/src/transform/glsl.cc @@ -27,7 +27,7 @@ #include "src/transform/loop_to_for_loop.h" #include "src/transform/manager.h" #include "src/transform/pad_array_elements.h" -#include "src/transform/promote_initializers_to_const_var.h" +#include "src/transform/promote_side_effects_to_decl.h" #include "src/transform/remove_phonies.h" #include "src/transform/simplify_pointers.h" #include "src/transform/single_entry_point.h" @@ -75,7 +75,11 @@ Output Glsl::Run(const Program* in, const DataMap& inputs) { manager.Add(); manager.Add(); manager.Add(); - manager.Add(); + + data.Add( + /* type_ctor_to_let */ true, /* dynamic_index_to_var */ false); + manager.Add(); + manager.Add(); manager.Add(); manager.Add(); diff --git a/src/transform/promote_initializers_to_const_var.cc b/src/transform/promote_initializers_to_const_var.cc deleted file mode 100644 index e6729473db..0000000000 --- a/src/transform/promote_initializers_to_const_var.cc +++ /dev/null @@ -1,203 +0,0 @@ -// Copyright 2021 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/transform/promote_initializers_to_const_var.h" - -#include -#include - -#include "src/program_builder.h" -#include "src/sem/block_statement.h" -#include "src/sem/call.h" -#include "src/sem/expression.h" -#include "src/sem/for_loop_statement.h" -#include "src/sem/statement.h" -#include "src/sem/type_constructor.h" - -TINT_INSTANTIATE_TYPEINFO(tint::transform::PromoteInitializersToConstVar); - -namespace tint { -namespace transform { - -namespace { - -/// Holds information about a for-loop that needs to be decomposed into a loop, -/// so that initializer declaration statements can be inserted before the -/// condition expression or continuing statement. -struct LoopInfo { - ast::StatementList cond_decls; - ast::StatementList cont_decls; -}; - -} // namespace - -PromoteInitializersToConstVar::PromoteInitializersToConstVar() = default; - -PromoteInitializersToConstVar::~PromoteInitializersToConstVar() = default; - -void PromoteInitializersToConstVar::Run(CloneContext& ctx, - const DataMap&, - DataMap&) { - auto& sem = ctx.src->Sem(); - // Scan the AST nodes for array and structure initializers which - // need to be promoted to their own constant declaration. - - // Note: Correct handling of nested expressions is guaranteed due to the - // depth-first traversal of the ast::Node::Clone() methods: - // - // The inner-most initializers are traversed first, and they are hoisted - // to const variables declared just above the statement of use. The outer - // initializer will then be hoisted, inserting themselves between the - // inner declaration and the statement of use. This pattern applies correctly - // to any nested depth. - // - // Depth-first traversal of the AST is guaranteed because AST nodes are fully - // immutable and require their children to be constructed first so their - // pointer can be passed to the parent's constructor. - - // For-loops that need to be decomposed to loops. - std::unordered_map loops; - - for (auto* node : ctx.src->ASTNodes().Objects()) { - if (auto* expr = node->As()) { - auto* ctor = ctx.src->Sem().Get(expr); - if (!ctor->Target()->Is()) { - continue; - } - auto* sem_stmt = ctor->Stmt(); - if (!sem_stmt) { - // Expression is outside of a statement. This usually means the - // expression is part of a global (module-scope) constant declaration. - // These must be constexpr, and so cannot contain the type of - // expressions that must be sanitized. - continue; - } - auto* stmt = sem_stmt->Declaration(); - - if (auto* src_var_decl = stmt->As()) { - if (src_var_decl->variable->constructor == expr) { - // This statement is just a variable declaration with the initializer - // as the constructor value. This is what we're attempting to - // transform to, and so ignore. - continue; - } - } - - auto* src_ty = ctor->Type(); - if (src_ty->IsAnyOf()) { - // Create a new symbol for the let - auto name = ctx.dst->Sym(); - // Construct the let that holds the hoisted initializer - auto* let = ctx.dst->Const(name, nullptr, ctx.Clone(expr)); - // Construct the let declaration statement - auto* let_decl = ctx.dst->Decl(let); - // Replace the initializer expression with a reference to the let - ctx.Replace(expr, ctx.dst->Expr(name)); - - if (auto* fl = sem_stmt->As()) { - // Expression used in for-loop condition. - // For-loop needs to be decomposed to a loop. - loops[fl].cond_decls.emplace_back(let_decl); - continue; - } - - auto* parent = sem_stmt->Parent(); // The statement's parent - if (auto* block = parent->As()) { - // Expression's statement sits in a block. Simple case. - // Insert the let before the parent statement - ctx.InsertBefore(block->Declaration()->statements, stmt, let_decl); - continue; - } - if (auto* fl = parent->As()) { - // Expression is used in a for-loop. These require special care. - if (fl->Declaration()->initializer == stmt) { - // Expression used in for-loop initializer. - // Insert the let above the for-loop. - ctx.InsertBefore(fl->Block()->Declaration()->statements, - fl->Declaration(), let_decl); - continue; - } - if (fl->Declaration()->continuing == stmt) { - // Expression used in for-loop continuing. - // For-loop needs to be decomposed to a loop. - loops[fl].cont_decls.emplace_back(let_decl); - continue; - } - TINT_ICE(Transform, ctx.dst->Diagnostics()) - << "unhandled use of expression in for-loop"; - } - - TINT_ICE(Transform, ctx.dst->Diagnostics()) - << "unhandled expression parent statement type: " - << parent->TypeInfo().name; - } - } - } - - if (!loops.empty()) { - // At least one for-loop needs to be transformed into a loop. - ctx.ReplaceAll( - [&](const ast::ForLoopStatement* stmt) -> const ast::Statement* { - if (auto* fl = sem.Get(stmt)) { - if (auto it = loops.find(fl); it != loops.end()) { - auto& info = it->second; - auto* for_loop = fl->Declaration(); - // For-loop needs to be decomposed to a loop. - // Build the loop body's statements. - // Start with any let declarations for the conditional expression. - auto body_stmts = info.cond_decls; - // If the for-loop has a condition, emit this next as: - // if (!cond) { break; } - if (auto* cond = for_loop->condition) { - // !condition - auto* not_cond = ctx.dst->create( - ast::UnaryOp::kNot, ctx.Clone(cond)); - // { break; } - auto* break_body = - ctx.dst->Block(ctx.dst->create()); - // if (!condition) { break; } - body_stmts.emplace_back(ctx.dst->If(not_cond, break_body)); - } - // Next emit the for-loop body - for (auto* body_stmt : for_loop->body->statements) { - body_stmts.emplace_back(ctx.Clone(body_stmt)); - } - - // Finally create the continuing block if there was one. - const ast::BlockStatement* continuing = nullptr; - if (auto* cont = for_loop->continuing) { - // Continuing block starts with any let declarations used by the - // continuing. - auto cont_stmts = info.cont_decls; - cont_stmts.emplace_back(ctx.Clone(cont)); - continuing = ctx.dst->Block(cont_stmts); - } - - auto* body = ctx.dst->Block(body_stmts); - auto* loop = ctx.dst->Loop(body, continuing); - if (auto* init = for_loop->initializer) { - return ctx.dst->Block(ctx.Clone(init), loop); - } - return loop; - } - } - return nullptr; - }); - } - - ctx.Clone(); -} - -} // namespace transform -} // namespace tint diff --git a/src/transform/promote_initializers_to_const_var_test.cc b/src/transform/promote_initializers_to_const_var_test.cc deleted file mode 100644 index 8e2dc254c6..0000000000 --- a/src/transform/promote_initializers_to_const_var_test.cc +++ /dev/null @@ -1,391 +0,0 @@ -// Copyright 2021 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/transform/promote_initializers_to_const_var.h" - -#include "src/transform/test_helper.h" - -namespace tint { -namespace transform { -namespace { - -using PromoteInitializersToConstVarTest = TransformTest; - -TEST_F(PromoteInitializersToConstVarTest, BasicArray) { - auto* src = R"( -fn f() { - var f0 = 1.0; - var f1 = 2.0; - var f2 = 3.0; - var f3 = 4.0; - var i = array(f0, f1, f2, f3)[2]; -} -)"; - - auto* expect = R"( -fn f() { - var f0 = 1.0; - var f1 = 2.0; - var f2 = 3.0; - var f3 = 4.0; - let tint_symbol = array(f0, f1, f2, f3); - var i = tint_symbol[2]; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, BasicStruct) { - auto* src = R"( -struct S { - a : i32; - b : f32; - c : vec3; -}; - -fn f() { - var x = S(1, 2.0, vec3()).b; -} -)"; - - auto* expect = R"( -struct S { - a : i32; - b : f32; - c : vec3; -} - -fn f() { - let tint_symbol = S(1, 2.0, vec3()); - var x = tint_symbol.b; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, ArrayInForLoopInit) { - auto* src = R"( -fn f() { - var insert_after = 1; - for(var i = array(0.0, 1.0, 2.0, 3.0)[2]; ; ) { - break; - } -} -)"; - - auto* expect = R"( -fn f() { - var insert_after = 1; - let tint_symbol = array(0.0, 1.0, 2.0, 3.0); - for(var i = tint_symbol[2]; ; ) { - break; - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, StructInForLoopInit) { - auto* src = R"( -struct S { - a : i32; - b : f32; - c : vec3; -}; - -fn f() { - var insert_after = 1; - for(var x = S(1, 2.0, vec3()).b; ; ) { - break; - } -} -)"; - - auto* expect = R"( -struct S { - a : i32; - b : f32; - c : vec3; -} - -fn f() { - var insert_after = 1; - let tint_symbol = S(1, 2.0, vec3()); - for(var x = tint_symbol.b; ; ) { - break; - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, ArrayInForLoopCond) { - auto* src = R"( -fn f() { - var f = 1.0; - for(; f == array(f)[0]; f = f + 1.0) { - var marker = 1; - } -} -)"; - - auto* expect = R"( -fn f() { - var f = 1.0; - loop { - let tint_symbol = array(f); - if (!((f == tint_symbol[0]))) { - break; - } - var marker = 1; - - continuing { - f = (f + 1.0); - } - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, ArrayInForLoopCont) { - auto* src = R"( -fn f() { - var f = 0.0; - for(; f < 10.0; f = f + array(1.0)[0]) { - var marker = 1; - } -} -)"; - - auto* expect = R"( -fn f() { - var f = 0.0; - loop { - if (!((f < 10.0))) { - break; - } - var marker = 1; - - continuing { - let tint_symbol = array(1.0); - f = (f + tint_symbol[0]); - } - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, ArrayInForLoopInitCondCont) { - auto* src = R"( -fn f() { - for(var f = array(0.0)[0]; - f < array(1.0)[0]; - f = f + array(2.0)[0]) { - var marker = 1; - } -} -)"; - - auto* expect = R"( -fn f() { - let tint_symbol = array(0.0); - { - var f = tint_symbol[0]; - loop { - let tint_symbol_1 = array(1.0); - if (!((f < tint_symbol_1[0]))) { - break; - } - var marker = 1; - - continuing { - let tint_symbol_2 = array(2.0); - f = (f + tint_symbol_2[0]); - } - } - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, ArrayInArrayArray) { - auto* src = R"( -fn f() { - var i = array, 2u>(array(1.0, 2.0), array(3.0, 4.0))[0][1]; -} -)"; - - auto* expect = R"( -fn f() { - let tint_symbol = array(1.0, 2.0); - let tint_symbol_1 = array(3.0, 4.0); - let tint_symbol_2 = array, 2u>(tint_symbol, tint_symbol_1); - var i = tint_symbol_2[0][1]; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, StructNested) { - auto* src = R"( -struct S1 { - a : i32; -}; - -struct S2 { - a : i32; - b : S1; - c : i32; -}; - -struct S3 { - a : S2; -}; - -fn f() { - var x = S3(S2(1, S1(2), 3)).a.b.a; -} -)"; - - auto* expect = R"( -struct S1 { - a : i32; -} - -struct S2 { - a : i32; - b : S1; - c : i32; -} - -struct S3 { - a : S2; -} - -fn f() { - let tint_symbol = S1(2); - let tint_symbol_1 = S2(1, tint_symbol, 3); - let tint_symbol_2 = S3(tint_symbol_1); - var x = tint_symbol_2.a.b.a; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, Mixed) { - auto* src = R"( -struct S1 { - a : i32; -}; - -struct S2 { - a : array; -}; - -fn f() { - var x = S2(array(S1(1), S1(2), S1(3))).a[1].a; -} -)"; - - auto* expect = R"( -struct S1 { - a : i32; -} - -struct S2 { - a : array; -} - -fn f() { - let tint_symbol = S1(1); - let tint_symbol_1 = S1(2); - let tint_symbol_2 = S1(3); - let tint_symbol_3 = array(tint_symbol, tint_symbol_1, tint_symbol_2); - let tint_symbol_4 = S2(tint_symbol_3); - var x = tint_symbol_4.a[1].a; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, NoChangeOnVarDecl) { - auto* src = R"( -struct S { - a : i32; - b : f32; - c : i32; -} - -fn f() { - var local_arr = array(0.0, 1.0, 2.0, 3.0); - var local_str = S(1, 2.0, 3); -} - -let module_arr : array = array(0.0, 1.0, 2.0, 3.0); - -let module_str : S = S(1, 2.0, 3); -)"; - - auto* expect = src; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(PromoteInitializersToConstVarTest, EmptyModule) { - auto* src = ""; - auto* expect = ""; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -} // namespace -} // namespace transform -} // namespace tint diff --git a/src/transform/promote_side_effects_to_decl.cc b/src/transform/promote_side_effects_to_decl.cc new file mode 100644 index 0000000000..037d35b5a6 --- /dev/null +++ b/src/transform/promote_side_effects_to_decl.cc @@ -0,0 +1,320 @@ +// Copyright 2021 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/transform/promote_side_effects_to_decl.h" + +#include +#include +#include + +#include "src/program_builder.h" +#include "src/sem/block_statement.h" +#include "src/sem/call.h" +#include "src/sem/expression.h" +#include "src/sem/for_loop_statement.h" +#include "src/sem/statement.h" +#include "src/sem/type_constructor.h" + +TINT_INSTANTIATE_TYPEINFO(tint::transform::PromoteSideEffectsToDecl); +TINT_INSTANTIATE_TYPEINFO(tint::transform::PromoteSideEffectsToDecl::Config); + +namespace tint { +namespace transform { + +/// Private implementation of PromoteSideEffectsToDecl transform +class PromoteSideEffectsToDecl::State { + private: + CloneContext& ctx; + const Config& cfg; + ProgramBuilder& b; + + /// Holds information about a for-loop that needs to be decomposed into a + /// loop, so that declaration statements can be inserted before the condition + /// expression or continuing statement. + struct LoopInfo { + ast::StatementList cond_decls; + ast::StatementList cont_decls; + }; + + // For-loops that need to be decomposed to loops. + std::unordered_map loops; + + // Inserts `decl` before `sem_expr`, possibly marking a for-loop to be + // converted to a loop. + bool InsertBefore(const sem::Expression* sem_expr, + const ast::VariableDeclStatement* decl) { + auto* sem_stmt = sem_expr->Stmt(); + auto* stmt = sem_stmt->Declaration(); + + if (auto* fl = sem_stmt->As()) { + // Expression used in for-loop condition. + // For-loop needs to be decomposed to a loop. + loops[fl].cond_decls.emplace_back(decl); + return true; + } + + auto* parent = sem_stmt->Parent(); // The statement's parent + if (auto* block = parent->As()) { + // Expression's statement sits in a block. Simple case. + // Insert the decl before the parent statement + ctx.InsertBefore(block->Declaration()->statements, stmt, decl); + return true; + } + + if (auto* fl = parent->As()) { + // Expression is used in a for-loop. These require special care. + if (fl->Declaration()->initializer == stmt) { + // Expression used in for-loop initializer. + // Insert the let above the for-loop. + ctx.InsertBefore(fl->Block()->Declaration()->statements, + fl->Declaration(), decl); + return true; + } + + if (fl->Declaration()->continuing == stmt) { + // Expression used in for-loop continuing. + // For-loop needs to be decomposed to a loop. + loops[fl].cont_decls.emplace_back(decl); + return true; + } + + TINT_ICE(Transform, b.Diagnostics()) + << "unhandled use of expression in for-loop"; + return false; + } + + TINT_ICE(Transform, b.Diagnostics()) + << "unhandled expression parent statement type: " + << parent->TypeInfo().name; + return false; + } + + // Hoists array and structure initializers to a constant variable, declared + // just before the statement of usage. + bool TypeConstructorToLet(const ast::CallExpression* expr) { + auto* ctor = ctx.src->Sem().Get(expr); + if (!ctor->Target()->Is()) { + return true; + } + auto* sem_stmt = ctor->Stmt(); + if (!sem_stmt) { + // Expression is outside of a statement. This usually means the + // expression is part of a global (module-scope) constant declaration. + // These must be constexpr, and so cannot contain the type of + // expressions that must be sanitized. + return true; + } + + auto* stmt = sem_stmt->Declaration(); + + if (auto* src_var_decl = stmt->As()) { + if (src_var_decl->variable->constructor == expr) { + // This statement is just a variable declaration with the + // initializer as the constructor value. This is what we're + // attempting to transform to, and so ignore. + return true; + } + } + + auto* src_ty = ctor->Type(); + if (!src_ty->IsAnyOf()) { + // We only care about array and struct initializers + return true; + } + + // Construct the let that holds the hoisted initializer + auto name = b.Sym(); + auto* let = b.Const(name, nullptr, ctx.Clone(expr)); + auto* let_decl = b.Decl(let); + + if (!InsertBefore(ctor, let_decl)) { + return false; + } + + // Replace the initializer expression with a reference to the let + ctx.Replace(expr, b.Expr(name)); + return true; + } + + // Extracts array and matrix values that are dynamically indexed to a + // temporary `var` local that is then indexed. + bool DynamicIndexToVar(const ast::IndexAccessorExpression* access_expr) { + auto* index_expr = access_expr->index; + auto* object_expr = access_expr->object; + auto& sem = ctx.src->Sem(); + + if (sem.Get(index_expr)->ConstantValue()) { + // Index expression resolves to a compile time value. + // As this isn't a dynamic index, we can ignore this. + return true; + } + + auto* indexed = sem.Get(object_expr); + if (!indexed->Type()->IsAnyOf()) { + // We only care about array and matrices. + return true; + } + + // Construct a `var` declaration to hold the value in memory. + // TODO(bclayton): group multiple accesses in the same object. + // e.g. arr[i] + arr[i+1] // Don't create two vars for this + auto var_name = b.Symbols().New("var_for_index"); + auto* var_decl = b.Decl(b.Var(var_name, nullptr, ctx.Clone(object_expr))); + + if (!InsertBefore(indexed, var_decl)) { + return false; + } + + // Replace the original index expression with the new `var`. + ctx.Replace(object_expr, b.Expr(var_name)); + return true; + } + + // Converts any for-loops marked for conversion to loops, inserting + // registered declaration statements before the condition or continuing + // statement. + void ForLoopsToLoops() { + if (loops.empty()) { + return; + } + + // At least one for-loop needs to be transformed into a loop. + ctx.ReplaceAll( + [&](const ast::ForLoopStatement* stmt) -> const ast::Statement* { + auto& sem = ctx.src->Sem(); + + if (auto* fl = sem.Get(stmt)) { + if (auto it = loops.find(fl); it != loops.end()) { + auto& info = it->second; + auto* for_loop = fl->Declaration(); + // For-loop needs to be decomposed to a loop. + // Build the loop body's statements. + // Start with any let declarations for the conditional + // expression. + auto body_stmts = info.cond_decls; + // If the for-loop has a condition, emit this next as: + // if (!cond) { break; } + if (auto* cond = for_loop->condition) { + // !condition + auto* not_cond = b.create( + ast::UnaryOp::kNot, ctx.Clone(cond)); + // { break; } + auto* break_body = b.Block(b.create()); + // if (!condition) { break; } + body_stmts.emplace_back(b.If(not_cond, break_body)); + } + // Next emit the for-loop body + for (auto* body_stmt : for_loop->body->statements) { + body_stmts.emplace_back(ctx.Clone(body_stmt)); + } + + // Finally create the continuing block if there was one. + const ast::BlockStatement* continuing = nullptr; + if (auto* cont = for_loop->continuing) { + // Continuing block starts with any let declarations used by + // the continuing. + auto cont_stmts = info.cont_decls; + cont_stmts.emplace_back(ctx.Clone(cont)); + continuing = b.Block(cont_stmts); + } + + auto* body = b.Block(body_stmts); + auto* loop = b.Loop(body, continuing); + if (auto* init = for_loop->initializer) { + return b.Block(ctx.Clone(init), loop); + } + return loop; + } + } + return nullptr; + }); + } + + public: + /// Constructor + /// @param ctx_in the CloneContext primed with the input program and + /// @param cfg_in the transform config + /// ProgramBuilder + explicit State(CloneContext& ctx_in, const Config& cfg_in) + : ctx(ctx_in), cfg(cfg_in), b(*ctx_in.dst) {} + + /// Runs the transform + void Run() { + // Scan the AST nodes for expressions that need to be promoted to their own + // constant or variable declaration. + + // Note: Correct handling of nested expressions is guaranteed due to the + // depth-first traversal of the ast::Node::Clone() methods: + // + // The inner-most expressions are traversed first, and they are hoisted + // to variables declared just above the statement of use. The outer + // expression will then be hoisted, inserting themselves between the + // inner declaration and the statement of use. This pattern applies + // correctly to any nested depth. + // + // Depth-first traversal of the AST is guaranteed because AST nodes are + // fully immutable and require their children to be constructed first so + // their pointer can be passed to the parent's constructor. + + for (auto* node : ctx.src->ASTNodes().Objects()) { + if (cfg.type_ctor_to_let) { + if (auto* call_expr = node->As()) { + if (!TypeConstructorToLet(call_expr)) { + return; + } + } + } + + if (cfg.dynamic_index_to_var) { + if (auto* access_expr = node->As()) { + if (!DynamicIndexToVar(access_expr)) { + return; + } + } + } + } + + ForLoopsToLoops(); + + ctx.Clone(); + } +}; + +PromoteSideEffectsToDecl::PromoteSideEffectsToDecl() = default; +PromoteSideEffectsToDecl::~PromoteSideEffectsToDecl() = default; + +void PromoteSideEffectsToDecl::Run(CloneContext& ctx, + const DataMap& inputs, + DataMap&) { + auto* cfg = inputs.Get(); + if (cfg == nullptr) { + ctx.dst->Diagnostics().add_error( + diag::System::Transform, + "missing transform data for " + std::string(TypeInfo().name)); + return; + } + + State state(ctx, *cfg); + state.Run(); +} + +PromoteSideEffectsToDecl::Config::Config(bool type_ctor_to_let_in, + bool dynamic_index_to_var_in) + : type_ctor_to_let(type_ctor_to_let_in), + dynamic_index_to_var(dynamic_index_to_var_in) {} + +PromoteSideEffectsToDecl::Config::~Config() = default; + +} // namespace transform +} // namespace tint diff --git a/src/transform/promote_initializers_to_const_var.h b/src/transform/promote_side_effects_to_decl.h similarity index 52% rename from src/transform/promote_initializers_to_const_var.h rename to src/transform/promote_side_effects_to_decl.h index b6150d7744..d2d36fdb5b 100644 --- a/src/transform/promote_initializers_to_const_var.h +++ b/src/transform/promote_side_effects_to_decl.h @@ -12,27 +12,46 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef SRC_TRANSFORM_PROMOTE_INITIALIZERS_TO_CONST_VAR_H_ -#define SRC_TRANSFORM_PROMOTE_INITIALIZERS_TO_CONST_VAR_H_ +#ifndef SRC_TRANSFORM_PROMOTE_SIDE_EFFECTS_TO_DECL_H_ +#define SRC_TRANSFORM_PROMOTE_SIDE_EFFECTS_TO_DECL_H_ #include "src/transform/transform.h" namespace tint { namespace transform { -/// A transform that hoists the array and structure initializers to a constant -/// variable, declared just before the statement of usage. This transform may -/// also decompose for-loops into loops so that let declarations can be emitted +/// A transform that hoists expressions with side-effects to a variable +/// declaration just before the statement of usage. This transform may also +/// decompose for-loops into loops so that let declarations can be emitted /// before loop condition expressions and/or continuing statements. /// @see crbug.com/tint/406 -class PromoteInitializersToConstVar - : public Castable { +class PromoteSideEffectsToDecl + : public Castable { public: /// Constructor - PromoteInitializersToConstVar(); + PromoteSideEffectsToDecl(); /// Destructor - ~PromoteInitializersToConstVar() override; + ~PromoteSideEffectsToDecl() override; + + /// Configuration options for the transform. + struct Config : public Castable { + /// Constructor + /// @param type_ctor_to_let whether to hoist type constructor expressions + /// to a let + /// @param dynamic_index_to_var whether to hoist dynamic indexed + /// expressions to a var + Config(bool type_ctor_to_let, bool dynamic_index_to_var); + + /// Destructor + ~Config() override; + + /// Whether to hoist type constructor expressions to a let + const bool type_ctor_to_let; + + /// Whether to hoist dynamic indexed expressions to a var + const bool dynamic_index_to_var; + }; protected: /// Runs the transform using the CloneContext built for transforming a @@ -42,9 +61,12 @@ class PromoteInitializersToConstVar /// @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) override; + + private: + class State; }; } // namespace transform } // namespace tint -#endif // SRC_TRANSFORM_PROMOTE_INITIALIZERS_TO_CONST_VAR_H_ +#endif // SRC_TRANSFORM_PROMOTE_SIDE_EFFECTS_TO_DECL_H_ diff --git a/src/transform/promote_side_effects_to_decl_test.cc b/src/transform/promote_side_effects_to_decl_test.cc new file mode 100644 index 0000000000..10f1e80f24 --- /dev/null +++ b/src/transform/promote_side_effects_to_decl_test.cc @@ -0,0 +1,756 @@ +// Copyright 2021 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/transform/promote_side_effects_to_decl.h" + +#include "src/transform/test_helper.h" + +namespace tint { +namespace transform { +namespace { + +using PromoteSideEffectsToDeclTest = TransformTest; + +TEST_F(PromoteSideEffectsToDeclTest, EmptyModule) { + auto* src = ""; + auto* expect = ""; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_BasicArray) { + auto* src = R"( +fn f() { + var f0 = 1.0; + var f1 = 2.0; + var f2 = 3.0; + var f3 = 4.0; + var i = array(f0, f1, f2, f3)[2]; +} +)"; + + auto* expect = R"( +fn f() { + var f0 = 1.0; + var f1 = 2.0; + var f2 = 3.0; + var f3 = 4.0; + let tint_symbol = array(f0, f1, f2, f3); + var i = tint_symbol[2]; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_BasicStruct) { + auto* src = R"( +struct S { + a : i32; + b : f32; + c : vec3; +}; + +fn f() { + var x = S(1, 2.0, vec3()).b; +} +)"; + + auto* expect = R"( +struct S { + a : i32; + b : f32; + c : vec3; +} + +fn f() { + let tint_symbol = S(1, 2.0, vec3()); + var x = tint_symbol.b; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_ArrayInForLoopInit) { + auto* src = R"( +fn f() { + var insert_after = 1; + for(var i = array(0.0, 1.0, 2.0, 3.0)[2]; ; ) { + break; + } +} +)"; + + auto* expect = R"( +fn f() { + var insert_after = 1; + let tint_symbol = array(0.0, 1.0, 2.0, 3.0); + for(var i = tint_symbol[2]; ; ) { + break; + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_StructInForLoopInit) { + auto* src = R"( +struct S { + a : i32; + b : f32; + c : vec3; +}; + +fn f() { + var insert_after = 1; + for(var x = S(1, 2.0, vec3()).b; ; ) { + break; + } +} +)"; + + auto* expect = R"( +struct S { + a : i32; + b : f32; + c : vec3; +} + +fn f() { + var insert_after = 1; + let tint_symbol = S(1, 2.0, vec3()); + for(var x = tint_symbol.b; ; ) { + break; + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_ArrayInForLoopCond) { + auto* src = R"( +fn f() { + var f = 1.0; + for(; f == array(f)[0]; f = f + 1.0) { + var marker = 1; + } +} +)"; + + auto* expect = R"( +fn f() { + var f = 1.0; + loop { + let tint_symbol = array(f); + if (!((f == tint_symbol[0]))) { + break; + } + var marker = 1; + + continuing { + f = (f + 1.0); + } + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_ArrayInForLoopCont) { + auto* src = R"( +fn f() { + var f = 0.0; + for(; f < 10.0; f = f + array(1.0)[0]) { + var marker = 1; + } +} +)"; + + auto* expect = R"( +fn f() { + var f = 0.0; + loop { + if (!((f < 10.0))) { + break; + } + var marker = 1; + + continuing { + let tint_symbol = array(1.0); + f = (f + tint_symbol[0]); + } + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_ArrayInForLoopInitCondCont) { + auto* src = R"( +fn f() { + for(var f = array(0.0)[0]; + f < array(1.0)[0]; + f = f + array(2.0)[0]) { + var marker = 1; + } +} +)"; + + auto* expect = R"( +fn f() { + let tint_symbol = array(0.0); + { + var f = tint_symbol[0]; + loop { + let tint_symbol_1 = array(1.0); + if (!((f < tint_symbol_1[0]))) { + break; + } + var marker = 1; + + continuing { + let tint_symbol_2 = array(2.0); + f = (f + tint_symbol_2[0]); + } + } + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_ArrayInArrayArray) { + auto* src = R"( +fn f() { + var i = array, 2u>(array(1.0, 2.0), array(3.0, 4.0))[0][1]; +} +)"; + + auto* expect = R"( +fn f() { + let tint_symbol = array(1.0, 2.0); + let tint_symbol_1 = array(3.0, 4.0); + let tint_symbol_2 = array, 2u>(tint_symbol, tint_symbol_1); + var i = tint_symbol_2[0][1]; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_StructNested) { + auto* src = R"( +struct S1 { + a : i32; +}; + +struct S2 { + a : i32; + b : S1; + c : i32; +}; + +struct S3 { + a : S2; +}; + +fn f() { + var x = S3(S2(1, S1(2), 3)).a.b.a; +} +)"; + + auto* expect = R"( +struct S1 { + a : i32; +} + +struct S2 { + a : i32; + b : S1; + c : i32; +} + +struct S3 { + a : S2; +} + +fn f() { + let tint_symbol = S1(2); + let tint_symbol_1 = S2(1, tint_symbol, 3); + let tint_symbol_2 = S3(tint_symbol_1); + var x = tint_symbol_2.a.b.a; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_Mixed) { + auto* src = R"( +struct S1 { + a : i32; +}; + +struct S2 { + a : array; +}; + +fn f() { + var x = S2(array(S1(1), S1(2), S1(3))).a[1].a; +} +)"; + + auto* expect = R"( +struct S1 { + a : i32; +} + +struct S2 { + a : array; +} + +fn f() { + let tint_symbol = S1(1); + let tint_symbol_1 = S1(2); + let tint_symbol_2 = S1(3); + let tint_symbol_3 = array(tint_symbol, tint_symbol_1, tint_symbol_2); + let tint_symbol_4 = S2(tint_symbol_3); + var x = tint_symbol_4.a[1].a; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, TypeCtorToLet_NoChangeOnVarDecl) { + auto* src = R"( +struct S { + a : i32; + b : f32; + c : i32; +} + +fn f() { + var local_arr = array(0.0, 1.0, 2.0, 3.0); + var local_str = S(1, 2.0, 3); +} + +let module_arr : array = array(0.0, 1.0, 2.0, 3.0); + +let module_str : S = S(1, 2.0, 3); +)"; + + auto* expect = src; + + DataMap data; + data.Add(/* type_ctor_to_let */ true, + /* dynamic_index_to_var */ false); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_ArrayIndexDynamic) { + auto* src = R"( +fn f() { + var i : i32; + let p = array(1, 2, 3, 4); + let x = p[i]; +} +)"; + + auto* expect = R"( +fn f() { + var i : i32; + let p = array(1, 2, 3, 4); + var var_for_index = p; + let x = var_for_index[i]; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_MatrixIndexDynamic) { + auto* src = R"( +fn f() { + var i : i32; + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + let x = p[i]; +} +)"; + + auto* expect = R"( +fn f() { + var i : i32; + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + var var_for_index = p; + let x = var_for_index[i]; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_ArrayIndexDynamicChain) { + auto* src = R"( +fn f() { + var i : i32; + var j : i32; + let p = array, 2>(array(1, 2), array(3, 4)); + let x = p[i][j]; +} +)"; + + // TODO(bclayton): Optimize this case: + // This output is not as efficient as it could be. + // We only actually need to hoist the inner-most array to a `var` + // (`var_for_index`), as later indexing operations will be working with + // references, not values. + auto* expect = R"( +fn f() { + var i : i32; + var j : i32; + let p = array, 2>(array(1, 2), array(3, 4)); + var var_for_index = p; + var var_for_index_1 = var_for_index[i]; + let x = var_for_index_1[j]; +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, + DynamicIndexToVar_ArrayIndexInForLoopInit) { + auto* src = R"( +fn f() { + var i : i32; + let p = array, 2>(array(1, 2), array(3, 4)); + for(let x = p[i]; ; ) { + break; + } +} +)"; + + auto* expect = R"( +fn f() { + var i : i32; + let p = array, 2>(array(1, 2), array(3, 4)); + var var_for_index = p; + for(let x = var_for_index[i]; ; ) { + break; + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, + DynamicIndexToVar_MatrixIndexInForLoopInit) { + auto* src = R"( +fn f() { + var i : i32; + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + for(let x = p[i]; ; ) { + break; + } +} +)"; + + auto* expect = R"( +fn f() { + var i : i32; + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + var var_for_index = p; + for(let x = var_for_index[i]; ; ) { + break; + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, + DynamicIndexToVar_ArrayIndexInForLoopCond) { + auto* src = R"( +fn f() { + var i : i32; + let p = array(1, 2); + for(; p[i] < 3; ) { + break; + } +} +)"; + + auto* expect = R"( +fn f() { + var i : i32; + let p = array(1, 2); + loop { + var var_for_index = p; + if (!((var_for_index[i] < 3))) { + break; + } + break; + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, + DynamicIndexToVar_MatrixIndexInForLoopCond) { + auto* src = R"( +fn f() { + var i : i32; + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + for(; p[i].x < 3.0; ) { + break; + } +} +)"; + + auto* expect = R"( +fn f() { + var i : i32; + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + loop { + var var_for_index = p; + if (!((var_for_index[i].x < 3.0))) { + break; + } + break; + } +} +)"; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_ArrayIndexLiteral) { + auto* src = R"( +fn f() { + let p = array(1, 2, 3, 4); + let x = p[1]; +} +)"; + + auto* expect = src; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_MatrixIndexLiteral) { + auto* src = R"( +fn f() { + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + let x = p[1]; +} +)"; + + auto* expect = src; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_ArrayIndexConstantLet) { + auto* src = R"( +fn f() { + let p = array(1, 2, 3, 4); + let c = 1; + let x = p[c]; +} +)"; + + auto* expect = src; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_MatrixIndexConstantLet) { + auto* src = R"( +fn f() { + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + let c = 1; + let x = p[c]; +} +)"; + + auto* expect = src; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, DynamicIndexToVar_ArrayIndexLiteralChain) { + auto* src = R"( +fn f() { + let a = array(1, 2); + let b = array(3, 4); + let p = array, 2>(a, b); + let x = p[0][1]; +} +)"; + + auto* expect = src; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(PromoteSideEffectsToDeclTest, + DynamicIndexToVar_MatrixIndexLiteralChain) { + auto* src = R"( +fn f() { + let p = mat2x2(1.0, 2.0, 3.0, 4.0); + let x = p[0][1]; +} +)"; + + auto* expect = src; + + DataMap data; + data.Add(/* type_ctor_to_let */ false, + /* dynamic_index_to_var */ true); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + +} // namespace +} // namespace transform +} // namespace tint diff --git a/src/transform/var_for_dynamic_index.cc b/src/transform/var_for_dynamic_index.cc deleted file mode 100644 index 192701fe3c..0000000000 --- a/src/transform/var_for_dynamic_index.cc +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2021 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/transform/var_for_dynamic_index.h" - -#include - -#include "src/program_builder.h" -#include "src/sem/array.h" -#include "src/sem/block_statement.h" -#include "src/sem/expression.h" -#include "src/sem/statement.h" -#include "src/transform/for_loop_to_loop.h" - -namespace tint { -namespace transform { - -VarForDynamicIndex::VarForDynamicIndex() = default; - -VarForDynamicIndex::~VarForDynamicIndex() = default; - -void VarForDynamicIndex::Run(CloneContext& ctx, const DataMap&, DataMap&) { - ProgramBuilder out; - if (!Requires(ctx)) { - return; - } - - auto& sem = ctx.src->Sem(); - - for (auto* node : ctx.src->ASTNodes().Objects()) { - if (auto* access_expr = node->As()) { - // Found an array accessor expression - auto* index_expr = access_expr->index; - auto* object_expr = access_expr->object; - - if (sem.Get(index_expr)->ConstantValue()) { - // Index expression resolves to a compile time value. - // As this isn't a dynamic index, we can ignore this. - continue; - } - - auto* indexed = sem.Get(object_expr); - if (!indexed->Type()->IsAnyOf()) { - // This transform currently only cares about array and matrices. - continue; - } - - // Construct a `var` declaration to hold the value in memory. - // TODO(bclayton): group multiple accesses in the same object. - // e.g. arr[i] + arr[i+1] // Don't create two vars for this - auto var_name = ctx.dst->Symbols().New("var_for_index"); - auto* var_decl = ctx.dst->Decl( - ctx.dst->Var(var_name, nullptr, ctx.Clone(object_expr))); - - // Statement that owns the expression - auto* stmt = indexed->Stmt(); - // Block that owns the statement - auto* block = stmt->Parent()->As(); - if (!block) { - TINT_ICE(Transform, ctx.dst->Diagnostics()) - << "statement parent was not a block"; - continue; - } - - // Insert the `var` declaration before the statement that performs the - // indexing. Note that for indexing chains, AST node ordering guarantees - // that the inner-most index variable will be placed first in the block. - ctx.InsertBefore(block->Declaration()->statements, stmt->Declaration(), - var_decl); - - // Replace the original index expression with the new `var`. - ctx.Replace(object_expr, ctx.dst->Expr(var_name)); - } - } - - ctx.Clone(); -} - -} // namespace transform -} // namespace tint diff --git a/src/transform/var_for_dynamic_index.h b/src/transform/var_for_dynamic_index.h deleted file mode 100644 index ece57511d6..0000000000 --- a/src/transform/var_for_dynamic_index.h +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2021 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_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_ -#define SRC_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_ - -#include "src/transform/transform.h" - -namespace tint { -namespace transform { - -/// A transform that extracts array and matrix values that are dynamically -/// indexed to a temporary `var` local before performing the index. This -/// transform is used by the SPIR-V writer as there is no SPIR-V instruction -/// that can dynamically index a non-pointer composite. -/// Requires the ForLoopToLoop transform to be run first. -class VarForDynamicIndex : public Transform { - public: - /// Constructor - VarForDynamicIndex(); - - /// Destructor - ~VarForDynamicIndex() override; - - protected: - /// 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) override; -}; - -} // namespace transform -} // namespace tint - -#endif // SRC_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_ diff --git a/src/transform/var_for_dynamic_index_test.cc b/src/transform/var_for_dynamic_index_test.cc deleted file mode 100644 index 0f386aeeeb..0000000000 --- a/src/transform/var_for_dynamic_index_test.cc +++ /dev/null @@ -1,327 +0,0 @@ -// Copyright 2021 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/transform/var_for_dynamic_index.h" -#include "src/transform/for_loop_to_loop.h" - -#include "src/transform/test_helper.h" - -namespace tint { -namespace transform { -namespace { - -using VarForDynamicIndexTest = TransformTest; - -TEST_F(VarForDynamicIndexTest, EmptyModule) { - auto* src = ""; - auto* expect = ""; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, ArrayIndexDynamic) { - auto* src = R"( -fn f() { - var i : i32; - let p = array(1, 2, 3, 4); - let x = p[i]; -} -)"; - - auto* expect = R"( -fn f() { - var i : i32; - let p = array(1, 2, 3, 4); - var var_for_index = p; - let x = var_for_index[i]; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, MatrixIndexDynamic) { - auto* src = R"( -fn f() { - var i : i32; - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - let x = p[i]; -} -)"; - - auto* expect = R"( -fn f() { - var i : i32; - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - var var_for_index = p; - let x = var_for_index[i]; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, ArrayIndexDynamicChain) { - auto* src = R"( -fn f() { - var i : i32; - var j : i32; - let p = array, 2>(array(1, 2), array(3, 4)); - let x = p[i][j]; -} -)"; - - // TODO(bclayton): Optimize this case: - // This output is not as efficient as it could be. - // We only actually need to hoist the inner-most array to a `var` - // (`var_for_index`), as later indexing operations will be working with - // references, not values. - - auto* expect = R"( -fn f() { - var i : i32; - var j : i32; - let p = array, 2>(array(1, 2), array(3, 4)); - var var_for_index = p; - var var_for_index_1 = var_for_index[i]; - let x = var_for_index_1[j]; -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, ArrayIndexInForLoopInit) { - auto* src = R"( -fn f() { - var i : i32; - let p = array, 2>(array(1, 2), array(3, 4)); - for(let x = p[i]; ; ) { - break; - } -} -)"; - - auto* expect = R"( -fn f() { - var i : i32; - let p = array, 2>(array(1, 2), array(3, 4)); - { - var var_for_index = p; - let x = var_for_index[i]; - loop { - break; - } - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, MatrixIndexInForLoopInit) { - auto* src = R"( -fn f() { - var i : i32; - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - for(let x = p[i]; ; ) { - break; - } -} -)"; - - auto* expect = R"( -fn f() { - var i : i32; - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - { - var var_for_index = p; - let x = var_for_index[i]; - loop { - break; - } - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, ArrayIndexInForLoopCond) { - auto* src = R"( -fn f() { - var i : i32; - let p = array(1, 2); - for(; p[i] < 3; ) { - break; - } -} -)"; - - auto* expect = R"( -fn f() { - var i : i32; - let p = array(1, 2); - loop { - var var_for_index = p; - if (!((var_for_index[i] < 3))) { - break; - } - break; - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, MatrixIndexInForLoopCond) { - auto* src = R"( -fn f() { - var i : i32; - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - for(; p[i].x < 3.0; ) { - break; - } -} -)"; - - auto* expect = R"( -fn f() { - var i : i32; - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - loop { - var var_for_index = p; - if (!((var_for_index[i].x < 3.0))) { - break; - } - break; - } -} -)"; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, ArrayIndexLiteral) { - auto* src = R"( -fn f() { - let p = array(1, 2, 3, 4); - let x = p[1]; -} -)"; - - auto* expect = src; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, MatrixIndexLiteral) { - auto* src = R"( -fn f() { - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - let x = p[1]; -} -)"; - - auto* expect = src; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, ArrayIndexConstantLet) { - auto* src = R"( -fn f() { - let p = array(1, 2, 3, 4); - let c = 1; - let x = p[c]; -} -)"; - - auto* expect = src; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, MatrixIndexConstantLet) { - auto* src = R"( -fn f() { - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - let c = 1; - let x = p[c]; -} -)"; - - auto* expect = src; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, ArrayIndexLiteralChain) { - auto* src = R"( -fn f() { - let p = array, 2>(array(1, 2), array(3, 4)); - let x = p[0][1]; -} -)"; - - auto* expect = src; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -TEST_F(VarForDynamicIndexTest, MatrixIndexLiteralChain) { - auto* src = R"( -fn f() { - let p = mat2x2(1.0, 2.0, 3.0, 4.0); - let x = p[0][1]; -} -)"; - - auto* expect = src; - - auto got = Run(src); - - EXPECT_EQ(expect, str(got)); -} - -} // namespace -} // namespace transform -} // namespace tint diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index bae3044f39..6b15ba6bca 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -57,7 +57,7 @@ #include "src/transform/manager.h" #include "src/transform/num_workgroups_from_uniform.h" #include "src/transform/pad_array_elements.h" -#include "src/transform/promote_initializers_to_const_var.h" +#include "src/transform/promote_side_effects_to_decl.h" #include "src/transform/remove_phonies.h" #include "src/transform/simplify_pointers.h" #include "src/transform/unshadow.h" @@ -191,7 +191,11 @@ SanitizedResult Sanitize( // will be transformed by CalculateArrayLength manager.Add(); manager.Add(); - manager.Add(); + + data.Add( + /* type_ctor_to_let */ true, /* dynamic_index_to_var */ false); + manager.Add(); + manager.Add(); manager.Add(); diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index d6ca1e4991..e25aa85866 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -63,7 +63,7 @@ #include "src/transform/manager.h" #include "src/transform/module_scope_var_to_entry_point_param.h" #include "src/transform/pad_array_elements.h" -#include "src/transform/promote_initializers_to_const_var.h" +#include "src/transform/promote_side_effects_to_decl.h" #include "src/transform/remove_phonies.h" #include "src/transform/simplify_pointers.h" #include "src/transform/unshadow.h" @@ -124,7 +124,7 @@ SanitizedResult Sanitize( bool disable_workgroup_init, const ArrayLengthFromUniformOptions& array_length_from_uniform) { transform::Manager manager; - transform::DataMap internal_inputs; + transform::DataMap data; // Build the config for the internal ArrayLengthFromUniform transform. transform::ArrayLengthFromUniform::Config array_length_from_uniform_cfg( @@ -162,7 +162,11 @@ SanitizedResult Sanitize( } manager.Add(); manager.Add(); - manager.Add(); + + data.Add( + /* type_ctor_to_let */ true, /* dynamic_index_to_var */ false); + manager.Add(); + manager.Add(); manager.Add(); manager.Add(); @@ -172,11 +176,11 @@ SanitizedResult Sanitize( // it assumes that the form of the array length argument is &var.array. manager.Add(); manager.Add(); - internal_inputs.Add( + data.Add( std::move(array_length_from_uniform_cfg)); - internal_inputs.Add( + data.Add( std::move(entry_point_io_cfg)); - auto out = manager.Run(in, internal_inputs); + auto out = manager.Run(in, data); SanitizedResult result; result.program = std::move(out.program); diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index f4b93acced..e633bb6b7a 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -47,10 +47,10 @@ #include "src/transform/fold_constants.h" #include "src/transform/for_loop_to_loop.h" #include "src/transform/manager.h" +#include "src/transform/promote_side_effects_to_decl.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" #include "src/transform/vectorize_scalar_matrix_constructors.h" #include "src/transform/zero_init_workgroup_memory.h" #include "src/utils/defer.h" @@ -271,7 +271,10 @@ SanitizedResult Sanitize(const Program* in, manager.Add(); manager.Add(); manager.Add(); - manager.Add(); + + data.Add( + /* type_ctor_to_let */ false, /* dynamic_index_to_var */ true); + manager.Add(); data.Add( transform::CanonicalizeEntryPointIO::Config( diff --git a/test/BUILD.gn b/test/BUILD.gn index 223ed34275..88d6554f43 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -320,7 +320,7 @@ tint_unittests_source_set("tint_unittests_transform_src") { "../src/transform/multiplanar_external_texture_test.cc", "../src/transform/num_workgroups_from_uniform_test.cc", "../src/transform/pad_array_elements_test.cc", - "../src/transform/promote_initializers_to_const_var_test.cc", + "../src/transform/promote_side_effects_to_decl_test.cc", "../src/transform/remove_phonies_test.cc", "../src/transform/remove_unreachable_statements_test.cc", "../src/transform/renamer_test.cc", @@ -330,7 +330,6 @@ tint_unittests_source_set("tint_unittests_transform_src") { "../src/transform/test_helper.h", "../src/transform/transform_test.cc", "../src/transform/unshadow_test.cc", - "../src/transform/var_for_dynamic_index_test.cc", "../src/transform/vectorize_scalar_matrix_constructors_test.cc", "../src/transform/vertex_pulling_test.cc", "../src/transform/wrap_arrays_in_structs_test.cc",