From 920bdcd0aabccc6cd281cc32efacd5eb83bc102d Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 6 May 2020 18:32:29 +0000 Subject: [PATCH] [spirv-reader] Refine selection construct concept Distinguish between selections constructs starting with with OpBranchConditional and those starting with OpSwitch. We'll use this in a followup CL to track break from a switch. Bug: tint:3 Change-Id: I8d000cb42325535a4937c84f83a83c98a9b8d4c5 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/21080 Reviewed-by: dan sinclair --- src/reader/spirv/construct.h | 20 ++++++++++---- src/reader/spirv/function.cc | 12 ++++---- src/reader/spirv/function.h | 17 ++++++++---- src/reader/spirv/function_cfg_test.cc | 40 +++++++++++++-------------- 4 files changed, 53 insertions(+), 36 deletions(-) diff --git a/src/reader/spirv/construct.h b/src/reader/spirv/construct.h index f458868bf5..a63437d73a 100644 --- a/src/reader/spirv/construct.h +++ b/src/reader/spirv/construct.h @@ -31,10 +31,16 @@ namespace spirv { struct Construct { /// Enumeration for the kinds of structured constructs. enum Kind { - kFunction, // The whole function. - kSelection, // A SPIR-V selection construct - kLoop, // A SPIR-V loop construct - kContinue, // A SPIR-V continue construct + /// The whole function. + kFunction, + /// A SPIR-V selection construct, header ending in OpBrancConditional + kIfSelection, + /// A SPIR-V selection construct, header ending in OpSwitch + kSwitchSelection, + /// A SPIR-V loop construct + kLoop, + /// A SPIR-V continue construct + kContinue, }; /// Constructor @@ -94,8 +100,10 @@ inline std::string ToString(Construct::Kind kind) { switch (kind) { case Construct::kFunction: return "Function"; - case Construct::kSelection: - return "Selection"; + case Construct::kIfSelection: + return "IfSelection"; + case Construct::kSwitchSelection: + return "SwitchSelection"; case Construct::kLoop: return "Loop"; case Construct::kContinue: diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 7d239230f7..0c20f730c5 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -791,7 +791,12 @@ bool FunctionEmitter::LabelControlFlowConstructs() { // From the interval rule, the selection construct consists of blocks // in the block order, starting at the header, until just before the // merge block. - top = push_construct(depth, Construct::kSelection, header, merge); + const auto branch_opcode = + header_info->basic_block->terminator()->opcode(); + const auto kind = (branch_opcode == SpvOpBranchConditional) + ? Construct::kIfSelection + : Construct::kSwitchSelection; + top = push_construct(depth, kind, header, merge); } } @@ -819,14 +824,11 @@ bool FunctionEmitter::FindSwitchCaseHeaders() { return false; } for (auto& construct : constructs_) { - if (construct->kind != Construct::kSelection) { + if (construct->kind != Construct::kSwitchSelection) { continue; } const auto* branch = GetBlockInfo(construct->begin_id)->basic_block->terminator(); - if (branch->opcode() != SpvOpSwitch) { - continue; - } // Mark the default block const auto default_id = branch->GetSingleWordInOperand(1); diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index e5bf677503..72fb2ab245 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -43,17 +43,24 @@ enum class EdgeKind { // A back-edge: An edge from a node to one of its ancestors in a depth-first // search from the entry block. kBack, - // An edge from a node to the merge block of the nearest enclosing structured - // construct. We write "ToMerge" to make it clear the destination block is - // the merge block, not the source block. - kToMerge, - // An edge from a node to the merge block of the nearest enclosing loop. + // An edge from a node to the merge block of the nearest enclosing switch, + // where there is no intervening loop. + kSwitchBreak, + // An edge from a node to the merge block of the nearest enclosing loop, where + // there is no intervening switch. // The source block is a "break block" as defined by SPIR-V. kLoopBreak, // An edge from a node in a loop body to the associated continue target, where // there are no other intervening loops or switches. // The source block is a "continue block" as defined by SPIR-V. kLoopContinue, + // An edge from a node to the merge block of the nearest enclosing structured + // 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, // 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 943045696e..5205d8b322 100644 --- a/src/reader/spirv/function_cfg_test.cc +++ b/src/reader/spirv/function_cfg_test.cc @@ -2847,7 +2847,7 @@ TEST_F(SpvParserTest, EXPECT_EQ(constructs.size(), 2u); EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,4) begin_id:10 end_id:0 depth:0 parent:null } - Construct{ Selection [0,3) begin_id:10 end_id:99 depth:1 parent:Function@10 } + Construct{ IfSelection [0,3) begin_id:10 end_id:99 depth:1 parent:Function@10 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get()); @@ -2894,7 +2894,7 @@ TEST_F( EXPECT_EQ(constructs.size(), 2u); EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,6) begin_id:5 end_id:0 depth:0 parent:null } - Construct{ Selection [1,4) begin_id:10 end_id:99 depth:1 parent:Function@5 } + Construct{ IfSelection [1,4) begin_id:10 end_id:99 depth:1 parent:Function@5 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(5)->construct, constructs[0].get()); @@ -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{ Selection [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 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get()); @@ -3070,7 +3070,7 @@ TEST_F(SpvParserTest, // it. EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,4) begin_id:10 end_id:0 depth:0 parent:null } - Construct{ Selection [0,2) begin_id:10 end_id:50 depth:1 parent:Function@10 } + Construct{ IfSelection [0,2) begin_id:10 end_id:50 depth:1 parent:Function@10 } Construct{ Continue [2,3) begin_id:50 end_id:99 depth:1 parent:Function@10 in-c:Continue@50 } })")) << constructs; // The block records the nearest enclosing construct. @@ -3117,7 +3117,7 @@ TEST_F(SpvParserTest, EXPECT_EQ(constructs.size(), 4u); EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,5) begin_id:10 end_id:0 depth:0 parent:null } - Construct{ Selection [0,2) begin_id:10 end_id:50 depth:1 parent:Function@10 } + 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 } })")) << constructs; @@ -3176,9 +3176,9 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_If_If) { EXPECT_EQ(constructs.size(), 4u); EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,9) begin_id:10 end_id:0 depth:0 parent:null } - Construct{ Selection [0,8) begin_id:10 end_id:99 depth:1 parent:Function@10 } - Construct{ Selection [1,3) begin_id:20 end_id:40 depth:2 parent:Selection@10 } - Construct{ Selection [5,7) begin_id:50 end_id:89 depth:2 parent:Selection@10 } + Construct{ IfSelection [0,8) begin_id:10 end_id:99 depth:1 parent:Function@10 } + Construct{ IfSelection [1,3) begin_id:20 end_id:40 depth:2 parent:IfSelection@10 } + Construct{ IfSelection [5,7) begin_id:50 end_id:89 depth:2 parent:IfSelection@10 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get()); @@ -3237,9 +3237,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{ Selection [0,7) begin_id:10 end_id:99 depth:1 parent:Function@10 } - Construct{ Selection [1,3) begin_id:50 end_id:89 depth:2 parent:Selection@10 } - Construct{ Selection [4,6) begin_id:20 end_id:49 depth:2 parent:Selection@10 } + Construct{ SwitchSelection [0,7) begin_id:10 end_id:99 depth:1 parent:Function@10 } + Construct{ IfSelection [1,3) begin_id:50 end_id:89 depth:2 parent:SwitchSelection@10 } + Construct{ IfSelection [4,6) begin_id:20 end_id:49 depth:2 parent:SwitchSelection@10 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get()); @@ -3286,8 +3286,8 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_If_Switch) { EXPECT_EQ(constructs.size(), 3u); EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,5) begin_id:10 end_id:0 depth:0 parent:null } - Construct{ Selection [0,4) begin_id:10 end_id:99 depth:1 parent:Function@10 } - Construct{ Selection [1,3) begin_id:20 end_id:89 depth:2 parent:Selection@10 } + 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 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get()); @@ -3397,7 +3397,7 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_Loop_If) { 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{ Selection [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-c-l:Loop@20 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[0].get()); @@ -3448,7 +3448,7 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_LoopContinue_If) { 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{ Selection [2,4) begin_id:30 end_id:49 depth:2 parent:Continue@30 in-c:Continue@30 } + 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. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[0].get()); @@ -3490,8 +3490,8 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_If_SingleBlockLoop) { EXPECT_EQ(constructs.size(), 3u); EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,4) begin_id:10 end_id:0 depth:0 parent:null } - Construct{ Selection [0,3) begin_id:10 end_id:99 depth:1 parent:Function@10 } - Construct{ Continue [1,2) begin_id:20 end_id:89 depth:2 parent:Selection@10 in-c:Continue@20 } + Construct{ IfSelection [0,3) begin_id:10 end_id:99 depth:1 parent:Function@10 } + Construct{ Continue [1,2) begin_id:20 end_id:89 depth:2 parent:IfSelection@10 in-c:Continue@20 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get()); @@ -3539,9 +3539,9 @@ TEST_F(SpvParserTest, LabelControlFlowConstructs_Nest_If_MultiBlockLoop) { EXPECT_EQ(constructs.size(), 4u); EXPECT_THAT(ToString(constructs), Eq(R"(ConstructList{ Construct{ Function [0,7) begin_id:10 end_id:0 depth:0 parent:null } - Construct{ Selection [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:Selection@10 in-c:Continue@40 } - Construct{ Loop [1,3) begin_id:20 end_id:40 depth:2 parent:Selection@10 in-c-l:Loop@20 } + 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 } })")) << constructs; // The block records the nearest enclosing construct. EXPECT_EQ(fe.GetBlockInfo(10)->construct, constructs[1].get());