Change IRContext::KillInst to delete instructions.
authorSteven Perron <stevenperron@google.com>
Tue, 21 Nov 2017 19:47:46 +0000 (14:47 -0500)
committerSteven Perron <stevenperron@google.com>
Mon, 4 Dec 2017 16:07:45 +0000 (11:07 -0500)
The current method of removing an instruction is to call ToNop.  The
problem with this is that it leaves around an instruction that later
passes will look at.  We should just delete the instruction.

In MemPass there is a utility routine called DCEInst.  It can delete
essentially any instruction, which can invalidate pointers now that they
are actually deleted.  The interface was changed to add a call back that
can be used to update any local data structures that contain
ir::Intruction*.

28 files changed:
source/link/linker.cpp
source/opt/aggressive_dead_code_elim_pass.cpp
source/opt/aggressive_dead_code_elim_pass.h
source/opt/basic_block.h
source/opt/common_uniform_elim_pass.cpp
source/opt/common_uniform_elim_pass.h
source/opt/dead_branch_elim_pass.cpp
source/opt/eliminate_dead_constant_pass.cpp
source/opt/fold_spec_constant_op_and_composite_pass.cpp
source/opt/insert_extract_elim.cpp
source/opt/instruction.cpp
source/opt/instruction.h
source/opt/instruction_list.h
source/opt/ir_context.cpp
source/opt/ir_context.h
source/opt/local_access_chain_convert_pass.cpp
source/opt/local_access_chain_convert_pass.h
source/opt/local_single_block_elim_pass.cpp
source/opt/local_single_store_elim_pass.cpp
source/opt/local_ssa_elim_pass.cpp
source/opt/mem_pass.cpp
source/opt/mem_pass.h
source/opt/merge_return_pass.cpp
source/opt/module.cpp
source/opt/remove_duplicates_pass.cpp
source/opt/strength_reduction_pass.cpp
source/opt/unify_const_pass.cpp
test/opt/def_use_test.cpp

