[vm] Cleanup Instruction::Effects(), prepare to cleanup Dependencies()

runtime/vm/redundancy_elimination.cc

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/redundancy_elimination.h"
 
 #include "vm/bit_vector.h"
 #include "vm/flow_graph.h"
 #include "vm/hash_map.h"
 #include "vm/il_printer.h"
 #include "vm/intermediate_language.h"
 #include "vm/stack_frame.h"
 
 namespace dart {
 
 DEFINE_FLAG(bool, dead_store_elimination, true, "Eliminate dead stores");
 DEFINE_FLAG(bool, load_cse, true, "Use redundant load elimination.");
 DEFINE_FLAG(bool,
             trace_load_optimization,
             false,
             "Print live sets for load optimization pass.");
 
 // Quick access to the current zone.
 #define Z (zone())
 
 class CSEInstructionMap : public ValueObject {
  public:
-  // Right now CSE and LICM track a single effect which is no longer used.
-  // TODO(alexmarkov): cleanup.
-  // Other effects like modifications of fields are tracked in a separate load
-  // forwarding pass via Alias structure.
-  COMPILE_ASSERT(EffectSet::kLastEffect == 1);
+  CSEInstructionMap() : map_() {}
+  explicit CSEInstructionMap(const CSEInstructionMap& other)
+      : ValueObject(), map_(other.map_) {}
 
-  CSEInstructionMap() : independent_(), dependent_() {}
-  explicit CSEInstructionMap(const CSEInstructionMap& other)
-      : ValueObject(),
-        independent_(other.independent_),
-        dependent_(other.dependent_) {}
-
-  void RemoveAffected(EffectSet effects) {
-    if (!effects.IsNone()) {
-      dependent_.Clear();
-    }
+  Instruction* Lookup(Instruction* other) const {
+    ASSERT(other->AllowsCSE());
+    ASSERT(other->Dependencies().IsNone());
+    return map_.LookupValue(other);
   }
 
-  Instruction* Lookup(Instruction* other) const {
-    return GetMapFor(other)->LookupValue(other);
+  void Insert(Instruction* instr) {
+    ASSERT(instr->AllowsCSE());
+    ASSERT(instr->Dependencies().IsNone());
+    return map_.Insert(instr);
   }
 
-  void Insert(Instruction* instr) { return GetMapFor(instr)->Insert(instr); }
-
  private:
   typedef DirectChainedHashMap<PointerKeyValueTrait<Instruction> > Map;
 
-  Map* GetMapFor(Instruction* instr) {
-    return instr->Dependencies().IsNone() ? &independent_ : &dependent_;
-  }
-
-  const Map* GetMapFor(Instruction* instr) const {
-    return instr->Dependencies().IsNone() ? &independent_ : &dependent_;
-  }
-
-  // All computations that are not affected by any side-effect.
-  // Majority of computations are not affected by anything and will be in
-  // this map.
-  Map independent_;
-
-  // All computations that are affected by side effect.
-  Map dependent_;
+  Map map_;
 };
 
 // Place describes an abstract location (e.g. field) that IR can load
 // from or store to.
 //
 // Places are also used to describe wild-card locations also known as aliases,
 // that essentially represent sets of places that alias each other. Places A
 // and B are said to alias each other if store into A can affect load from B.
 //
 // We distinguish the following aliases:
@@ skipping 1510 matching lines @@
           // Check if this load needs renumbering because of the intrablock
           // load forwarding.
           const Place* canonical = aliased_set_->LookupCanonical(&place);
           if ((canonical != NULL) &&
               (canonical->id() != instr->AsDefinition()->place_id())) {
             instr->AsDefinition()->set_place_id(canonical->id());
           }
         }
 
         // If instruction has effects then kill all loads affected.
-        if (!instr->Effects().IsNone()) {
+        if (instr->HasSideEffects()) {
           kill->AddAll(aliased_set_->aliased_by_effects());
           // There is no need to clear out_values when removing values from GEN
           // set because only those values that are in the GEN set
           // will ever be used.
           gen->RemoveAll(aliased_set_->aliased_by_effects());
           continue;
         }
 
         Definition* defn = instr->AsDefinition();
         if (defn == NULL) {
@@ skipping 738 matching lines @@
 }
 
 bool DominatorBasedCSE::OptimizeRecursive(FlowGraph* graph,
                                           BlockEntryInstr* block,
                                           CSEInstructionMap* map) {
   bool changed = false;
   for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
     Instruction* current = it.Current();
     if (current->AllowsCSE()) {
       Instruction* replacement = map->Lookup(current);
+      ASSERT((replacement == NULL) || replacement->AllowsCSE());
       if ((replacement != NULL) &&
           graph->block_effects()->IsAvailableAt(replacement, block)) {
         // Replace current with lookup result.
         graph->ReplaceCurrentInstruction(&it, current, replacement);
         changed = true;
         continue;
       }
 
       // For simplicity we assume that instruction either does not depend on
       // anything or does not affect anything. If this is not the case then
       // we should first remove affected instructions from the map and
       // then add instruction to the map so that it does not kill itself.
-      ASSERT(current->Effects().IsNone() || current->Dependencies().IsNone());
+      ASSERT(!current->HasSideEffects() || current->Dependencies().IsNone());
       map->Insert(current);
     }
-
-    map->RemoveAffected(current->Effects());
   }
 
   // Process children in the dominator tree recursively.
   intptr_t num_children = block->dominated_blocks().length();
   if (num_children != 0) {
     graph->thread()->CheckForSafepoint();
   }
   for (intptr_t i = 0; i < num_children; ++i) {
     BlockEntryInstr* child = block->dominated_blocks()[i];
     if (i < num_children - 1) {
@@ skipping 126 matching lines @@
             }
             exposed_stores->Add(instr);
           }
           // Interfering stores kill only loads from the same place.
           kill->Add(instr->place_id());
           live_in->Remove(instr->place_id());
           continue;
         }
 
         // Handle side effects, deoptimization and function return.
-        if (!instr->Effects().IsNone() || instr->CanDeoptimize() ||
+        if (instr->HasSideEffects() || instr->CanDeoptimize() ||
             instr->IsThrow() || instr->IsReThrow() || instr->IsReturn()) {
           // Instructions that return from the function, instructions with side
           // effects and instructions that can deoptimize are considered as
           // loads from all places.
           live_in->CopyFrom(all_places);
           if (instr->IsThrow() || instr->IsReThrow() || instr->IsReturn()) {
             // Initialize live-out for exit blocks since it won't be computed
             // otherwise during the fixed point iteration.
             live_out->CopyFrom(all_places);
           }
@@ skipping 794 matching lines @@
       if (to_index == 0) {
         join->phis_ = NULL;
       } else {
         join->phis_->TruncateTo(to_index);
       }
     }
   }
 }
 
 }  // namespace dart