From 50c29f6d38b3d471bc4036faae11020530538d86 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 1 Nov 2022 17:03:05 +0000 Subject: [PATCH] [IR] Add support for `fallthrough`. This CL adds `fallthrough` support to the IR building. Fallthrough is deprecated but needs to be supported until removed from Tint. Bug: tint:1718 Change-Id: I52ce4cc9953384a450ad09422b2ba38943284a42 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/107802 Commit-Queue: Dan Sinclair Reviewed-by: Ben Clayton Kokoro: Kokoro --- src/tint/ir/builder_impl.cc | 26 ++++++++++++-- src/tint/ir/builder_impl.h | 15 +++++--- src/tint/ir/builder_impl_test.cc | 59 ++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/tint/ir/builder_impl.cc b/src/tint/ir/builder_impl.cc index 895e316a4f..135602975d 100644 --- a/src/tint/ir/builder_impl.cc +++ b/src/tint/ir/builder_impl.cc @@ -19,6 +19,7 @@ #include "src/tint/ast/break_if_statement.h" #include "src/tint/ast/break_statement.h" #include "src/tint/ast/continue_statement.h" +#include "src/tint/ast/fallthrough_statement.h" #include "src/tint/ast/function.h" #include "src/tint/ast/if_statement.h" #include "src/tint/ast/return_statement.h" @@ -204,7 +205,7 @@ bool BuilderImpl::EmitStatement(const ast::Statement* stmt) { // [&](const ast::CallStatement* c) { }, [&](const ast::ContinueStatement* c) { return EmitContinue(c); }, // [&](const ast::DiscardStatement* d) { }, - // [&](const ast::FallthroughStatement*) { }, + [&](const ast::FallthroughStatement*) { return EmitFallthrough(); }, [&](const ast::IfStatement* i) { return EmitIf(i); }, [&](const ast::LoopStatement* l) { return EmitLoop(l); }, // [&](const ast::ForLoopStatement* l) { }, @@ -324,12 +325,25 @@ bool BuilderImpl::EmitSwitch(const ast::SwitchStatement* stmt) { { FlowStackScope scope(this, switch_node); + // TODO(crbug.com/tint/1644): This can be simplifed when fallthrough is removed, a single + // loop can be used to iterate each body statement and emit for the case. Two loops are + // needed in order to have the target for a fallthrough. for (const auto* c : stmt->body) { - current_flow_block_ = builder_.CreateCase(switch_node, c->selectors); - if (!EmitStatement(c->body)) { + builder_.CreateCase(switch_node, c->selectors); + } + + for (size_t i = 0; i < stmt->body.Length(); ++i) { + current_flow_block_ = switch_node->cases[i].start_target; + if (i < (stmt->body.Length() - 1)) { + fallthrough_target_ = switch_node->cases[i + 1].start_target; + } + + if (!EmitStatement(stmt->body[i]->body)) { return false; } BranchToIfNeeded(switch_node->merge_target); + + fallthrough_target_ = nullptr; } } current_flow_block_ = nullptr; @@ -419,4 +433,10 @@ bool BuilderImpl::EmitBreakIf(const ast::BreakIfStatement* stmt) { return true; } +bool BuilderImpl::EmitFallthrough() { + TINT_ASSERT(IR, fallthrough_target_ != nullptr); + BranchTo(fallthrough_target_); + return true; +} + } // namespace tint::ir diff --git a/src/tint/ir/builder_impl.h b/src/tint/ir/builder_impl.h index ac76bd7cf4..83bb8a29b0 100644 --- a/src/tint/ir/builder_impl.h +++ b/src/tint/ir/builder_impl.h @@ -104,24 +104,28 @@ class BuilderImpl { /// Emits a switch statement /// @param stmt the switch statement - /// @returns true if successfull, false otherwise. + /// @returns true if successful, false otherwise. bool EmitSwitch(const ast::SwitchStatement* stmt); /// Emits a break statement /// @param stmt the break statement - /// @returns true if successfull, false otherwise. + /// @returns true if successful, false otherwise. bool EmitBreak(const ast::BreakStatement* stmt); /// Emits a continue statement /// @param stmt the continue statement - /// @returns true if successfull, false otherwise. + /// @returns true if successful, false otherwise. bool EmitContinue(const ast::ContinueStatement* stmt); /// Emits a break-if statement /// @param stmt the break-if statement - /// @returns true if successfull, false otherwise. + /// @returns true if successful, false otherwise. bool EmitBreakIf(const ast::BreakIfStatement* stmt); + /// Emits a fallthrough statement + /// @returns true if successful, false otherwise + bool EmitFallthrough(); + /// Retrieve the IR Flow node for a given AST node. /// @param n the node to lookup /// @returns the FlowNode for the given ast::Node or nullptr if it doesn't exist. @@ -150,6 +154,9 @@ class BuilderImpl { Block* current_flow_block_ = nullptr; Function* current_function_ = nullptr; + // TODO(crbug.com/tint/1644): Remove this when fallthrough is removed. + Block* fallthrough_target_ = nullptr; + /// Map from ast nodes to flow nodes, used to retrieve the flow node for a given AST node. /// Used for testing purposes. std::unordered_map ast_to_flow_; diff --git a/src/tint/ir/builder_impl_test.cc b/src/tint/ir/builder_impl_test.cc index 826cd77ef7..d044067055 100644 --- a/src/tint/ir/builder_impl_test.cc +++ b/src/tint/ir/builder_impl_test.cc @@ -1039,5 +1039,64 @@ TEST_F(IRBuilderImplTest, Switch_AllReturn) { EXPECT_EQ(flow->merge_target->branch_target, nullptr); } +// TODO(crbug.com/tint/1644): Remove when fallthrough is removed. +TEST_F(IRBuilderImplTest, Switch_Fallthrough) { + // func -> switch -> case 1 + // -> case 2 + // -> default + // + // [case 1] -> switch merge + // [case 2] -> default + // [default] -> switch merge + // [switch merge] -> func end + // + auto* ast_switch = + Switch(1_i, utils::Vector{Case(utils::Vector{CaseSelector(0_i)}, Block()), + Case(utils::Vector{CaseSelector(1_i)}, Block(Fallthrough())), + DefaultCase(Block())}); + + WrapInFunction(ast_switch); + auto& b = Build(); + + auto r = b.Build(); + ASSERT_TRUE(r) << b.error(); + auto m = r.Move(); + + auto* ir_switch = b.FlowNodeForAstNode(ast_switch); + ASSERT_NE(ir_switch, nullptr); + ASSERT_TRUE(ir_switch->Is()); + + auto* flow = ir_switch->As(); + ASSERT_NE(flow->merge_target, nullptr); + ASSERT_EQ(3u, flow->cases.Length()); + + ASSERT_EQ(1u, m.functions.Length()); + auto* func = m.functions[0]; + + ASSERT_EQ(1u, flow->cases[0].selectors.Length()); + ASSERT_TRUE(flow->cases[0].selectors[0]->expr->Is()); + EXPECT_EQ(0_i, flow->cases[0].selectors[0]->expr->As()->value); + + ASSERT_EQ(1u, flow->cases[1].selectors.Length()); + ASSERT_TRUE(flow->cases[1].selectors[0]->expr->Is()); + EXPECT_EQ(1_i, flow->cases[1].selectors[0]->expr->As()->value); + + ASSERT_EQ(1u, flow->cases[2].selectors.Length()); + EXPECT_TRUE(flow->cases[2].selectors[0]->IsDefault()); + + EXPECT_EQ(1u, flow->inbound_branches.Length()); + EXPECT_EQ(1u, flow->cases[0].start_target->inbound_branches.Length()); + EXPECT_EQ(1u, flow->cases[1].start_target->inbound_branches.Length()); + EXPECT_EQ(2u, flow->cases[2].start_target->inbound_branches.Length()); + EXPECT_EQ(2u, flow->merge_target->inbound_branches.Length()); + EXPECT_EQ(1u, func->end_target->inbound_branches.Length()); + + EXPECT_EQ(func->start_target->branch_target, ir_switch); + EXPECT_EQ(flow->cases[0].start_target->branch_target, flow->merge_target); + EXPECT_EQ(flow->cases[1].start_target->branch_target, flow->cases[2].start_target); + EXPECT_EQ(flow->cases[2].start_target->branch_target, flow->merge_target); + EXPECT_EQ(flow->merge_target->branch_target, func->end_target); +} + } // namespace } // namespace tint::ir