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 730ec37..b822580 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 4704d40..62973b0 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 024d82c..3112fbc 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 c84206c..ba1552a 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 721b953..8f4c95b 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 6f1e65f..6415208 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 90ef32d..52199ba 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 5c60f34..5e299c6 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 dc33eda..5f8a66b 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 d249e42..43d34fa 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 5713772..da52c95 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 08c5b77..1bd1344 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 d9d94b3..2b7b931 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 f516d0e..6576a0d 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 7e2a20f..a6a2209 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 b5729bb..81854f4 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 b8862a8..98f009a 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 3ef8e0d..f86f0aa 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 e4e67ed..78d1ace 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 02ee979..2e4135b 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 2af09bf..0d39a4f 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 c02e179..4966288 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 c63bedd..f8cf25c 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 9d46a1b..aaf0e20 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 168c322..6dc84ea 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 75118a3..6edb6f0 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 ace53de..2667573 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 3533de1..fd1a9ec 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"