Fix infinite loop in dead-branch-elimination (#1891)

* Create structed cfg analysis.

There are lots of optimization that have to traverse the CFG in a
structured order just because it wants to know which constructs a
basic block in contained in.  This adds extra complexity to these
optimizations, for causes too much refactoring of older optimizations.

To help with this problem, I have written an analysis that can give this
information.

* Identify branches breaking from loops.

Dead branch elimination does a search for a conditional branch to the
end of the current selection construct.  This search assumes that the
only way to leave the construct is through the merge node.  But that is
not true.  The code can jump to the merge node of a loop that contains
the construct.

The search needs to take this into consideration.

Android.mk

@@ -144,6 +144,7 @@ SPVTOOLS_OPT_SRC_FILES := \
 		source/opt/strength_reduction_pass.cpp \
 		source/opt/strip_debug_info_pass.cpp \
 		source/opt/strip_reflect_info_pass.cpp \
+		source/opt/struct_cfg_analysis.cpp \
 		source/opt/type_manager.cpp \
 		source/opt/types.cpp \
 		source/opt/unify_const_pass.cpp \

BUILD.gn

@@ -599,6 +599,8 @@ static_library("spvtools_opt") {
     "source/opt/strip_debug_info_pass.h",
     "source/opt/strip_reflect_info_pass.cpp",
     "source/opt/strip_reflect_info_pass.h",
+    "source/opt/struct_cfg_analysis.cpp",
+    "source/opt/struct_cfg_analysis.h",
     "source/opt/tree_iterator.h",
     "source/opt/type_manager.cpp",
     "source/opt/type_manager.h",

source/opt/CMakeLists.txt

@@ -91,6 +91,7 @@ add_library(SPIRV-Tools-opt
   strength_reduction_pass.h
   strip_debug_info_pass.h
   strip_reflect_info_pass.h
+  struct_cfg_analysis.h
   tree_iterator.h
   type_manager.h
   types.h
@@ -175,6 +176,7 @@ add_library(SPIRV-Tools-opt
   strength_reduction_pass.cpp
   strip_debug_info_pass.cpp
   strip_reflect_info_pass.cpp
+  struct_cfg_analysis.cpp
   type_manager.cpp
   types.cpp
   unify_const_pass.cpp

source/opt/dead_branch_elim_pass.cpp

@@ -24,6 +24,7 @@
 #include "source/cfa.h"
 #include "source/opt/ir_context.h"
 #include "source/opt/iterator.h"
+#include "source/opt/struct_cfg_analysis.h"
 #include "source/util/make_unique.h"
 
 namespace spvtools {
@@ -92,6 +93,8 @@ BasicBlock* DeadBranchElimPass::GetParentBlock(uint32_t id) {
 
 bool DeadBranchElimPass::MarkLiveBlocks(
     Function* func, std::unordered_set<BasicBlock*>* live_blocks) {
+  StructuredCFGAnalysis cfgAnalysis(context());
+
   std::unordered_set<BasicBlock*> continues;
   std::vector<BasicBlock*> stack;
   stack.push_back(&*func->begin());
@@ -160,7 +163,8 @@ bool DeadBranchElimPass::MarkLiveBlocks(
       Instruction* mergeInst = block->GetMergeInst();
       if (mergeInst && mergeInst->opcode() == SpvOpSelectionMerge) {
         Instruction* first_break = FindFirstExitFromSelectionMerge(
-            live_lab_id, mergeInst->GetSingleWordInOperand(0));
+            live_lab_id, mergeInst->GetSingleWordInOperand(0),
+            cfgAnalysis.LoopMergeBlock(live_lab_id));
         if (first_break == nullptr) {
           context()->KillInst(mergeInst);
         } else {
@@ -435,7 +439,7 @@ Pass::Status DeadBranchElimPass::Process() {
 }
 
 Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
-    uint32_t start_block_id, uint32_t merge_block_id) {
+    uint32_t start_block_id, uint32_t merge_block_id, uint32_t loop_merge_id) {
   // To find the "first" exit, we follow branches looking for a conditional
   // branch that is not in a nested construct and is not the header of a new
   // construct.  We follow the control flow from |start_block_id| to find the
@@ -446,14 +450,73 @@ Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
     uint32_t next_block_id = 0;
     switch (branch->opcode()) {
       case SpvOpBranchConditional:
+        next_block_id = start_block->MergeBlockIdIfAny();
+        if (next_block_id == 0) {
+          // If a possible target is the |loop_merge_id|, which is not the
+          // current merge node, then we have to continue the search with the
+          // other target.
+          for (uint32_t i = 1; i < 3; i++) {
+            if (branch->GetSingleWordInOperand(i) == loop_merge_id &&
+                loop_merge_id != merge_block_id) {
+              next_block_id = branch->GetSingleWordInOperand(3 - i);
+              break;
+            }
+          }
+
+          if (next_block_id == 0) {
+            return branch;
+          }
+        }
+        break;
       case SpvOpSwitch:
         next_block_id = start_block->MergeBlockIdIfAny();
         if (next_block_id == 0) {
+          // A switch with no merge instructions can have at most 3 targets:
+          //   a. merge_block_id
+          //   b. loop_merge_id
+          //   c. 1 block inside the current region.
+          //
+          // This leads to a number of cases of what to do.
+          //
+          // 1. Does not jump to a block inside of the current construct.  In
+          // this case, there is not conditional break, so we should return
+          // |nullptr|.
+          //
+          // 2. Jumps to |merge_block_id| and a block inside the current
+          // construct.  In this case, this branch conditionally break to the
+          // end of the current construct, so return the current branch.
+          //
+          // 3.  Otherwise, this branch may break, but not to the current merge
+          // block.  So we continue with the block that is inside the loop.
+
+          bool found_break = false;
+          for (uint32_t i = 1; i < branch->NumInOperands(); i += 2) {
+            uint32_t target = branch->GetSingleWordInOperand(i);
+            if (target == merge_block_id) {
+              found_break = true;
+            } else if (target != loop_merge_id) {
+              next_block_id = branch->GetSingleWordInOperand(i);
+            }
+          }
+
+          if (next_block_id == 0) {
+            // Case 1.
+            return nullptr;
+          }
+
+          if (found_break) {
+            // Case 2.
             return branch;
           }
+
+          // The fall through is case 3.
+        }
         break;
       case SpvOpBranch:
         next_block_id = branch->GetSingleWordInOperand(0);
+        if (next_block_id == loop_merge_id) {
+          return nullptr;
+        }
         break;
       default:
         return nullptr;

source/opt/dead_branch_elim_pass.h

@@ -132,15 +132,19 @@ class DeadBranchElimPass : public MemPass {
   void FixBlockOrder();
 
   // Return the first branch instruction that is a conditional branch to
-  // |merge_block_id|. Returns |nullptr| if not such branch exists. If there are
+  // |merge_block_id|. Returns |nullptr| if no such branch exists. If there are
   // multiple such branches, the first one is the one that would be executed
   // first when running the code.  That is, the one that dominates all of the
   // others.
   //
   // |start_block_id| must be a block whose innermost containing merge construct
   // has |merge_block_id| as the merge block.
+  //
+  // |loop_merge_id| is the merge block id of the innermost loop containing
+  // |start_block_id|.
   Instruction* FindFirstExitFromSelectionMerge(uint32_t start_block_id,
-                                               uint32_t merge_block_id);
+                                               uint32_t merge_block_id,
+                                               uint32_t loop_merge_id);
 };
 
 }  // namespace opt

source/opt/struct_cfg_analysis.cpp

@@ -0,0 +1,104 @@
+// Copyright (c) 2018 Google LLC.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "source/opt/struct_cfg_analysis.h"
+
+namespace {
+const uint32_t kMergeNodeIndex = 0;
+}
+
+namespace spvtools {
+namespace opt {
+
+StructuredCFGAnalysis::StructuredCFGAnalysis(IRContext* ctx) : context_(ctx) {
+  // If this is not a shader, there are no merge instructions, and not
+  // structured CFG to analyze.
+  if (!context_->get_feature_mgr()->HasCapability(SpvCapabilityShader)) {
+    return;
+  }
+
+  for (auto& func : *context_->module()) {
+    AddBlocksInFunction(&func);
+  }
+}
+
+void StructuredCFGAnalysis::AddBlocksInFunction(Function* func) {
+  std::list<BasicBlock*> order;
+  context_->cfg()->ComputeStructuredOrder(func, &*func->begin(), &order);
+
+  struct TraversalInfo {
+    ConstructInfo cinfo;
+    uint32_t merge_node;
+  };
+
+  // Set up a stack to keep track of currently active constructs.
+  std::vector<TraversalInfo> state;
+  state.emplace_back();
+  state[0].cinfo.containing_construct = 0;
+  state[0].cinfo.containing_loop = 0;
+  state[0].merge_node = 0;
+
+  for (BasicBlock* block : order) {
+    if (context_->cfg()->IsPseudoEntryBlock(block) ||
+        context_->cfg()->IsPseudoExitBlock(block)) {
+      continue;
+    }
+
+    if (block->id() == state.back().merge_node) {
+      state.pop_back();
+    }
+
+    bb_to_construct_.emplace(std::make_pair(block->id(), state.back().cinfo));
+
+    if (Instruction* merge_inst = block->GetMergeInst()) {
+      TraversalInfo new_state;
+      new_state.merge_node =
+          merge_inst->GetSingleWordInOperand(kMergeNodeIndex);
+      new_state.cinfo.containing_construct = block->id();
+
+      if (merge_inst->opcode() == SpvOpLoopMerge) {
+        new_state.cinfo.containing_loop = block->id();
+      } else {
+        new_state.cinfo.containing_loop = state.back().cinfo.containing_loop;
+      }
+
+      state.emplace_back(new_state);
+    }
+  }
+}
+
+uint32_t StructuredCFGAnalysis::MergeBlock(uint32_t bb_id) {
+  uint32_t header_id = ContainingConstruct(bb_id);
+  if (header_id == 0) {
+    return 0;
+  }
+
+  BasicBlock* header = context_->cfg()->block(header_id);
+  Instruction* merge_inst = header->GetMergeInst();
+  return merge_inst->GetSingleWordInOperand(kMergeNodeIndex);
+}
+
+uint32_t StructuredCFGAnalysis::LoopMergeBlock(uint32_t bb_id) {
+  uint32_t header_id = ContainingLoop(bb_id);
+  if (header_id == 0) {
+    return 0;
+  }
+
+  BasicBlock* header = context_->cfg()->block(header_id);
+  Instruction* merge_inst = header->GetMergeInst();
+  return merge_inst->GetSingleWordInOperand(kMergeNodeIndex);
+}
+
+}  // namespace opt
+}  // namespace spvtools

source/opt/struct_cfg_analysis.h

@@ -0,0 +1,89 @@
+// Copyright (c) 2018 Google LLC.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SOURCE_OPT_STRUCT_CFG_ANALYSIS_H_
+#define SOURCE_OPT_STRUCT_CFG_ANALYSIS_H_
+
+#include <unordered_map>
+
+#include "ir_context.h"
+
+namespace spvtools {
+namespace opt {
+
+// An analysis that, for each basic block, finds the constructs in which it is
+// contained, so we can easily get headers and merge nodes.
+class StructuredCFGAnalysis {
+ public:
+  explicit StructuredCFGAnalysis(IRContext* ctx);
+
+  // Returns the id of the header of the innermost merge construct
+  // that contains |bb_id|.  Returns |0| if |bb_id| is not contained in any
+  // merge construct.
+  uint32_t ContainingConstruct(uint32_t bb_id) {
+    auto it = bb_to_construct_.find(bb_id);
+    if (it == bb_to_construct_.end()) {
+      return 0;
+    }
+    return it->second.containing_construct;
+  }
+
+  // Returns the id of the merge block of the innermost merge construct
+  // that contains |bb_id|.  Returns |0| if |bb_id| is not contained in any
+  // merge construct.
+  uint32_t MergeBlock(uint32_t bb_id);
+
+  // Returns the id of the header of the innermost loop construct
+  // that contains |bb_id|.  Return |0| if |bb_id| is not contained in any loop
+  // construct.
+  uint32_t ContainingLoop(uint32_t bb_id) {
+    auto it = bb_to_construct_.find(bb_id);
+    if (it == bb_to_construct_.end()) {
+      return 0;
+    }
+    return it->second.containing_loop;
+  }
+
+  // Returns the id of the merge block of the innermost loop construct
+  // that contains |bb_id|.  Return |0| if |bb_id| is not contained in any loop
+  // construct.
+  uint32_t LoopMergeBlock(uint32_t bb_id);
+
+ private:
+  // Struct used to hold the information for a basic block.
+  // |containing_construct| is the header for the innermost containing
+  // construct, or 0 if no such construct exists.  It could be a selection
+  // construct or a loop construct. |containing_loop| is the innermost
+  // containing loop construct, or 0 if the basic bloc is not in a loop.  If the
+  // basic block is in a selection construct that is contained in a loop
+  // construct, then these two values will not be the same.
+  struct ConstructInfo {
+    uint32_t containing_construct;
+    uint32_t containing_loop;
+  };
+
+  // Populates |bb_to_construct_| with the innermost containing merge and loop
+  // constructs for each basic block in |func|.
+  void AddBlocksInFunction(Function* func);
+
+  IRContext* context_;
+
+  // A map from a basic block to the headers of its inner most containing
+  // constructs.
+  std::unordered_map<uint32_t, ConstructInfo> bb_to_construct_;
+};
+
+}  // namespace opt
+}  // namespace spvtools
+#endif  // SOURCE_OPT_STRUCT_CFG_ANALYSIS_H_

test/opt/CMakeLists.txt

@@ -337,3 +337,8 @@ add_spvtools_unittest(TARGET combine_access_chains
   LIBS SPIRV-Tools-opt
 )
 
+add_spvtools_unittest(TARGET struct_cfg_analysis
+        SRCS struct_cfg_analysis_test.cpp
+        LIBS SPIRV-Tools-opt
+)
+

test/opt/dead_branch_elim_test.cpp

@@ -2193,6 +2193,302 @@ OpFunctionEnd
 
   SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
 }
+
+TEST_F(DeadBranchElimTest, SelectionMergeWithConditionalExit) {
+  // Checks that if a selection merge construct contains a conditional branch
+  // to the merge node, then we keep the OpSelectionMerge on that branch.
+  const std::string predefs = R"(
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+%void = OpTypeVoid
+%func_type = OpTypeFunction %void
+%bool = OpTypeBool
+%true = OpConstantTrue %bool
+%uint = OpTypeInt 32 0
+%undef_int = OpUndef %uint
+)";
+
+  const std::string body =
+      R"(
+; CHECK: OpLoopMerge [[loop_merge:%\w+]]
+; CHECK-NEXT: OpBranch [[bb1:%\w+]]
+; CHECK: [[bb1]] = OpLabel
+; CHECK-NEXT: OpBranch [[bb2:%\w+]]
+; CHECK: [[bb2]] = OpLabel
+; CHECK-NEXT: OpSelectionMerge [[sel_merge:%\w+]] None
+; CHECK-NEXT: OpSwitch {{%\w+}} [[sel_merge]] 1 [[bb3:%\w+]]
+; CHECK: [[bb3]] = OpLabel
+; CHECK-NEXT: OpBranch [[sel_merge]]
+; CHECK: [[sel_merge]] = OpLabel
+; CHECK-NEXT: OpBranch [[loop_merge]]
+; CHECK: [[loop_merge]] = OpLabel
+; CHECK-NEXT: OpReturn
+%main = OpFunction %void None %func_type
+%entry_bb = OpLabel
+OpBranch %loop_header
+%loop_header = OpLabel
+OpLoopMerge %loop_merge %cont None
+OpBranch %bb1
+%bb1 = OpLabel
+OpSelectionMerge %sel_merge None
+OpBranchConditional %true %bb2 %bb4
+%bb2 = OpLabel
+OpSwitch %undef_int %sel_merge 1 %bb3
+%bb3 = OpLabel
+OpBranch %sel_merge
+%bb4 = OpLabel
+OpBranch %sel_merge
+%sel_merge = OpLabel
+OpBranch %loop_merge
+%cont = OpLabel
+OpBranch %loop_header
+%loop_merge = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
+}
+
+TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoop) {
+  // Checks  that if a selection merge construct contains a conditional branch
+  // to a loop surrounding the selection merge, then we do not keep the
+  // OpSelectionMerge instruction.
+  const std::string predefs = R"(
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+%void = OpTypeVoid
+%func_type = OpTypeFunction %void
+%bool = OpTypeBool
+%true = OpConstantTrue %bool
+%undef_bool = OpUndef %bool
+)";
+
+  const std::string body =
+      R"(
+; CHECK: OpLoopMerge [[loop_merge:%\w+]]
+; CHECK-NEXT: OpBranch [[bb1:%\w+]]
+; CHECK: [[bb1]] = OpLabel
+; CHECK-NEXT: OpBranch [[bb2:%\w+]]
+; CHECK: [[bb2]] = OpLabel
+; CHECK-NEXT: OpBranchConditional {{%\w+}} [[bb3:%\w+]] [[loop_merge]]
+; CHECK: [[bb3]] = OpLabel
+; CHECK-NEXT: OpBranch [[sel_merge:%\w+]]
+; CHECK: [[sel_merge]] = OpLabel
+; CHECK-NEXT: OpBranch [[loop_merge]]
+; CHECK: [[loop_merge]] = OpLabel
+; CHECK-NEXT: OpReturn
+%main = OpFunction %void None %func_type
+%entry_bb = OpLabel
+OpBranch %loop_header
+%loop_header = OpLabel
+OpLoopMerge %loop_merge %cont None
+OpBranch %bb1
+%bb1 = OpLabel
+OpSelectionMerge %sel_merge None
+OpBranchConditional %true %bb2 %bb4
+%bb2 = OpLabel
+OpBranchConditional %undef_bool %bb3 %loop_merge
+%bb3 = OpLabel
+OpBranch %sel_merge
+%bb4 = OpLabel
+OpBranch %sel_merge
+%sel_merge = OpLabel
+OpBranch %loop_merge
+%cont = OpLabel
+OpBranch %loop_header
+%loop_merge = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
+}
+
+TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoop2) {
+  // Same as |SelectionMergeWithExitToLoop|, except the swith goes to the loop
+  // merge or the selection merge.  In this case, we do not need an
+  // OpSelectionMerge either.
+  const std::string predefs = R"(
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+%void = OpTypeVoid
+%func_type = OpTypeFunction %void
+%bool = OpTypeBool
+%true = OpConstantTrue %bool
+%undef_bool = OpUndef %bool
+)";
+
+  const std::string body =
+      R"(
+; CHECK: OpLoopMerge [[loop_merge:%\w+]]
+; CHECK-NEXT: OpBranch [[bb1:%\w+]]
+; CHECK: [[bb1]] = OpLabel
+; CHECK-NEXT: OpBranch [[bb2:%\w+]]
+; CHECK: [[bb2]] = OpLabel
+; CHECK-NEXT: OpBranchConditional {{%\w+}} [[sel_merge:%\w+]] [[loop_merge]]
+; CHECK: [[sel_merge]] = OpLabel
+; CHECK-NEXT: OpBranch [[loop_merge]]
+; CHECK: [[loop_merge]] = OpLabel
+; CHECK-NEXT: OpReturn
+%main = OpFunction %void None %func_type
+%entry_bb = OpLabel
+OpBranch %loop_header
+%loop_header = OpLabel
+OpLoopMerge %loop_merge %cont None
+OpBranch %bb1
+%bb1 = OpLabel
+OpSelectionMerge %sel_merge None
+OpBranchConditional %true %bb2 %bb4
+%bb2 = OpLabel
+OpBranchConditional %undef_bool %sel_merge %loop_merge
+%bb4 = OpLabel
+OpBranch %sel_merge
+%sel_merge = OpLabel
+OpBranch %loop_merge
+%cont = OpLabel
+OpBranch %loop_header
+%loop_merge = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
+}
+
+TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoop3) {
+  // Checks that if a selection merge construct contains a conditional branch
+  // to the merge of a surrounding loop, the selection merge, and another block
+  // inside the selection merge, then we must keep the OpSelectionMerge
+  // instruction on that branch.
+  const std::string predefs = R"(
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+%void = OpTypeVoid
+%func_type = OpTypeFunction %void
+%bool = OpTypeBool
+%true = OpConstantTrue %bool
+%uint = OpTypeInt 32 0
+%undef_int = OpUndef %uint
+)";
+
+  const std::string body =
+      R"(
+; CHECK: OpLoopMerge [[loop_merge:%\w+]]
+; CHECK-NEXT: OpBranch [[bb1:%\w+]]
+; CHECK: [[bb1]] = OpLabel
+; CHECK-NEXT: OpBranch [[bb2:%\w+]]
+; CHECK: [[bb2]] = OpLabel
+; CHECK-NEXT: OpSelectionMerge [[sel_merge:%\w+]] None
+; CHECK-NEXT: OpSwitch {{%\w+}} [[sel_merge]] 0 [[loop_merge]] 1 [[bb3:%\w+]]
+; CHECK: [[bb3]] = OpLabel
+; CHECK-NEXT: OpBranch [[sel_merge]]
+; CHECK: [[sel_merge]] = OpLabel
+; CHECK-NEXT: OpBranch [[loop_merge]]
+; CHECK: [[loop_merge]] = OpLabel
+; CHECK-NEXT: OpReturn
+%main = OpFunction %void None %func_type
+%entry_bb = OpLabel
+OpBranch %loop_header
+%loop_header = OpLabel
+OpLoopMerge %loop_merge %cont None
+OpBranch %bb1
+%bb1 = OpLabel
+OpSelectionMerge %sel_merge None
+OpBranchConditional %true %bb2 %bb4
+%bb2 = OpLabel
+OpSwitch %undef_int %sel_merge 0 %loop_merge 1 %bb3
+%bb3 = OpLabel
+OpBranch %sel_merge
+%bb4 = OpLabel
+OpBranch %sel_merge
+%sel_merge = OpLabel
+OpBranch %loop_merge
+%cont = OpLabel
+OpBranch %loop_header
+%loop_merge = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
+}
+
+TEST_F(DeadBranchElimTest, SelectionMergeWithExitToLoop4) {
+  // Same as |SelectionMergeWithExitToLoop|, execept the branch in the selection
+  // construct is an |OpSwitch| instead of an |OpConditionalBranch|.  The
+  // OpSelectionMerge instruction is not needed in this case either.
+  const std::string predefs = R"(
+OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+OpSource GLSL 140
+%void = OpTypeVoid
+%func_type = OpTypeFunction %void
+%bool = OpTypeBool
+%true = OpConstantTrue %bool
+%uint = OpTypeInt 32 0
+%undef_int = OpUndef %uint
+)";
+
+  const std::string body =
+      R"(
+; CHECK: OpLoopMerge [[loop_merge:%\w+]]
+; CHECK-NEXT: OpBranch [[bb1:%\w+]]
+; CHECK: [[bb1]] = OpLabel
+; CHECK-NEXT: OpBranch [[bb2:%\w+]]
+; CHECK: [[bb2]] = OpLabel
+; CHECK-NEXT: OpSwitch {{%\w+}} [[bb3:%\w+]] 0 [[loop_merge]] 1 [[bb3:%\w+]]
+; CHECK: [[bb3]] = OpLabel
+; CHECK-NEXT: OpBranch [[sel_merge:%\w+]]
+; CHECK: [[sel_merge]] = OpLabel
+; CHECK-NEXT: OpBranch [[loop_merge]]
+; CHECK: [[loop_merge]] = OpLabel
+; CHECK-NEXT: OpReturn
+%main = OpFunction %void None %func_type
+%entry_bb = OpLabel
+OpBranch %loop_header
+%loop_header = OpLabel
+OpLoopMerge %loop_merge %cont None
+OpBranch %bb1
+%bb1 = OpLabel
+OpSelectionMerge %sel_merge None
+OpBranchConditional %true %bb2 %bb4
+%bb2 = OpLabel
+OpSwitch %undef_int %bb3 0 %loop_merge 1 %bb3
+%bb3 = OpLabel
+OpBranch %sel_merge
+%bb4 = OpLabel
+OpBranch %sel_merge
+%sel_merge = OpLabel
+OpBranch %loop_merge
+%cont = OpLabel
+OpBranch %loop_header
+%loop_merge = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
+}
 #endif
 
 // TODO(greg-lunarg): Add tests to verify handling of these cases:

test/opt/struct_cfg_analysis_test.cpp

@@ -0,0 +1,466 @@
+// Copyright (c) 2018 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <string>
+
+#include "gmock/gmock.h"
+#include "source/opt/struct_cfg_analysis.h"
+#include "test/opt/assembly_builder.h"
+#include "test/opt/pass_fixture.h"
+#include "test/opt/pass_utils.h"
+
+namespace spvtools {
+namespace opt {
+namespace {
+
+using StructCFGAnalysisTest = PassTest<::testing::Test>;
+
+TEST_F(StructCFGAnalysisTest, BBInSelection) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%bool_undef = OpUndef %bool
+%uint = OpTypeInt 32 0
+%uint_undef = OpUndef %uint
+%void_func = OpTypeFunction %void
+%main = OpFunction %void None %void_func
+%1 = OpLabel
+OpSelectionMerge %3 None
+OpBranchConditional %undef_bool %2 %3
+%2 = OpLabel
+OpBranch %3
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  StructuredCFGAnalysis analysis(context.get());
+
+  // The header is not in the construct.
+  EXPECT_EQ(analysis.ContainingConstruct(1), 0);
+  EXPECT_EQ(analysis.ContainingLoop(1), 0);
+  EXPECT_EQ(analysis.MergeBlock(1), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(1), 0);
+
+  // BB2 is in the construct.
+  EXPECT_EQ(analysis.ContainingConstruct(2), 1);
+  EXPECT_EQ(analysis.ContainingLoop(2), 0);
+  EXPECT_EQ(analysis.MergeBlock(2), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(2), 0);
+
+  // The merge node is not in the construct.
+  EXPECT_EQ(analysis.ContainingConstruct(3), 0);
+  EXPECT_EQ(analysis.ContainingLoop(3), 0);
+  EXPECT_EQ(analysis.MergeBlock(3), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(3), 0);
+}
+
+TEST_F(StructCFGAnalysisTest, BBInLoop) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%bool_undef = OpUndef %bool
+%uint = OpTypeInt 32 0
+%uint_undef = OpUndef %uint
+%void_func = OpTypeFunction %void
+%main = OpFunction %void None %void_func
+%entry_lab = OpLabel
+OpBranch %1
+%1 = OpLabel
+OpLoopMerge %3 %4 None
+OpBranchConditional %undef_bool %2 %3
+%2 = OpLabel
+OpBranch %3
+%4 = OpLabel
+OpBranch %1
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  StructuredCFGAnalysis analysis(context.get());
+
+  // The header is not in the construct.
+  EXPECT_EQ(analysis.ContainingConstruct(1), 0);
+  EXPECT_EQ(analysis.ContainingLoop(1), 0);
+  EXPECT_EQ(analysis.MergeBlock(1), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(1), 0);
+
+  // BB2 is in the construct.
+  EXPECT_EQ(analysis.ContainingConstruct(2), 1);
+  EXPECT_EQ(analysis.ContainingLoop(2), 1);
+  EXPECT_EQ(analysis.MergeBlock(2), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(2), 3);
+
+  // The merge node is not in the construct.
+  EXPECT_EQ(analysis.ContainingConstruct(3), 0);
+  EXPECT_EQ(analysis.ContainingLoop(3), 0);
+  EXPECT_EQ(analysis.MergeBlock(3), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(3), 0);
+
+  // The continue block is in the construct.
+  EXPECT_EQ(analysis.ContainingConstruct(4), 1);
+  EXPECT_EQ(analysis.ContainingLoop(4), 1);
+  EXPECT_EQ(analysis.MergeBlock(4), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(4), 3);
+}
+
+TEST_F(StructCFGAnalysisTest, SelectionInLoop) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%bool_undef = OpUndef %bool
+%uint = OpTypeInt 32 0
+%uint_undef = OpUndef %uint
+%void_func = OpTypeFunction %void
+%main = OpFunction %void None %void_func
+%entry_lab = OpLabel
+OpBranch %1
+%1 = OpLabel
+OpLoopMerge %3 %4 None
+OpBranchConditional %undef_bool %2 %3
+%2 = OpLabel
+OpSelectionMerge %6 None
+OpBranchConditional %undef_bool %5 %6
+%5 = OpLabel
+OpBranch %6
+%6 = OpLabel
+OpBranch %3
+%4 = OpLabel
+OpBranch %1
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  StructuredCFGAnalysis analysis(context.get());
+
+  // The loop header is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(1), 0);
+  EXPECT_EQ(analysis.ContainingLoop(1), 0);
+  EXPECT_EQ(analysis.MergeBlock(1), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(1), 0);
+
+  // Selection header is in the loop only.
+  EXPECT_EQ(analysis.ContainingConstruct(2), 1);
+  EXPECT_EQ(analysis.ContainingLoop(2), 1);
+  EXPECT_EQ(analysis.MergeBlock(2), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(2), 3);
+
+  // The loop merge node is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(3), 0);
+  EXPECT_EQ(analysis.ContainingLoop(3), 0);
+  EXPECT_EQ(analysis.MergeBlock(3), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(3), 0);
+
+  // The continue block is in the loop only.
+  EXPECT_EQ(analysis.ContainingConstruct(4), 1);
+  EXPECT_EQ(analysis.ContainingLoop(4), 1);
+  EXPECT_EQ(analysis.MergeBlock(4), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(4), 3);
+
+  // BB5 is in the selection fist and the loop.
+  EXPECT_EQ(analysis.ContainingConstruct(5), 2);
+  EXPECT_EQ(analysis.ContainingLoop(5), 1);
+  EXPECT_EQ(analysis.MergeBlock(5), 6);
+  EXPECT_EQ(analysis.LoopMergeBlock(5), 3);
+
+  // The selection merge is in the loop only.
+  EXPECT_EQ(analysis.ContainingConstruct(6), 1);
+  EXPECT_EQ(analysis.ContainingLoop(6), 1);
+  EXPECT_EQ(analysis.MergeBlock(6), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(6), 3);
+}
+
+TEST_F(StructCFGAnalysisTest, LoopInSelection) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%bool_undef = OpUndef %bool
+%uint = OpTypeInt 32 0
+%uint_undef = OpUndef %uint
+%void_func = OpTypeFunction %void
+%main = OpFunction %void None %void_func
+%entry_lab = OpLabel
+OpBranch %1
+%1 = OpLabel
+OpSelectionMerge %3 None
+OpBranchConditional %undef_bool %2 %3
+%2 = OpLabel
+OpLoopMerge %4 %5 None
+OpBranchConditional %undef_bool %4 %6
+%5 = OpLabel
+OpBranch %2
+%6 = OpLabel
+OpBranch %4
+%4 = OpLabel
+OpBranch %3
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  StructuredCFGAnalysis analysis(context.get());
+
+  // The selection header is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(1), 0);
+  EXPECT_EQ(analysis.ContainingLoop(1), 0);
+  EXPECT_EQ(analysis.MergeBlock(1), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(1), 0);
+
+  // Loop header is in the selection only.
+  EXPECT_EQ(analysis.ContainingConstruct(2), 1);
+  EXPECT_EQ(analysis.ContainingLoop(2), 0);
+  EXPECT_EQ(analysis.MergeBlock(2), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(2), 0);
+
+  // The selection merge node is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(3), 0);
+  EXPECT_EQ(analysis.ContainingLoop(3), 0);
+  EXPECT_EQ(analysis.MergeBlock(3), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(3), 0);
+
+  // The loop merge is in the selection only.
+  EXPECT_EQ(analysis.ContainingConstruct(4), 1);
+  EXPECT_EQ(analysis.ContainingLoop(4), 0);
+  EXPECT_EQ(analysis.MergeBlock(4), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(4), 0);
+
+  // The loop continue target is in the loop.
+  EXPECT_EQ(analysis.ContainingConstruct(5), 2);
+  EXPECT_EQ(analysis.ContainingLoop(5), 2);
+  EXPECT_EQ(analysis.MergeBlock(5), 4);
+  EXPECT_EQ(analysis.LoopMergeBlock(5), 4);
+
+  // BB6 is in the loop.
+  EXPECT_EQ(analysis.ContainingConstruct(6), 2);
+  EXPECT_EQ(analysis.ContainingLoop(6), 2);
+  EXPECT_EQ(analysis.MergeBlock(6), 4);
+  EXPECT_EQ(analysis.LoopMergeBlock(6), 4);
+}
+
+TEST_F(StructCFGAnalysisTest, SelectionInSelection) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%bool_undef = OpUndef %bool
+%uint = OpTypeInt 32 0
+%uint_undef = OpUndef %uint
+%void_func = OpTypeFunction %void
+%main = OpFunction %void None %void_func
+%entry_lab = OpLabel
+OpBranch %1
+%1 = OpLabel
+OpSelectionMerge %3 None
+OpBranchConditional %undef_bool %2 %3
+%2 = OpLabel
+OpSelectionMerge %4 None
+OpBranchConditional %undef_bool %4 %5
+%5 = OpLabel
+OpBranch %4
+%4 = OpLabel
+OpBranch %3
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  StructuredCFGAnalysis analysis(context.get());
+
+  // The outer selection header is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(1), 0);
+  EXPECT_EQ(analysis.ContainingLoop(1), 0);
+  EXPECT_EQ(analysis.MergeBlock(1), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(1), 0);
+
+  // The inner header is in the outer selection.
+  EXPECT_EQ(analysis.ContainingConstruct(2), 1);
+  EXPECT_EQ(analysis.ContainingLoop(2), 0);
+  EXPECT_EQ(analysis.MergeBlock(2), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(2), 0);
+
+  // The outer merge node is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(3), 0);
+  EXPECT_EQ(analysis.ContainingLoop(3), 0);
+  EXPECT_EQ(analysis.MergeBlock(3), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(3), 0);
+
+  // The inner merge is in the outer selection.
+  EXPECT_EQ(analysis.ContainingConstruct(4), 1);
+  EXPECT_EQ(analysis.ContainingLoop(4), 0);
+  EXPECT_EQ(analysis.MergeBlock(4), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(4), 0);
+
+  // BB5 is in the inner selection.
+  EXPECT_EQ(analysis.ContainingConstruct(5), 2);
+  EXPECT_EQ(analysis.ContainingLoop(5), 0);
+  EXPECT_EQ(analysis.MergeBlock(5), 4);
+  EXPECT_EQ(analysis.LoopMergeBlock(5), 0);
+}
+
+TEST_F(StructCFGAnalysisTest, LoopInLoop) {
+  const std::string text = R"(
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%bool_undef = OpUndef %bool
+%uint = OpTypeInt 32 0
+%uint_undef = OpUndef %uint
+%void_func = OpTypeFunction %void
+%main = OpFunction %void None %void_func
+%entry_lab = OpLabel
+OpBranch %1
+%1 = OpLabel
+OpLoopMerge %3 %7 None
+OpBranchConditional %undef_bool %2 %3
+%2 = OpLabel
+OpLoopMerge %4 %5 None
+OpBranchConditional %undef_bool %4 %6
+%5 = OpLabel
+OpBranch %2
+%6 = OpLabel
+OpBranch %4
+%4 = OpLabel
+OpBranch %3
+%7 = OpLabel
+OpBranch %1
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  StructuredCFGAnalysis analysis(context.get());
+
+  // The outer loop header is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(1), 0);
+  EXPECT_EQ(analysis.ContainingLoop(1), 0);
+  EXPECT_EQ(analysis.MergeBlock(1), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(1), 0);
+
+  // The inner loop header is in the outer loop.
+  EXPECT_EQ(analysis.ContainingConstruct(2), 1);
+  EXPECT_EQ(analysis.ContainingLoop(2), 1);
+  EXPECT_EQ(analysis.MergeBlock(2), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(2), 3);
+
+  // The outer merge node is not in either construct.
+  EXPECT_EQ(analysis.ContainingConstruct(3), 0);
+  EXPECT_EQ(analysis.ContainingLoop(3), 0);
+  EXPECT_EQ(analysis.MergeBlock(3), 0);
+  EXPECT_EQ(analysis.LoopMergeBlock(3), 0);
+
+  // The inner merge is in the outer loop.
+  EXPECT_EQ(analysis.ContainingConstruct(4), 1);
+  EXPECT_EQ(analysis.ContainingLoop(4), 1);
+  EXPECT_EQ(analysis.MergeBlock(4), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(4), 3);
+
+  // The inner continue target is in the inner loop.
+  EXPECT_EQ(analysis.ContainingConstruct(5), 2);
+  EXPECT_EQ(analysis.ContainingLoop(5), 2);
+  EXPECT_EQ(analysis.MergeBlock(5), 4);
+  EXPECT_EQ(analysis.LoopMergeBlock(5), 4);
+
+  // BB6 is in the loop.
+  EXPECT_EQ(analysis.ContainingConstruct(6), 2);
+  EXPECT_EQ(analysis.ContainingLoop(6), 2);
+  EXPECT_EQ(analysis.MergeBlock(6), 4);
+  EXPECT_EQ(analysis.LoopMergeBlock(6), 4);
+
+  // The outer continue target is in the outer loop.
+  EXPECT_EQ(analysis.ContainingConstruct(7), 1);
+  EXPECT_EQ(analysis.ContainingLoop(7), 1);
+  EXPECT_EQ(analysis.MergeBlock(7), 3);
+  EXPECT_EQ(analysis.LoopMergeBlock(7), 3);
+}
+
+TEST_F(StructCFGAnalysisTest, KernelTest) {
+  const std::string text = R"(
+OpCapability Kernel
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+%void = OpTypeVoid
+%bool = OpTypeBool
+%bool_undef = OpUndef %bool
+%void_func = OpTypeFunction %void
+%main = OpFunction %void None %void_func
+%1 = OpLabel
+OpBranchConditional %undef_bool %2 %3
+%2 = OpLabel
+OpBranch %3
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  std::unique_ptr<IRContext> context =
+      BuildModule(SPV_ENV_UNIVERSAL_1_1, nullptr, text,
+                  SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+
+  StructuredCFGAnalysis analysis(context.get());
+
+  // No structured control flow, so none of the basic block are in any
+  // construct.
+  for (uint32_t i = 1; i <= 3; i++) {
+    EXPECT_EQ(analysis.ContainingConstruct(i), 0);
+    EXPECT_EQ(analysis.ContainingLoop(i), 0);
+    EXPECT_EQ(analysis.MergeBlock(i), 0);
+    EXPECT_EQ(analysis.LoopMergeBlock(i), 0);
+  }
+}
+
+}  // namespace
+}  // namespace opt
+}  // namespace spvtools