Fix PromoteSideEffectsToDeclTest converting for loop to loop with nested hoisted lets
When converting a for-loop to a loop, we were not cloning the for-loop's
body, but rather the statements within it. This worked fine, except if
we also hoisted a variable to a let within that body, which requires the
body to be cloned for the 'insert before' to work. This change clones
the for-loop body, which fixes the problem, but introduces a block in
the destination AST, which is ugly, but not incorrect.
Bug: tint:1300
Change-Id: I478244d87f8cf58837102004242ba1c835e21710
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/78821
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
diff --git a/src/transform/promote_side_effects_to_decl.cc b/src/transform/promote_side_effects_to_decl.cc
index 6274187..520e845 100644
--- a/src/transform/promote_side_effects_to_decl.cc
+++ b/src/transform/promote_side_effects_to_decl.cc
@@ -242,9 +242,7 @@
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));
- }
+ body_stmts.emplace_back(ctx.Clone(for_loop->body));
// Finally create the continuing block if there was one.
const ast::BlockStatement* continuing = nullptr;
diff --git a/src/transform/promote_side_effects_to_decl_test.cc b/src/transform/promote_side_effects_to_decl_test.cc
index 9c7a7fe..c520aa1 100644
--- a/src/transform/promote_side_effects_to_decl_test.cc
+++ b/src/transform/promote_side_effects_to_decl_test.cc
@@ -184,7 +184,9 @@
if (!((f == tint_symbol[0]))) {
break;
}
- var marker = 1;
+ {
+ var marker = 1;
+ }
continuing {
f = (f + 1.0);
@@ -218,7 +220,9 @@
if (!((f < 10.0))) {
break;
}
- var marker = 1;
+ {
+ var marker = 1;
+ }
continuing {
let tint_symbol = array<f32, 1u>(1.0);
@@ -257,7 +261,9 @@
if (!((f < tint_symbol_1[0]))) {
break;
}
- var marker = 1;
+ {
+ var marker = 1;
+ }
continuing {
let tint_symbol_2 = array<f32, 1u>(2.0);
@@ -678,7 +684,9 @@
if (!((var_for_index[i] < 3))) {
break;
}
- break;
+ {
+ break;
+ }
}
}
)";
@@ -712,11 +720,56 @@
if (!((var_for_index[i].x < 3.0))) {
break;
}
+ {
+ break;
+ }
+ }
+}
+)";
+
+ DataMap data;
+ data.Add<PromoteSideEffectsToDecl::Config>(/* type_ctor_to_let */ false,
+ /* dynamic_index_to_var */ true);
+ auto got = Run<PromoteSideEffectsToDecl>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PromoteSideEffectsToDeclTest,
+ DynamicIndexToVar_MatrixIndexInForLoopCondWithNestedIndex) {
+ 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; ) {
+ if (p[i].x < 1.0) {
+ var marker = 1;
+ }
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;
+ }
+ {
+ var var_for_index_1 = p;
+ if ((var_for_index_1[i].x < 1.0)) {
+ var marker = 1;
+ }
+ break;
+ }
+ }
+}
+)";
+
DataMap data;
data.Add<PromoteSideEffectsToDecl::Config>(/* type_ctor_to_let */ false,
/* dynamic_index_to_var */ true);
diff --git a/test/statements/for/condition/array_ctor.wgsl.expected.glsl b/test/statements/for/condition/array_ctor.wgsl.expected.glsl
index 42330bb..c02c4fb 100644
--- a/test/statements/for/condition/array_ctor.wgsl.expected.glsl
+++ b/test/statements/for/condition/array_ctor.wgsl.expected.glsl
@@ -12,6 +12,8 @@
if (!((i < tint_symbol[0]))) {
break;
}
+ {
+ }
}
}
diff --git a/test/statements/for/condition/array_ctor.wgsl.expected.hlsl b/test/statements/for/condition/array_ctor.wgsl.expected.hlsl
index 409e7c4..141cd27 100644
--- a/test/statements/for/condition/array_ctor.wgsl.expected.hlsl
+++ b/test/statements/for/condition/array_ctor.wgsl.expected.hlsl
@@ -10,5 +10,7 @@
if (!((i < tint_symbol[0]))) {
break;
}
+ {
+ }
}
}
diff --git a/test/statements/for/condition/array_ctor.wgsl.expected.msl b/test/statements/for/condition/array_ctor.wgsl.expected.msl
index 2ec1a68..5aefd59 100644
--- a/test/statements/for/condition/array_ctor.wgsl.expected.msl
+++ b/test/statements/for/condition/array_ctor.wgsl.expected.msl
@@ -12,6 +12,8 @@
if (!((i < tint_symbol.arr[0]))) {
break;
}
+ {
+ }
}
}
diff --git a/test/statements/for/condition/struct_ctor.wgsl.expected.glsl b/test/statements/for/condition/struct_ctor.wgsl.expected.glsl
index 853168f..ad8c2cb 100644
--- a/test/statements/for/condition/struct_ctor.wgsl.expected.glsl
+++ b/test/statements/for/condition/struct_ctor.wgsl.expected.glsl
@@ -16,6 +16,8 @@
if (!((i < tint_symbol.i))) {
break;
}
+ {
+ }
}
}
diff --git a/test/statements/for/condition/struct_ctor.wgsl.expected.hlsl b/test/statements/for/condition/struct_ctor.wgsl.expected.hlsl
index c9d9ff0..d80f79a 100644
--- a/test/statements/for/condition/struct_ctor.wgsl.expected.hlsl
+++ b/test/statements/for/condition/struct_ctor.wgsl.expected.hlsl
@@ -14,5 +14,7 @@
if (!((i < tint_symbol.i))) {
break;
}
+ {
+ }
}
}
diff --git a/test/statements/for/condition/struct_ctor.wgsl.expected.msl b/test/statements/for/condition/struct_ctor.wgsl.expected.msl
index a370f25..a89dfc2 100644
--- a/test/statements/for/condition/struct_ctor.wgsl.expected.msl
+++ b/test/statements/for/condition/struct_ctor.wgsl.expected.msl
@@ -12,6 +12,8 @@
if (!((i < tint_symbol.i))) {
break;
}
+ {
+ }
}
}
diff --git a/test/statements/for/continuing/array_ctor.wgsl.expected.glsl b/test/statements/for/continuing/array_ctor.wgsl.expected.glsl
index d40e419..76006fd 100644
--- a/test/statements/for/continuing/array_ctor.wgsl.expected.glsl
+++ b/test/statements/for/continuing/array_ctor.wgsl.expected.glsl
@@ -12,6 +12,8 @@
break;
}
{
+ }
+ {
int tint_symbol[1] = int[1](1);
i = (i + tint_symbol[0]);
}
diff --git a/test/statements/for/continuing/array_ctor.wgsl.expected.hlsl b/test/statements/for/continuing/array_ctor.wgsl.expected.hlsl
index e24b634..7348a72 100644
--- a/test/statements/for/continuing/array_ctor.wgsl.expected.hlsl
+++ b/test/statements/for/continuing/array_ctor.wgsl.expected.hlsl
@@ -10,6 +10,8 @@
break;
}
{
+ }
+ {
const int tint_symbol[1] = {1};
i = (i + tint_symbol[0]);
}
diff --git a/test/statements/for/continuing/array_ctor.wgsl.expected.msl b/test/statements/for/continuing/array_ctor.wgsl.expected.msl
index 5339dfa..ac6ffc2 100644
--- a/test/statements/for/continuing/array_ctor.wgsl.expected.msl
+++ b/test/statements/for/continuing/array_ctor.wgsl.expected.msl
@@ -12,6 +12,8 @@
break;
}
{
+ }
+ {
tint_array_wrapper const tint_symbol = {.arr={1}};
i = as_type<int>((as_type<uint>(i) + as_type<uint>(tint_symbol.arr[0])));
}
diff --git a/test/statements/for/continuing/struct_ctor.wgsl.expected.glsl b/test/statements/for/continuing/struct_ctor.wgsl.expected.glsl
index 014a37a..ca9a56c 100644
--- a/test/statements/for/continuing/struct_ctor.wgsl.expected.glsl
+++ b/test/statements/for/continuing/struct_ctor.wgsl.expected.glsl
@@ -17,6 +17,8 @@
break;
}
{
+ }
+ {
S tint_symbol = S(1);
i = (i + tint_symbol.i);
}
diff --git a/test/statements/for/continuing/struct_ctor.wgsl.expected.hlsl b/test/statements/for/continuing/struct_ctor.wgsl.expected.hlsl
index 671f26f..4dd0cf2 100644
--- a/test/statements/for/continuing/struct_ctor.wgsl.expected.hlsl
+++ b/test/statements/for/continuing/struct_ctor.wgsl.expected.hlsl
@@ -15,6 +15,8 @@
break;
}
{
+ }
+ {
const S tint_symbol = {1};
i = (i + tint_symbol.i);
}
diff --git a/test/statements/for/continuing/struct_ctor.wgsl.expected.msl b/test/statements/for/continuing/struct_ctor.wgsl.expected.msl
index a022488..08603f7 100644
--- a/test/statements/for/continuing/struct_ctor.wgsl.expected.msl
+++ b/test/statements/for/continuing/struct_ctor.wgsl.expected.msl
@@ -13,6 +13,8 @@
break;
}
{
+ }
+ {
S const tint_symbol = {.i=1};
i = as_type<int>((as_type<uint>(i) + as_type<uint>(tint_symbol.i)));
}
