Adding support for switch removal in ADCE

* Updated code to handle switches
* Enabled disabled test and added a couple new ones

diff --git a/CHANGES b/CHANGES
index f9de60e..fb7ecdd 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,8 @@ Revision history for SPIRV-Tools
 
 v2018.0-dev 2018-01-12
  - Start v2018.0-dev
+ - Optimizer:
+   - Aggressive dead code elimination now removes OpSwitch constructs
  - Fixes:
    - PR 1198: Optimizer: Fix CCP in presence of matrix constants.
    - #1199: Optimizer: Fix CCP: don't propagate spec constants.

diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp
index 6ba59ee..511a12f 100644 (file)
--- a/source/opt/aggressive_dead_code_elim_pass.cpp
+++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -136,9 +136,8 @@ bool AggressiveDCEPass::AllExtensionsSupported() const {
 
 bool AggressiveDCEPass::IsDead(ir::Instruction* inst) {
   if (IsLive(inst)) return false;
-  if (inst->IsBranch() &&
-      !IsStructuredIfOrLoopHeader(context()->get_instr_block(inst), nullptr,
-                                  nullptr, nullptr))
+  if (inst->IsBranch() && !IsStructuredHeader(context()->get_instr_block(inst),
+                                              nullptr, nullptr, nullptr))
     return false;
   return true;
 }
@@ -173,18 +172,14 @@ void AggressiveDCEPass::ProcessLoad(uint32_t varId) {
   live_local_vars_.insert(varId);
 }
 
-bool AggressiveDCEPass::IsStructuredIfOrLoopHeader(ir::BasicBlock* bp,
-                                                   ir::Instruction** mergeInst,
-                                                   ir::Instruction** branchInst,
-                                                   uint32_t* mergeBlockId) {
+bool AggressiveDCEPass::IsStructuredHeader(ir::BasicBlock* bp,
+                                           ir::Instruction** mergeInst,
+                                           ir::Instruction** branchInst,
+                                           uint32_t* mergeBlockId) {
   if (!bp) return false;
   ir::Instruction* mi = bp->GetMergeInst();
   if (mi == nullptr) return false;
   ir::Instruction* bri = &*bp->tail();
-  // Make sure it is not a Switch
-  if (mi->opcode() == SpvOpSelectionMerge &&
-      bri->opcode() != SpvOpBranchConditional)
-    return false;
   if (branchInst != nullptr) *branchInst = bri;
   if (mergeInst != nullptr) *mergeInst = mi;
   if (mergeBlockId != nullptr) *mergeBlockId = mi->GetSingleWordInOperand(0);
@@ -211,7 +206,7 @@ void AggressiveDCEPass::ComputeBlock2HeaderMaps(
     ir::Instruction* branchInst;
     uint32_t mergeBlockId;
     bool is_header =
-        IsStructuredIfOrLoopHeader(*bi, &mergeInst, &branchInst, &mergeBlockId);
+        IsStructuredHeader(*bi, &mergeInst, &branchInst, &mergeBlockId);
     // If this is a loop header, update state first so the block will map to
     // the loop.
     if (is_header && mergeInst->opcode() == SpvOpLoopMerge) {
@@ -246,8 +241,6 @@ void AggressiveDCEPass::AddBreaksAndContinuesToWorklist(
       mergeId, [&loopMerge, this](ir::Instruction* user) {
         // A branch to the merge block can only be a break if it is nested in
         // the current loop
-        SpvOp op = user->opcode();
-        if (op != SpvOpBranchConditional && op != SpvOpBranch) return;
         ir::Instruction* branchInst = user;
         while (true) {
           ir::BasicBlock* blk = context()->get_instr_block(branchInst);
@@ -269,9 +262,9 @@ void AggressiveDCEPass::AddBreaksAndContinuesToWorklist(
   get_def_use_mgr()->ForEachUser(contId, [&contId,
                                           this](ir::Instruction* user) {
     SpvOp op = user->opcode();
-    if (op == SpvOpBranchConditional) {
-      // A conditional branch can only be a continue if it does not have a merge
-      // instruction or its merge block is not the continue block.
+    if (op == SpvOpBranchConditional || op == SpvOpSwitch) {
+      // A conditional branch or switch can only be a continue if it does not
+      // have a merge instruction or its merge block is not the continue block.
       ir::Instruction* hdrMerge = branch2merge_[user];
       if (hdrMerge != nullptr && hdrMerge->opcode() == SpvOpSelectionMerge) {
         uint32_t hdrMergeId =
@@ -353,14 +346,11 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
               ii->GetSingleWordInOperand(kLoopMergeMergeBlockIdInIdx));
         } break;
         case SpvOpSelectionMerge: {
-          auto brii = ii;
-          ++brii;
-          bool is_structured_if = brii->opcode() == SpvOpBranchConditional;
-          assume_branches_live.push(!is_structured_if);
+          assume_branches_live.push(false);
           currentMergeBlockId.push(
               ii->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx));
-          if (!is_structured_if) AddToWorklist(&*ii);
         } break;
+        case SpvOpSwitch:
         case SpvOpBranch:
         case SpvOpBranchConditional: {
           if (assume_branches_live.top()) AddToWorklist(&*ii);

diff --git a/source/opt/aggressive_dead_code_elim_pass.h b/source/opt/aggressive_dead_code_elim_pass.h
index 4c49986..3669752 100644 (file)
--- a/source/opt/aggressive_dead_code_elim_pass.h
+++ b/source/opt/aggressive_dead_code_elim_pass.h
@@ -94,15 +94,13 @@ class AggressiveDCEPass : public MemPass {
   // If |varId| is local, mark all stores of varId as live.
   void ProcessLoad(uint32_t varId);
 
-  // If |bp| is structured if or loop header block, return true and set
-  // |mergeInst| to the merge instruction, |branchInst| to the conditional
-  // branch and |mergeBlockId| to the merge block if they are not nullptr.
-  // Any of |mergeInst|, |branchInst| or |mergeBlockId| may be a null pointer.
-  // Returns false if |bp| is a null pointer.
-  bool IsStructuredIfOrLoopHeader(ir::BasicBlock* bp,
-                                  ir::Instruction** mergeInst,
-                                  ir::Instruction** branchInst,
-                                  uint32_t* mergeBlockId);
+  // If |bp| is structured header block, returns true and sets |mergeInst| to
+  // the merge instruction, |branchInst| to the branch and |mergeBlockId| to the
+  // merge block if they are not nullptr.  Any of |mergeInst|, |branchInst| or
+  // |mergeBlockId| may be a null pointer.  Returns false if |bp| is a null
+  // pointer.
+  bool IsStructuredHeader(ir::BasicBlock* bp, ir::Instruction** mergeInst,
+                          ir::Instruction** branchInst, uint32_t* mergeBlockId);
 
   // Initialize block2headerBranch_ and branch2merge_ using |structuredOrder|
   // to order blocks.

diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp
index 8099a0a..72e0de6 100644 (file)
--- a/test/opt/aggressive_dead_code_elim_test.cpp
+++ b/test/opt/aggressive_dead_code_elim_test.cpp
@@ -1484,9 +1484,7 @@ OpFunctionEnd
       predefs_before + func_before, predefs_after + func_after, true, true);
 }
 
-// This test fails. OpSwitch is not handled by ADCE.
-// (https://github.com/KhronosGroup/SPIRV-Tools/issues/1021).
-TEST_F(AggressiveDCETest, DISABLED_EliminateDeadSwitch) {
+TEST_F(AggressiveDCETest, EliminateDeadSwitch) {
   // #version 450
   //
   // layout(location = 0) in vec4 BaseColor;
@@ -1567,24 +1565,20 @@ OpDecorate %x Location 1
 OpDecorate %BaseColor Location 0
 OpDecorate %OutColor Location 0
 %void = OpTypeVoid
-%8 = OpTypeFunction %void
+%3 = OpTypeFunction %void
 %int = OpTypeInt 32 1
 %_ptr_Input_int = OpTypePointer Input %int
 %x = OpVariable %_ptr_Input_int Input
 %float = OpTypeFloat 32
-%_ptr_Function_float = OpTypePointer Function %float
 %v4float = OpTypeVector %float 4
 %_ptr_Input_v4float = OpTypePointer Input %v4float
 %BaseColor = OpVariable %_ptr_Input_v4float Input
-%uint = OpTypeInt 32 0
-%uint_1 = OpConstant %uint 1
-%_ptr_Input_float = OpTypePointer Input %float
 %_ptr_Output_v4float = OpTypePointer Output %v4float
 %OutColor = OpVariable %_ptr_Output_v4float Output
 %float_1 = OpConstant %float 1
-%20 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
-%main = OpFunction %void None %8
-%21 = OpLabel
+%27 = OpConstantComposite %v4float %float_1 %float_1 %float_1 %float_1
+%main = OpFunction %void None %3
+%5 = OpLabel
 OpBranch %11
 %11 = OpLabel
 OpStore %OutColor %27
@@ -1592,6 +1586,7 @@ OpReturn
 OpFunctionEnd
 )";
 
+  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
   SinglePassRunAndCheck<opt::AggressiveDCEPass>(before, after, true, true);
 }
 
@@ -3769,6 +3764,93 @@ OpFunctionEnd
   SinglePassRunAndCheck<opt::AggressiveDCEPass>(before, after, true, true);
 }
 
+#ifdef SPIRV_EFFCEE
+TEST_F(AggressiveDCETest, DeadNestedSwitch) {
+  const std::string text = R"(
+; CHECK: OpLabel
+; CHECK: OpBranch [[block:%\w+]]
+; CHECK-NOT: OpSwitch
+; CHECK-NEXT: [[block]] = OpLabel
+; CHECK: OpBranch [[block:%\w+]]
+; CHECK-NOT: OpSwitch
+; CHECK-NEXT: [[block]] = OpLabel
+; CHECK-NEXT: OpStore
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %func "func" %x
+OpExecutionMode %func OriginUpperLeft
+OpName %func "func"
+%void = OpTypeVoid
+%1 = OpTypeFunction %void
+%uint = OpTypeInt 32 0
+%uint_0 = OpConstant %uint 0
+%uint_ptr_Output = OpTypePointer Output %uint
+%uint_ptr_Input = OpTypePointer Input %uint
+%x = OpVariable %uint_ptr_Output Output
+%a = OpVariable %uint_ptr_Input Input
+%func = OpFunction %void None %1
+%entry = OpLabel
+OpBranch %header
+%header = OpLabel
+%ld = OpLoad %uint %a
+OpLoopMerge %merge %continue None
+OpBranch %postheader
+%postheader = OpLabel
+; This switch doesn't require an OpSelectionMerge and is nested in the dead loop.
+OpSwitch %ld %merge 0 %extra 1 %continue
+%extra = OpLabel
+OpBranch %continue
+%continue = OpLabel
+OpBranch %header
+%merge = OpLabel
+OpStore %x %uint_0
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<opt::AggressiveDCEPass>(text, true);
+}
+#endif  //  SPIRV_EFFCEE
+
+TEST_F(AggressiveDCETest, LiveNestedSwitch) {
+  const std::string text = R"(OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %func "func" %3
+OpExecutionMode %func OriginUpperLeft
+OpName %func "func"
+%void = OpTypeVoid
+%1 = OpTypeFunction %void
+%uint = OpTypeInt 32 0
+%uint_0 = OpConstant %uint 0
+%uint_1 = OpConstant %uint 1
+%_ptr_Output_uint = OpTypePointer Output %uint
+%_ptr_Input_uint = OpTypePointer Input %uint
+%3 = OpVariable %_ptr_Output_uint Output
+%10 = OpVariable %_ptr_Input_uint Input
+%func = OpFunction %void None %1
+%11 = OpLabel
+OpBranch %12
+%12 = OpLabel
+%13 = OpLoad %uint %10
+OpLoopMerge %14 %15 None
+OpBranch %16
+%16 = OpLabel
+OpSwitch %13 %14 0 %17 1 %15
+%17 = OpLabel
+OpStore %3 %uint_1
+OpBranch %15
+%15 = OpLabel
+OpBranch %12
+%14 = OpLabel
+OpStore %3 %uint_0
+OpReturn
+OpFunctionEnd
+)";
+
+  SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
+  SinglePassRunAndCheck<opt::AggressiveDCEPass>(text, text, false, true);
+}
+
 TEST_F(AggressiveDCETest, BasicDeleteDeadFunction) {
   // The function Dead should be removed because it is never called.
   const std::vector<const char*> common_code = {