ADCE: Empty Loop Elimination

This entirely eliminates loops which do not contain live code.

source/opt/aggressive_dead_code_elim_pass.cpp

@@ -101,21 +101,20 @@ void AggressiveDCEPass::ProcessLoad(uint32_t varId) {
   live_local_vars_.insert(varId);
 }
 
-bool AggressiveDCEPass::IsStructuredIfHeader(ir::BasicBlock* bp,
-                                             ir::Instruction** mergeInst,
-                                             ir::Instruction** branchInst,
-                                             uint32_t* mergeBlockId) {
-  auto ii = bp->end();
-  --ii;
-  if (ii->opcode() != SpvOpBranchConditional) return false;
-  if (ii == bp->begin()) return false;
-  if (branchInst != nullptr) *branchInst = &*ii;
-  --ii;
-  if (ii->opcode() != SpvOpSelectionMerge) return false;
-  if (mergeInst != nullptr) *mergeInst = &*ii;
-  if (mergeBlockId != nullptr)
-    *mergeBlockId =
-        ii->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx);
+bool AggressiveDCEPass::IsStructuredIfOrLoopHeader(ir::BasicBlock* bp,
+                                                   ir::Instruction** mergeInst,
+                                                   ir::Instruction** branchInst,
+                                                   uint32_t* mergeBlockId) {
+  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);
   return true;
 }
 
@@ -130,17 +129,27 @@ void AggressiveDCEPass::ComputeBlock2HeaderMaps(
   currentBranchInst.push(nullptr);
   currentMergeBlockId.push(0);
   for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); ++bi) {
+    // If leaving an if or loop, update stacks
     if ((*bi)->id() == currentMergeBlockId.top()) {
       currentMergeBlockId.pop();
       currentMergeInst.pop();
       currentBranchInst.pop();
     }
-    block2headerMerge_[*bi] = currentMergeInst.top();
-    block2headerBranch_[*bi] = currentBranchInst.top();
     ir::Instruction* mergeInst;
     ir::Instruction* branchInst;
     uint32_t mergeBlockId;
-    if (IsStructuredIfHeader(*bi, &mergeInst, &branchInst, &mergeBlockId)) {
+    bool is_header =
+        IsStructuredIfOrLoopHeader(*bi, &mergeInst, &branchInst, &mergeBlockId);
+    // If there is live code in loop header, the loop is live
+    if (is_header && mergeInst->opcode() == SpvOpLoopMerge) {
+      currentMergeBlockId.push(mergeBlockId);
+      currentMergeInst.push(mergeInst);
+      currentBranchInst.push(branchInst);
+    }
+    block2headerMerge_[*bi] = currentMergeInst.top();
+    block2headerBranch_[*bi] = currentBranchInst.top();
+    // If there is live code following if header, the if is live
+    if (is_header && mergeInst->opcode() == SpvOpSelectionMerge) {
       currentMergeBlockId.push(mergeBlockId);
       currentMergeInst.push(mergeInst);
       currentBranchInst.push(branchInst);
@@ -180,19 +189,23 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
   ComputeBlock2HeaderMaps(structuredOrder);
   bool modified = false;
   // Add instructions with external side effects to worklist. Also add branches
-  // EXCEPT those immediately contained in an "if" selection construct.
+  // EXCEPT those immediately contained in an "if" selection construct or a loop
+  // or continue construct.
   // TODO(greg-lunarg): Handle Frexp, Modf more optimally
   call_in_func_ = false;
   func_is_entry_point_ = false;
   private_stores_.clear();
-  // Stacks to keep track of when we are inside an if-construct. When not
-  // immediately inside an in-construct,  we must assume all branches are live.
+  // Stacks to keep track of when we are inside an if- or loop-construct.
+  // When not immediately inside an if- or loop-construct,  we must assume
+  // all branches are live as we may be inside of a control construct (ie
+  // switch) which is not part of the ADCE analysis.
   std::stack<bool> assume_branches_live;
   std::stack<uint32_t> currentMergeBlockId;
   // Push sentinel values on stack for when outside of any control flow.
   assume_branches_live.push(true);
   currentMergeBlockId.push(0);
   for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); ++bi) {
+    // If exiting if or loop, update stacks
     if ((*bi)->id() == currentMergeBlockId.top()) {
       assume_branches_live.pop();
       currentMergeBlockId.pop();
@@ -212,12 +225,9 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
             AddToWorklist(&*ii);
         } break;
         case SpvOpLoopMerge: {
-          // Assume loops live (for now)
-          // TODO(greg-lunarg): Add dead loop elimination
-          assume_branches_live.push(true);
+          assume_branches_live.push(false);
           currentMergeBlockId.push(
               ii->GetSingleWordInOperand(kLoopMergeMergeBlockIdInIdx));
-          AddToWorklist(&*ii);
         } break;
         case SpvOpSelectionMerge: {
           auto brii = ii;
@@ -268,18 +278,32 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
   while (!worklist_.empty()) {
     ir::Instruction* liveInst = worklist_.front();
     // Add all operand instructions if not already live
-    liveInst->ForEachInId([this](const uint32_t* iid) {
+    liveInst->ForEachInId([&liveInst, this](const uint32_t* iid) {
       ir::Instruction* inInst = get_def_use_mgr()->GetDef(*iid);
+      // Do not add label if an operand of a branch. This is not needed
+      // as part of live code discovery and can create false live code,
+      // for example, the branch to a header of a loop.
+      if (inInst->opcode() == SpvOpLabel && liveInst->IsBranch()) return;
       if (!IsLive(inInst)) AddToWorklist(inInst);
     });
-    // If in a structured if construct, add the controlling conditional branch
-    // and its merge. Any containing if construct is marked live when the
-    // the merge and branch are processed out of the worklist.
+    // If in a structured if or loop construct, add the controlling
+    // conditional branch and its merge. Any containing control construct
+    // is marked live when the merge and branch are processed out of the
+    // worklist.
     ir::BasicBlock* blk = inst2block_[liveInst];
     ir::Instruction* branchInst = block2headerBranch_[blk];
     if (branchInst != nullptr && !IsLive(branchInst)) {
       AddToWorklist(branchInst);
-      AddToWorklist(block2headerMerge_[blk]);
+      ir::Instruction* mergeInst = block2headerMerge_[blk];
+      AddToWorklist(mergeInst);
+      // If in a loop, mark all branches targeting merge block
+      // and continue block as live.
+      if (mergeInst->opcode() == SpvOpLoopMerge) {
+        AddBranchesToWorklist(
+            mergeInst->GetSingleWordInOperand(kLoopMergeMergeBlockIdInIdx));
+        AddBranchesToWorklist(
+            mergeInst->GetSingleWordInOperand(kLoopMergeContinueBlockIdInIdx));
+      }
     }
     // If local load, add all variable's stores if variable not already live
     if (liveInst->opcode() == SpvOpLoad) {
@@ -297,27 +321,19 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
         ProcessLoad(varId);
       });
     }
-    // If loop merge, add all branches to continue and merge blocks
-    // to worklist
-    else if (liveInst->opcode() == SpvOpLoopMerge) {
-      AddBranchesToWorklist(
-          liveInst->GetSingleWordInOperand(kLoopMergeContinueBlockIdInIdx));
-      AddBranchesToWorklist(
-          liveInst->GetSingleWordInOperand(kLoopMergeMergeBlockIdInIdx));
-    }
     // If function parameter, treat as if it's result id is loaded from
     else if (liveInst->opcode() == SpvOpFunctionParameter) {
       ProcessLoad(liveInst->result_id());
     }
     worklist_.pop();
   }
-  // Mark all non-live instructions dead, except branches which are not
-  // at the end of an if-header, which indicate a dead if.
+  // Mark all non-live instructions dead except non-structured branches, which
+  // now should be considered live unless their block is deleted.
   for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); ++bi) {
     for (auto ii = (*bi)->begin(); ii != (*bi)->end(); ++ii) {
       if (IsLive(&*ii)) continue;
       if (ii->IsBranch() &&
-          !IsStructuredIfHeader(*bi, nullptr, nullptr, nullptr))
+          !IsStructuredIfOrLoopHeader(*bi, nullptr, nullptr, nullptr))
         continue;
       dead_insts_.insert(&*ii);
     }
@@ -363,15 +379,15 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
       if (dead_insts_.find(inst) == dead_insts_.end()) return;
       // If dead instruction is selection merge, remember merge block
       // for new branch at end of block
-      if (inst->opcode() == SpvOpSelectionMerge)
-        mergeBlockId =
-            inst->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx);
+      if (inst->opcode() == SpvOpSelectionMerge ||
+          inst->opcode() == SpvOpLoopMerge)
+        mergeBlockId = inst->GetSingleWordInOperand(0);
       context()->KillInst(inst);
       modified = true;
     });