index 730ec376373e3c973e3a57be6d8dfbf21500a613..b8225809e9f582bec752b64335c4ad26900174fa 100644 (file)
@@ -705,9 +705,14 @@ static spv_result_t RemoveLinkageSpecificInstructions(
 
   // Remove declarations of imported variables
   for (const auto& linking_entry : linkings_to_do) {
-    for (auto& inst : linked_context->types_values())
-      if (inst.result_id() == linking_entry.imported_symbol.id)
-        linked_context->KillInst(&inst);
+    auto next = linked_context->types_values_begin();
+    for (auto inst = next; inst != linked_context->types_values_end();
+         inst = next) {
+      ++next;
+      if (inst->result_id() == linking_entry.imported_symbol.id) {
+        linked_context->KillInst(&*inst);
+      }
+    }
   }
 
   // Remove import linkage attributes
index 4704d406bc13c54c998757cb7da01d9e28c037ec..62973b00cb5e6e67adf11a77a1bd2a5a597fa141 100644 (file)
@@ -81,11 +81,10 @@ bool AggressiveDCEPass::AllExtensionsSupported() const {
   return true;
 }
 
-bool AggressiveDCEPass::KillInstIfTargetDead(ir::Instruction* inst) {
+bool AggressiveDCEPass::IsTargetDead(ir::Instruction* inst) {
   const uint32_t tId = inst->GetSingleWordInOperand(0);
   const ir::Instruction* tInst = get_def_use_mgr()->GetDef(tId);
   if (dead_insts_.find(tInst) != dead_insts_.end()) {
-    context()->KillInst(inst);
     return true;
   }
   return false;
@@ -326,28 +325,50 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
   // Remove debug and annotation statements referencing dead instructions.
   // This must be done before killing the instructions, otherwise there are
   // dead objects in the def/use database.
-  for (auto& di : get_module()->debugs2()) {
-    if (di.opcode() != SpvOpName) continue;
-    if (KillInstIfTargetDead(&di)) modified = true;
+  ir::Instruction* instruction = &*get_module()->debug2_begin();
+  while (instruction) {
+    if (instruction->opcode() != SpvOpName) {
+      instruction = instruction->NextNode();
+      continue;
+    }
+
+    if (IsTargetDead(instruction)) {
+      instruction = context()->KillInst(instruction);
+      modified = true;
+    } else {
+      instruction = instruction->NextNode();
+    }
   }
-  for (auto& ai : get_module()->annotations()) {
-    if (ai.opcode() != SpvOpDecorate && ai.opcode() != SpvOpDecorateId)
+
+  instruction = &*get_module()->annotation_begin();
+  while (instruction) {
+    if (instruction->opcode() != SpvOpDecorate &&
+        instruction->opcode() != SpvOpDecorateId) {
+      instruction = instruction->NextNode();
       continue;
-    if (KillInstIfTargetDead(&ai)) modified = true;
+    }
+
+    if (IsTargetDead(instruction)) {
+      instruction = context()->KillInst(instruction);
+      modified = true;
+    } else {
+      instruction = instruction->NextNode();
+    }
   }
+
   // Kill dead instructions and remember dead blocks
   for (auto bi = structuredOrder.begin(); bi != structuredOrder.end();) {
     uint32_t mergeBlockId = 0;
-    for (auto ii = (*bi)->begin(); ii != (*bi)->end(); ++ii) {
-      if (dead_insts_.find(&*ii) == dead_insts_.end()) continue;
+    (*bi)->ForEachInst([this, &modified, &mergeBlockId](ir::Instruction* inst) {
+      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 (ii->opcode() == SpvOpSelectionMerge)
+      if (inst->opcode() == SpvOpSelectionMerge)
         mergeBlockId =
-            ii->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx);
-      context()->KillInst(&*ii);
+            inst->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx);
+      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.
index 024d82caff5a5748b264a0ffc0cfc0d406ae5cea..3112fbcf2c56c4c272cdbc6344f57b111ec35ebf 100644 (file)
@@ -78,9 +78,9 @@ class AggressiveDCEPass : public MemPass {
   // Return true if all extensions in this module are supported by this pass.
   bool AllExtensionsSupported() const;
 
-  // Kill debug or annotation |inst| if target operand is dead. Return true
-  // if inst killed.
-  bool KillInstIfTargetDead(ir::Instruction* inst);
+  // Returns true if |inst| is dead.  An instruction is dead if its result id
+  // is used in decoration or debug instructions only.
+  bool IsTargetDead(ir::Instruction* inst);
 
   // If |varId| is local, mark all stores of varId as live.
   void ProcessLoad(uint32_t varId);
index c84206cd4a1fa2137b36aa8f8844c33a87bae2d0..ba1552ab420d20cea32654b66def80a376e15357 100644 (file)
@@ -166,7 +166,16 @@ inline void BasicBlock::AddInstructions(BasicBlock* bp) {
 inline void BasicBlock::ForEachInst(const std::function<void(Instruction*)>& f,
                                     bool run_on_debug_line_insts) {
   if (label_) label_->ForEachInst(f, run_on_debug_line_insts);
-  for (auto& inst : insts_) inst.ForEachInst(f, run_on_debug_line_insts);
+  if (insts_.empty()) {
+    return;
+  }
+
+  Instruction* inst = &insts_.front();
+  while (inst != nullptr) {
+    Instruction* next_instruction = inst->NextNode();
+    inst->ForEachInst(f, run_on_debug_line_insts);
+    inst = next_instruction;
+  }
 }
 
 inline void BasicBlock::ForEachInst(
index 721b953690aa9fcbc4909822ea5127ef6c5534ee..8f4c95b4830ab0cb22e237708feb715aaeec6693 100644 (file)
@@ -207,16 +207,16 @@ void CommonUniformElimPass::DeleteIfUseless(ir::Instruction* inst) {
   }
 }
 
-void CommonUniformElimPass::ReplaceAndDeleteLoad(ir::Instruction* loadInst,
-                                                 uint32_t replId,
-                                                 ir::Instruction* ptrInst) {
+ir::Instruction* CommonUniformElimPass::ReplaceAndDeleteLoad(
+    ir::Instruction* loadInst, uint32_t replId, ir::Instruction* ptrInst) {
   const uint32_t loadId = loadInst->result_id();
   context()->KillNamesAndDecorates(loadId);
   (void)context()->ReplaceAllUsesWith(loadId, replId);
   // remove load instruction
-  context()->KillInst(loadInst);
+  ir::Instruction* next_instruction = context()->KillInst(loadInst);
   // if access chain, see if it can be removed as well
   if (IsNonPtrAccessChain(ptrInst->opcode())) DeleteIfUseless(ptrInst);
+  return next_instruction;
 }
 
 void CommonUniformElimPass::GenACLoadRepl(
@@ -281,29 +281,27 @@ bool CommonUniformElimPass::IsConstantIndexAccessChain(ir::Instruction* acp) {
 bool CommonUniformElimPass::UniformAccessChainConvert(ir::Function* func) {
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
-    for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
-      if (ii->opcode() != SpvOpLoad) continue;
+    for (ir::Instruction* inst = &*bi->begin(); inst; inst = inst->NextNode()) {
+      if (inst->opcode() != SpvOpLoad) continue;
       uint32_t varId;
-      ir::Instruction* ptrInst = GetPtr(&*ii, &varId);
+      ir::Instruction* ptrInst = GetPtr(inst, &varId);
       if (!IsNonPtrAccessChain(ptrInst->opcode())) continue;
       // Do not convert nested access chains
       if (ptrInst->GetSingleWordInOperand(kAccessChainPtrIdInIdx) != varId)
         continue;
       if (!IsUniformVar(varId)) continue;
       if (!IsConstantIndexAccessChain(ptrInst)) continue;
-      if (HasUnsupportedDecorates(ii->result_id())) continue;
+      if (HasUnsupportedDecorates(inst->result_id())) continue;
       if (HasUnsupportedDecorates(ptrInst->result_id())) continue;
-      if (IsVolatileLoad(*ii)) continue;
+      if (IsVolatileLoad(*inst)) continue;
       if (IsAccessChainToVolatileStructType(*ptrInst)) continue;
       std::vector<std::unique_ptr<ir::Instruction>> newInsts;
       uint32_t replId;
       GenACLoadRepl(ptrInst, &newInsts, &replId);
-      ReplaceAndDeleteLoad(&*ii, replId, ptrInst);
-      ++ii;
-      ii = ii.InsertBefore(std::move(newInsts));
-      ++ii;
+      inst = ReplaceAndDeleteLoad(inst, replId, ptrInst);
+      inst = inst->InsertBefore(std::move(newInsts));
       modified = true;
-    }
+    };
   }
   return modified;
 }
@@ -371,15 +369,15 @@ bool CommonUniformElimPass::CommonUniformLoadElimination(ir::Function* func) {
       mergeBlockId = 0;
       insertItr = bp->begin();
     }
-    for (auto ii = bp->begin(); ii != bp->end(); ++ii) {
-      if (ii->opcode() != SpvOpLoad) continue;
+    for (ir::Instruction* inst = &*bp->begin(); inst; inst = inst->NextNode()) {
+      if (inst->opcode() != SpvOpLoad) continue;
       uint32_t varId;
-      ir::Instruction* ptrInst = GetPtr(&*ii, &varId);
+      ir::Instruction* ptrInst = GetPtr(inst, &varId);
       if (ptrInst->opcode() != SpvOpVariable) continue;
       if (!IsUniformVar(varId)) continue;
       if (IsSamplerOrImageVar(varId)) continue;
-      if (HasUnsupportedDecorates(ii->result_id())) continue;
-      if (IsVolatileLoad(*ii)) continue;
+      if (HasUnsupportedDecorates(inst->result_id())) continue;
+      if (IsVolatileLoad(*inst)) continue;
       uint32_t replId;
       const auto uItr = uniform2load_id_.find(varId);
       if (uItr != uniform2load_id_.end()) {
@@ -387,13 +385,13 @@ bool CommonUniformElimPass::CommonUniformLoadElimination(ir::Function* func) {
       } else {
         if (mergeBlockId == 0) {
           // Load is in dominating block; just remember it
-          uniform2load_id_[varId] = ii->result_id();
+          uniform2load_id_[varId] = inst->result_id();
           continue;
         } else {
           // Copy load into most recent dominating block and remember it
           replId = TakeNextId();
           std::unique_ptr<ir::Instruction> newLoad(new ir::Instruction(
-              context(), SpvOpLoad, ii->type_id(), replId,
+              context(), SpvOpLoad, inst->type_id(), replId,
               {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {varId}}}));
           get_def_use_mgr()->AnalyzeInstDefUse(&*newLoad);
           insertItr = insertItr.InsertBefore(std::move(newLoad));
@@ -401,7 +399,7 @@ bool CommonUniformElimPass::CommonUniformLoadElimination(ir::Function* func) {
           uniform2load_id_[varId] = replId;
         }
       }
-      ReplaceAndDeleteLoad(&*ii, replId, ptrInst);
+      inst = ReplaceAndDeleteLoad(inst, replId, ptrInst);
       modified = true;
     }
     // If we are outside of any control construct and entering one, remember
@@ -417,24 +415,24 @@ bool CommonUniformElimPass::CommonUniformLoadElimBlock(ir::Function* func) {
   bool modified = false;
   for (auto& blk : *func) {
     uniform2load_id_.clear();
-    for (auto ii = blk.begin(); ii != blk.end(); ++ii) {
-      if (ii->opcode() != SpvOpLoad) continue;
+    for (ir::Instruction* inst = &*blk.begin(); inst; inst = inst->NextNode()) {
+      if (inst->opcode() != SpvOpLoad) continue;
       uint32_t varId;
-      ir::Instruction* ptrInst = GetPtr(&*ii, &varId);
+      ir::Instruction* ptrInst = GetPtr(inst, &varId);
       if (ptrInst->opcode() != SpvOpVariable) continue;
       if (!IsUniformVar(varId)) continue;
       if (!IsSamplerOrImageVar(varId)) continue;
-      if (HasUnsupportedDecorates(ii->result_id())) continue;
-      if (IsVolatileLoad(*ii)) continue;
+      if (HasUnsupportedDecorates(inst->result_id())) continue;
+      if (IsVolatileLoad(*inst)) continue;
       uint32_t replId;
       const auto uItr = uniform2load_id_.find(varId);
       if (uItr != uniform2load_id_.end()) {
         replId = uItr->second;
       } else {
-        uniform2load_id_[varId] = ii->result_id();
+        uniform2load_id_[varId] = inst->result_id();
         continue;
       }
-      ReplaceAndDeleteLoad(&*ii, replId, ptrInst);
+      inst = ReplaceAndDeleteLoad(inst, replId, ptrInst);
       modified = true;
     }
   }
index 6f1e65fc9006beb5e6ac593dc92f2eb7c3cb530c..64152089264049e1fa6037e0536a544ad70730c3 100644 (file)
@@ -91,8 +91,9 @@ class CommonUniformElimPass : public Pass {
 
   // Replace all instances of load's id with replId and delete load
   // and its access chain, if any
-  void ReplaceAndDeleteLoad(ir::Instruction* loadInst, uint32_t replId,
-                            ir::Instruction* ptrInst);
+  ir::Instruction* ReplaceAndDeleteLoad(ir::Instruction* loadInst,
+                                        uint32_t replId,
+                                        ir::Instruction* ptrInst);
 
   // For the (constant index) access chain ptrInst, create an
   // equivalent load and extract
index 90ef32d87aa71f18df160948b1a5c7955dfcadcc..52199babb802174d4e1109b5611d85511d017add 100644 (file)
@@ -258,17 +258,21 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
     if (liveLabId != dLabId) deadPreds.insert(*bi);
 
     // Update phi instructions in terminating block.
-    for (auto pii = (*dbi)->begin();; ++pii) {
+    ir::Instruction* inst = &*(*dbi)->begin();
+    while (inst) {
       // Skip NoOps, break at end of phis
-      SpvOp op = pii->opcode();
-      if (op == SpvOpNop) continue;
+      SpvOp op = inst->opcode();
+      if (op == SpvOpNop) {
+        inst = inst->NextNode();
+        continue;
+      }
       if (op != SpvOpPhi) break;
       // Count phi's live predecessors with lcnt and remember last one
       // with lidx.
       uint32_t lcnt = 0;
       uint32_t lidx = 0;
       uint32_t icnt = 0;
-      pii->ForEachInId([&deadPreds, &icnt, &lcnt, &lidx, this](uint32_t* idp) {
+      inst->ForEachInId([&deadPreds, &icnt, &lcnt, &lidx, this](uint32_t* idp) {
         if (icnt % 2 == 1) {
           if (deadPreds.find(cfg()->block(*idp)) == deadPreds.end()) {
             ++lcnt;
@@ -280,7 +284,7 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
       // If just one live predecessor, replace resultid with live value id.
       uint32_t replId;
       if (lcnt == 1) {
-        replId = pii->GetSingleWordInOperand(lidx);
+        replId = inst->GetSingleWordInOperand(lidx);
       } else {
         // Otherwise create new phi eliminating dead predecessor entries
         assert(lcnt > 1);
@@ -288,7 +292,7 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
         std::vector<ir::Operand> phi_in_opnds;
         icnt = 0;
         uint32_t lastId;
-        pii->ForEachInId(
+        inst->ForEachInId(
             [&deadPreds, &icnt, &phi_in_opnds, &lastId, this](uint32_t* idp) {
               if (icnt % 2 == 1) {
                 if (deadPreds.find(cfg()->block(*idp)) == deadPreds.end()) {
@@ -303,15 +307,14 @@ bool DeadBranchElimPass::EliminateDeadBranches(ir::Function* func) {
               ++icnt;
             });
         std::unique_ptr<ir::Instruction> newPhi(new ir::Instruction(
-            context(), SpvOpPhi, pii->type_id(), replId, phi_in_opnds));
+            context(), SpvOpPhi, inst->type_id(), replId, phi_in_opnds));
         get_def_use_mgr()->AnalyzeInstDefUse(&*newPhi);
-        pii = pii.InsertBefore(std::move(newPhi));
-        ++pii;
+        inst->InsertBefore(std::move(newPhi));
       }
-      const uint32_t phiId = pii->result_id();
+      const uint32_t phiId = inst->result_id();
       context()->KillNamesAndDecorates(phiId);
       (void)context()->ReplaceAllUsesWith(phiId, replId);
-      context()->KillInst(&*pii);
+      inst = context()->KillInst(inst);
     }
   }
 
index 5c60f3444d554f4983bcd07a87810f56faca125b..5e299c6d29807cb98cf770168e9d9253f761702a 100644 (file)
@@ -91,27 +91,10 @@ Pass::Status EliminateDeadConstantPass::Process(ir::IRContext* irContext) {
     working_list.erase(inst);
   }
 
-  // Find all annotation and debug instructions that are referencing dead
-  // constants.
-  std::unordered_set<ir::Instruction*> dead_others;
-  for (auto* dc : dead_consts) {
-    irContext->get_def_use_mgr()->ForEachUser(
-        dc, [&dead_others](ir::Instruction* user) {
-          SpvOp op = user->opcode();
-          if (ir::IsAnnotationInst(op) || ir::IsDebug1Inst(op) ||
-              ir::IsDebug2Inst(op) || ir::IsDebug3Inst(op)) {
-            dead_others.insert(user);
-          }
-        });
-  }
-
   // Turn all dead instructions and uses of them to nop
   for (auto* dc : dead_consts) {
     irContext->KillDef(dc->result_id());
   }
-  for (auto* da : dead_others) {
-    irContext->KillInst(da);
-  }
   return dead_consts.empty() ? Status::SuccessWithoutChange
                              : Status::SuccessWithChange;
 }
index dc33eda76145d46e8adeca81439474ed3226d93f..5f8a66b78ed99ea0efebcc33a6fbd0e458da105d 100644 (file)
@@ -282,10 +282,12 @@ Pass::Status FoldSpecConstantOpAndCompositePass::ProcessImpl(
   // the dependee Spec Constants, all its dependent constants must have been
   // processed and all its dependent Spec Constants should have been folded if
   // possible.
-  for (ir::Module::inst_iterator inst_iter = irContext->types_values_begin();
+  ir::Module::inst_iterator next_inst = irContext->types_values_begin();
+  for (ir::Module::inst_iterator inst_iter = next_inst;
        // Need to re-evaluate the end iterator since we may modify the list of
        // instructions in this section of the module as the process goes.
-       inst_iter != irContext->types_values_end(); ++inst_iter) {
+       inst_iter != irContext->types_values_end(); inst_iter = next_inst) {
+    ++next_inst;
     ir::Instruction* inst = &*inst_iter;
     // Collect constant values of normal constants and process the
     // OpSpecConstantOp and OpSpecConstantComposite instructions if possible.
index d249e421290e0e4b0ab21401335d3e21be35cf8e..43d34fa78098e349baec066b498b57e1cb293eb0 100644 (file)
@@ -66,25 +66,26 @@ bool InsertExtractElimPass::IsVectorType(uint32_t typeId) {
 bool InsertExtractElimPass::EliminateInsertExtract(ir::Function* func) {
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
-    for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
-      switch (ii->opcode()) {
+    ir::Instruction* inst = &*bi->begin();
+    while (inst) {
+      switch (inst->opcode()) {
         case SpvOpCompositeExtract: {
-          uint32_t cid = ii->GetSingleWordInOperand(kExtractCompositeIdInIdx);
+          uint32_t cid = inst->GetSingleWordInOperand(kExtractCompositeIdInIdx);
           ir::Instruction* cinst = get_def_use_mgr()->GetDef(cid);
           uint32_t replId = 0;
           // Offset of extract indices being compared to insert indices.
           // Offset increases as indices are matched.
           uint32_t extOffset = 0;
           while (cinst->opcode() == SpvOpCompositeInsert) {
-            if (ExtInsMatch(&*ii, cinst, extOffset)) {
+            if (ExtInsMatch(inst, cinst, extOffset)) {
               // Match! Use inserted value as replacement
               replId = cinst->GetSingleWordInOperand(kInsertObjectIdInIdx);
               break;
-            } else if (ExtInsConflict(&*ii, cinst, extOffset)) {
+            } else if (ExtInsConflict(inst, cinst, extOffset)) {
               // If extract has fewer indices than the insert, stop searching.
               // Otherwise increment offset of extract indices considered and
               // continue searching through the inserted value
-              if (ii->NumInOperands() - extOffset <
+              if (inst->NumInOperands() - extOffset <
                   cinst->NumInOperands() - 1) {
                 break;
               } else {
@@ -105,15 +106,15 @@ bool InsertExtractElimPass::EliminateInsertExtract(ir::Function* func) {
           // vector composition, and additional CompositeInsert.
           if ((cinst->opcode() == SpvOpCompositeConstruct ||
                cinst->opcode() == SpvOpConstantComposite) &&
-              (*ii).NumInOperands() - extOffset == 2) {
-            uint32_t compIdx = (*ii).GetSingleWordInOperand(extOffset + 1);
+              inst->NumInOperands() - extOffset == 2) {
+            uint32_t compIdx = inst->GetSingleWordInOperand(extOffset + 1);
             if (IsVectorType(cinst->type_id())) {
               if (compIdx < cinst->NumInOperands()) {
                 uint32_t i = 0;
                 for (; i <= compIdx; i++) {
                   uint32_t compId = cinst->GetSingleWordInOperand(i);
                   ir::Instruction* compInst = get_def_use_mgr()->GetDef(compId);
-                  if (compInst->type_id() != (*ii).type_id()) break;
+                  if (compInst->type_id() != inst->type_id()) break;
                 }
                 if (i > compIdx)
                   replId = cinst->GetSingleWordInOperand(compIdx);
@@ -123,13 +124,16 @@ bool InsertExtractElimPass::EliminateInsertExtract(ir::Function* func) {
             }
           }
           if (replId != 0) {
-            const uint32_t extId = ii->result_id();
+            const uint32_t extId = inst->result_id();
             (void)context()->ReplaceAllUsesWith(extId, replId);
-            context()->KillInst(&*ii);
+            inst = context()->KillInst(inst);
             modified = true;
+          } else {
+            inst = inst->NextNode();
           }
         } break;
         default:
+          inst = inst->NextNode();
           break;
       }
     }
index 571377267a1fe703a5a2ed2842c160654a802421..da52c95c9b6fad821b5a2292b0cb92c3523c6c47 100644 (file)
@@ -354,5 +354,20 @@ bool Instruction::IsReadOnlyVariableKernel() const {
   uint32_t storage_class = GetSingleWordInOperand(kVariableStorageClassIndex);
   return storage_class == SpvStorageClassUniformConstant;
 }
+
+Instruction* Instruction::InsertBefore(
+    std::vector<std::unique_ptr<Instruction>>&& list) {
+  Instruction* first_node = list.front().get();
+  for (auto& i : list) {
+    i.release()->InsertBefore(this);
+  }
+  list.clear();
+  return first_node;
+}
+
+Instruction* Instruction::InsertBefore(std::unique_ptr<Instruction>&& i) {
+  i.get()->InsertBefore(this);
+  return i.release();
+}
 }  // namespace ir
 }  // namespace spvtools
index 08c5b77ee8916be604eacb9a9a2168e137b94312..1bd1344e6aa636d9546537bbacd2ca865aae1b7c 100644 (file)
@@ -312,6 +312,10 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
   inline bool operator!=(const Instruction&) const;
   inline bool operator<(const Instruction&) const;
 
+  Instruction* InsertBefore(std::vector<std::unique_ptr<Instruction>>&& list);
+  Instruction* InsertBefore(std::unique_ptr<Instruction>&& i);
+  using utils::IntrusiveNodeBase<Instruction>::InsertBefore;
+
  private:
   // Returns the total count of result type id and result id.
   uint32_t TypeResultIdCount() const {
index d9d94b34763e1ea6a383b877c70e1ffccba25e34..2b7b931f1f37bf1793090291bd8ed95aa4508f9b 100644 (file)
@@ -101,6 +101,18 @@ class InstructionList : public utils::IntrusiveList<Instruction> {
 
   // Same as in the base class, except it will delete the data as well.
   inline void clear();
+
+  // Runs the given function |f| on the instructions in the list and optionally
+  // on the preceding debug line instructions.
+  inline void ForEachInst(
+      const std::function<void(Instruction*)>& f,
+      bool run_on_debug_line_insts) {
+    auto next = begin();
+    for( auto i = next; i != end(); i = next ) {
+      ++next;
+      i->ForEachInst(f, run_on_debug_line_insts);
+    }
+  }
 };
 
 InstructionList::~InstructionList() { clear(); }
index f516d0edb8860a34e53104e6e1fa8ecd3cf1864f..6576a0d4fb65e37293f3a4fa326f0d203430e712 100644 (file)
@@ -61,9 +61,9 @@ void IRContext::InvalidateAnalyses(IRContext::Analysis analyses_to_invalidate) {
   valid_analyses_ = Analysis(valid_analyses_ & ~analyses_to_invalidate);
 }
 
-void IRContext::KillInst(ir::Instruction* inst) {
+Instruction* IRContext::KillInst(ir::Instruction* inst) {
   if (!inst) {
-    return;
+    return nullptr;
   }
 
   KillNamesAndDecorates(inst);
@@ -83,7 +83,17 @@ void IRContext::KillInst(ir::Instruction* inst) {
     }
   }
 
-  inst->ToNop();
+  Instruction* next_instruction = nullptr;
+  if (inst->IsInAList()) {
+    next_instruction = inst->NextNode();
+    inst->RemoveFromList();
+    delete inst;
+  } else {
+    // Needed for instructions that are not part of a list like OpLabels,
+    // OpFunction, OpFunctionEnd, etc..
+    inst->ToNop();
+  }
+  return next_instruction;
 }
 
 bool IRContext::KillDef(uint32_t id) {
@@ -189,12 +199,19 @@ void IRContext::KillNamesAndDecorates(uint32_t id) {
     KillInst(inst);
   }
 
-  for (auto& di : debugs2()) {
-    if (di.opcode() == SpvOpMemberName || di.opcode() == SpvOpName) {
-      if (di.GetSingleWordInOperand(0) == id) {
-        KillInst(&di);
+  Instruction* debug_inst = &*debug2_begin();
+  while (debug_inst) {
+    bool killed_inst = false;
+    if (debug_inst->opcode() == SpvOpMemberName ||
+        debug_inst->opcode() == SpvOpName) {
+      if (debug_inst->GetSingleWordInOperand(0) == id) {
+        debug_inst = KillInst(debug_inst);
+        killed_inst = true;
       }
     }
+    if (!killed_inst) {
+      debug_inst = debug_inst->NextNode();
+    }
   }
 }
 
index 7e2a20f8d2f843330871b7d12280356de53693f1..a6a2209fb7641380a2035db2f1cfb7366858fd84 100644 (file)
@@ -107,6 +107,8 @@ class IRContext {
   // Iterators for extension instructions contained in this module.
   inline Module::inst_iterator ext_inst_import_begin();
   inline Module::inst_iterator ext_inst_import_end();
+  inline IteratorRange<Module::inst_iterator> ext_inst_imports();
+  inline IteratorRange<Module::const_inst_iterator> ext_inst_imports() const;
 
   // There are several kinds of debug instructions, according to where they can
   // appear in the logical layout of a module:
@@ -218,16 +220,28 @@ class IRContext {
   // Invalidates the analyses marked in |analyses_to_invalidate|.
   void InvalidateAnalyses(Analysis analyses_to_invalidate);
 
-  // Turns the instruction defining the given |id| into a Nop. Returns true on
+  // Deletes the instruction defining the given |id|. Returns true on
   // success, false if the given |id| is not defined at all. This method also
-  // erases both the uses of |id| and the information of this |id|-generating
-  // instruction's uses of its operands.
+  // erases the name, decorations, and defintion of |id|.
+  //
+  // Pointers and iterators pointing to the deleted instructions become invalid.
+  // However other pointers and iterators are still valid.
   bool KillDef(uint32_t id);
 
-  // Turns the given instruction |inst| to a Nop. This method erases the
+  // Deletes the given instruction |inst|. This method erases the
   // information of the given instruction's uses of its operands. If |inst|
-  // defines an result id, the uses of the result id will also be erased.
-  void KillInst(ir::Instruction* inst);
+  // defines a result id, its name and decorations will also be deleted.
+  //
+  // Pointer and iterator pointing to the deleted instructions become invalid.
+  // However other pointers and iterators are still valid.
+  //
+  // Note that if an instruction is not in an instruction list, the memory may
+  // not be safe to delete, so the instruction is turned into a OpNop instead.
+  // This can happen with OpLabel.
+  //
+  // Returns a pointer to the instruction after |inst| or |nullptr| if no such
+  // instruction exists.
+  Instruction* KillInst(ir::Instruction* inst);
 
   // Returns true if all of the given analyses are valid.
   bool AreAnalysesValid(Analysis set) { return (set & valid_analyses_) == set; }
@@ -455,6 +469,14 @@ Module::inst_iterator IRContext::ext_inst_import_end() {
   return module()->ext_inst_import_end();
 }
 
+IteratorRange<Module::inst_iterator> IRContext::ext_inst_imports() {
+  return module()->ext_inst_imports();
+}
+
+IteratorRange<Module::const_inst_iterator> IRContext::ext_inst_imports() const {
+  return ((const Module*)module_.get())->ext_inst_imports();
+}
+
 Module::inst_iterator IRContext::debug1_begin() {
   return module()->debug1_begin();
 }
index b5729bb626195454202c9428045d9942039dc443..81854f404777465789a076597464fcf8ae83b30b 100644 (file)
@@ -31,14 +31,6 @@ const uint32_t kTypeIntWidthInIdx = 0;
 
 }  // anonymous namespace
 
-void LocalAccessChainConvertPass::DeleteIfUseless(ir::Instruction* inst) {
-  const uint32_t resId = inst->result_id();
-  assert(resId != 0);
-  if (HasOnlyNamesAndDecorates(resId)) {
-    context()->KillInst(inst);
-  }
-}
-
 void LocalAccessChainConvertPass::BuildAndAppendInst(
     SpvOp opcode, uint32_t typeId, uint32_t resultId,
     const std::vector<ir::Operand>& in_opnds,
@@ -201,6 +193,7 @@ bool LocalAccessChainConvertPass::ConvertLocalAccessChains(ir::Function* func) {
   // extract and insert sequences
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
+    std::vector<ir::Instruction*> dead_instructions;
     for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
       switch (ii->opcode()) {
         case SpvOpLoad: {
@@ -210,7 +203,9 @@ bool LocalAccessChainConvertPass::ConvertLocalAccessChains(ir::Function* func) {
           if (!IsTargetVar(varId)) break;
           std::vector<std::unique_ptr<ir::Instruction>> newInsts;
           uint32_t replId = GenAccessChainLoadReplacement(ptrInst, &newInsts);
-          ReplaceAndDeleteLoad(&*ii, replId);
+          context()->KillNamesAndDecorates(&*ii);
+          context()->ReplaceAllUsesWith(ii->result_id(), replId);
+          dead_instructions.push_back(&*ii);
           ++ii;
           ii = ii.InsertBefore(std::move(newInsts));
           ++ii;
@@ -224,8 +219,7 @@ bool LocalAccessChainConvertPass::ConvertLocalAccessChains(ir::Function* func) {
           std::vector<std::unique_ptr<ir::Instruction>> newInsts;
           uint32_t valId = ii->GetSingleWordInOperand(kStoreValIdInIdx);
           GenAccessChainStoreReplacement(ptrInst, valId, &newInsts);
-          context()->KillInst(&*ii);
-          DeleteIfUseless(ptrInst);
+          dead_instructions.push_back(&*ii);
           ++ii;
           ii = ii.InsertBefore(std::move(newInsts));
           ++ii;
@@ -236,6 +230,18 @@ bool LocalAccessChainConvertPass::ConvertLocalAccessChains(ir::Function* func) {
           break;
       }
     }
+
+    while (!dead_instructions.empty()) {
+      ir::Instruction* inst = dead_instructions.back();
+      dead_instructions.pop_back();
+      DCEInst(inst, [&dead_instructions](ir::Instruction* other_inst) {
+        auto i = std::find(dead_instructions.begin(), dead_instructions.end(),
+                           other_inst);
+        if (i != dead_instructions.end()) {
+          dead_instructions.erase(i);
+        }
+      });
+    }
   }
   return modified;
 }
index b8862a88dcac30620dba3f614a5abc6609154951..98f009a89eceead0af9bf397103a7a1d736583cf 100644 (file)
@@ -57,9 +57,6 @@ class LocalAccessChainConvertPass : public MemPass {
   // variables.
   void FindTargetVars(ir::Function* func);
 
-  // Delete |inst| if it has no uses. Assumes |inst| has a non-zero resultId.
-  void DeleteIfUseless(ir::Instruction* inst);
-
   // Build instruction from |opcode|, |typeId|, |resultId|, and |in_opnds|.
   // Append to |newInsts|.
   void BuildAndAppendInst(
index 3ef8e0d01a516afab3fe8cb3a87afc1289dd4df1..f86f0aa0e6ba8b1c7e293f1ea510f5f1df88f2a5 100644 (file)
@@ -51,12 +51,15 @@ bool LocalSingleBlockLoadStoreElimPass::HasOnlySupportedRefs(uint32_t ptrId) {
 bool LocalSingleBlockLoadStoreElimPass::LocalSingleBlockLoadStoreElim(
     ir::Function* func) {
   // Perform local store/load and load/load elimination on each block
+  std::vector<ir::Instruction*> dead_instructions;
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
     var2store_.clear();
     var2load_.clear();
     pinned_vars_.clear();
-    for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
+    auto next = bi->begin();
+    for (auto ii = next; ii != bi->end(); ii = next) {
+      ++next;
       switch (ii->opcode()) {
         case SpvOpStore: {
           // Verify store variable is target type
@@ -70,7 +73,7 @@ bool LocalSingleBlockLoadStoreElimPass::LocalSingleBlockLoadStoreElim(
             if (pinned_vars_.find(varId) == pinned_vars_.end()) {
               auto si = var2store_.find(varId);
               if (si != var2store_.end()) {
-                context()->KillInst(si->second);
+                dead_instructions.push_back(si->second);
               }
             }
             var2store_[varId] = &*ii;
@@ -102,7 +105,9 @@ bool LocalSingleBlockLoadStoreElimPass::LocalSingleBlockLoadStoreElim(
           }
           if (replId != 0) {
             // replace load's result id and delete load
-            ReplaceAndDeleteLoad(&*ii, replId);
+            context()->KillNamesAndDecorates(&*ii);
+            context()->ReplaceAllUsesWith(ii->result_id(), replId);
+            dead_instructions.push_back(&*ii);
             modified = true;
           } else {
             if (ptrInst->opcode() == SpvOpVariable)
@@ -121,12 +126,37 @@ bool LocalSingleBlockLoadStoreElimPass::LocalSingleBlockLoadStoreElim(
           break;
       }
     }
+
+    while (!dead_instructions.empty()) {
+      ir::Instruction* inst = dead_instructions.back();
+      dead_instructions.pop_back();
+      DCEInst(inst, [&dead_instructions](ir::Instruction* other_inst) {
+        auto i = std::find(dead_instructions.begin(), dead_instructions.end(),
+                           other_inst);
+        if (i != dead_instructions.end()) {
+          dead_instructions.erase(i);
+        }
+      });
+    }
+
     // Go back and delete useless stores in block
     // TODO(greg-lunarg): Consider moving DCE into separate pass
+    std::vector<ir::Instruction*> dead_stores;
     for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
       if (ii->opcode() != SpvOpStore) continue;
       if (IsLiveStore(&*ii)) continue;
-      DCEInst(&*ii);
+      dead_stores.push_back(&*ii);
+    }
+
+    while (!dead_stores.empty()) {
+      ir::Instruction* inst = dead_stores.back();
+      dead_stores.pop_back();
+      DCEInst(inst, [&dead_stores](ir::Instruction* other_inst) {
+        auto i = std::find(dead_stores.begin(), dead_stores.end(), other_inst);
+        if (i != dead_stores.end()) {
+          dead_stores.erase(i);
+        }
+      });
     }
   }
   return modified;
index e4e67edde7d5d9d7130d182b98ea3a6ce18ef834..78d1ace02c7d22ba24e00022046a6d67ca79b8f4 100644 (file)
@@ -174,6 +174,7 @@ bool LocalSingleStoreElimPass::SingleStoreProcess(ir::Function* func) {
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
     uint32_t instIdx = 0;
+    std::vector<ir::Instruction*> dead_instructions;
     for (auto ii = bi->begin(); ii != bi->end(); ++ii, ++instIdx) {
       if (ii->opcode() != SpvOpLoad) continue;
       uint32_t varId;
@@ -190,21 +191,56 @@ bool LocalSingleStoreElimPass::SingleStoreProcess(ir::Function* func) {
       // Use store value as replacement id
       uint32_t replId = vsi->second->GetSingleWordInOperand(kStoreValIdInIdx);
       // replace all instances of the load's id with the SSA value's id
-      ReplaceAndDeleteLoad(&*ii, replId);
+      context()->KillNamesAndDecorates(&*ii);
+      context()->ReplaceAllUsesWith(ii->result_id(), replId);
+      dead_instructions.push_back(&*ii);
       modified = true;
     }
+
+    // Define the function that will update the data structures as instructions
+    // are deleted.
+    auto update_function = [&dead_instructions,
+                            this](ir::Instruction* other_inst) {
+      // Update dead_instructions.
+      auto i = std::find(dead_instructions.begin(), dead_instructions.end(),
+                         other_inst);
+      if (i != dead_instructions.end()) {
+        dead_instructions.erase(i);
+      }
+
+      // Update the variable to store map.
+      if (other_inst->opcode() != SpvOpStore) {
+        return;
+      }
+
+      uint32_t id;
+      GetPtr(other_inst, &id);
+      auto store = ssa_var2store_.find(id);
+      if (store != ssa_var2store_.end()) {
+        ssa_var2store_.erase(store);
+      }
+    };
+
+    while (!dead_instructions.empty()) {
+      ir::Instruction* inst = dead_instructions.back();
+      dead_instructions.pop_back();
+      DCEInst(inst, update_function);
+    }
   }
   return modified;
 }
 
 bool LocalSingleStoreElimPass::SingleStoreDCE() {
   bool modified = false;
+  std::unordered_set<ir::Instruction*> already_deleted;
   for (auto v : ssa_var2store_) {
     // check that it hasn't already been DCE'd
-    if (v.second->opcode() != SpvOpStore) continue;
+    if (already_deleted.find(v.second) != already_deleted.end()) continue;
     if (non_ssa_vars_.find(v.first) != non_ssa_vars_.end()) continue;
     if (!IsLiveStore(v.second)) {
-      DCEInst(v.second);
+      DCEInst(v.second, [&already_deleted](ir::Instruction* inst) {
+        already_deleted.insert(inst);
+      });
       modified = true;
     }
   }
index 02ee9794ddb72d1ec88f212d55a6671ab668d2a5..2e4135baf65a131069e1332d85847f532c0c9bc2 100644 (file)
@@ -29,15 +29,28 @@ bool LocalMultiStoreElimPass::EliminateMultiStoreLocal(ir::Function* func) {
   // Remove all target variable stores.
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end(); ++bi) {
+    std::vector<ir::Instruction*> dead_instructions;
     for (auto ii = bi->begin(); ii != bi->end(); ++ii) {
       if (ii->opcode() != SpvOpStore) continue;
       uint32_t varId;
       (void)GetPtr(&*ii, &varId);
       if (!IsTargetVar(varId)) continue;
       assert(!HasLoads(varId));
-      DCEInst(&*ii);
+      dead_instructions.push_back(&*ii);
       modified = true;
     }
+
+    while (!dead_instructions.empty()) {
+      ir::Instruction* inst = dead_instructions.back();
+      dead_instructions.pop_back();
+      DCEInst(inst, [&dead_instructions](ir::Instruction* other_inst) {
+        auto i = std::find(dead_instructions.begin(), dead_instructions.end(),
+                           other_inst);
+        if (i != dead_instructions.end()) {
+          dead_instructions.erase(i);
+        }
+      });
+    }
   }
 
   return modified;
index 2af09bffc10d560fcfb60ab0cf551f8390421377..0d39a4f65da5f218b3ebde6ce319415a83f72379 100644 (file)
@@ -183,7 +183,8 @@ void MemPass::AddStores(uint32_t ptr_id, std::queue<ir::Instruction*>* insts) {
   });
 }
 
-void MemPass::DCEInst(ir::Instruction* inst) {
+void MemPass::DCEInst(ir::Instruction* inst,
+                      const function<void(ir::Instruction * )>& call_back) {
   std::queue<ir::Instruction*> deadInsts;
   deadInsts.push(inst);
   while (!deadInsts.empty()) {
@@ -199,6 +200,9 @@ void MemPass::DCEInst(ir::Instruction* inst) {
     uint32_t varId = 0;
     // Remember variable if dead load
     if (di->opcode() == SpvOpLoad) (void)GetPtr(di, &varId);
+    if (call_back) {
+      call_back(di);
+    }
     context()->KillInst(di);
     // For all operands with no remaining uses, add their instruction
     // to the dead instruction queue.
@@ -212,13 +216,6 @@ void MemPass::DCEInst(ir::Instruction* inst) {
   }
 }
 
-void MemPass::ReplaceAndDeleteLoad(ir::Instruction* loadInst, uint32_t replId) {
-  const uint32_t loadId = loadInst->result_id();
-  context()->KillNamesAndDecorates(loadId);
-  (void)context()->ReplaceAllUsesWith(loadId, replId);
-  DCEInst(loadInst);
-}
-
 MemPass::MemPass() {}
 
 bool MemPass::HasOnlySupportedRefs(uint32_t varId) {
@@ -573,19 +570,21 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) {
     SSABlockInit(bi);
     ir::BasicBlock* bp = *bi;
     const uint32_t label = bp->id();
-    for (auto ii = bp->begin(); ii != bp->end(); ++ii) {
-      switch (ii->opcode()) {
+    ir::Instruction* inst = &*bp->begin();
+    while (inst) {
+      ir::Instruction* next_instruction = inst->NextNode();
+      switch (inst->opcode()) {
         case SpvOpStore: {
           uint32_t varId;
-          (void)GetPtr(&*ii, &varId);
+          (void)GetPtr(inst, &varId);
           if (!IsTargetVar(varId)) break;
           // Register new stored value for the variable
           label2ssa_map_[label][varId] =
-              ii->GetSingleWordInOperand(kStoreValIdInIdx);
+              inst->GetSingleWordInOperand(kStoreValIdInIdx);
         } break;
         case SpvOpLoad: {
           uint32_t varId;
-          (void)GetPtr(&*ii, &varId);
+          (void)GetPtr(inst, &varId);
           if (!IsTargetVar(varId)) break;
           uint32_t replId = 0;
           const auto ssaItr = label2ssa_map_.find(label);
@@ -601,12 +600,15 @@ Pass::Status MemPass::InsertPhiInstructions(ir::Function* func) {
           // Replace load's id with the last stored value id for variable
           // and delete load. Kill any names or decorates using id before
           // replacing to prevent incorrect replacement in those instructions.
-          const uint32_t loadId = ii->result_id();
+          const uint32_t loadId = inst->result_id();
           context()->KillNamesAndDecorates(loadId);
           (void)context()->ReplaceAllUsesWith(loadId, replId);
-          context()->KillInst(&*ii);
+          context()->KillInst(inst);
         } break;
-        default: { } break; }
+        default:
+          break;
+      }
+      inst = next_instruction;
     }
     visitedBlocks_.insert(label);
     // Look for successor backedge and patch phis in loop header
index c02e1793b91021d8152761f98d48a17463d62568..496628810a99d17232d720d2d9967fe0ce7e0f78 100644 (file)
@@ -88,11 +88,8 @@ class MemPass : public Pass {
   // Delete |inst| and iterate DCE on all its operands if they are now
   // useless. If a load is deleted and its variable has no other loads,
   // delete all its variable's stores.
-  void DCEInst(ir::Instruction* inst);
-
-  // Replace all instances of |loadInst|'s id with |replId| and delete
-  // |loadInst|.
-  void ReplaceAndDeleteLoad(ir::Instruction* loadInst, uint32_t replId);
+  void DCEInst(ir::Instruction* inst,
+               const std::function<void(ir::Instruction * )>&);
 
   // Call all the cleanup helper functions on |func|.
   bool CFGCleanup(ir::Function* func);
index c63beddb2af8bf273cae5d3a966c8c1ed3601b88..f8cf25c749f8d17a2ed8445e72a962b847e21d96 100644 (file)
@@ -114,8 +114,10 @@ bool MergeReturnPass::MergeReturnBlocks(
   // Replace returns with branches
   for (auto block : returnBlocks) {
     context()->KillInst(&*block->tail());
-    block->tail()->SetOpcode(SpvOpBranch);
-    block->tail()->ReplaceOperands({{SPV_OPERAND_TYPE_ID, {returnId}}});
+    std::unique_ptr<ir::Instruction> new_instruction(
+        new ir::Instruction(context(), SpvOpBranch, 0,
+                            0, {{SPV_OPERAND_TYPE_ID, {returnId}}}));
+    block->AddInstruction(std::move(new_instruction));
     uses_to_update.push_back(&*block->tail());
     uses_to_update.push_back(block->GetLabelInst());
   }
index 9d46a1b6fde666cdf0a353e55260a306f7f4427c..aaf0e20baa9ed3433e6b0bf092916ecf4181da0c 100644 (file)
@@ -71,18 +71,18 @@ void Module::AddGlobalValue(SpvOp opcode, uint32_t result_id,
 
 void Module::ForEachInst(const std::function<void(Instruction*)>& f,
                          bool run_on_debug_line_insts) {
-#define DELEGATE(i) i.ForEachInst(f, run_on_debug_line_insts)
-  for (auto& i : capabilities_) DELEGATE(i);
-  for (auto& i : extensions_) DELEGATE(i);
-  for (auto& i : ext_inst_imports_) DELEGATE(i);
+#define DELEGATE(list) list.ForEachInst(f, run_on_debug_line_insts)
+  DELEGATE(capabilities_);
+  DELEGATE(extensions_);
+  DELEGATE(ext_inst_imports_);
   if (memory_model_) memory_model_->ForEachInst(f, run_on_debug_line_insts);
-  for (auto& i : entry_points_) DELEGATE(i);
-  for (auto& i : execution_modes_) DELEGATE(i);
-  for (auto& i : debugs1_) DELEGATE(i);
-  for (auto& i : debugs2_) DELEGATE(i);
-  for (auto& i : debugs3_) DELEGATE(i);
-  for (auto& i : annotations_) DELEGATE(i);
-  for (auto& i : types_values_) DELEGATE(i);
+  DELEGATE(entry_points_);
+  DELEGATE(execution_modes_);
+  DELEGATE(debugs1_);
+  DELEGATE(debugs2_);
+  DELEGATE(debugs3_);
+  DELEGATE(annotations_);
+  DELEGATE(types_values_);
   for (auto& i : functions_) i->ForEachInst(f, run_on_debug_line_insts);
 #undef DELEGATE
 }
index 168c322e9effb5f7c593e4cfe72683cdaa416cf0..6dc84ea58fe10669f046ab066e2950c760cfb32f 100644 (file)
@@ -48,18 +48,20 @@ bool RemoveDuplicatesPass::RemoveDuplicateCapabilities(
     ir::IRContext* irContext) const {
   bool modified = false;
 
+  if (irContext->capabilities().empty()) {
+    return modified;
+  }
+
   std::unordered_set<uint32_t> capabilities;
-  for (auto i = irContext->capability_begin();
-       i != irContext->capability_end();) {
+  for (auto* i = &*irContext->capability_begin(); i;) {
     auto res = capabilities.insert(i->GetSingleWordOperand(0u));
 
     if (res.second) {
       // Never seen before, keep it.
-      ++i;
+      i = i->NextNode();
     } else {
       // It's a duplicate, remove it.
-      irContext->KillInst(&*i);
-      i = i.Erase();
+      i = irContext->KillInst(i);
       modified = true;
     }
   }
@@ -71,20 +73,22 @@ bool RemoveDuplicatesPass::RemoveDuplicatesExtInstImports(
     ir::IRContext* irContext) const {
   bool modified = false;
 
+  if (irContext->ext_inst_imports().empty()) {
+    return modified;
+  }
+
   std::unordered_map<std::string, SpvId> extInstImports;
-  for (auto i = irContext->ext_inst_import_begin();
-       i != irContext->ext_inst_import_end();) {
+  for (auto* i = &*irContext->ext_inst_import_begin(); i;) {
     auto res = extInstImports.emplace(
         reinterpret_cast<const char*>(i->GetInOperand(0u).words.data()),
         i->result_id());
     if (res.second) {
       // Never seen before, keep it.
-      ++i;
+      i = i->NextNode();
     } else {
       // It's a duplicate, remove it.
       irContext->ReplaceAllUsesWith(i->result_id(), res.first->second);
-      irContext->KillInst(&*i);
-      i = i.Erase();
+      i = irContext->KillInst(i);
       modified = true;
     }
   }
@@ -96,14 +100,16 @@ bool RemoveDuplicatesPass::RemoveDuplicateTypes(
     ir::IRContext* irContext) const {
   bool modified = false;
 
-  std::vector<Instruction> visitedTypes;
+  if (irContext->types_values().empty()) {
+    return modified;
+  }
 
-  for (auto i = irContext->types_values_begin();
-       i != irContext->types_values_end();) {
+  std::vector<Instruction> visitedTypes;
+  for (auto* i = &*irContext->types_values_begin(); i;) {
     // We only care about types.
     if (!spvOpcodeGeneratesType((i->opcode())) &&
         i->opcode() != SpvOpTypeForwardPointer) {
-      ++i;
+      i = i->NextNode();
       continue;
     }
 
@@ -120,13 +126,12 @@ bool RemoveDuplicatesPass::RemoveDuplicateTypes(
     if (idToKeep == 0u) {
       // This is a never seen before type, keep it around.
       visitedTypes.emplace_back(*i);
-      ++i;
+      i = i->NextNode();
     } else {
       // The same type has already been seen before, remove this one.
       irContext->ReplaceAllUsesWith(i->result_id(), idToKeep);
       modified = true;
-      irContext->KillInst(&*i);
-      i = i.Erase();
+      i = irContext->KillInst(i);
     }
   }
 
@@ -146,8 +151,7 @@ bool RemoveDuplicatesPass::RemoveDuplicateDecorations(
   std::vector<const Instruction*> visitedDecorations;
 
   opt::analysis::DecorationManager decorationManager(irContext->module());
-  for (auto i = irContext->annotation_begin();
-       i != irContext->annotation_end();) {
+  for (auto* i = &*irContext->annotation_begin(); i;) {
     // Is the current decoration equal to one of the decorations we have aready
     // visited?
     bool alreadyVisited = false;
@@ -161,11 +165,11 @@ bool RemoveDuplicatesPass::RemoveDuplicateDecorations(
     if (!alreadyVisited) {
       // This is a never seen before decoration, keep it around.
       visitedDecorations.emplace_back(&*i);
-      ++i;
+      i = i->NextNode();
     } else {
       // The same decoration has already been seen before, remove this one.
       modified = true;
-      i = i.Erase();
+      i = irContext->KillInst(i);
     }
   }
 
index 75118a3d0ab2c406d978ddf9a77f27d6486c0a9d..6edb6f0a6dcc40345aa7cc9355232f96ccbd0ca2 100644 (file)
@@ -68,20 +68,20 @@ Pass::Status StrengthReductionPass::Process(ir::IRContext* c) {
 }
 
 bool StrengthReductionPass::ReplaceMultiplyByPowerOf2(
-    ir::BasicBlock::iterator* instPtr) {
-  ir::BasicBlock::iterator& inst = *instPtr;
-  assert(inst->opcode() == SpvOp::SpvOpIMul &&
+    ir::BasicBlock::iterator* inst) {
+  assert((*inst)->opcode() == SpvOp::SpvOpIMul &&
          "Only works for multiplication of integers.");
   bool modified = false;
 
   // Currently only works on 32-bit integers.
-  if (inst->type_id() != int32_type_id_ && inst->type_id() != uint32_type_id_) {
+  if ((*inst)->type_id() != int32_type_id_ &&
+      (*inst)->type_id() != uint32_type_id_) {
     return modified;
   }
 
   // Check the operands for a constant that is a power of 2.
   for (int i = 0; i < 2; i++) {
-    uint32_t opId = inst->GetSingleWordInOperand(i);
+    uint32_t opId = (*inst)->GetSingleWordInOperand(i);
     ir::Instruction* opInst = get_def_use_mgr()->GetDef(opId);
     if (opInst->opcode() == SpvOp::SpvOpConstant) {
       // We found a constant operand.
@@ -95,22 +95,24 @@ bool StrengthReductionPass::ReplaceMultiplyByPowerOf2(
         // Create the new instruction.
         uint32_t newResultId = TakeNextId();
         std::vector<ir::Operand> newOperands;
-        newOperands.push_back(inst->GetInOperand(1 - i));
+        newOperands.push_back((*inst)->GetInOperand(1 - i));
         ir::Operand shiftOperand(spv_operand_type_t::SPV_OPERAND_TYPE_ID,
                                  {shiftConstResultId});
         newOperands.push_back(shiftOperand);
         std::unique_ptr<ir::Instruction> newInstruction(
             new ir::Instruction(context(), SpvOp::SpvOpShiftLeftLogical,
-                                inst->type_id(), newResultId, newOperands));
+                                (*inst)->type_id(), newResultId, newOperands));
 
         // Insert the new instruction and update the data structures.
-        inst = inst.InsertBefore(std::move(newInstruction));
-        get_def_use_mgr()->AnalyzeInstDefUse(&*inst);
-        ++inst;
-        context()->ReplaceAllUsesWith(inst->result_id(), newResultId);
+        (*inst) = (*inst).InsertBefore(std::move(newInstruction));
+        get_def_use_mgr()->AnalyzeInstDefUse(&*(*inst));
+        ++(*inst);
+        context()->ReplaceAllUsesWith((*inst)->result_id(), newResultId);
 
         // Remove the old instruction.
-        context()->KillInst(&*inst);
+        ir::Instruction* inst_to_delete = &*(*inst);
+        --(*inst);
+        context()->KillInst(inst_to_delete);
 
         // We do not want to replace the instruction twice if both operands
         // are constants that are a power of 2.  So we break here.
index ace53de0c4b4e0f025245d2701ce619beaed5d47..266757339c10cdc988824b26820b9b8130fa9584 100644 (file)
@@ -108,10 +108,12 @@ Pass::Status UnifyConstantPass::Process(ir::IRContext* c) {
   bool modified = false;
   ResultIdTrie defined_constants;
 
-  for (ir::Instruction& inst : context()->types_values()) {
+  for( ir::Instruction* next_instruction, *inst = &*(context()->types_values_begin()); inst; inst = next_instruction) {
+    next_instruction = inst->NextNode();
+
     // Do not handle the instruction when there are decorations upon the result
     // id.
-    if (get_def_use_mgr()->GetAnnotations(inst.result_id()).size() != 0) {
+    if (get_def_use_mgr()->GetAnnotations(inst->result_id()).size() != 0) {
       continue;
     }
 
@@ -133,7 +135,7 @@ Pass::Status UnifyConstantPass::Process(ir::IRContext* c) {
     // used in key arrays will be the ids of the unified constants, when
     // processing is up to a descendant. This makes comparing the key array
     // always valid for judging duplication.
-    switch (inst.opcode()) {
+    switch (inst->opcode()) {
       case SpvOp::SpvOpConstantTrue:
       case SpvOp::SpvOpConstantFalse:
       case SpvOp::SpvOpConstant:
@@ -151,12 +153,12 @@ Pass::Status UnifyConstantPass::Process(ir::IRContext* c) {
       // same so are unifiable.
       case SpvOp::SpvOpSpecConstantOp:
       case SpvOp::SpvOpSpecConstantComposite: {
-        uint32_t id = defined_constants.LookupEquivalentResultFor(inst);
-        if (id != inst.result_id()) {
+        uint32_t id = defined_constants.LookupEquivalentResultFor(*inst);
+        if (id != inst->result_id()) {
           // The constant is a duplicated one, use the cached constant to
           // replace the uses of this duplicated one, then turn it to nop.
-          context()->ReplaceAllUsesWith(inst.result_id(), id);
-          context()->KillInst(&inst);
+          context()->ReplaceAllUsesWith(inst->result_id(), id);
+          context()->KillInst(inst);
           modified = true;
         }
         break;
index 3533de1908086264d04aab4ebe13f5212b1c0a4e..fd1a9ec3cc73299eaeba765b05371342f6286da7 100644 (file)
@@ -1014,11 +1014,8 @@ INSTANTIATE_TEST_CASE_P(
         "%5 = OpTypeMatrix %3 3 "
         "%6 = OpTypeMatrix %2 3",
         {1, 3, 5, 10}, // ids to kill
-        "OpNop\n"
         "%2 = OpTypeVector %1 2\n"
-        "OpNop\n"
         "%4 = OpTypeVector %1 4\n"
-        "OpNop\n"
         "%6 = OpTypeMatrix %2 3",
         {
           { // defs
@@ -1049,8 +1046,6 @@ INSTANTIATE_TEST_CASE_P(
 
          "%5 = OpLabel\n"
          "%7 = OpPhi %6 %8 %4 %9 %5\n"
-              "OpNop\n"
-              "OpNop\n"
         "%13 = OpFAdd %10 %11 %12\n"
         "%17 = OpSLessThan %16 %7 %18\n"
               "OpLoopMerge %19 %5 None\n"
@@ -1164,7 +1159,6 @@ INSTANTIATE_TEST_CASE_P(
         "%6 = OpLabel\n"
              "OpBranch %7\n"
         "%7 = OpLabel\n"
-             "OpNop\n"
              "OpBranch %7\n"
              "OpFunctionEnd",
         {
@@ -1484,7 +1478,6 @@ INSTANTIATE_TEST_CASE_P(
         "     OpReturn "
         "     OpFunctionEnd",
         {3, 5, 7},
-        "OpNop\n"
         "%1 = OpTypeFunction %3\n"
         "%2 = OpFunction %1 None %3\n"
         "%4 = OpLabel\n"