Analyze uses for all instructions. (#1937)

* Analyze uses for all instructions.

The def-use manager needs to fill in the `inst_to_used_ids_` field for
every instruction.  This means we have to analyze the uses for every
instruction, even if they do not have any uses.

This mistake was not found earlier because there was a typo in the
equality check for def-use managers.  No new tests are needed.

While looking into this I found redundant work in block merge.  Cleaning
that up at the same time.

* Fix other transformations

Aggressive dead code elimination did not update the OpGroupDecorate
and the OpGroupMemberDecorate instructions properly when they are
updated.  That is fixed.

Dead branch elimination did not analyze the OpUnreachable instructions
that is would add.  That is taken care of.

source/opt/aggressive_dead_code_elim_pass.cpp

@@ -638,6 +638,7 @@ bool AggressiveDCEPass::ProcessGlobalValues() {
         // Go through the targets of this group decorate. Remove each dead
         // target. If all targets are dead, remove this decoration.
         bool dead = true;
+        bool removed_operand = false;
         for (uint32_t i = 1; i < annotation->NumOperands();) {
           Instruction* opInst =
               get_def_use_mgr()->GetDef(annotation->GetSingleWordOperand(i));
@@ -645,6 +646,7 @@ bool AggressiveDCEPass::ProcessGlobalValues() {
             // Don't increment |i|.
             annotation->RemoveOperand(i);
             modified = true;
+            removed_operand = true;
           } else {
             i++;
             dead = false;
@@ -653,6 +655,8 @@ bool AggressiveDCEPass::ProcessGlobalValues() {
         if (dead) {
           context()->KillInst(annotation);
           modified = true;
+        } else if (removed_operand) {
+          context()->UpdateDefUse(annotation);
         }
         break;
       }
@@ -661,6 +665,7 @@ bool AggressiveDCEPass::ProcessGlobalValues() {
         // dead target (and member index). If all targets are dead, remove this
         // decoration.
         bool dead = true;
+        bool removed_operand = false;
         for (uint32_t i = 1; i < annotation->NumOperands();) {
           Instruction* opInst =
               get_def_use_mgr()->GetDef(annotation->GetSingleWordOperand(i));
@@ -669,6 +674,7 @@ bool AggressiveDCEPass::ProcessGlobalValues() {
             annotation->RemoveOperand(i + 1);
             annotation->RemoveOperand(i);
             modified = true;
+            removed_operand = true;
           } else {
             i += 2;
             dead = false;
@@ -677,6 +683,8 @@ bool AggressiveDCEPass::ProcessGlobalValues() {
         if (dead) {
           context()->KillInst(annotation);
           modified = true;
+        } else if (removed_operand) {
+          context()->UpdateDefUse(annotation);
         }
         break;
       }

source/opt/block_merge_pass.cpp

@@ -24,19 +24,6 @@
 namespace spvtools {
 namespace opt {
 
-void BlockMergePass::KillInstAndName(Instruction* inst) {
-  std::vector<Instruction*> to_kill;
-  get_def_use_mgr()->ForEachUser(inst, [&to_kill](Instruction* user) {
-    if (user->opcode() == SpvOpName) {
-      to_kill.push_back(user);
-    }
-  });
-  for (auto i : to_kill) {
-    context()->KillInst(i);
-  }
-  context()->KillInst(inst);
-}
-
 bool BlockMergePass::MergeBlocks(Function* func) {
   bool modified = false;
   for (auto bi = func->begin(); bi != func->end();) {
@@ -117,7 +104,7 @@ bool BlockMergePass::MergeBlocks(Function* func) {
       }
     }
     context()->ReplaceAllUsesWith(lab_id, bi->id());
-    KillInstAndName(sbi->GetLabelInst());
+    context()->KillInst(sbi->GetLabelInst());
     (void)sbi.Erase();
     // Reprocess block.
     modified = true;

source/opt/block_merge_pass.h

@@ -48,8 +48,6 @@ class BlockMergePass : public Pass {
   }
 
  private:
-  // Kill any OpName instruction referencing |inst|, then kill |inst|.
-  void KillInstAndName(Instruction* inst);
 
   // Search |func| for blocks which have a single Branch to a block
   // with no other predecessors. Merge these blocks into a single block.

source/opt/dead_branch_elim_pass.cpp

@@ -331,7 +331,8 @@ bool DeadBranchElimPass::EraseDeadBlocks(
         ebi->AddInstruction(
             MakeUnique<Instruction>(context(), SpvOpUnreachable, 0, 0,
                                     std::initializer_list<Operand>{}));
-        context()->set_instr_block(&*ebi->tail(), &*ebi);
+        context()->AnalyzeUses(ebi->terminator());
+        context()->set_instr_block(ebi->terminator(), &*ebi);
         modified = true;
       }
       ++ebi;

source/opt/def_use_manager.cpp

@@ -278,7 +278,7 @@ bool operator==(const DefUseManager& lhs, const DefUseManager& rhs) {
     return false;
   }
 
-  if (lhs.inst_to_used_ids_ != lhs.inst_to_used_ids_) {
+  if (lhs.inst_to_used_ids_ != rhs.inst_to_used_ids_) {
     return false;
   }
   return true;

source/opt/loop_unroller.cpp

@@ -841,7 +841,7 @@ void LoopUnrollerUtilsImpl::AssignNewResultIds(BasicBlock* basic_block) {
   // Assign a new id to the label.
   state_.new_inst[basic_block->GetLabelInst()->result_id()] = new_label_id;
   basic_block->GetLabelInst()->SetResultId(new_label_id);
-  def_use_mgr->AnalyzeInstDef(basic_block->GetLabelInst());
+  def_use_mgr->AnalyzeInstDefUse(basic_block->GetLabelInst());
 
   for (Instruction& inst : *basic_block) {
     uint32_t old_id = inst.result_id();