-    // If a structured if was deleted, add a branch to its merge block,
-    // and traverse to the merge block, continuing processing there.
-    // The block still exists as the OpLabel at least is still intact.
+    // If a structured if or loop was deleted, add a branch to its merge
+    // block, and traverse to the merge block and continue processing there.
+    // We know the block still exists because the label is not deleted.
     if (mergeBlockId != 0) {
       AddBranch(mergeBlockId, *bi);
       for (++bi; (*bi)->id() != mergeBlockId; ++bi) {

source/opt/aggressive_dead_code_elim_pass.h

@@ -85,11 +85,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 header block, return true and set |branchInst|
-  // to the conditional branch and |mergeBlockId| to the merge block.
-  bool IsStructuredIfHeader(ir::BasicBlock* bp, ir::Instruction** mergeInst,
-                            ir::Instruction** branchInst,
-                            uint32_t* mergeBlockId);
+  // 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.
+  bool IsStructuredIfOrLoopHeader(ir::BasicBlock* bp,
+                                  ir::Instruction** mergeInst,
+                                  ir::Instruction** branchInst,
+                                  uint32_t* mergeBlockId);
 
   // Initialize block2branch_ and block2merge_ using |structuredOrder| to
   // order blocks.
@@ -101,7 +103,7 @@ class AggressiveDCEPass : public MemPass {
   // Add branch to |labelId| to end of block |bp|.
   void AddBranch(uint32_t labelId, ir::BasicBlock* bp);
 
-  // Add all branches to |labelId| to worklist if not already live
+  // Add all branches targeting |labelId| to worklist if not already live
   void AddBranchesToWorklist(uint32_t labelId);
 
   // For function |func|, mark all Stores to non-function-scope variables