[ir] Add conditions to if and switch nodes.
This CL updates the if and switch nodes to store the condition value in
a register. The EmitExpression is updated to return a Register and the
builder updated to emit the expressions for the if, break-if, while,
and switch expressions.
Bug: tint:1718
Change-Id: Ie710812c74e8b9423a4aa997db451d9cdf304feb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/110784
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>
diff --git a/src/tint/ir/builder_impl.cc b/src/tint/ir/builder_impl.cc
index 4525b33..b2dab25 100644
--- a/src/tint/ir/builder_impl.cc
+++ b/src/tint/ir/builder_impl.cc
@@ -234,7 +234,12 @@
bool BuilderImpl::EmitIf(const ast::IfStatement* stmt) {
auto* if_node = builder.CreateIf(stmt);
- // TODO(dsinclair): Emit the condition expression into the current block
+ // Emit the if condition into the end of the preceeding block
+ auto reg = EmitExpression(stmt->condition);
+ if (!reg) {
+ return false;
+ }
+ if_node->condition = reg.Get();
BranchTo(if_node);
@@ -243,8 +248,6 @@
{
FlowStackScope scope(this, if_node);
- // TODO(dsinclair): set if condition register into if flow node
-
current_flow_block = if_node->true_target;
if (!EmitStatement(stmt->body)) {
return false;
@@ -323,13 +326,17 @@
current_flow_block = loop_node->start_target;
- // TODO(dsinclair): Emit the instructions for the condition
+ // Emit the while condition into the start target of the loop
+ auto reg = EmitExpression(stmt->condition);
+ if (!reg) {
+ return false;
+ }
// Create an if (cond) {} else {break;} control flow
auto* if_node = builder.CreateIf(nullptr);
builder.Branch(if_node->true_target, if_node->merge_target);
builder.Branch(if_node->false_target, loop_node->merge_target);
- // TODO(dsinclair): set if condition register into if flow node
+ if_node->condition = reg.Get();
BranchTo(if_node);
@@ -367,13 +374,17 @@
current_flow_block = loop_node->start_target;
if (stmt->condition) {
- // TODO(dsinclair): Emit the instructions for the condition
+ // Emit the condition into the target target of the loop
+ auto reg = EmitExpression(stmt->condition);
+ if (!reg) {
+ return false;
+ }
// Create an if (cond) {} else {break;} control flow
auto* if_node = builder.CreateIf(nullptr);
builder.Branch(if_node->true_target, if_node->merge_target);
builder.Branch(if_node->false_target, loop_node->merge_target);
- // TODO(dsinclair): set if condition register into if flow node
+ if_node->condition = reg.Get();
BranchTo(if_node);
current_flow_block = if_node->merge_target;
@@ -401,7 +412,12 @@
bool BuilderImpl::EmitSwitch(const ast::SwitchStatement* stmt) {
auto* switch_node = builder.CreateSwitch(stmt);
- // TODO(dsinclair): Emit the condition expression into the current block
+ // Emit the condition into the preceeding block
+ auto reg = EmitExpression(stmt->condition);
+ if (!reg) {
+ return false;
+ }
+ switch_node->condition = reg.Get();
BranchTo(switch_node);
@@ -466,7 +482,12 @@
bool BuilderImpl::EmitBreakIf(const ast::BreakIfStatement* stmt) {
auto* if_node = builder.CreateIf(stmt);
- // TODO(dsinclair): Emit the condition expression into the current block
+ // Emit the break-if condition into the end of the preceeding block
+ auto reg = EmitExpression(stmt->condition);
+ if (!reg) {
+ return false;
+ }
+ if_node->condition = reg.Get();
BranchTo(if_node);
@@ -478,8 +499,6 @@
auto* loop = current_control->As<Loop>();
- // TODO(dsinclair): set if condition register into if flow node
-
current_flow_block = if_node->true_target;
BranchTo(loop->merge_target);
@@ -496,7 +515,7 @@
return true;
}
-bool BuilderImpl::EmitExpression(const ast::Expression* expr) {
+utils::Result<Register> BuilderImpl::EmitExpression(const ast::Expression* expr) {
return tint::Switch(
expr,
// [&](const ast::IndexAccessorExpression* a) { return EmitIndexAccessor(a); },
@@ -512,7 +531,7 @@
diagnostics_.add_warning(
tint::diag::System::IR,
"unknown expression type: " + std::string(expr->TypeInfo().name), expr->source);
- return false;
+ return utils::Failure;
});
}
diff --git a/src/tint/ir/builder_impl.h b/src/tint/ir/builder_impl.h
index f1113dc..18c4d3e 100644
--- a/src/tint/ir/builder_impl.h
+++ b/src/tint/ir/builder_impl.h
@@ -139,7 +139,7 @@
/// Emits an expression
/// @param expr the expression to emit
/// @returns true if successful, false otherwise
- bool EmitExpression(const ast::Expression* expr);
+ utils::Result<Register> EmitExpression(const ast::Expression* expr);
/// Emits a variable
/// @param var the variable to emit
diff --git a/src/tint/ir/builder_impl_test.cc b/src/tint/ir/builder_impl_test.cc
index f10b285..04c8d00 100644
--- a/src/tint/ir/builder_impl_test.cc
+++ b/src/tint/ir/builder_impl_test.cc
@@ -80,8 +80,6 @@
ASSERT_NE(ir_if, nullptr);
EXPECT_TRUE(ir_if->Is<ir::If>());
- // TODO(dsinclair): check condition
-
auto* flow = ir_if->As<ir::If>();
ASSERT_NE(flow->true_target, nullptr);
ASSERT_NE(flow->false_target, nullptr);
@@ -101,6 +99,11 @@
EXPECT_EQ(flow->true_target->branch_target, flow->merge_target);
EXPECT_EQ(flow->false_target->branch_target, flow->merge_target);
EXPECT_EQ(flow->merge_target->branch_target, func->end_target);
+
+ // Check condition
+ auto op = flow->condition;
+ ASSERT_TRUE(op.IsBool());
+ EXPECT_TRUE(op.AsBool());
}
TEST_F(IR_BuilderImplTest, IfStatement_TrueReturns) {
@@ -497,6 +500,11 @@
EXPECT_EQ(func->start_target->branch_target, ir_loop);
EXPECT_EQ(loop_flow->merge_target->branch_target, nullptr);
+
+ // Check condition
+ auto op = if_flow->condition;
+ ASSERT_TRUE(op.IsBool());
+ EXPECT_TRUE(op.AsBool());
}
TEST_F(IR_BuilderImplTest, Loop_WithOnlyReturn) {
@@ -937,6 +945,11 @@
EXPECT_EQ(if_flow->merge_target->branch_target, flow->continuing_target);
EXPECT_EQ(flow->continuing_target->branch_target, flow->start_target);
EXPECT_EQ(flow->merge_target->branch_target, func->end_target);
+
+ // Check condition
+ auto op = if_flow->condition;
+ ASSERT_TRUE(op.IsBool());
+ EXPECT_FALSE(op.AsBool());
}
TEST_F(IR_BuilderImplTest, While_Return) {
@@ -1056,6 +1069,11 @@
EXPECT_EQ(if_flow->merge_target->branch_target, flow->continuing_target);
EXPECT_EQ(flow->continuing_target->branch_target, flow->start_target);
EXPECT_EQ(flow->merge_target->branch_target, func->end_target);
+
+ // Check condition
+ auto op = if_flow->condition;
+ ASSERT_TRUE(op.IsBool());
+ EXPECT_FALSE(op.AsBool());
}
TEST_F(IR_BuilderImplTest, For_NoInitCondOrContinuing) {
@@ -1151,6 +1169,11 @@
EXPECT_EQ(flow->cases[1].start_target->branch_target, flow->merge_target);
EXPECT_EQ(flow->cases[2].start_target->branch_target, flow->merge_target);
EXPECT_EQ(flow->merge_target->branch_target, func->end_target);
+
+ // Check condition
+ auto op = flow->condition;
+ ASSERT_TRUE(op.IsI32());
+ EXPECT_EQ(1_i, op.AsI32());
}
TEST_F(IR_BuilderImplTest, Switch_OnlyDefault) {
diff --git a/src/tint/ir/debug.cc b/src/tint/ir/debug.cc
index 749c654..bcac46d 100644
--- a/src/tint/ir/debug.cc
+++ b/src/tint/ir/debug.cc
@@ -221,7 +221,7 @@
Walk(b->branch_target);
},
[&](const ir::Switch* s) {
- indent() << "Switch" << std::endl;
+ indent() << "Switch (" << s->condition.AsString() << ")" << std::endl;
{
ScopedIndent switch_indent(&indent_size);
@@ -237,15 +237,15 @@
Walk(s->merge_target);
},
[&](const ir::If* i) {
- indent() << "if" << std::endl;
+ indent() << "if (" << i->condition.AsString() << ")" << std::endl;
{
ScopedIndent if_indent(&indent_size);
ScopedStopNode scope(&stop_nodes, i->merge_target);
- indent() << "If true" << std::endl;
+ indent() << "true branch" << std::endl;
Walk(i->true_target);
- indent() << "If false" << std::endl;
+ indent() << "false branch" << std::endl;
Walk(i->false_target);
}
diff --git a/src/tint/ir/if.h b/src/tint/ir/if.h
index 2d28aa1..109b990 100644
--- a/src/tint/ir/if.h
+++ b/src/tint/ir/if.h
@@ -17,6 +17,7 @@
#include "src/tint/ast/if_statement.h"
#include "src/tint/ir/flow_node.h"
+#include "src/tint/ir/register.h"
// Forward declarations
namespace tint::ir {
@@ -43,6 +44,8 @@
/// An block to reconvert the true/false barnches. The block always exists, but there maybe no
/// branches into it. (e.g. if both branches `return`)
Block* merge_target = nullptr;
+ /// Register holding the condition result
+ Register condition;
};
} // namespace tint::ir
diff --git a/src/tint/ir/register.cc b/src/tint/ir/register.cc
index 815cb02..ba156e2 100644
--- a/src/tint/ir/register.cc
+++ b/src/tint/ir/register.cc
@@ -16,6 +16,8 @@
namespace tint::ir {
+Register::Register() : kind_(Kind::kUninitialized), data_(Id(0)) {}
+
Register::Register(Id id) : kind_(Kind::kTemp), data_(id) {}
Register::Register(f32 f) : kind_(Kind::kF32), data_(f) {}
@@ -57,6 +59,8 @@
return "%v" + std::to_string(AsVarData().id);
case Kind::kBool:
return AsBool() ? "true" : "false";
+ case Kind::kUninitialized:
+ break;
}
return "unknown register";
}
diff --git a/src/tint/ir/register.h b/src/tint/ir/register.h
index dd56aa9..54d863d 100644
--- a/src/tint/ir/register.h
+++ b/src/tint/ir/register.h
@@ -44,6 +44,10 @@
};
/// Constructor
+ /// Creates a uninitialized register
+ Register();
+
+ /// Constructor
/// @param id the id for the register
explicit Register(Id id);
@@ -134,6 +138,8 @@
private:
/// The type of the register
enum class Kind {
+ /// A uninitialized register
+ kUninitialized,
/// A temporary allocated register
kTemp,
/// A f32 register
diff --git a/src/tint/ir/register_test.cc b/src/tint/ir/register_test.cc
index a19a773..af4b659 100644
--- a/src/tint/ir/register_test.cc
+++ b/src/tint/ir/register_test.cc
@@ -132,5 +132,17 @@
EXPECT_FALSE(r.IsBool());
}
+TEST_F(IR_RegisterTest, uninitialized) {
+ Register r;
+
+ EXPECT_FALSE(r.IsF32());
+ EXPECT_FALSE(r.IsF16());
+ EXPECT_FALSE(r.IsI32());
+ EXPECT_FALSE(r.IsU32());
+ EXPECT_FALSE(r.IsTemp());
+ EXPECT_FALSE(r.IsVar());
+ EXPECT_FALSE(r.IsBool());
+}
+
} // namespace
} // namespace tint::ir
diff --git a/src/tint/ir/switch.h b/src/tint/ir/switch.h
index 39d3d06..73284cf 100644
--- a/src/tint/ir/switch.h
+++ b/src/tint/ir/switch.h
@@ -17,6 +17,7 @@
#include "src/tint/ir/block.h"
#include "src/tint/ir/flow_node.h"
+#include "src/tint/ir/register.h"
// Forward declarations
namespace tint::ast {
@@ -50,6 +51,9 @@
/// The switch case statements
utils::Vector<Case, 4> cases;
+
+ /// Register holding the condition result
+ Register condition;
};
} // namespace tint::ir
