[hlsl-writer] Emit LogicalAnd and LogicalOr operations
This Cl emits the required code to handle LogicalAnd and LogicalOr short
circuting in HLSL.
Bug: tint:192
Change-Id: I30a88c207f7864e0a3c856d2dae1420c7ff35eb4
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/27782
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: David Neto <dneto@google.com>
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index a2b15ea..bba6c0f 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -161,7 +161,7 @@
std::string GeneratorImpl::generate_name(const std::string& prefix) {
std::string name = prefix;
uint32_t i = 0;
- while (namer_.IsMapped(name)) {
+ while (namer_.IsMapped(name) || namer_.IsRemapped(name)) {
name = prefix + "_" + std::to_string(i);
++i;
}
@@ -302,8 +302,97 @@
bool GeneratorImpl::EmitBinary(std::ostream& pre,
std::ostream& out,
ast::BinaryExpression* expr) {
- out << "(";
+ if (expr->op() == ast::BinaryOp::kLogicalAnd) {
+ std::ostringstream lhs_out;
+ if (!EmitExpression(pre, lhs_out, expr->lhs())) {
+ return false;
+ }
+ auto name = generate_name(kTempNamePrefix);
+ make_indent(pre);
+ pre << "bool " << name << " = false;" << std::endl;
+ make_indent(pre);
+ pre << "if (" << lhs_out.str() << ") {" << std::endl;
+ {
+ increment_indent();
+
+ std::ostringstream rhs_out;
+ if (!EmitExpression(pre, rhs_out, expr->rhs())) {
+ return false;
+ }
+ make_indent(pre);
+ pre << "if (" << rhs_out.str() << ") {" << std::endl;
+ {
+ increment_indent();
+
+ make_indent(pre);
+ pre << name << " = true;" << std::endl;
+
+ decrement_indent();
+ }
+ make_indent(pre);
+ pre << "}" << std::endl;
+
+ decrement_indent();
+ }
+ make_indent(pre);
+ pre << "}" << std::endl;
+
+ out << "(" << name << ")";
+ return true;
+ }
+ if (expr->op() == ast::BinaryOp::kLogicalOr) {
+ std::ostringstream lhs_out;
+ if (!EmitExpression(pre, lhs_out, expr->lhs())) {
+ return false;
+ }
+
+ auto name = generate_name(kTempNamePrefix);
+ make_indent(pre);
+ pre << "bool " << name << " = false;" << std::endl;
+ make_indent(pre);
+ pre << "if (" << lhs_out.str() << ") {" << std::endl;
+ {
+ increment_indent();
+
+ make_indent(pre);
+ pre << name << " = true;" << std::endl;
+
+ decrement_indent();
+ }
+
+ make_indent(pre);
+ pre << "} else {" << std::endl;
+ {
+ increment_indent();
+
+ std::ostringstream rhs_out;
+ if (!EmitExpression(pre, rhs_out, expr->rhs())) {
+ return false;
+ }
+ make_indent(pre);
+ pre << "if (" << rhs_out.str() << ") {" << std::endl;
+ {
+ increment_indent();
+
+ make_indent(pre);
+ pre << name << " = true;" << std::endl;
+
+ decrement_indent();
+ }
+ make_indent(pre);
+ pre << "}" << std::endl;
+
+ decrement_indent();
+ }
+ make_indent(pre);
+ pre << "}" << std::endl;
+
+ out << "(" << name << ")";
+ return true;
+ }
+
+ out << "(";
if (!EmitExpression(pre, out, expr->lhs())) {
return false;
}
@@ -320,13 +409,11 @@
out << "^";
break;
case ast::BinaryOp::kLogicalAnd:
- // TODO(dsinclair): Implement support ...
- error_ = "&& not supported yet";
+ case ast::BinaryOp::kLogicalOr: {
+ // These are both handled above.
+ assert(false);
return false;
- case ast::BinaryOp::kLogicalOr:
- // TODO(dsinclair): Implement support ...
- error_ = "|| not supported yet";
- return false;
+ }
case ast::BinaryOp::kEqual:
out << "==";
break;
diff --git a/src/writer/hlsl/generator_impl_binary_test.cc b/src/writer/hlsl/generator_impl_binary_test.cc
index 024aa42..c1b04bb 100644
--- a/src/writer/hlsl/generator_impl_binary_test.cc
+++ b/src/writer/hlsl/generator_impl_binary_test.cc
@@ -24,6 +24,8 @@
namespace hlsl {
namespace {
+using HlslGeneratorImplTest_Binary = TestHelper;
+
struct BinaryData {
const char* result;
ast::BinaryOp op;
@@ -52,8 +54,6 @@
BinaryData{"(left & right)", ast::BinaryOp::kAnd},
BinaryData{"(left | right)", ast::BinaryOp::kOr},
BinaryData{"(left ^ right)", ast::BinaryOp::kXor},
- // BinaryData{"(left && right)", ast::BinaryOp::kLogicalAnd},
- // BinaryData{"(left || right)", ast::BinaryOp::kLogicalOr},
BinaryData{"(left == right)", ast::BinaryOp::kEqual},
BinaryData{"(left != right)", ast::BinaryOp::kNotEqual},
BinaryData{"(left < right)", ast::BinaryOp::kLessThan},
@@ -68,6 +68,85 @@
BinaryData{"(left / right)", ast::BinaryOp::kDivide},
BinaryData{"(left % right)", ast::BinaryOp::kModulo}));
+TEST_F(HlslGeneratorImplTest_Binary, Logical_And) {
+ auto left = std::make_unique<ast::IdentifierExpression>("left");
+ auto right = std::make_unique<ast::IdentifierExpression>("right");
+
+ ast::BinaryExpression expr(ast::BinaryOp::kLogicalAnd, std::move(left),
+ std::move(right));
+
+ ASSERT_TRUE(gen().EmitExpression(pre(), out(), &expr)) << gen().error();
+ EXPECT_EQ(result(), "(_tint_tmp)");
+ EXPECT_EQ(pre_result(), R"(bool _tint_tmp = false;
+if (left) {
+ if (right) {
+ _tint_tmp = true;
+ }
+}
+)");
+}
+
+TEST_F(HlslGeneratorImplTest_Binary, Logical_Multi) {
+ // (a && b) || (c || d)
+ auto a = std::make_unique<ast::IdentifierExpression>("a");
+ auto b = std::make_unique<ast::IdentifierExpression>("b");
+ auto c = std::make_unique<ast::IdentifierExpression>("c");
+ auto d = std::make_unique<ast::IdentifierExpression>("d");
+
+ ast::BinaryExpression expr(
+ ast::BinaryOp::kLogicalOr,
+ std::make_unique<ast::BinaryExpression>(ast::BinaryOp::kLogicalAnd,
+ std::move(a), std::move(b)),
+ std::make_unique<ast::BinaryExpression>(ast::BinaryOp::kLogicalOr,
+ std::move(c), std::move(d)));
+
+ ASSERT_TRUE(gen().EmitExpression(pre(), out(), &expr)) << gen().error();
+ EXPECT_EQ(result(), "(_tint_tmp_0)");
+ EXPECT_EQ(pre_result(), R"(bool _tint_tmp = false;
+if (a) {
+ if (b) {
+ _tint_tmp = true;
+ }
+}
+bool _tint_tmp_0 = false;
+if ((_tint_tmp)) {
+ _tint_tmp_0 = true;
+} else {
+ bool _tint_tmp_1 = false;
+ if (c) {
+ _tint_tmp_1 = true;
+ } else {
+ if (d) {
+ _tint_tmp_1 = true;
+ }
+ }
+ if ((_tint_tmp_1)) {
+ _tint_tmp_0 = true;
+ }
+}
+)");
+}
+
+TEST_F(HlslGeneratorImplTest_Binary, Logical_Or) {
+ auto left = std::make_unique<ast::IdentifierExpression>("left");
+ auto right = std::make_unique<ast::IdentifierExpression>("right");
+
+ ast::BinaryExpression expr(ast::BinaryOp::kLogicalOr, std::move(left),
+ std::move(right));
+
+ ASSERT_TRUE(gen().EmitExpression(pre(), out(), &expr)) << gen().error();
+ EXPECT_EQ(result(), "(_tint_tmp)");
+ EXPECT_EQ(pre_result(), R"(bool _tint_tmp = false;
+if (left) {
+ _tint_tmp = true;
+} else {
+ if (right) {
+ _tint_tmp = true;
+ }
+}
+)");
+}
+
} // namespace
} // namespace hlsl
} // namespace writer
diff --git a/src/writer/hlsl/namer.cc b/src/writer/hlsl/namer.cc
index 8c04e91..a577d63 100644
--- a/src/writer/hlsl/namer.cc
+++ b/src/writer/hlsl/namer.cc
@@ -664,8 +664,7 @@
uint32_t i = 0;
// Make sure the ident name wasn't assigned by a remapping.
while (true) {
- auto remap_it = remapped_names_.find(ret_name);
- if (remap_it == remapped_names_.end()) {
+ if (!IsRemapped(ret_name)) {
break;
}
ret_name = name + "_" + std::to_string(i);
@@ -683,6 +682,11 @@
return it != name_map_.end();
}
+bool Namer::IsRemapped(const std::string& name) {
+ auto it = remapped_names_.find(name);
+ return it != remapped_names_.end();
+}
+
void Namer::RegisterRemappedName(const std::string& name) {
remapped_names_.insert(name);
}
diff --git a/src/writer/hlsl/namer.h b/src/writer/hlsl/namer.h
index be3b521..b5da343 100644
--- a/src/writer/hlsl/namer.h
+++ b/src/writer/hlsl/namer.h
@@ -39,11 +39,16 @@
/// @param name the name to register
void RegisterRemappedName(const std::string& name);
- /// Returns if the given name has been mapped alread
+ /// Returns if the given name has been mapped already
/// @param name the name to check
/// @returns true if the name has been mapped
bool IsMapped(const std::string& name);
+ /// Returns if the given name has been remapped already
+ /// @param name the name to check
+ /// @returns true if the name has been remapped
+ bool IsRemapped(const std::string& name);
+
private:
/// Map of original name to new name. The two names may be the same.
std::unordered_map<std::string, std::string> name_map_;
