[spirv-reader] Classify kSwitchBreak from deep in control flow

This also refactors break detection.

Bug: tint:3
Change-Id: I3a3e01c8d76d7c6fc2a14b3dbff136acd487e802
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/21220
Reviewed-by: dan sinclair <dsinclair@google.com>

src/reader/spirv/construct.cc

@@ -26,18 +26,36 @@ Construct::Construct(const Construct* the_parent,
                      uint32_t the_begin_pos,
                      uint32_t the_end_pos)
     : parent(the_parent),
+      enclosing_loop(
+          // Compute the enclosing loop construct. Doing this in the
+          // constructor member list lets us make the member const.
+          // Compare parent depth because loop and continue are siblings and
+          // it's incidental which will appear on the stack first.
+          the_kind == kLoop
+              ? this
+              : ((parent && parent->depth < the_depth) ? parent->enclosing_loop
+                                                       : nullptr)),
       enclosing_continue(
           // Compute the enclosing continue construct. Doing this in the
           // constructor member list lets us make the member const.
-          the_kind == kContinue
+          // Compare parent depth because loop and continue are siblings and
+          // it's incidental which will appear on the stack first.
+          the_kind == kContinue ? this
+                                : ((parent && parent->depth < the_depth)
+                                       ? parent->enclosing_continue
+                                       : nullptr)),
+      enclosing_loop_or_continue_or_switch(
+          // Compute the enclosing loop or continue or switch construct.
+          // Doing this in the constructor member list lets us make the
+          // member const.
+          // Compare parent depth because loop and continue are siblings and
+          // it's incidental which will appear on the stack first.
+          (the_kind == kLoop || the_kind == kContinue ||
+           the_kind == kSwitchSelection)
               ? this
-              : (parent ? parent->enclosing_continue : nullptr)),
+              : ((parent && parent->depth < the_depth)
-      enclosing_loop_or_continue(
+                     ? parent->enclosing_loop_or_continue_or_switch
-          // Compute the enclosing loop or continue construct. Doing this
+                     : nullptr)),
-          // in the constructor member list lets us make the member const.
-          (the_kind == kLoop || the_kind == kContinue)
-              ? this
-              : (parent ? parent->enclosing_loop_or_continue : nullptr)),
       depth(the_depth),
       kind(the_kind),
       begin_id(the_begin_id),

src/reader/spirv/construct.h

