diff --git a/src/tint/ir/builder_impl.cc b/src/tint/ir/builder_impl.cc index 4525b33603..b2dab25699 100644 --- a/src/tint/ir/builder_impl.cc +++ b/src/tint/ir/builder_impl.cc @@ -234,7 +234,12 @@ bool BuilderImpl::EmitBlock(const ast::BlockStatement* block) { 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 @@ bool BuilderImpl::EmitIf(const ast::IfStatement* stmt) { { 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 @@ bool BuilderImpl::EmitWhile(const ast::WhileStatement* stmt) { 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 @@ bool BuilderImpl::EmitForLoop(const ast::ForLoopStatement* stmt) { 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::EmitForLoop(const ast::ForLoopStatement* stmt) { 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::EmitContinue(const ast::ContinueStatement*) { 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 @@ bool BuilderImpl::EmitBreakIf(const ast::BreakIfStatement* stmt) { auto* loop = current_control->As(); - // 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 @@ bool BuilderImpl::EmitBreakIf(const ast::BreakIfStatement* stmt) { return true; } -bool BuilderImpl::EmitExpression(const ast::Expression* expr) { +utils::Result BuilderImpl::EmitExpression(const ast::Expression* expr) { return tint::Switch( expr, // [&](const ast::IndexAccessorExpression* a) { return EmitIndexAccessor(a); }, @@ -512,7 +531,7 @@ bool BuilderImpl::EmitExpression(const ast::Expression* expr) { 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 f1113dc2a4..18c4d3e960 100644 --- a/src/tint/ir/builder_impl.h +++ b/src/tint/ir/builder_impl.h @@ -139,7 +139,7 @@ class BuilderImpl { /// Emits an expression /// @param expr the expression to emit /// @returns true if successful, false otherwise - bool EmitExpression(const ast::Expression* expr); + utils::Result 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 f10b2854fe..04c8d00916 100644 --- a/src/tint/ir/builder_impl_test.cc +++ b/src/tint/ir/builder_impl_test.cc @@ -80,8 +80,6 @@ TEST_F(IR_BuilderImplTest, IfStatement) { ASSERT_NE(ir_if, nullptr); EXPECT_TRUE(ir_if->Is()); - // TODO(dsinclair): check condition - auto* flow = ir_if->As(); ASSERT_NE(flow->true_target, nullptr); ASSERT_NE(flow->false_target, nullptr); @@ -101,6 +99,11 @@ TEST_F(IR_BuilderImplTest, IfStatement) { 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 @@ TEST_F(IR_BuilderImplTest, Loop_WithReturn) { 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 @@ TEST_F(IR_BuilderImplTest, While) { 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 @@ TEST_F(IR_BuilderImplTest, DISABLED_For) { 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 @@ TEST_F(IR_BuilderImplTest, Switch) { 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 749c65436d..bcac46d9bf 100644 --- a/src/tint/ir/debug.cc +++ b/src/tint/ir/debug.cc @@ -221,7 +221,7 @@ std::string Debug::AsString(const Module* mod) { 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 @@ std::string Debug::AsString(const Module* mod) { 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 2d28aa17d7..109b990dce 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 @@ class If : public Castable { /// 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 815cb02d80..ba156e237e 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 @@ std::string Register::AsString() const { 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 dd56aa9488..54d863d1f6 100644 --- a/src/tint/ir/register.h +++ b/src/tint/ir/register.h @@ -43,6 +43,10 @@ class Register { // TODO(dsinclair): Should var type data be stored here along side the variable info? }; + /// Constructor + /// Creates a uninitialized register + Register(); + /// Constructor /// @param id the id for the register explicit Register(Id id); @@ -134,6 +138,8 @@ class Register { 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 a19a773cc9..af4b659d2e 100644 --- a/src/tint/ir/register_test.cc +++ b/src/tint/ir/register_test.cc @@ -132,5 +132,17 @@ TEST_F(IR_RegisterTest, var) { 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 39d3d06b4c..73284cf81b 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 @@ class Switch : public Castable { /// The switch case statements utils::Vector cases; + + /// Register holding the condition result + Register condition; }; } // namespace tint::ir