[msl-writer][hlsl-writer] Pull loop variables out with continuing.
If there is a continuing block we pull the variables declared in the
loop up into the scope outside the loop. This allows those variables to
be used in the continuing block.
We pull out all variables instead of detecting ones which are only used
in continuing as that's easier and still correct.
Bug: tint:187, tint:186
Change-Id: I1de0e36111a236ff04a323cf9777bc79e67afa77
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/27620
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: David Neto <dneto@google.com>
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index ca3a445..a6f454d 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -1470,6 +1470,19 @@
make_indent(out);
out << "bool " << guard << " = true;" << std::endl;
+
+ // A continuing block may use variables declared in the method body. As a
+ // first pass, if we have a continuing, we pull all declarations outside
+ // the for loop into the continuing scope. Then, the variable declarations
+ // will be turned into assignments.
+ for (const auto& s : *(stmt->body())) {
+ if (!s->IsVariableDecl()) {
+ continue;
+ }
+ if (!EmitVariable(out, s->AsVariableDecl()->variable(), true)) {
+ return false;
+ }
+ }
}
make_indent(out);
@@ -1490,6 +1503,26 @@
}
for (const auto& s : *(stmt->body())) {
+ // If we have a continuing block we've already emitted the variable
+ // declaration before the loop, so treat it as an assignment.
+ if (s->IsVariableDecl() && stmt->has_continuing()) {
+ make_indent(out);
+
+ auto* var = s->AsVariableDecl()->variable();
+ out << var->name() << " = ";
+ if (var->constructor() != nullptr) {
+ if (!EmitExpression(out, var->constructor())) {
+ return false;
+ }
+ } else {
+ if (!EmitZeroValue(out, var->type())) {
+ return false;
+ }
+ }
+ out << ";" << std::endl;
+ continue;
+ }
+
if (!EmitStatement(out, s.get())) {
return false;
}
@@ -1831,7 +1864,7 @@
return EmitSwitch(out, stmt->AsSwitch());
}
if (stmt->IsVariableDecl()) {
- return EmitVariable(out, stmt->AsVariableDecl()->variable());
+ return EmitVariable(out, stmt->AsVariableDecl()->variable(), false);
}
error_ = "unknown statement type: " + stmt->str();
@@ -1982,7 +2015,9 @@
return true;
}
-bool GeneratorImpl::EmitVariable(std::ostream& out, ast::Variable* var) {
+bool GeneratorImpl::EmitVariable(std::ostream& out,
+ ast::Variable* var,
+ bool skip_constructor) {
make_indent(out);
// TODO(dsinclair): Handle variable decorations
@@ -2001,7 +2036,7 @@
out << " " << var->name();
}
- if (var->constructor() != nullptr) {
+ if (!skip_constructor && var->constructor() != nullptr) {
out << " = ";
if (!EmitExpression(out, var->constructor())) {
return false;
diff --git a/src/writer/hlsl/generator_impl.h b/src/writer/hlsl/generator_impl.h
index e087c77..ba3f60b 100644
--- a/src/writer/hlsl/generator_impl.h
+++ b/src/writer/hlsl/generator_impl.h
@@ -255,8 +255,11 @@
/// Handles generating a variable
/// @param out the output stream
/// @param var the variable to generate
+ /// @param skip_constructor set true if the constructor should be skipped
/// @returns true if the variable was emitted
- bool EmitVariable(std::ostream& out, ast::Variable* var);
+ bool EmitVariable(std::ostream& out,
+ ast::Variable* var,
+ bool skip_constructor);
/// Handles generating a program scope constant variable
/// @param out the output stream
/// @param var the variable to emit
diff --git a/src/writer/hlsl/generator_impl_loop_test.cc b/src/writer/hlsl/generator_impl_loop_test.cc
index 54d600e..576b7b2 100644
--- a/src/writer/hlsl/generator_impl_loop_test.cc
+++ b/src/writer/hlsl/generator_impl_loop_test.cc
@@ -122,9 +122,28 @@
)");
}
-// TODO(dsinclair): Handle pulling declared variables up and out of the for() if
-// there is a continuing block.
-TEST_F(HlslGeneratorImplTest_Loop, DISABLED_Emit_LoopWithVarUsedInContinuing) {
+TEST_F(HlslGeneratorImplTest_Loop, Emit_LoopWithVarUsedInContinuing) {
+ // loop {
+ // var lhs : f32 = 2.4;
+ // var other : f32;
+ // continuing {
+ // lhs = rhs
+ // }
+ // }
+ //
+ // ->
+ // {
+ // float lhs;
+ // float other;
+ // for (;;) {
+ // if (continuing) {
+ // lhs = rhs;
+ // }
+ // lhs = 2.4f;
+ // other = 0.0f;
+ // }
+ // }
+
ast::type::F32Type f32;
auto var = std::make_unique<ast::Variable>(
@@ -150,8 +169,9 @@
ASSERT_TRUE(gen().EmitStatement(out(), &outer)) << gen().error();
EXPECT_EQ(result(), R"( {
- float lhs;
bool tint_hlsl_is_first_1 = true;
+ float lhs;
+ float other;
for(;;) {
if (!tint_hlsl_is_first_1) {
lhs = rhs;
@@ -159,7 +179,7 @@
tint_hlsl_is_first_1 = false;
lhs = 2.40000010f;
- float other;
+ other = 0.0f;
}
}
)");
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index ce0d14e..6bc21fd 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -1462,6 +1462,19 @@
make_indent();
out_ << "bool " << guard << " = true;" << std::endl;
+
+ // A continuing block may use variables declared in the method body. As a
+ // first pass, if we have a continuing, we pull all declarations outside
+ // the for loop into the continuing scope. Then, the variable declarations
+ // will be turned into assignments.
+ for (const auto& s : *(stmt->body())) {
+ if (!s->IsVariableDecl()) {
+ continue;
+ }
+ if (!EmitVariable(s->AsVariableDecl()->variable(), true)) {
+ return false;
+ }
+ }
}
make_indent();
@@ -1482,6 +1495,26 @@
}
for (const auto& s : *(stmt->body())) {
+ // If we have a continuing block we've already emitted the variable
+ // declaration before the loop, so treat it as an assignment.
+ if (s->IsVariableDecl() && stmt->has_continuing()) {
+ make_indent();
+
+ auto* var = s->AsVariableDecl()->variable();
+ out_ << var->name() << " = ";
+ if (var->constructor() != nullptr) {
+ if (!EmitExpression(var->constructor())) {
+ return false;
+ }
+ } else {
+ if (!EmitZeroValue(var->type())) {
+ return false;
+ }
+ }
+ out_ << ";" << std::endl;
+ continue;
+ }
+
if (!EmitStatement(s.get())) {
return false;
}
@@ -1652,7 +1685,7 @@
return EmitSwitch(stmt->AsSwitch());
}
if (stmt->IsVariableDecl()) {
- return EmitVariable(stmt->AsVariableDecl()->variable());
+ return EmitVariable(stmt->AsVariableDecl()->variable(), false);
}
error_ = "unknown statement type: " + stmt->str();
@@ -1815,7 +1848,7 @@
return true;
}
-bool GeneratorImpl::EmitVariable(ast::Variable* var) {
+bool GeneratorImpl::EmitVariable(ast::Variable* var, bool skip_constructor) {
make_indent();
// TODO(dsinclair): Handle variable decorations
@@ -1834,7 +1867,7 @@
out_ << " " << var->name();
}
- if (var->constructor() != nullptr) {
+ if (!skip_constructor && var->constructor() != nullptr) {
out_ << " = ";
if (!EmitExpression(var->constructor())) {
return false;
diff --git a/src/writer/msl/generator_impl.h b/src/writer/msl/generator_impl.h
index 1a9e0b9..b574b43 100644
--- a/src/writer/msl/generator_impl.h
+++ b/src/writer/msl/generator_impl.h
@@ -200,8 +200,9 @@
bool EmitUnaryOp(ast::UnaryOpExpression* expr);
/// Handles generating a variable
/// @param var the variable to generate
+ /// @param skip_constructor set true if the constructor should be skipped
/// @returns true if the variable was emitted
- bool EmitVariable(ast::Variable* var);
+ bool EmitVariable(ast::Variable* var, bool skip_constructor);
/// Handles generating a program scope constant variable
/// @param var the variable to emit
/// @returns true if the variable was emitted
diff --git a/src/writer/msl/generator_impl_loop_test.cc b/src/writer/msl/generator_impl_loop_test.cc
index 1cf6609..c11efdc 100644
--- a/src/writer/msl/generator_impl_loop_test.cc
+++ b/src/writer/msl/generator_impl_loop_test.cc
@@ -132,9 +132,28 @@
)");
}
-// TODO(dsinclair): Handle pulling declared variables up and out of the for() if
-// there is a continuing block.
-TEST_F(MslGeneratorImplTest, DISABLED_Emit_LoopWithVarUsedInContinuing) {
+TEST_F(MslGeneratorImplTest, Emit_LoopWithVarUsedInContinuing) {
+ // loop {
+ // var lhs : f32 = 2.4;
+ // var other : f32;
+ // continuing {
+ // lhs = rhs
+ // }
+ // }
+ //
+ // ->
+ // {
+ // float lhs;
+ // float other;
+ // for (;;) {
+ // if (continuing) {
+ // lhs = rhs;
+ // }
+ // lhs = 2.4f;
+ // other = 0.0f;
+ // }
+ // }
+
ast::type::F32Type f32;
auto var = std::make_unique<ast::Variable>(
@@ -155,16 +174,17 @@
continuing->append(std::make_unique<ast::AssignmentStatement>(
std::move(lhs), std::move(rhs)));
- ast::LoopStatement outer(std::move(body), std::move(continuing));
-
ast::Module m;
GeneratorImpl g(&m);
g.increment_indent();
+ ast::LoopStatement outer(std::move(body), std::move(continuing));
+
ASSERT_TRUE(g.EmitStatement(&outer)) << g.error();
EXPECT_EQ(g.result(), R"( {
- float lhs;
bool tint_msl_is_first_1 = true;
+ float lhs;
+ float other;
for(;;) {
if (!tint_msl_is_first_1) {
lhs = rhs;
@@ -172,7 +192,7 @@
tint_msl_is_first_1 = false;
lhs = 2.40000010f;
- float other;
+ other = 0.0f;
}
}
)");
