diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index f8360b952d..dad106c41c 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -118,7 +118,7 @@ // // If CT(H) exists, then: // -// Pos(H) <= Pos(CT(H)), with equality exactly for single-block loops +// Pos(H) <= Pos(CT(H)) // Pos(CT(H)) < Pos(M) // // This gives us the fundamental ordering of blocks in relation to a @@ -140,15 +140,16 @@ // where H and d-e-f: blocks in the selection construct // where M(H) and n-o-p...: blocks after the selection construct // -// Schematically, for a single-block loop construct headed by H, there are -// blocks in order from left to right: +// Schematically, for a loop construct headed by H that is its own +// continue construct, the blocks in order from left to right: // -// ...a-b-c H M(H) n-o-p... +// ...a-b-c H=CT(H) d-e-f M(H) n-o-p... // // where ...a-b-c: blocks before the loop // where H is the continue construct; CT(H)=H, and the loop construct -// is *empty* where M(H) and n-o-p...: blocks after the loop and -// continue constructs +// is *empty* +// where d-e-f... are other blocks in the continue construct +// where M(H) and n-o-p...: blocks after the continue construct // // Schematically, for a multi-block loop construct headed by H, there are // blocks in order from left to right: @@ -767,16 +768,14 @@ bool FunctionEmitter::RegisterMerges() { if (block_id == succ) is_single_block_loop = true; }); - block_info->is_single_block_loop = is_single_block_loop; const auto ct = block_info->continue_for_header; - if (is_single_block_loop && ct != block_id) { + block_info->is_continue_entire_loop = ct == block_id; + if (is_single_block_loop && !block_info->is_continue_entire_loop) { return Fail() << "Block " << block_id << " branches to itself but is not its own continue target"; - } else if (!is_single_block_loop && ct == block_id) { - return Fail() << "Loop header block " << block_id - << " declares itself as its own continue target, but " - "does not branch to itself"; } + // It's valid for a the header of a multi-block loop header to declare + // itself as its own continue target. } return success(); } @@ -799,7 +798,7 @@ bool FunctionEmitter::VerifyHeaderContinueMergeOrder() { // Pos(H) < Pos(M(H)) // // If CT(H) exists, then: - // Pos(H) <= Pos(CT(H)), with equality exactly for single-block loops + // Pos(H) <= Pos(CT(H)) // Pos(CT(H)) < Pos(M) // for (auto block_id : block_order_) { @@ -830,12 +829,8 @@ bool FunctionEmitter::VerifyHeaderContinueMergeOrder() { // Furthermore, this is a loop header. const auto* ct_info = GetBlockInfo(ct); const auto ct_pos = ct_info->pos; - // Pos(H) <= Pos(CT(H)), with equality only for single-block loops. - if (header_info->is_single_block_loop && ct_pos != header_pos) { - Fail() << "Internal error: Single block loop. CT pos is not the " - "header pos. Should have already checked this"; - } - if (!header_info->is_single_block_loop && (ct_pos <= header_pos)) { + // Pos(H) <= Pos(CT(H)) + if (ct_pos < header_pos) { Fail() << "Loop header " << header << " does not dominate its continue target " << ct; } @@ -867,20 +862,24 @@ bool FunctionEmitter::LabelControlFlowConstructs() { // be the associated header. Pop it off. // b. When you reach a header, push it on the stack. // c. When you reach a continue target, push it on the stack. - // (A block can be both a header and a continue target, in the case - // of a single-block loop, in which case it should also be its - // own backedge block.) + // (A block can be both a header and a continue target.) // c. When you reach a block with an edge branching backward (in the // structured order) to block T: // T should be a loop header, and the top of the stack should be a // continue target associated with T. // This is the end of the continue construct. Pop the continue // target off the stack. - // (Note: We pop the merge off first because a merge block that marks + // + // Note: A loop header can declare itself as its own continue target. + // + // Note: For a single-block loop, that block is a header, its own + // continue target, and its own backedge block. + // + // Note: We pop the merge off first because a merge block that marks // the end of one construct can be a single-block loop. So that block // is a merge, a header, a continue target, and a backedge block. // But we want to finish processing of the merge before dealing with - // the loop.) + // the loop. // // In the same scan, mark each basic block with the nearest enclosing // header: the most recent header for which we haven't reached its merge @@ -963,8 +962,10 @@ bool FunctionEmitter::LabelControlFlowConstructs() { // in the block order, starting at the continue target, until just // before the merge block. top = push_construct(depth, Construct::kContinue, ct, merge); - // A single block loop has an empty loop construct. - if (!header_info->is_single_block_loop) { + // A loop header that is its own continue target will have an + // empty loop construct. Only create a loop construct when + // the continue target is *not* the same as the loop header. + if (header != ct) { // From the interval rule, the loop construct consists of blocks // in the block order, starting at the header, until just // before the continue target. @@ -1706,9 +1707,9 @@ bool FunctionEmitter::EmitBasicBlock(const BlockInfo& block_info) { // - It can't be kFunction, because there is only one of those, and it was // already on the stack at the outermost level. // - We have at most one of kIfSelection, kSwitchSelection, or kLoop because - // each of those is headed by a block with a merge instruction, and the - // kIfSelection and kSwitchSelection header blocks end in different branch - // instructions. + // each of those is headed by a block with a merge instruction (OpLoopMerge + // for kLoop, and OpSelectionMerge for the others), and the kIfSelection and + // kSwitchSelection header blocks end in different branch instructions. // - A kContinue can contain a kContinue // This is possible in Vulkan SPIR-V, but Tint disallows this by the rule // that a block can be continue target for at most one header block. See @@ -1723,13 +1724,23 @@ bool FunctionEmitter::EmitBasicBlock(const BlockInfo& block_info) { // starting at the first block of a continue construct. // // The kContinue can't be the child of the other because either: - // - Either it would be a single block loop but in that case there is no - // kLoop construct for it, by construction. - // - The kContinue is in a loop that is not single-block; and the - // selection contains the kContinue block but not the loop block. That - // breaks dominance rules. That is, the continue target is dominated by - // that loop header, and so gets found on the outside before the - // selection is found. The selection is inside the outer loop. + // - The other can't be kLoop because: + // - If the kLoop is for a different loop then the kContinue, then + // the kContinue must be its own loop header, and so the same + // block is two different loops. That's a contradiction. + // - If the kLoop is for a the same loop, then this is a contradiction + // because a kContinue and its kLoop have disjoint block sets. + // - The other construct can't be a selection because: + // - The kContinue construct is the entire loop, i.e. the continue + // target is its own loop header block. But then the continue target + // has an OpLoopMerge instruction, which contradicts this block being + // a selection header. + // - The kContinue is in a multi-block loop that is has a non-empty + // kLoop; and the selection contains the kContinue block but not the + // loop block. That breaks dominance rules. That is, the continue + // target is dominated by that loop header, and so gets found by the + // block traversal on the outside before the selection is found. The + // selection is inside the outer loop. // // So we fall into one of the following cases: // - We are entering 0 or 1 constructs, or @@ -1792,7 +1803,7 @@ bool FunctionEmitter::EmitBasicBlock(const BlockInfo& block_info) { break; case Construct::kContinue: - if (block_info.is_single_block_loop) { + if (block_info.is_continue_entire_loop) { if (!EmitLoopStart(construct)) { return false; } diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index 068657a0a9..7044b0ff76 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -108,9 +108,10 @@ struct BlockInfo { /// If this block is a continue target, then this is the ID of the loop /// header. uint32_t header_for_continue = 0; - /// Is this block a single-block loop: A loop header that declares itself - /// as its own continue target, and has branch to itself. - bool is_single_block_loop = false; + /// Is this block a continue target which is its own loop header block? + /// In this case the continue construct is the entire loop. The associated + /// "loop construct" is empty, and not represented. + bool is_continue_entire_loop = false; /// The immediately enclosing structured construct. If this block is not /// in the block order at all, then this is still nullptr. @@ -185,7 +186,7 @@ inline std::ostream& operator<<(std::ostream& o, const BlockInfo& bi) { << " merge_for_header: " << bi.merge_for_header << " continue_for_header: " << bi.continue_for_header << " header_for_merge: " << bi.header_for_merge - << " single_block_loop: " << int(bi.is_single_block_loop) << "}"; + << " is_continue_entire_loop: " << int(bi.is_continue_entire_loop) << "}"; return o; } @@ -310,9 +311,9 @@ class FunctionEmitter { /// @returns true if terminators are sane bool TerminatorsAreSane(); - /// Populates merge-header cross-links and the |is_single_block_loop| member - /// of BlockInfo. Also verifies that merge instructions go to blocks in - /// the same function. Assumes basic blocks have been registered, and + /// Populates merge-header cross-links and the |is_continue_entire_loop| + /// member of BlockInfo. Also verifies that merge instructions go to blocks + /// in the same function. Assumes basic blocks have been registered, and /// terminators are sane. /// @returns false if registration fails bool RegisterMerges(); diff --git a/src/reader/spirv/function_cfg_test.cc b/src/reader/spirv/function_cfg_test.cc index bb736210b8..d24542778d 100644 --- a/src/reader/spirv/function_cfg_test.cc +++ b/src/reader/spirv/function_cfg_test.cc @@ -374,7 +374,7 @@ TEST_F(SpvParserTest, RegisterMerges_NoMerges) { EXPECT_EQ(bi->continue_for_header, 0u); EXPECT_EQ(bi->header_for_merge, 0u); EXPECT_EQ(bi->header_for_continue, 0u); - EXPECT_FALSE(bi->is_single_block_loop); + EXPECT_FALSE(bi->is_continue_entire_loop); } TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) { @@ -405,7 +405,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) { EXPECT_EQ(bi10->continue_for_header, 0u); EXPECT_EQ(bi10->header_for_merge, 0u); EXPECT_EQ(bi10->header_for_continue, 0u); - EXPECT_FALSE(bi10->is_single_block_loop); + EXPECT_FALSE(bi10->is_continue_entire_loop); // Middle block is neither header nor merge const auto* bi20 = fe.GetBlockInfo(20); @@ -414,7 +414,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) { EXPECT_EQ(bi20->continue_for_header, 0u); EXPECT_EQ(bi20->header_for_merge, 0u); EXPECT_EQ(bi20->header_for_continue, 0u); - EXPECT_FALSE(bi20->is_single_block_loop); + EXPECT_FALSE(bi20->is_continue_entire_loop); // Merge block points to the header const auto* bi99 = fe.GetBlockInfo(99); @@ -423,7 +423,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_BranchConditional) { EXPECT_EQ(bi99->continue_for_header, 0u); EXPECT_EQ(bi99->header_for_merge, 10u); EXPECT_EQ(bi99->header_for_continue, 0u); - EXPECT_FALSE(bi99->is_single_block_loop); + EXPECT_FALSE(bi99->is_continue_entire_loop); } TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) { @@ -454,7 +454,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) { EXPECT_EQ(bi10->continue_for_header, 0u); EXPECT_EQ(bi10->header_for_merge, 0u); EXPECT_EQ(bi10->header_for_continue, 0u); - EXPECT_FALSE(bi10->is_single_block_loop); + EXPECT_FALSE(bi10->is_continue_entire_loop); // Middle block is neither header nor merge const auto* bi20 = fe.GetBlockInfo(20); @@ -463,7 +463,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) { EXPECT_EQ(bi20->continue_for_header, 0u); EXPECT_EQ(bi20->header_for_merge, 0u); EXPECT_EQ(bi20->header_for_continue, 0u); - EXPECT_FALSE(bi20->is_single_block_loop); + EXPECT_FALSE(bi20->is_continue_entire_loop); // Merge block points to the header const auto* bi99 = fe.GetBlockInfo(99); @@ -472,7 +472,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodSelectionMerge_Switch) { EXPECT_EQ(bi99->continue_for_header, 0u); EXPECT_EQ(bi99->header_for_merge, 10u); EXPECT_EQ(bi99->header_for_continue, 0u); - EXPECT_FALSE(bi99->is_single_block_loop); + EXPECT_FALSE(bi99->is_continue_entire_loop); } TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) { @@ -503,7 +503,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) { EXPECT_EQ(bi10->continue_for_header, 0u); EXPECT_EQ(bi10->header_for_merge, 0u); EXPECT_EQ(bi10->header_for_continue, 0u); - EXPECT_FALSE(bi10->is_single_block_loop); + EXPECT_FALSE(bi10->is_continue_entire_loop); // Single block loop is its own continue, and marked as single block loop. const auto* bi20 = fe.GetBlockInfo(20); @@ -512,7 +512,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) { EXPECT_EQ(bi20->continue_for_header, 20u); EXPECT_EQ(bi20->header_for_merge, 0u); EXPECT_EQ(bi20->header_for_continue, 20u); - EXPECT_TRUE(bi20->is_single_block_loop); + EXPECT_TRUE(bi20->is_continue_entire_loop); // Merge block points to the header const auto* bi99 = fe.GetBlockInfo(99); @@ -521,10 +521,64 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_SingleBlockLoop) { EXPECT_EQ(bi99->continue_for_header, 0u); EXPECT_EQ(bi99->header_for_merge, 20u); EXPECT_EQ(bi99->header_for_continue, 0u); - EXPECT_FALSE(bi99->is_single_block_loop); + EXPECT_FALSE(bi99->is_continue_entire_loop); } -TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) { +TEST_F(SpvParserTest, + RegisterMerges_GoodLoopMerge_MultiBlockLoop_ContinueIsHeader) { + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %20 None + OpBranch %40 + + %40 = OpLabel + OpBranch %20 + + %99 = OpLabel + OpReturn + + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + fe.RegisterBasicBlocks(); + EXPECT_TRUE(fe.RegisterMerges()); + + // Loop header points to continue (itself) and merge + const auto* bi20 = fe.GetBlockInfo(20); + ASSERT_NE(bi20, nullptr); + EXPECT_EQ(bi20->merge_for_header, 99u); + EXPECT_EQ(bi20->continue_for_header, 20u); + EXPECT_EQ(bi20->header_for_merge, 0u); + EXPECT_EQ(bi20->header_for_continue, 20u); + EXPECT_TRUE(bi20->is_continue_entire_loop); + + // Backedge block, but is not a declared header, merge, or continue + const auto* bi40 = fe.GetBlockInfo(40); + ASSERT_NE(bi40, nullptr); + EXPECT_EQ(bi40->merge_for_header, 0u); + EXPECT_EQ(bi40->continue_for_header, 0u); + EXPECT_EQ(bi40->header_for_merge, 0u); + EXPECT_EQ(bi40->header_for_continue, 0u); + EXPECT_FALSE(bi40->is_continue_entire_loop); + + // Merge block points to the header + const auto* bi99 = fe.GetBlockInfo(99); + ASSERT_NE(bi99, nullptr); + EXPECT_EQ(bi99->merge_for_header, 0u); + EXPECT_EQ(bi99->continue_for_header, 0u); + EXPECT_EQ(bi99->header_for_merge, 20u); + EXPECT_EQ(bi99->header_for_continue, 0u); + EXPECT_FALSE(bi99->is_continue_entire_loop); +} + +TEST_F(SpvParserTest, + RegisterMerges_GoodLoopMerge_MultiBlockLoop_ContinueIsNotHeader_Branch) { auto* p = parser(test::Assemble(CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -558,7 +612,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) { EXPECT_EQ(bi20->continue_for_header, 40u); EXPECT_EQ(bi20->header_for_merge, 0u); EXPECT_EQ(bi20->header_for_continue, 0u); - EXPECT_FALSE(bi20->is_single_block_loop); + EXPECT_FALSE(bi20->is_continue_entire_loop); // Continue block points to header const auto* bi40 = fe.GetBlockInfo(40); @@ -567,7 +621,7 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) { EXPECT_EQ(bi40->continue_for_header, 0u); EXPECT_EQ(bi40->header_for_merge, 0u); EXPECT_EQ(bi40->header_for_continue, 20u); - EXPECT_FALSE(bi40->is_single_block_loop); + EXPECT_FALSE(bi40->is_continue_entire_loop); // Merge block points to the header const auto* bi99 = fe.GetBlockInfo(99); @@ -576,11 +630,12 @@ TEST_F(SpvParserTest, RegisterMerges_GoodLoopMerge_MultiBlockLoop_Branch) { EXPECT_EQ(bi99->continue_for_header, 0u); EXPECT_EQ(bi99->header_for_merge, 20u); EXPECT_EQ(bi99->header_for_continue, 0u); - EXPECT_FALSE(bi99->is_single_block_loop); + EXPECT_FALSE(bi99->is_continue_entire_loop); } -TEST_F(SpvParserTest, - RegisterMerges_GoodLoopMerge_MultiBlockLoop_BranchConditional) { +TEST_F( + SpvParserTest, + RegisterMerges_GoodLoopMerge_MultiBlockLoop_ContinueIsNotHeader_BranchConditional) { auto* p = parser(test::Assemble(CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -614,7 +669,7 @@ TEST_F(SpvParserTest, EXPECT_EQ(bi20->continue_for_header, 40u); EXPECT_EQ(bi20->header_for_merge, 0u); EXPECT_EQ(bi20->header_for_continue, 0u); - EXPECT_FALSE(bi20->is_single_block_loop); + EXPECT_FALSE(bi20->is_continue_entire_loop); // Continue block points to header const auto* bi40 = fe.GetBlockInfo(40); @@ -623,7 +678,7 @@ TEST_F(SpvParserTest, EXPECT_EQ(bi40->continue_for_header, 0u); EXPECT_EQ(bi40->header_for_merge, 0u); EXPECT_EQ(bi40->header_for_continue, 20u); - EXPECT_FALSE(bi40->is_single_block_loop); + EXPECT_FALSE(bi40->is_continue_entire_loop); // Merge block points to the header const auto* bi99 = fe.GetBlockInfo(99); @@ -632,7 +687,7 @@ TEST_F(SpvParserTest, EXPECT_EQ(bi99->continue_for_header, 0u); EXPECT_EQ(bi99->header_for_merge, 20u); EXPECT_EQ(bi99->header_for_continue, 0u); - EXPECT_FALSE(bi99->is_single_block_loop); + EXPECT_FALSE(bi99->is_continue_entire_loop); } TEST_F(SpvParserTest, RegisterMerges_SelectionMerge_BadTerminator) { @@ -920,33 +975,6 @@ TEST_F(SpvParserTest, RegisterMerges_SingleBlockLoop_NotItsOwnContinue) { Eq("Block 20 branches to itself but is not its own continue target")); } -TEST_F(SpvParserTest, RegisterMerges_NotSingleBlockLoop_IsItsOwnContinue) { - auto* p = parser(test::Assemble(CommonTypes() + R"( - %100 = OpFunction %void None %voidfn - - %10 = OpLabel - OpBranch %20 - - %20 = OpLabel - OpLoopMerge %99 %20 None - OpBranchConditional %cond %30 %99 - - %30 = OpLabel - OpBranch %20 - - %99 = OpLabel - OpReturn - - OpFunctionEnd - )")); - ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); - FunctionEmitter fe(p, *spirv_function(100)); - fe.RegisterBasicBlocks(); - EXPECT_FALSE(fe.RegisterMerges()); - EXPECT_THAT(p->error(), Eq("Loop header block 20 declares itself as its own " - "continue target, but does not branch to itself")); -} - TEST_F(SpvParserTest, ComputeBlockOrder_OneBlock) { auto* p = parser(test::Assemble(CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -3026,7 +3054,9 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_SingleBlockLoop) { EXPECT_EQ(fe.GetBlockInfo(99)->construct, constructs[0].get()); } -TEST_F(SpvParserTest, LabelControlFlowConstructs_MultiBlockLoop) { +TEST_F(SpvParserTest, + LabelControlFlowConstructs_MultiBlockLoop_HeaderIsNotContinue) { + // In this case, we have a continue construct and a non-empty loop construct. auto assembly = CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -3073,6 +3103,54 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_MultiBlockLoop) { EXPECT_EQ(fe.GetBlockInfo(99)->construct, constructs[0].get()); } +TEST_F(SpvParserTest, + LabelControlFlowConstructs_MultiBlockLoop_HeaderIsContinue) { + // In this case, we have only a continue construct and no loop construct. + auto assembly = CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %20 None + OpBranchConditional %cond %30 %99 + + %30 = OpLabel + OpBranch %40 + + %40 = OpLabel + OpBranch %50 + + %50 = OpLabel + OpBranch %20 + + %99 = OpLabel + OpReturn + + OpFunctionEnd +)"; + auto* p = parser(test::Assemble(assembly)); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + fe.RegisterBasicBlocks(); + fe.ComputeBlockOrderAndPositions(); + fe.RegisterMerges(); + EXPECT_TRUE(fe.LabelControlFlowConstructs()); + const auto& constructs = fe.constructs(); + EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ + Construct{ Function [0,6) begin_id:10 end_id:0 depth:0 parent:null } + Construct{ Continue [1,5) begin_id:20 end_id:99 depth:1 parent:Function@10 in-c:Continue@20 } +})")) << constructs; + // The block records the nearest enclosing construct. + EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[0].get()); + EXPECT_EQ(fe.GetBlockInfo(20)->construct, constructs[1].get()); + EXPECT_EQ(fe.GetBlockInfo(30)->construct, constructs[1].get()); + EXPECT_EQ(fe.GetBlockInfo(40)->construct, constructs[1].get()); + EXPECT_EQ(fe.GetBlockInfo(50)->construct, constructs[1].get()); + EXPECT_EQ(fe.GetBlockInfo(99)->construct, constructs[0].get()); +} + TEST_F(SpvParserTest, LabelControlFlowConstructs_MergeBlockIsAlsoSingleBlockLoop) { auto assembly = CommonTypes() + R"( @@ -4482,8 +4560,9 @@ TEST_F(SpvParserTest, EXPECT_EQ(bi40->succ_edge[20], EdgeKind::kBack); } -TEST_F(SpvParserTest, - ClassifyCFGEdges_BackEdge_MultiBlockLoop_MultiBlockContinueConstruct) { +TEST_F( + SpvParserTest, + ClassifyCFGEdges_BackEdge_MultiBlockLoop_MultiBlockContinueConstruct_ContinueIsNotHeader) { auto assembly = CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -4517,6 +4596,42 @@ TEST_F(SpvParserTest, EXPECT_EQ(bi50->succ_edge[20], EdgeKind::kBack); } +TEST_F( + SpvParserTest, + ClassifyCFGEdges_BackEdge_MultiBlockLoop_MultiBlockContinueConstruct_ContinueIsHeader) { + auto assembly = CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpBranch %20 + + %20 = OpLabel + OpLoopMerge %99 %20 None ; continue target + OpBranch %30 + + %30 = OpLabel + OpBranch %40 + + %40 = OpLabel + OpBranch %50 + + %50 = OpLabel + OpBranchConditional %cond %20 %99 ; good back edge + + %99 = OpLabel ; outer merge + OpReturn +)"; + auto* p = parser(test::Assemble(assembly)); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(FlowClassifyCFGEdges(&fe)) << p->error(); + + auto* bi50 = fe.GetBlockInfo(50); + ASSERT_NE(bi50, nullptr); + EXPECT_EQ(bi50->succ_edge.count(20), 1u); + EXPECT_EQ(bi50->succ_edge[20], EdgeKind::kBack); +} + TEST_F(SpvParserTest, ClassifyCFGEdges_PrematureExitFromContinueConstruct) { auto assembly = CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -8534,6 +8649,63 @@ Return{} )")) << ToString(fe.ast_body()); } +TEST_F(SpvParserTest, EmitBody_Loop_MultiBlockContinueIsEntireLoop) { + // Test case where both branches exit. e.g both go to merge. + auto* p = parser(test::Assemble(CommonTypes() + R"( + %100 = OpFunction %void None %voidfn + + %10 = OpLabel + OpStore %var %uint_0 + OpBranch %20 + + %20 = OpLabel ; its own continue target + OpStore %var %uint_1 + OpLoopMerge %99 %20 None + OpBranch %80 + + %80 = OpLabel + OpStore %var %uint_2 + OpBranchConditional %cond %99 %20 + + %99 = OpLabel + OpStore %var %uint_3 + 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{0} +} +Loop{ + Assignment{ + Identifier{var} + ScalarConstructor{1} + } + Assignment{ + Identifier{var} + ScalarConstructor{2} + } + If{ + ( + ScalarConstructor{false} + ) + { + Break{} + } + } +} +Assignment{ + Identifier{var} + ScalarConstructor{3} +} +Return{} +)")) << ToString(fe.ast_body()); +} + TEST_F(SpvParserTest, EmitBody_Loop_Never) { // Test case where both branches exit. e.g both go to merge. auto* p = parser(test::Assemble(CommonTypes() + R"(