From 0ba35798c393eb3a43eb7e8b8a1098f5ac65cb61 Mon Sep 17 00:00:00 2001
From: Steven Perron <stevenperron@google.com>
Date: Mon, 22 Oct 2018 13:59:20 -0400
Subject: [PATCH] Fix dead branch elim infinite loop. (#1997)

When looking for a break from a selection construct, we do not need to
look inside nested constructs.  However, if a loop header has an
unconditional branch, then we enter the loop.  Entering the loop causes
an infinite loop because we keep going through the loop.

The solution is to look for a merge block, if one exsits, even for block
terminated by an OpBranch.

Fixes #1979.
---
 source/opt/dead_branch_elim_pass.cpp | 14 ++++--
 test/opt/dead_branch_elim_test.cpp   | 67 ++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/source/opt/dead_branch_elim_pass.cpp b/source/opt/dead_branch_elim_pass.cpp
index e3836279c..3b3cd5e80 100644
--- a/source/opt/dead_branch_elim_pass.cpp
+++ b/source/opt/dead_branch_elim_pass.cpp
@@ -514,9 +514,17 @@ Instruction* DeadBranchElimPass::FindFirstExitFromSelectionMerge(
         }
         break;
       case SpvOpBranch:
-        next_block_id = branch->GetSingleWordInOperand(0);
-        if (next_block_id == loop_merge_id) {
-          return nullptr;
+        // Need to check if this is the header of a loop nested in the
+        // selection construct.
+        next_block_id = start_block->MergeBlockIdIfAny();
+        if (next_block_id == 0) {
+          next_block_id = branch->GetSingleWordInOperand(0);
+          if (next_block_id == loop_merge_id) {
+            // We break out of the selection, but to the merge of a loop
+            // containing the selection.  There is no break to the merge of the
+            // selection construct.
+            return nullptr;
+          }
         }
         break;
       default:
diff --git a/test/opt/dead_branch_elim_test.cpp b/test/opt/dead_branch_elim_test.cpp
index b4aff709a..204f949ae 100644
--- a/test/opt/dead_branch_elim_test.cpp
+++ b/test/opt/dead_branch_elim_test.cpp
@@ -2492,6 +2492,73 @@ OpFunctionEnd
   SinglePassRunAndMatch<DeadBranchElimPass>(predefs + body, true);
 }
 
+TEST_F(DeadBranchElimTest, SelectionMergeWithNestedLoop) {
+  const std::string body =
+      R"(
+; CHECK: OpSelectionMerge [[merge1:%\w+]]
+; CHECK: [[merge1]] = OpLabel
+; CHECK-NEXT: OpBranch [[preheader:%\w+]]
+; CHECK: [[preheader]] = OpLabel
+; CHECK-NOT: OpLabel
+; CHECK: OpBranch [[header:%\w+]]
+; CHECK: [[header]] = OpLabel
+; CHECK-NOT: OpLabel
+; CHECK: OpLoopMerge [[merge2:%\w+]]
+; CHECK: [[merge2]] = OpLabel
+; CHECK-NEXT: OpUnreachable
+                 OpCapability Shader
+            %1 = OpExtInstImport "GLSL.std.450"
+                 OpMemoryModel Logical GLSL450
+                 OpEntryPoint Fragment %main "main"
+                 OpExecutionMode %main OriginUpperLeft
+                 OpSource ESSL 310
+                 OpName %main "main"
+                 OpName %h "h"
+                 OpName %i "i"
+         %void = OpTypeVoid
+            %3 = OpTypeFunction %void
+         %bool = OpTypeBool
+  %_ptr_Function_bool = OpTypePointer Function %bool
+         %true = OpConstantTrue %bool
+          %int = OpTypeInt 32 1
+  %_ptr_Function_int = OpTypePointer Function %int
+        %int_1 = OpConstant %int 1
+        %int_0 = OpConstant %int 0
+           %27 = OpUndef %bool
+         %main = OpFunction %void None %3
+            %5 = OpLabel
+            %h = OpVariable %_ptr_Function_bool Function
+            %i = OpVariable %_ptr_Function_int Function
+                 OpSelectionMerge %11 None
+                 OpBranchConditional %27 %10 %11
+           %10 = OpLabel
+                 OpBranch %11
+           %11 = OpLabel
+                 OpSelectionMerge %14 None
+                 OpBranchConditional %true %13 %14
+           %13 = OpLabel
+                 OpStore %i %int_1
+                 OpBranch %19
+           %19 = OpLabel
+                 OpLoopMerge %21 %22 None
+                 OpBranch %23
+           %23 = OpLabel
+           %26 = OpSGreaterThan %bool %int_1 %int_0
+                 OpBranchConditional %true %20 %21
+           %20 = OpLabel
+                 OpBranch %22
+           %22 = OpLabel
+                 OpBranch %19
+           %21 = OpLabel
+                 OpBranch %14
+           %14 = OpLabel
+                 OpReturn
+                 OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<DeadBranchElimPass>(body, true);
+}
+
 // TODO(greg-lunarg): Add tests to verify handling of these cases:
 //
 //    More complex control flow