From 27d42ede4ea694c18d82e5cd06284d5a8ad46eb1 Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 3 Jun 2020 13:43:03 +0000 Subject: [PATCH] [spirv-reader]: Support OpBranch Bug: tint:3 Change-Id: I39d03f4fc29c7b60dc09d0bafc3afaec754671a0 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/22425 Reviewed-by: dan sinclair --- src/reader/spirv/function.cc | 47 ++- src/reader/spirv/function_cfg_test.cc | 543 +++++++++++++++++++++++++- 2 files changed, 580 insertions(+), 10 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index e3ef47426d..eb1e6fc7ea 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -27,7 +27,10 @@ #include "src/ast/as_expression.h" #include "src/ast/assignment_statement.h" #include "src/ast/binary_expression.h" +#include "src/ast/break_statement.h" +#include "src/ast/continue_statement.h" #include "src/ast/else_statement.h" +#include "src/ast/fallthrough_statement.h" #include "src/ast/identifier_expression.h" #include "src/ast/if_statement.h" #include "src/ast/kill_statement.h" @@ -1786,6 +1789,39 @@ bool FunctionEmitter::EmitNormalTerminator(const BlockInfo& block_info) { } } return true; + case SpvOpBranch: { + const auto dest = terminator.GetSingleWordInOperand(0); + const auto kind = block_info.succ_edge.find(dest)->second; + switch (kind) { + case EdgeKind::kBack: + // Nothing to do. The loop backedge is implicit. + return true; + case EdgeKind::kSwitchBreak: + case EdgeKind::kLoopBreak: + AddStatement(std::make_unique()); + return true; + case EdgeKind::kLoopContinue: + // An unconditional continue to the next block is redundant and ugly. + // Skip it in that case. + if (GetBlockInfo(dest)->pos == 1 + block_info.pos) { + return true; + } + // Otherwise, emit a regular continue statement. + AddStatement(std::make_unique()); + return true; + case EdgeKind::kIfBreak: + // For an unconditional branch, the break out to an if-selection + // merge block is implicit. + return true; + case EdgeKind::kCaseFallThrough: + AddStatement(std::make_unique()); + return true; + case EdgeKind::kForward: + // Unconditional forward branch is implicit. + return true; + } + break; + } default: break; } @@ -1842,8 +1878,8 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { auto combinatorial_expr = MaybeEmitCombinatorialValue(inst); if (combinatorial_expr.expr != nullptr) { if (def_use_mgr_->NumUses(&inst) == 1) { - // If it's used once, then defer emitting the expression until it's used. - // Any supporting statements have already been emitted. + // If it's used once, then defer emitting the expression until it's + // used. Any supporting statements have already been emitted. singly_used_values_.insert( std::make_pair(inst.result_id(), std::move(combinatorial_expr))); return success(); @@ -1984,10 +2020,9 @@ TypedExpression FunctionEmitter::MakeAccessChain( } // A SPIR-V access chain is a single instruction with multiple indices - // walking down into composites. The Tint AST represents this as ever-deeper - // nested indexing expresions. - // Start off with an expression for the base, and then bury that inside - // nested indexing expressions. + // walking down into composites. The Tint AST represents this as + // ever-deeper nested indexing expresions. Start off with an expression + // for the base, and then bury that inside nested indexing expressions. TypedExpression current_expr(MakeOperand(inst, 0)); const auto constants = constant_mgr_->GetOperandConstants(&inst); diff --git a/src/reader/spirv/function_cfg_test.cc b/src/reader/spirv/function_cfg_test.cc index b9bc306d45..4a710508a3 100644 --- a/src/reader/spirv/function_cfg_test.cc +++ b/src/reader/spirv/function_cfg_test.cc @@ -7631,12 +7631,117 @@ TEST_F(SpvParserTest, DISABLED_EmitBody_Loop_FalseToBody) { // TODO(dneto): Needs break-if } -TEST_F(SpvParserTest, DISABLED_EmitBody_Loop_NestedIfContinue) { - // TODO(dneto): Needs "continue" terminator support +TEST_F(SpvParserTest, EmitBody_Loop_NestedIfContinue) { + // By construction, it has to come from nested code. + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %80 None + OpBranch %30 + + %30 = OpLabel + OpSelectionMerge %50 None + OpBranchConditional %cond %40 %50 + + %40 = OpLabel + OpStore %var %uint_1 + OpBranch %80 ; continue edge + + %50 = OpLabel ; inner selection merge + OpStore %var %uint_2 + OpBranch %80 + + %80 = OpLabel ; continue target + OpStore %var %uint_3 + OpBranch %20 + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + If{ + ( + ScalarConstructor{false} + ) + { + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + Continue{} + } + } + Else{ + { + } + } + Assignment{ + Identifier{var} + ScalarConstructor{2} + } + continuing { + Assignment{ + Identifier{var} + ScalarConstructor{3} + } + } +} +Return{} +)")) << ToString(fe.ast_body()); } -TEST_F(SpvParserTest, DISABLED_EmitBody_Loop_BodyAlwaysBreaks) { - // TODO(dneto): Needs "continue" terminator support +TEST_F(SpvParserTest, EmitBody_Loop_BodyAlwaysBreaks) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %80 None + OpBranch %30 + + %30 = OpLabel + OpStore %var %uint_1 + OpBranch %99 ; break is here + + %80 = OpLabel + OpStore %var %uint_2 + OpBranch %20 ; backedge + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + Break{} + continuing { + Assignment{ + Identifier{var} + ScalarConstructor{2} + } + } +} +Return{} +)")) << ToString(fe.ast_body()); } TEST_F(SpvParserTest, DISABLED_EmitBody_Loop_BodyConditionallyBreaks) { @@ -8062,6 +8167,436 @@ TEST_F(SpvParserTest, EmitBody_Unreachable_InNonVoidFunction) { )")) << ToString(fe.ast_body()); } +TEST_F(SpvParserTest, EmitBody_Branch_BackEdge_MultiBlockLoop) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %80 None + OpBranch %80 + + %80 = OpLabel + OpStore %var %uint_1 + OpBranch %20 ; here is one + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + continuing { + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + } +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F(SpvParserTest, EmitBody_Branch_BackEdge_SingleBlockLoop) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpStore %var %uint_1 + OpLoopMerge %99 %20 None + OpBranch %20 ; backedge in single block loop + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + Assignment{ + Identifier{var} + ScalarConstructor{1} + } +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F(SpvParserTest, DISABLED_EmitBody_Branch_SwitchBreak) { + // TODO(dneto): support switch first. +} + +TEST_F(SpvParserTest, EmitBody_Branch_LoopBreak_SingleBlockLoop) { + // This case is impossible. The loop must have a backedge, +} + +TEST_F(SpvParserTest, EmitBody_Branch_LoopBreak_MultiBlockLoop_FromBody) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %80 None + OpBranch %30 + + %30 = OpLabel + OpStore %var %uint_1 + OpBranch %99 ; break is here + + %80 = OpLabel + OpStore %var %uint_2 + OpBranch %20 ; backedge + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + Break{} + continuing { + Assignment{ + Identifier{var} + ScalarConstructor{2} + } + } +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F( + SpvParserTest, + EmitBody_Branch_LoopBreak_MultiBlockLoop_FromContinueConstructConditional) { + // This case is invalid because the backedge block doesn't post-dominate the + // continue target. + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %30 None + OpBranch %30 + + %30 = OpLabel ; continue target; also an if-header + OpSelectionMerge %80 None + OpBranchConditional %cond %40 %80 + + %40 = OpLabel + OpBranch %99 ; break, inside a nested if. + + %80 = OpLabel + OpBranch %20 ; backedge + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_FALSE(fe.EmitBody()) << p->error(); + EXPECT_THAT(p->error(), + Eq("Invalid exit (40->99) from continue construct: 40 is not the " + "last block in the continue construct starting at 30 " + "(violates post-dominance rule)")); +} + +TEST_F(SpvParserTest, + EmitBody_Branch_LoopBreak_MultiBlockLoop_FromContinueConstructEnd) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %80 None + OpBranch %80 + + %80 = OpLabel ; continue target + OpStore %var %uint_1 + OpBranch %99 ; should be a backedge + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + continuing { + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + Break{} + } +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F(SpvParserTest, EmitBody_Branch_LoopContinue_LastInLoopConstruct) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %80 None + OpBranch %30 + + %30 = OpLabel + OpStore %var %uint_1 + OpBranch %80 ; continue edge from last block before continue target + + %80 = OpLabel ; continue target + OpStore %var %uint_2 + OpBranch %20 + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + continuing { + Assignment{ + Identifier{var} + ScalarConstructor{2} + } + } +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F(SpvParserTest, EmitBody_Branch_LoopContinue_BeforeLast) { + // By construction, it has to come from nested code. + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %80 None + OpBranch %30 + + %30 = OpLabel + OpSelectionMerge %50 None + OpBranchConditional %cond %40 %50 + + %40 = OpLabel + OpStore %var %uint_1 + OpBranch %80 ; continue edge + + %50 = OpLabel ; inner selection merge + OpStore %var %uint_2 + OpBranch %80 + + %80 = OpLabel ; continue target + OpStore %var %uint_3 + OpBranch %20 + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Loop{ + If{ + ( + ScalarConstructor{false} + ) + { + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + Continue{} + } + } + Else{ + { + } + } + Assignment{ + Identifier{var} + ScalarConstructor{2} + } + continuing { + Assignment{ + Identifier{var} + ScalarConstructor{3} + } + } +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F(SpvParserTest, EmitBody_Branch_IfBreak_FromThen) { + // When unconditional, the if-break must be last in the then clause. + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpSelectionMerge %99 None + OpBranchConditional %cond %30 %99 + + %30 = OpLabel + OpStore %var %uint_1 + OpBranch %99 + + %99 = OpLabel + OpStore %var %uint_2 + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(If{ + ( + ScalarConstructor{false} + ) + { + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + } +} +Else{ + { + } +} +Assignment{ + Identifier{var} + ScalarConstructor{2} +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F(SpvParserTest, EmitBody_Branch_IfBreak_FromElse) { + // When unconditional, the if-break must be last in the else clause. + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpSelectionMerge %99 None + OpBranchConditional %cond %99 %30 + + %30 = OpLabel + OpStore %var %uint_1 + OpBranch %99 + + %99 = OpLabel + OpStore %var %uint_2 + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(If{ + ( + ScalarConstructor{false} + ) + { + } +} +Else{ + { + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + } +} +Assignment{ + Identifier{var} + ScalarConstructor{2} +} +Return{} +)")) << ToString(fe.ast_body()); +} + +TEST_F(SpvParserTest, DISABLED_EmitBody_Branch_CaseFallthrough) { + // TODO(dneto): support switch first. +} + +TEST_F(SpvParserTest, EmitBody_Branch_Forward) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpStore %var %uint_1 + OpBranch %99 ; forward + + %99 = OpLabel + OpStore %var %uint_2 + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), Eq(R"(Assignment{ + Identifier{var} + ScalarConstructor{1} +} +Assignment{ + Identifier{var} + ScalarConstructor{2} +} +Return{} +)")) << ToString(fe.ast_body()); +} + } // namespace } // namespace spirv } // namespace reader