diff --git a/src/reader/spirv/construct.h b/src/reader/spirv/construct.h index 55ea5fcccf..2437e4ee79 100644 --- a/src/reader/spirv/construct.h +++ b/src/reader/spirv/construct.h @@ -25,21 +25,61 @@ namespace tint { namespace reader { namespace spirv { -/// A structured construct, consisting of a set of basic blocks. +/// A structured control flow construct, consisting of a set of basic blocks. /// A construct is a span of blocks in the computed block order, /// and will appear contiguously in the WGSL source. +/// +/// SPIR-V (2.11 Structured Control Flow) defines: +/// - loop construct +/// - continue construct +/// - selection construct +/// We also define a "function construct" consisting of all the basic blocks in +/// the function. +/// +/// The first block in a construct (by computed block order) is called a +/// "header". For the constructs defined by SPIR-V, the header block is the +/// basic block containing the merge instruction. The header for the function +/// construct is the entry block of the function. +/// +/// Given two constructs A and B, we say "A encloses B" if B is a subset of A, +/// i.e. if every basic block in B is also in A. Note that a construct encloses +/// itself. +/// +/// In a valid SPIR-V module, constructs will nest, meaning given +/// constructs A and B, either A encloses B, or B encloses A, or +/// or they are disjoint (have no basic blocks in commont). +/// +/// A loop in a high level language translates into either: +// +/// - a single-block loop, where the loop header branches back to itself. +/// In this case this single-block loop consists only of the *continue +/// construct*. There is no "loop construct" for this case. +// +/// - a multi-block loop, where the loop back-edge is different from the loop +/// header. +/// This case has both a non-empty loop construct containing at least the +/// loop header, and a non-empty continue construct, containing at least the +/// back-edge block. +/// +/// We care about two kinds of selection constructs: +/// +/// - if-selection: where the header block ends in OpBranchConditional +/// +/// - switch-selection: where the header block ends in OpSwitch +/// struct Construct { /// Enumeration for the kinds of structured constructs. enum Kind { /// The whole function. kFunction, - /// A SPIR-V selection construct, header ending in OpBrancConditional + /// A SPIR-V selection construct, header basic block ending in + /// OpBrancConditional. kIfSelection, - /// A SPIR-V selection construct, header ending in OpSwitch + /// A SPIR-V selection construct, header basic block ending in OpSwitch. kSwitchSelection, - /// A SPIR-V loop construct + /// A SPIR-V loop construct. kLoop, - /// A SPIR-V continue construct + /// A SPIR-V continue construct. kContinue, }; @@ -65,20 +105,20 @@ struct Construct { return begin_pos <= pos && pos < end_pos; } - /// The nearest enclosing construct (other than itself), or nullptr if + /// The nearest enclosing construct other than itself, or nullptr if /// this construct represents the entire function. const Construct* const parent = nullptr; - /// The nearest enclosing loop construct, if one exists. A construct - /// encloses itself. + /// The nearest enclosing loop construct, if one exists. Points to |this| + /// when this is a loop construct. const Construct* const enclosing_loop = nullptr; - /// The nearest enclosing continue construct, if one exists. A construct - /// encloses itself. + /// The nearest enclosing continue construct, if one exists. Points to + /// |this| when this is a contnue construct. const Construct* const enclosing_continue = nullptr; /// 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. + /// of such an enclosing construct. Points to |this| when this is + /// a loop construct, a continue construct, or a switch-selection construct. const Construct* const enclosing_loop_or_continue_or_switch = nullptr; /// Control flow nesting depth. The entry block is at nesting depth 0. diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 2176aaee20..b3756b6c2b 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -1090,7 +1090,7 @@ bool FunctionEmitter::ClassifyCFGEdges() { 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; + edge_kind = EdgeKind::kIfBreak; } } diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index 51c32c5be1..00a8641496 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -58,9 +58,7 @@ enum class EdgeKind { // construct, but which is neither a kSwitchBreak or a kLoopBreak. // This can only occur for an "if" selection, i.e. where the selection // header ends in OpBranchConditional. - // TODO(dneto): Rename this to kIfBreak after we correctly classify edges - // as kSwitchBreak. - kToMerge, + kIfBreak, // An edge from one switch case to the next sibling switch case. kCaseFallThrough, // None of the above. By structured control flow rules, there can be at most diff --git a/src/reader/spirv/function_cfg_test.cc b/src/reader/spirv/function_cfg_test.cc index 2db7687e23..8fe36fe33b 100644 --- a/src/reader/spirv/function_cfg_test.cc +++ b/src/reader/spirv/function_cfg_test.cc @@ -4550,7 +4550,7 @@ TEST_F(SpvParserTest, ClassifyCFGEdges_LoopBreak_FromContinueConstructHeader) { EXPECT_EQ(bi->succ_edge[99], EdgeKind::kLoopBreak); } -TEST_F(SpvParserTest, ClassifyCFGEdges_ToMerge_FromIfHeader) { +TEST_F(SpvParserTest, ClassifyCFGEdges_IfBreak_FromIfHeader) { auto assembly = CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -4571,11 +4571,11 @@ TEST_F(SpvParserTest, ClassifyCFGEdges_ToMerge_FromIfHeader) { auto* bi = fe.GetBlockInfo(20); ASSERT_NE(bi, nullptr); - EXPECT_EQ(bi->succ_edge.count(99), 1u); - EXPECT_EQ(bi->succ_edge[99], EdgeKind::kToMerge); + EXPECT_EQ(bi->succ_edge.count(99), 1); + EXPECT_EQ(bi->succ_edge[99], EdgeKind::kIfBreak); } -TEST_F(SpvParserTest, ClassifyCFGEdges_ToMerge_FromIfThenElse) { +TEST_F(SpvParserTest, ClassifyCFGEdges_IfBreak_FromIfThenElse) { auto assembly = CommonTypes() + R"( %100 = OpFunction %void None %voidfn @@ -4600,14 +4600,14 @@ TEST_F(SpvParserTest, ClassifyCFGEdges_ToMerge_FromIfThenElse) { // Then clause auto* bi20 = fe.GetBlockInfo(20); ASSERT_NE(bi20, nullptr); - EXPECT_EQ(bi20->succ_edge.count(99), 1u); - EXPECT_EQ(bi20->succ_edge[99], EdgeKind::kToMerge); + EXPECT_EQ(bi20->succ_edge.count(99), 1); + EXPECT_EQ(bi20->succ_edge[99], EdgeKind::kIfBreak); // Else clause auto* bi50 = fe.GetBlockInfo(50); ASSERT_NE(bi50, nullptr); - EXPECT_EQ(bi50->succ_edge.count(99), 1u); - EXPECT_EQ(bi50->succ_edge[99], EdgeKind::kToMerge); + EXPECT_EQ(bi50->succ_edge.count(99), 1); + EXPECT_EQ(bi50->succ_edge[99], EdgeKind::kIfBreak); } TEST_F(SpvParserTest, ClassifyCFGEdges_SwitchBreak_FromSwitchCaseDirect) { @@ -6011,8 +6011,8 @@ TEST_F(SpvParserTest, ClassifyCFGEdges_Pathological_Forward_Premerge) { auto* bi60 = fe.GetBlockInfo(60); ASSERT_NE(bi60, nullptr); - EXPECT_EQ(bi60->succ_edge.count(99), 1u); - EXPECT_EQ(bi60->succ_edge[99], EdgeKind::kToMerge); + EXPECT_EQ(bi60->succ_edge.count(99), 1); + EXPECT_EQ(bi60->succ_edge[99], EdgeKind::kIfBreak); } TEST_F(SpvParserTest, ClassifyCFGEdges_Pathological_Forward_Regardless) { @@ -6042,8 +6042,8 @@ TEST_F(SpvParserTest, ClassifyCFGEdges_Pathological_Forward_Regardless) { auto* bi20 = fe.GetBlockInfo(20); ASSERT_NE(bi20, nullptr); - EXPECT_EQ(bi20->succ_edge.count(99), 1u); - EXPECT_EQ(bi20->succ_edge[99], EdgeKind::kToMerge); + EXPECT_EQ(bi20->succ_edge.count(99), 1); + EXPECT_EQ(bi20->succ_edge[99], EdgeKind::kIfBreak); } } // namespace