@@ -68,12 +68,18 @@ struct Construct {
   /// The nearest enclosing construct (other than itself), or nullptr if
   /// this construct represents the entire function.
   const Construct* const parent = nullptr;
-  /// The nearest continue construct, if one exists. A construct encloses
+  /// The nearest enclosing loop construct, if one exists. A construct
-  /// itself.
+  /// encloses itself.
+  const Construct* const enclosing_loop = nullptr;
+  /// The nearest enclosing continue construct, if one exists. A construct
+  /// encloses itself.
   const Construct* const enclosing_continue = nullptr;
-  /// The nearest enclosing loop or continue construct, if one exists.
+  /// The nearest enclosing loop construct or continue construct or
+  /// switch-selection construct, if one exists. The signficance is
+  /// that a high level language "break" will branch to the merge block
+  /// of such an enclosing construct.
   /// A construct encloses itself.
-  const Construct* const enclosing_loop_or_continue = nullptr;
+  const Construct* const enclosing_loop_or_continue_or_switch = nullptr;
 
   /// Control flow nesting depth. The entry block is at nesting depth 0.
   const int depth = 0;
@@ -136,12 +142,17 @@ inline std::ostream& operator<<(std::ostream& o, const Construct& c) {
 
   o << " parent:" << ToStringBrief(c.parent);
 
+  if (c.enclosing_loop) {
+    o << " in-l:" << ToStringBrief(c.enclosing_loop);
+  }
+
   if (c.enclosing_continue) {
     o << " in-c:" << ToStringBrief(c.enclosing_continue);
   }
 
-  if (c.enclosing_loop_or_continue != c.enclosing_continue) {
+  if ((c.enclosing_loop_or_continue_or_switch != c.enclosing_loop) &&
-    o << " in-c-l:" << ToStringBrief(c.enclosing_loop_or_continue);
+      (c.enclosing_loop_or_continue_or_switch != c.enclosing_continue)) {
+    o << " in-c-l-s:" << ToStringBrief(c.enclosing_loop_or_continue_or_switch);
   }
 
   o << " }";

src/reader/spirv/function.cc

@@ -927,13 +927,20 @@ bool FunctionEmitter::FindSwitchCaseHeaders() {
   return success();
 }
 
-BlockInfo* FunctionEmitter::HeaderForLoopOrContinue(const Construct* c) {
+BlockInfo* FunctionEmitter::HeaderIfBreakable(const Construct* c) {
-  if (c->kind == Construct::kLoop) {
+  if (c == nullptr) {
-    return GetBlockInfo(c->begin_id);
+    return nullptr;
   }
-  if (c->kind == Construct::kContinue) {
+  switch (c->kind) {
-    auto* continue_block = GetBlockInfo(c->begin_id);
+    case Construct::kLoop:
-    return GetBlockInfo(continue_block->header_for_continue);
+    case Construct::kSwitchSelection:
+      return GetBlockInfo(c->begin_id);
+    case Construct::kContinue: {
+      const auto* continue_target = GetBlockInfo(c->begin_id);
+      return GetBlockInfo(continue_target->header_for_continue);
+    }
+    default:
+      break;
   }
   return nullptr;
 }
@@ -1051,44 +1058,42 @@ bool FunctionEmitter::ClassifyCFGEdges() {
 
         // Check valid structured exit cases.
 
-        const auto& header_info = *GetBlockInfo(src_construct.begin_id);
+        if (edge_kind == EdgeKind::kForward) {
-        if (dest == header_info.merge_for_header) {
+          // Check for a 'break' from a loop or from a switch.
-          // Branch to construct's merge block.
+          const auto* breakable_header = HeaderIfBreakable(
-          const bool src_is_loop_header = header_info.continue_for_header != 0;
+              src_construct.enclosing_loop_or_continue_or_switch);
-          const bool src_is_continue_header =
+          if (breakable_header != nullptr) {
-              header_info.header_for_continue != 0;
+            if (dest == breakable_header->merge_for_header) {
-          if (src_is_loop_header || src_is_continue_header) {
+              // It's a break.
-            edge_kind = EdgeKind::kLoopBreak;
+              edge_kind = (breakable_header->construct->kind ==
-          } else if (src_construct.kind == Construct::kSwitchSelection) {
+                           Construct::kSwitchSelection)
-            edge_kind = EdgeKind::kSwitchBreak;
+                              ? EdgeKind::kSwitchBreak
-          } else {
+                              : EdgeKind::kLoopBreak;
-            edge_kind = EdgeKind::kToMerge;
-          }
-        } else {
-          const auto* loop_or_continue_construct =
-              src_construct.enclosing_loop_or_continue;
-          if (loop_or_continue_construct != nullptr) {
-            // Check for break block or continue block.
-            const auto* loop_header_info =
-                HeaderForLoopOrContinue(loop_or_continue_construct);
-            if (loop_header_info == nullptr) {
-              return Fail() << "internal error: invalid construction of loop "
-                               "related to block "
-                            << loop_or_continue_construct->begin_id;
             }
-            if (dest == loop_header_info->merge_for_header) {
+          }
-              // It's a break block for the innermost loop.
+        }
-              edge_kind = EdgeKind::kLoopBreak;
+
-            } else if (dest == loop_header_info->continue_for_header) {
+        if (edge_kind == EdgeKind::kForward) {
-              // It's a continue block for the innermost loop construct.
+          // Check for a 'continue' from within a loop.
-              // In this case loop_or_continue_construct can't be a continue
+          const auto* loop_header =
-              // construct, because then the branch to the continue target is
+              HeaderIfBreakable(src_construct.enclosing_loop);
-              // a backedge, and this code is only looking at forward edges.
+          if (loop_header != nullptr) {
+            if (dest == loop_header->continue_for_header) {
+              // It's a continue.
               edge_kind = EdgeKind::kLoopContinue;
             }
           }
         }
 
+        if (edge_kind == EdgeKind::kForward) {
+          const auto& header_info = *GetBlockInfo(src_construct.begin_id);
+          if (dest == header_info.merge_for_header) {
+            // Branch to construct's merge block.  The loop break and
+            // switch break cases have already been covered.
+            edge_kind = EdgeKind::kToMerge;
+          }
+        }
+
         // A forward edge into a case construct that comes from something
         // other than the OpSwitch is actually a fallthrough.
         if (edge_kind == EdgeKind::kForward) {

src/reader/spirv/function.h

@@ -289,12 +289,17 @@ class FunctionEmitter {
   ast::type::Type* GetVariableStoreType(
       const spvtools::opt::Instruction& var_decl_inst);
 
-  /// Finds the loop header block for a loop construct or continue construct.
+  /// Finds the header block for a structured construct that we can "break"
-  /// The loop header block is the block with the corresponding OpLoopMerge
+  /// out from, from deeply nested control flow, if such a block exists.
-  /// instruction.
+  /// If the construct is:
-  /// @param c the loop or continue construct
+  ///  - a switch selection: return the selection header (ending in OpSwitch)
-  /// @returns the block info for the loop header.
+  ///  - a loop construct: return the loop header block
-  BlockInfo* HeaderForLoopOrContinue(const Construct* c);
+  ///  - a continue construct: return the loop header block
+  /// Otherwise, return nullptr.
+  /// @param c a structured construct, or nullptr
+  /// @returns the block info for the structured header we can "break" from,
+  /// or nullptr
+  BlockInfo* HeaderIfBreakable(const Construct* c);
 
   ParserImpl& parser_impl_;
   ast::Module& ast_module_;

src/reader/spirv/function_cfg_test.cc

@@ -2938,7 +2938,7 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_SwitchSelection) {
   EXPECT_EQ(constructs.size(), 2u);
   EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
   Construct{ Function [0,5) begin_id:10 end_id:0 depth:0 parent:null }
-  Construct{ SwitchSelection [0,4) begin_id:10 end_id:99 depth:1 parent:Function@10 }
+  Construct{ SwitchSelection [0,4) begin_id:10 end_id:99 depth:1 parent:Function@10 in-c-l-s:SwitchSelection@10 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());
@@ -3018,12 +3018,10 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_MultiBlockLoop) {
   fe.RegisterMerges();
   EXPECT_TRUE(fe.LabelControlFlowConstructs());
   const auto& constructs = fe.constructs();
-  // A single-block loop consists *only* of a continue target with one block in
-  // it.
   EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
   Construct{ Function [0,6) begin_id:10 end_id:0 depth:0 parent:null }
   Construct{ Continue [3,5) begin_id:40 end_id:99 depth:1 parent:Function@10 in-c:Continue@40 }
-  Construct{ Loop [1,3) begin_id:20 end_id:40 depth:1 parent:Function@10 in-c-l:Loop@20 }
+  Construct{ Loop [1,3) begin_id:20 end_id:40 depth:1 parent:Function@10 in-l:Loop@20 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[0].get());
@@ -3119,7 +3117,7 @@ TEST_F(SpvParserTest,
   Construct{ Function [0,5) begin_id:10 end_id:0 depth:0 parent:null }
   Construct{ IfSelection [0,2) begin_id:10 end_id:50 depth:1 parent:Function@10 }
   Construct{ Continue [3,4) begin_id:60 end_id:99 depth:1 parent:Function@10 in-c:Continue@60 }
-  Construct{ Loop [2,3) begin_id:50 end_id:60 depth:1 parent:Function@10 in-c-l:Loop@50 }
+  Construct{ Loop [2,3) begin_id:50 end_id:60 depth:1 parent:Function@10 in-l:Loop@50 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());
@@ -3237,9 +3235,9 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_Switch_If) {
   // The ordering among siblings depends on the computed block order.
   EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
   Construct{ Function [0,8) begin_id:10 end_id:0 depth:0 parent:null }
-  Construct{ SwitchSelection [0,7) begin_id:10 end_id:99 depth:1 parent:Function@10 }
+  Construct{ SwitchSelection [0,7) begin_id:10 end_id:99 depth:1 parent:Function@10 in-c-l-s:SwitchSelection@10 }
-  Construct{ IfSelection [1,3) begin_id:50 end_id:89 depth:2 parent:SwitchSelection@10 }
+  Construct{ IfSelection [1,3) begin_id:50 end_id:89 depth:2 parent:SwitchSelection@10 in-c-l-s:SwitchSelection@10 }
-  Construct{ IfSelection [4,6) begin_id:20 end_id:49 depth:2 parent:SwitchSelection@10 }
+  Construct{ IfSelection [4,6) begin_id:20 end_id:49 depth:2 parent:SwitchSelection@10 in-c-l-s:SwitchSelection@10 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());
@@ -3287,7 +3285,7 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_If_Switch) {
   EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
   Construct{ Function [0,5) begin_id:10 end_id:0 depth:0 parent:null }
   Construct{ IfSelection [0,4) begin_id:10 end_id:99 depth:1 parent:Function@10 }
-  Construct{ SwitchSelection [1,3) begin_id:20 end_id:89 depth:2 parent:IfSelection@10 }
+  Construct{ SwitchSelection [1,3) begin_id:20 end_id:89 depth:2 parent:IfSelection@10 in-c-l-s:SwitchSelection@20 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());
@@ -3341,8 +3339,8 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_Loop_Loop) {
   EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
   Construct{ Function [0,8) begin_id:10 end_id:0 depth:0 parent:null }
   Construct{ Continue [4,6) begin_id:50 end_id:89 depth:1 parent:Function@10 in-c:Continue@50 }
-  Construct{ Loop [1,4) begin_id:20 end_id:50 depth:1 parent:Function@10 in-c-l:Loop@20 }
+  Construct{ Loop [1,4) begin_id:20 end_id:50 depth:1 parent:Function@10 in-l:Loop@20 }
-  Construct{ Continue [2,3) begin_id:30 end_id:40 depth:2 parent:Loop@20 in-c:Continue@30 }
+  Construct{ Continue [2,3) begin_id:30 end_id:40 depth:2 parent:Loop@20 in-l:Loop@20 in-c:Continue@30 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[0].get());
@@ -3396,8 +3394,8 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_Loop_If) {
   EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
   Construct{ Function [0,7) begin_id:10 end_id:0 depth:0 parent:null }
   Construct{ Continue [5,6) begin_id:80 end_id:99 depth:1 parent:Function@10 in-c:Continue@80 }
-  Construct{ Loop [1,5) begin_id:20 end_id:80 depth:1 parent:Function@10 in-c-l:Loop@20 }
+  Construct{ Loop [1,5) begin_id:20 end_id:80 depth:1 parent:Function@10 in-l:Loop@20 }
-  Construct{ IfSelection [2,4) begin_id:30 end_id:49 depth:2 parent:Loop@20 in-c-l:Loop@20 }
+  Construct{ IfSelection [2,4) begin_id:30 end_id:49 depth:2 parent:Loop@20 in-l:Loop@20 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[0].get());
@@ -3447,7 +3445,7 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_LoopContinue_If) {
   EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{
   Construct{ Function [0,6) begin_id:10 end_id:0 depth:0 parent:null }
   Construct{ Continue [2,5) begin_id:30 end_id:99 depth:1 parent:Function@10 in-c:Continue@30 }
-  Construct{ Loop [1,2) begin_id:20 end_id:30 depth:1 parent:Function@10 in-c-l:Loop@20 }
+  Construct{ Loop [1,2) begin_id:20 end_id:30 depth:1 parent:Function@10 in-l:Loop@20 }
   Construct{ IfSelection [2,4) begin_id:30 end_id:49 depth:2 parent:Continue@30 in-c:Continue@30 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
@@ -3541,7 +3539,7 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_If_MultiBlockLoop) {
   Construct{ Function [0,7) begin_id:10 end_id:0 depth:0 parent:null }
   Construct{ IfSelection [0,6) begin_id:10 end_id:99 depth:1 parent:Function@10 }
   Construct{ Continue [3,5) begin_id:40 end_id:89 depth:2 parent:IfSelection@10 in-c:Continue@40 }
-  Construct{ Loop [1,3) begin_id:20 end_id:40 depth:2 parent:IfSelection@10 in-c-l:Loop@20 }
+  Construct{ Loop [1,3) begin_id:20 end_id:40 depth:2 parent:IfSelection@10 in-l:Loop@20 }
 })")) << constructs;
   // The block records the nearest enclosing construct.
   EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());
@@ -4715,6 +4713,135 @@ TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromSwitchDefaultIsMerge) {
   EXPECT_EQ(bi->succ_edge[99], EdgeKind::kSwitchBreak);
 }
 
+TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromNestedIf_Unconditional) {
+  auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpSelectionMerge %99 None
+     OpSwitch %selector %99 20 %20
+
+     %20 = OpLabel
+     OpSelectionMerge %80 None
+     OpBranchConditional %cond %30 %80
+
+     %30 = OpLabel
+     OpBranch %99
+
+     %80 = OpLabel ; inner merge
+     OpBranch %99
+
+     %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));
+
+  auto* bi = fe.GetBlockInfo(30);
+  ASSERT_NE(bi, nullptr);
+  EXPECT_EQ(bi->succ_edge.count(99), 1);
+  EXPECT_EQ(bi->succ_edge[99], EdgeKind::kSwitchBreak);
+}
+
+TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromNestedIf_Conditional) {
+  auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpSelectionMerge %99 None
+     OpSwitch %selector %99 20 %20
+
+     %20 = OpLabel
+     OpSelectionMerge %80 None
+     OpBranchConditional %cond %30 %80
+
+     %30 = OpLabel
+     OpBranchConditional %cond2 %99 %80 ; break-if
+
+     %80 = OpLabel ; inner merge
+     OpBranch %99
+
+     %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));
+
+  auto* bi = fe.GetBlockInfo(30);
+  ASSERT_NE(bi, nullptr);
+  EXPECT_EQ(bi->succ_edge.count(99), 1);
+  EXPECT_EQ(bi->succ_edge[99], EdgeKind::kSwitchBreak);
+}
+
+TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromNestedLoop_IsError) {
+  // It's an error because the break can only go as far as the loop.
+  auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpSelectionMerge %99 None
+     OpSwitch %selector %99 20 %20
+
+     %20 = OpLabel
+     OpLoopMerge %80 %70 None
+     OpBranchConditional %cond %30 %80
+
+     %30 = OpLabel ; in loop construct
+     OpBranch %99 ; break
+
+     %70 = OpLabel
+     OpBranch %20
+
+     %80 = OpLabel
+     OpBranch %99
+
+     %99 = OpLabel ; outer merge
+     OpReturn
+)";
+  auto* p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
+  EXPECT_THAT(p->error(),
+              Eq("Branch from block 30 to block 99 is an invalid exit from "
+                 "construct starting at block 20; branch bypasses block 70"));
+}
+
+TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromNestedSwitch_IsError) {
+  // It's an error because the break can only go as far as inner switch
+  auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpSelectionMerge %99 None
+     OpSwitch %selector %99 20 %20
+
+     %20 = OpLabel
+     OpSelectionMerge %80 None
+     OpSwitch %selector %80 30 %30
+
+     %30 = OpLabel
+     OpBranch %99 ; break
+
+     %80 = OpLabel
+     OpBranch %99
+
+     %99 = OpLabel ; outer merge
+     OpReturn
+)";
+  auto* p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
+  EXPECT_THAT(p->error(),
+              Eq("Branch from block 30 to block 99 is an invalid exit from "
+                 "construct starting at block 20; branch bypasses block 80"));
+}
+
 TEST_F(SpvParserTest, ClassifyCFGEdges_LoopBreak_FromLoopBody) {
   auto assembly = CommonTypes() + R"(
      %100 = OpFunction %void None %voidfn
@@ -4811,6 +4938,125 @@ TEST_F(SpvParserTest, ClassifyCFGEdges_LoopBreak_FromLoopBodyDirect) {
   EXPECT_EQ(bi->succ_edge[99], EdgeKind::kLoopBreak);
 }
 
+TEST_F(SpvParserTest,
+       ClassifyCFGEdges_LoopBreak_FromLoopBodyNestedSelection_Unconditional) {
+  auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpBranch %20
+
+     %20 = OpLabel
+     OpLoopMerge %99 %80 None
+     OpBranchConditional %cond %30 %99
+
+     %30 = OpLabel
+     OpSelectionMerge %50 None
+     OpBranchConditional %cond2 %40 %50
+
+     %40 = OpLabel
+     OpBranch %99 ; deeply nested break
+
+     %50 = OpLabel ; inner merge
+     OpBranch %80
+
+     %80 = OpLabel
+     OpBranch %20  ; backedge
+
+     %99 = OpLabel
+     OpReturn
+)";
+  auto* p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_TRUE(FlowClassifyCFGEdges(&fe));
+
+  auto* bi = fe.GetBlockInfo(40);
+  ASSERT_NE(bi, nullptr);
+  EXPECT_EQ(bi->succ_edge.count(99), 1);
+  EXPECT_EQ(bi->succ_edge[99], EdgeKind::kLoopBreak);
+}
+
+TEST_F(SpvParserTest,
+       ClassifyCFGEdges_LoopBreak_FromLoopBodyNestedSelection_Conditional) {
+  auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpBranch %20
+
+     %20 = OpLabel
+     OpLoopMerge %99 %80 None
+     OpBranchConditional %cond %30 %99
+
+     %30 = OpLabel
+     OpSelectionMerge %50 None
+     OpBranchConditional %cond2 %40 %50
+
+     %40 = OpLabel
+     OpBranchConditional %cond3 %99 %50 ; break-if
+
+     %50 = OpLabel ; inner merge
+     OpBranch %80
+
+     %80 = OpLabel
+     OpBranch %20  ; backedge
+
+     %99 = OpLabel
+     OpReturn
+)";
+  auto* p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_TRUE(FlowClassifyCFGEdges(&fe));
+
+  auto* bi = fe.GetBlockInfo(40);
+  ASSERT_NE(bi, nullptr);
+  EXPECT_EQ(bi->succ_edge.count(99), 1);
+  EXPECT_EQ(bi->succ_edge[99], EdgeKind::kLoopBreak);
+}
+
+TEST_F(SpvParserTest,
+       ClassifyCFGEdges_LoopBreak_FromContinueConstructNestedFlow_IsError) {
+  auto assembly = CommonTypes() + R"(
+     %100 = OpFunction %void None %voidfn
+
+     %10 = OpLabel
+     OpBranch %20
+
+     %20 = OpLabel
+     OpLoopMerge %99 %40 None
+     OpBranchConditional %cond %30 %99
+
+     %30 = OpLabel
+     OpBranch %40
+
+     %40 = OpLabel ; continue construct
+     OpSelectionMerge %79 None
+     OpBranchConditional %cond2 %50 %79
+
+     %50 = OpLabel
+     OpBranchConditional %cond3 %99 %79 ; attempt to break to 99 should fail
+
+     %79 = OpLabel
+     OpBranch %80  ; inner merge
+
+     %80 = OpLabel
+     OpBranch %20  ; backedge
+
+     %99 = OpLabel
+     OpReturn
+)";
+  auto* p = parser(test::Assemble(assembly));
+  ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
+  FunctionEmitter fe(p, *spirv_function(100));
+  EXPECT_FALSE(FlowClassifyCFGEdges(&fe));
+  EXPECT_THAT(p->error(),
+              Eq("Invalid exit (50->99) from continue construct: 50 is not the "
+                 "last block in the continue construct starting at 40 "
+                 "(violates post-dominance rule)"));
+}
+
 TEST_F(SpvParserTest, ClassifyCFGEdges_LoopContinue_LoopBodyToContinue) {
   auto assembly = CommonTypes() + R"(
      %100 = OpFunction %void None %voidfn
@@ -4949,7 +5195,7 @@ TEST_F(SpvParserTest,
   auto* p = parser(test::Assemble(assembly));
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
   FunctionEmitter fe(p, *spirv_function(100));
-  EXPECT_TRUE(FlowClassifyCFGEdges(&fe));
+  EXPECT_TRUE(FlowClassifyCFGEdges(&fe)) << p->error();
 
   auto* bi = fe.GetBlockInfo(40);
   ASSERT_NE(bi, nullptr);
@@ -5064,7 +5310,7 @@ TEST_F(SpvParserTest,
   auto* p = parser(test::Assemble(assembly));
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error();
   FunctionEmitter fe(p, *spirv_function(100));
-  EXPECT_TRUE(FlowClassifyCFGEdges(&fe));
+  EXPECT_TRUE(FlowClassifyCFGEdges(&fe)) << p->error();
 
   auto* bi = fe.GetBlockInfo(40);
   ASSERT_NE(bi, nullptr);