[spirv-reader] Allow multi-block continue construct being the entire loop

The special case is not "single-block-loop" but rather the
case where a continue target is also its own loop header.
This can occur for single-block loops and multi-block loops.

Bug: tint:3
Change-Id: I4af1410793caf8b26a1f781e221fc0b395f07aa3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/24320
Reviewed-by: dan sinclair <dsinclair@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index f8360b9..dad106c 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 @@
           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 @@
   //      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 @@
     // 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 @@
   //           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 @@
         // 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 @@
   // - 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 @@
   //   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 @@
         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 068657a..7044b0f 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -108,9 +108,10 @@
   /// 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 @@
     << " 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 @@
   /// @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 bb73621..d245427 100644
--- a/src/reader/spirv/function_cfg_test.cc
+++ b/src/reader/spirv/function_cfg_test.cc
@@ -374,7 +374,7 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
       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 @@
   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
 
@@ -3074,6 +3104,54 @@
 }
 
 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"(
      %100 = OpFunction %void None %voidfn
@@ -4482,8 +4560,9 @@
   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 @@
   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 @@
 )")) << 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"(