[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>

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<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 @@ bool BuilderImpl::EmitBreakIf(const ast::BreakIfStatement* stmt) {
     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 @@ 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;
         });
 }
 

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<Register> EmitExpression(const ast::Expression* expr);
 
     /// Emits a variable
     /// @param var the variable to emit

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<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 @@ 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) {

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);
                 }
 

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<If, FlowNode> {
     /// 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

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";
 }

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

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

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<Switch, FlowNode> {
 
     /// The switch case statements
     utils::Vector<Case, 4> cases;
+
+    /// Register holding the condition result
+    Register condition;
 };
 
 }  // namespace tint::ir