Reland: "Use stability to only conditionally flush information from the CheckMaps table."

BUG=
R=ishell@chromium.org

Original CL: https://codereview.chromium.org/153823003

Review URL: https://codereview.chromium.org/153653007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19342 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/code-stubs-hydrogen.cc

@@ -295,7 +295,8 @@ HValue* CodeStubGraphBuilder<ToNumberStub>::BuildCodeStub() {
   // Check if the parameter is already a SMI or heap number.
   IfBuilder if_number(this);
   if_number.If<HIsSmiAndBranch>(value);
-  if_number.OrIf<HCompareMap>(value, isolate()->factory()->heap_number_map());
+  if_number.OrIf<HCompareMap>(value, isolate()->factory()->heap_number_map(),
+                              top_info());
   if_number.Then();
 
   // Return the number.
@@ -358,7 +359,8 @@ HValue* CodeStubGraphBuilder<FastCloneShallowArrayStub>::BuildCodeStub() {
     HValue* elements = AddLoadElements(boilerplate);
 
     IfBuilder if_fixed_cow(this);
-    if_fixed_cow.If<HCompareMap>(elements, factory->fixed_cow_array_map());
+    if_fixed_cow.If<HCompareMap>(elements, factory->fixed_cow_array_map(),
+                                 top_info());
     if_fixed_cow.Then();
     push_value = BuildCloneShallowArray(boilerplate,
                                         allocation_site,
@@ -369,7 +371,7 @@ HValue* CodeStubGraphBuilder<FastCloneShallowArrayStub>::BuildCodeStub() {
     if_fixed_cow.Else();
 
     IfBuilder if_fixed(this);
-    if_fixed.If<HCompareMap>(elements, factory->fixed_array_map());
+    if_fixed.If<HCompareMap>(elements, factory->fixed_array_map(), top_info());
     if_fixed.Then();
     push_value = BuildCloneShallowArray(boilerplate,
                                         allocation_site,

src/compiler.cc

@@ -1185,7 +1185,7 @@ Handle<Code> Compiler::GetOptimizedCode(Handle<JSFunction> function,
   if (FLAG_trace_opt) {
     PrintF("[failed to optimize ");
     function->PrintName();
-    PrintF("]\n");
+    PrintF(": %s]\n", GetBailoutReason(info->bailout_reason()));
   }
 
   if (isolate->has_pending_exception()) isolate->clear_pending_exception();

src/hydrogen-check-elimination.cc

@@ -50,6 +50,7 @@ struct HCheckTableEntry {
   HValue* object_;  // The object being approximated. NULL => invalid entry.
   HInstruction* check_;  // The last check instruction.
   MapSet maps_;          // The set of known maps for the object.
+  bool is_stable_;
 };
 
 
@@ -103,9 +104,10 @@ class HCheckTable : public ZoneObject {
       }
       default: {
         // If the instruction changes maps uncontrollably, drop everything.
-        if (instr->CheckChangesFlag(kMaps) ||
-            instr->CheckChangesFlag(kOsrEntries)) {
-          Kill();
+        if (instr->CheckChangesFlag(kOsrEntries)) {
+          Reset();
+        } else if (instr->CheckChangesFlag(kMaps)) {
+          KillUnstableEntries();
         }
       }
       // Improvements possible:
@@ -154,6 +156,7 @@ class HCheckTable : public ZoneObject {
       if (old_entry->check_ != NULL &&
           old_entry->check_->block()->Dominates(succ)) {
         new_entry->check_ = old_entry->check_;
+        new_entry->is_stable_ = old_entry->is_stable_;
       } else {
         // Leave it NULL till we meet a new check instruction for this object
         // in the control flow.
@@ -175,7 +178,8 @@ class HCheckTable : public ZoneObject {
         HCheckTableEntry* pred_entry = copy->Find(phi_operand);
         if (pred_entry != NULL) {
           // Create an entry for a phi in the table.
-          copy->Insert(phi, NULL, pred_entry->maps_->Copy(phase_->zone()));
+          copy->Insert(phi, NULL, pred_entry->maps_->Copy(phase_->zone()),
+                       pred_entry->is_stable_);
         }
       }
     }
@@ -193,12 +197,13 @@ class HCheckTable : public ZoneObject {
         if (is_true_branch) {
           // Learn on the true branch of if(CompareMap(x)).
           if (entry == NULL) {
-            copy->Insert(object, cmp, cmp->map());
+            copy->Insert(object, cmp, cmp->map(), cmp->is_stable());
           } else {
             MapSet list = new(phase_->zone()) UniqueSet<Map>();
             list->Add(cmp->map(), phase_->zone());
             entry->maps_ = list;
             entry->check_ = cmp;
+            entry->is_stable_ = cmp->is_stable();
           }
         } else {
           // Learn on the false branch of if(CompareMap(x)).
@@ -217,10 +222,10 @@ class HCheckTable : public ZoneObject {
         HCheckTableEntry* re = copy->Find(right);
         if (le == NULL) {
           if (re != NULL) {
-            copy->Insert(left, NULL, re->maps_->Copy(zone));
+            copy->Insert(left, NULL, re->maps_->Copy(zone), re->is_stable_);
           }
         } else if (re == NULL) {
-          copy->Insert(right, NULL, le->maps_->Copy(zone));
+          copy->Insert(right, NULL, le->maps_->Copy(zone), le->is_stable_);
         } else {
           MapSet intersect = le->maps_->Intersect(re->maps_, zone);
           le->maps_ = intersect;
@@ -247,8 +252,7 @@ class HCheckTable : public ZoneObject {
                      HBasicBlock* pred_block, Zone* zone) {
     if (that->size_ == 0) {
       // If the other state is empty, simply reset.
-      size_ = 0;
-      cursor_ = 0;
+      Reset();
     } else {
       int pred_index = succ->PredecessorIndexOf(pred_block);
       bool compact = false;
@@ -271,6 +275,8 @@ class HCheckTable : public ZoneObject {
         } else {
           this_entry->maps_ =
               this_entry->maps_->Union(that_entry->maps_, phase_->zone());
+          this_entry->is_stable_ =
+              this_entry->is_stable_ && that_entry->is_stable_;
           if (this_entry->check_ != that_entry->check_) {
             this_entry->check_ = NULL;
           }
@@ -351,7 +357,8 @@ class HCheckTable : public ZoneObject {
       }
     } else {
       // No entry; insert a new one.
-      Insert(object, instr, instr->map_set().Copy(phase_->zone()));
+      Insert(object, instr, instr->map_set().Copy(phase_->zone()),
+             instr->is_stable());
     }
   }
 
@@ -413,7 +420,8 @@ class HCheckTable : public ZoneObject {
       }
     } else {
       // No prior information.
-      Insert(object, instr, map);
+      // TODO(verwaest): Tag map constants with stability.
+      Insert(object, instr, map, false);
     }
   }
 
@@ -430,12 +438,14 @@ class HCheckTable : public ZoneObject {
     if (instr->has_transition()) {
       // This store transitions the object to a new map.
       Kill(object);
-      Insert(object, NULL, MapConstant(instr->transition()));
+      Insert(object, NULL, MapConstant(instr->transition()),
+             instr->is_stable());
     } else if (IsMapAccess(instr->access())) {
       // This is a store directly to the map field of the object.
       Kill(object);
       if (!instr->value()->IsConstant()) return;
-      Insert(object, NULL, MapConstant(instr->value()));
+      // TODO(verwaest): Tag with stability.
+      Insert(object, NULL, MapConstant(instr->value()), false);
     } else {
       // If the instruction changes maps, it should be handled above.
       CHECK(!instr->CheckChangesFlag(kMaps));
@@ -485,12 +495,26 @@ class HCheckTable : public ZoneObject {
     }
   }
 
-  // Kill everything in the table.
-  void Kill() {
+  // Reset the table.
+  void Reset() {
     size_ = 0;
     cursor_ = 0;
   }
 
+  // Kill everything in the table.
+  void KillUnstableEntries() {
+    bool compact = false;
+    for (int i = 0; i < size_; i++) {
+      HCheckTableEntry* entry = &entries_[i];
+      ASSERT(entry->object_ != NULL);
+      if (!entry->is_stable_) {
+        entry->object_ = NULL;
+        compact = true;
+      }
+    }
+    if (compact) Compact();
+  }
+
   // Kill everything in the table that may alias {object}.
   void Kill(HValue* object) {
     bool compact = false;
@@ -572,17 +596,24 @@ class HCheckTable : public ZoneObject {
     return entry == NULL ? NULL : entry->maps_;
   }
 
-  void Insert(HValue* object, HInstruction* check, Unique<Map> map) {
+  void Insert(HValue* object,
+              HInstruction* check,
+              Unique<Map> map,
+              bool is_stable) {
     MapSet list = new(phase_->zone()) UniqueSet<Map>();
     list->Add(map, phase_->zone());
-    Insert(object, check, list);
+    Insert(object, check, list, is_stable);
   }
 
-  void Insert(HValue* object, HInstruction* check, MapSet maps) {
+  void Insert(HValue* object,
+              HInstruction* check,
+              MapSet maps,
+              bool is_stable) {
     HCheckTableEntry* entry = &entries_[cursor_++];
     entry->object_ = object;
     entry->check_ = check;
     entry->maps_ = maps;
+    entry->is_stable_ = is_stable;
     // If the table becomes full, wrap around and overwrite older entries.
     if (cursor_ == kMaxTrackedObjects) cursor_ = 0;
     if (size_ < kMaxTrackedObjects) size_++;
@@ -612,8 +643,7 @@ class HCheckTable : public ZoneObject {
 class HCheckMapsEffects : public ZoneObject {
  public:
   explicit HCheckMapsEffects(Zone* zone)
-    : maps_stored_(false),
-      stores_(5, zone) { }
+    : stores_(5, zone) { }
 
   inline bool Disabled() {
     return false;  // Effects are _not_ disabled.
@@ -621,27 +651,22 @@ class HCheckMapsEffects : public ZoneObject {
 
   // Process a possibly side-effecting instruction.
   void Process(HInstruction* instr, Zone* zone) {
-    switch (instr->opcode()) {
-      case HValue::kStoreNamedField: {
-        stores_.Add(HStoreNamedField::cast(instr), zone);
-        break;
-      }
-      case HValue::kOsrEntry: {
-        // Kill everything. Loads must not be hoisted past the OSR entry.
-        maps_stored_ = true;
-      }
-      default: {
-        maps_stored_ |= (instr->CheckChangesFlag(kMaps) |
-                         instr->CheckChangesFlag(kElementsKind));
-      }
+    if (instr->IsStoreNamedField()) {
+      stores_.Add(HStoreNamedField::cast(instr), zone);
+    } else {
+      flags_.Add(instr->ChangesFlags());
     }
   }
 
   // Apply these effects to the given check elimination table.
   void Apply(HCheckTable* table) {
-    if (maps_stored_) {
+    if (flags_.Contains(kOsrEntries)) {
+      table->Reset();
+      return;
+    }
+    if (flags_.Contains(kMaps) || flags_.Contains(kElementsKind)) {
       // Uncontrollable map modifications; kill everything.
-      table->Kill();
+      table->KillUnstableEntries();
       return;
     }
 
@@ -656,14 +681,14 @@ class HCheckMapsEffects : public ZoneObject {
 
   // Union these effects with the other effects.
   void Union(HCheckMapsEffects* that, Zone* zone) {
-    maps_stored_ |= that->maps_stored_;
+    flags_.Add(that->flags_);
     for (int i = 0; i < that->stores_.length(); i++) {
       stores_.Add(that->stores_[i], zone);
     }
   }
 
  private:
-  bool maps_stored_ : 1;
+  GVNFlagSet flags_;
   ZoneList<HStoreNamedField*> stores_;
 };
 
@@ -680,7 +705,7 @@ void HCheckEliminationPhase::Run() {
   } else {
     // Perform only local analysis.
     for (int i = 0; i < graph()->blocks()->length(); i++) {
-      table->Kill();
+      table->Reset();
       engine.AnalyzeOneBlock(graph()->blocks()->at(i), table);
     }
   }

src/hydrogen-instructions.cc

@@ -3107,7 +3107,7 @@ HCheckMaps* HCheckMaps::New(Zone* zone,
                             CompilationInfo* info,
                             HValue* typecheck) {
   HCheckMaps* check_map = new(zone) HCheckMaps(value, zone, typecheck);
-  check_map->Add(map, zone);
+  check_map->Add(map, info, zone);
   if (map->CanOmitMapChecks() &&
       value->IsConstant() &&
       HConstant::cast(value)->HasMap(map)) {

src/hydrogen-instructions.h

@@ -1508,8 +1508,10 @@ class HBranch V8_FINAL : public HUnaryControlInstruction {
 
 class HCompareMap V8_FINAL : public HUnaryControlInstruction {
  public:
-  DECLARE_INSTRUCTION_FACTORY_P2(HCompareMap, HValue*, Handle<Map>);
-  DECLARE_INSTRUCTION_FACTORY_P4(HCompareMap, HValue*, Handle<Map>,
+  DECLARE_INSTRUCTION_FACTORY_P3(HCompareMap, HValue*, Handle<Map>,
+                                 CompilationInfo*);
+  DECLARE_INSTRUCTION_FACTORY_P5(HCompareMap, HValue*, Handle<Map>,
+                                 CompilationInfo*,
                                  HBasicBlock*, HBasicBlock*);
 
   virtual bool KnownSuccessorBlock(HBasicBlock** block) V8_OVERRIDE {
@@ -1535,6 +1537,10 @@ class HCompareMap V8_FINAL : public HUnaryControlInstruction {
     return Representation::Tagged();
   }
 
+  bool is_stable() const {
+    return is_stable_;
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(CompareMap)
 
  protected:
@@ -1543,14 +1549,22 @@ class HCompareMap V8_FINAL : public HUnaryControlInstruction {
  private:
   HCompareMap(HValue* value,
               Handle<Map> map,
+              CompilationInfo* info,
               HBasicBlock* true_target = NULL,
               HBasicBlock* false_target = NULL)
       : HUnaryControlInstruction(value, true_target, false_target),
         known_successor_index_(kNoKnownSuccessorIndex), map_(Unique<Map>(map)) {
     ASSERT(!map.is_null());
+    is_stable_ = map->is_stable();
+
+    if (FLAG_check_elimination && is_stable_) {
+      map->AddDependentCompilationInfo(
+          DependentCode::kPrototypeCheckGroup, info);
+    }
   }
 
   int known_successor_index_;
+  bool is_stable_;
   Unique<Map> map_;
 };
 
@@ -2633,10 +2647,11 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
                          HValue* typecheck = NULL);
   static HCheckMaps* New(Zone* zone, HValue* context,
                          HValue* value, SmallMapList* maps,
+                         CompilationInfo* info,
                          HValue* typecheck = NULL) {
     HCheckMaps* check_map = new(zone) HCheckMaps(value, zone, typecheck);
     for (int i = 0; i < maps->length(); i++) {
-      check_map->Add(maps->at(i), zone);
+      check_map->Add(maps->at(i), info, zone);
     }
     return check_map;
   }
@@ -2668,6 +2683,10 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
     return has_migration_target_;
   }
 
+  bool is_stable() const {
+    return is_stable_;
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(CheckMaps)
 
  protected:
@@ -2678,8 +2697,15 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
   virtual int RedefinedOperandIndex() { return 0; }
 
  private:
-  void Add(Handle<Map> map, Zone* zone) {
+  void Add(Handle<Map> map, CompilationInfo* info, Zone* zone) {
     map_set_.Add(Unique<Map>(map), zone);
+    is_stable_ = is_stable_ && map->is_stable();
+
+    if (FLAG_check_elimination && is_stable_) {
+      map->AddDependentCompilationInfo(
+          DependentCode::kPrototypeCheckGroup, info);
+    }
+
     if (!has_migration_target_ && map->is_migration_target()) {
       has_migration_target_ = true;
       SetChangesFlag(kNewSpacePromotion);
@@ -2689,7 +2715,7 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
   // Clients should use one of the static New* methods above.
   HCheckMaps(HValue* value, Zone *zone, HValue* typecheck)
       : HTemplateInstruction<2>(value->type()),
-        omit_(false), has_migration_target_(false) {
+        omit_(false), has_migration_target_(false), is_stable_(true) {
     SetOperandAt(0, value);
     // Use the object value for the dependency if NULL is passed.
     SetOperandAt(1, typecheck != NULL ? typecheck : value);
@@ -2702,6 +2728,7 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
 
   bool omit_;
   bool has_migration_target_;
+  bool is_stable_;
   UniqueSet<Map> map_set_;
 };
 
@@ -6477,6 +6504,16 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> {
     }
     SetOperandAt(2, map_constant);
     has_transition_ = true;
+    is_stable_ = map->is_stable();
+
+    if (FLAG_check_elimination && is_stable_) {
+      map->AddDependentCompilationInfo(
+          DependentCode::kPrototypeCheckGroup, info);
+    }
+  }
+
+  bool is_stable() const {
+    return is_stable_;
   }
 
   bool NeedsWriteBarrier() {
@@ -6515,6 +6552,7 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> {
         new_space_dominator_(NULL),
         write_barrier_mode_(UPDATE_WRITE_BARRIER),
         has_transition_(false),
+        is_stable_(false),
         store_mode_(store_mode) {
     // Stores to a non existing in-object property are allowed only to the
     // newly allocated objects (via HAllocate or HInnerAllocatedObject).
@@ -6530,6 +6568,7 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> {
   HValue* new_space_dominator_;
   WriteBarrierMode write_barrier_mode_ : 1;
   bool has_transition_ : 1;
+  bool is_stable_ : 1;
   StoreFieldOrKeyedMode store_mode_ : 1;
 };
 

src/hydrogen.cc

@@ -1390,7 +1390,8 @@ HValue* HGraphBuilder::BuildCopyElementsOnWrite(HValue* object,
 
   IfBuilder cow_checker(this);
 
-  cow_checker.If<HCompareMap>(elements, factory->fixed_cow_array_map());
+  cow_checker.If<HCompareMap>(
+      elements, factory->fixed_cow_array_map(), top_info());
   cow_checker.Then();
 
   HValue* capacity = AddLoadFixedArrayLength(elements);
@@ -1707,7 +1708,7 @@ HValue* HGraphBuilder::BuildNumberToString(HValue* object, Type* type) {
       // Check if the object is a heap number.
       IfBuilder if_objectisnumber(this);
       HValue* objectisnumber = if_objectisnumber.If<HCompareMap>(
-          object, isolate()->factory()->heap_number_map());
+          object, isolate()->factory()->heap_number_map(), top_info());
       if_objectisnumber.Then();
       {
         // Compute hash for heap number similar to double_get_hash().
@@ -5651,13 +5652,15 @@ void HOptimizedGraphBuilder::HandlePolymorphicNamedFieldAccess(
     HValue* dependency;
     if (info.type()->Is(Type::Number())) {
       Handle<Map> heap_number_map = isolate()->factory()->heap_number_map();
-      compare = New<HCompareMap>(object, heap_number_map, if_true, if_false);
+      compare = New<HCompareMap>(object, heap_number_map, top_info(),
+                                 if_true, if_false);
       dependency = smi_check;
     } else if (info.type()->Is(Type::String())) {
       compare = New<HIsStringAndBranch>(object, if_true, if_false);
       dependency = compare;
     } else {
-      compare = New<HCompareMap>(object, info.map(), if_true, if_false);
+      compare = New<HCompareMap>(object, info.map(), top_info(),
+                                 if_true, if_false);
       dependency = compare;
     }
     FinishCurrentBlock(compare);
@@ -6297,7 +6300,7 @@ HInstruction* HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad(
   }
   if (!has_double_maps && !has_smi_or_object_maps) return NULL;
 
-  HCheckMaps* checked_object = Add<HCheckMaps>(object, maps);
+  HCheckMaps* checked_object = Add<HCheckMaps>(object, maps, top_info());
   // FAST_ELEMENTS is considered more general than FAST_HOLEY_SMI_ELEMENTS.
   // If we've seen both, the consolidated load must use FAST_HOLEY_ELEMENTS.
   ElementsKind consolidated_elements_kind = has_seen_holey_elements
@@ -6395,7 +6398,7 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(
     HBasicBlock* this_map = graph()->CreateBasicBlock();
     HBasicBlock* other_map = graph()->CreateBasicBlock();
     HCompareMap* mapcompare =
-        New<HCompareMap>(object, map, this_map, other_map);
+        New<HCompareMap>(object, map, top_info(), this_map, other_map);
     FinishCurrentBlock(mapcompare);
 
     set_current_block(this_map);
@@ -6603,7 +6606,7 @@ HInstruction* HOptimizedGraphBuilder::BuildNamedAccess(
       checked_object =
           Add<HCheckInstanceType>(object, HCheckInstanceType::IS_STRING);
     } else {
-      checked_object = Add<HCheckMaps>(object, types);
+      checked_object = Add<HCheckMaps>(object, types, top_info());
     }
     return BuildMonomorphicAccess(
         &info, object, checked_object, value, ast_id, return_id);
@@ -6855,11 +6858,13 @@ void HOptimizedGraphBuilder::HandlePolymorphicCallNamed(
     Handle<Map> map = info.map();
     if (info.type()->Is(Type::Number())) {
       Handle<Map> heap_number_map = isolate()->factory()->heap_number_map();
-      compare = New<HCompareMap>(receiver, heap_number_map, if_true, if_false);
+      compare = New<HCompareMap>(receiver, heap_number_map, top_info(),
+                                 if_true, if_false);
     } else if (info.type()->Is(Type::String())) {
       compare = New<HIsStringAndBranch>(receiver, if_true, if_false);
     } else {
-      compare = New<HCompareMap>(receiver, map, if_true, if_false);
+      compare = New<HCompareMap>(receiver, map, top_info(),
+                                 if_true, if_false);
     }
     FinishCurrentBlock(compare);
 
@@ -7704,7 +7709,7 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle<JSFunction> function,
     case kCallApiFunction:
     case kCallApiMethod:
       // Need to check that none of the receiver maps could have changed.
-      Add<HCheckMaps>(receiver, receiver_maps);
+      Add<HCheckMaps>(receiver, receiver_maps, top_info());
       // Need to ensure the chain between receiver and api_holder is intact.
       if (holder_lookup == CallOptimization::kHolderFound) {
         AddCheckPrototypeMaps(api_holder, receiver_maps->first());

test/cctest/test-deoptimization.cc

@@ -538,6 +538,7 @@ TEST(DeoptimizeCompare) {
 
 TEST(DeoptimizeLoadICStoreIC) {
   i::FLAG_concurrent_recompilation = false;
+  i::FLAG_check_elimination = false;
   LocalContext env;
   v8::HandleScope scope(env->GetIsolate());
 
@@ -619,6 +620,7 @@ TEST(DeoptimizeLoadICStoreIC) {
 
 TEST(DeoptimizeLoadICStoreICNested) {
   i::FLAG_concurrent_recompilation = false;
+  i::FLAG_check_elimination = false;
   LocalContext env;
   v8::HandleScope scope(env->GetIsolate());
 

test/mjsunit/regress/regress-map-invalidation-2.js

@@ -47,7 +47,9 @@ function g() {
 var fun = g();
 fun(false, c);
 fun(false, c);
+%OptimizeFunctionOnNextCall(fun);
 fun(false, c);
+%TryMigrateInstance(c);
 %OptimizeFunctionOnNextCall(fun);
 fun(false, c);
 fun(true, c);