From: verwaest@chromium.org Date: Tue, 25 Feb 2014 16:11:58 +0000 (+0000) Subject: Revert "Use stability to only conditionally flush information from the CheckMaps... X-Git-Tag: upstream/4.7.83~10564 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d5caecccc5307b45de997083a78a6c848513350b;p=platform%2Fupstream%2Fv8.git Revert "Use stability to only conditionally flush information from the CheckMaps table." R=ishell@chromium.org Review URL: https://codereview.chromium.org/180023002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19548 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index e1d11fb..6bb0118 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -299,8 +299,7 @@ HValue* CodeStubGraphBuilder::BuildCodeStub() { // Check if the parameter is already a SMI or heap number. IfBuilder if_number(this); if_number.If(value); - if_number.OrIf(value, isolate()->factory()->heap_number_map(), - top_info()); + if_number.OrIf(value, isolate()->factory()->heap_number_map()); if_number.Then(); // Return the number. @@ -363,8 +362,7 @@ HValue* CodeStubGraphBuilder::BuildCodeStub() { HValue* elements = AddLoadElements(boilerplate); IfBuilder if_fixed_cow(this); - if_fixed_cow.If(elements, factory->fixed_cow_array_map(), - top_info()); + if_fixed_cow.If(elements, factory->fixed_cow_array_map()); if_fixed_cow.Then(); push_value = BuildCloneShallowArray(boilerplate, allocation_site, @@ -375,7 +373,7 @@ HValue* CodeStubGraphBuilder::BuildCodeStub() { if_fixed_cow.Else(); IfBuilder if_fixed(this); - if_fixed.If(elements, factory->fixed_array_map(), top_info()); + if_fixed.If(elements, factory->fixed_array_map()); if_fixed.Then(); push_value = BuildCloneShallowArray(boilerplate, allocation_site, diff --git a/src/hydrogen-check-elimination.cc b/src/hydrogen-check-elimination.cc index 6e6a84c..0ea0c3a 100644 --- a/src/hydrogen-check-elimination.cc +++ b/src/hydrogen-check-elimination.cc @@ -50,7 +50,6 @@ 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_; }; @@ -104,10 +103,9 @@ class HCheckTable : public ZoneObject { } default: { // If the instruction changes maps uncontrollably, drop everything. - if (instr->CheckChangesFlag(kOsrEntries)) { - Reset(); - } else if (instr->CheckChangesFlag(kMaps)) { - KillUnstableEntries(); + if (instr->CheckChangesFlag(kMaps) || + instr->CheckChangesFlag(kOsrEntries)) { + Kill(); } } // Improvements possible: @@ -152,7 +150,6 @@ class HCheckTable : public ZoneObject { HCheckTableEntry* new_entry = ©->entries_[i]; new_entry->object_ = old_entry->object_; new_entry->maps_ = old_entry->maps_->Copy(phase_->zone()); - new_entry->is_stable_ = old_entry->is_stable_; // Keep the check if the existing check's block dominates the successor. if (old_entry->check_ != NULL && old_entry->check_->block()->Dominates(succ)) { @@ -178,8 +175,7 @@ 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()), - pred_entry->is_stable_); + copy->Insert(phi, NULL, pred_entry->maps_->Copy(phase_->zone())); } } } @@ -197,13 +193,12 @@ 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(), cmp->is_stable()); + copy->Insert(object, cmp, cmp->map()); } else { MapSet list = new(phase_->zone()) UniqueSet(); 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)). @@ -222,10 +217,10 @@ class HCheckTable : public ZoneObject { HCheckTableEntry* re = copy->Find(right); if (le == NULL) { if (re != NULL) { - copy->Insert(left, NULL, re->maps_->Copy(zone), re->is_stable_); + copy->Insert(left, NULL, re->maps_->Copy(zone)); } } else if (re == NULL) { - copy->Insert(right, NULL, le->maps_->Copy(zone), le->is_stable_); + copy->Insert(right, NULL, le->maps_->Copy(zone)); } else { MapSet intersect = le->maps_->Intersect(re->maps_, zone); le->maps_ = intersect; @@ -252,7 +247,8 @@ class HCheckTable : public ZoneObject { HBasicBlock* pred_block, Zone* zone) { if (that->size_ == 0) { // If the other state is empty, simply reset. - Reset(); + size_ = 0; + cursor_ = 0; } else { int pred_index = succ->PredecessorIndexOf(pred_block); bool compact = false; @@ -275,8 +271,6 @@ 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; } @@ -359,8 +353,7 @@ class HCheckTable : public ZoneObject { } } else { // No entry; insert a new one. - Insert(object, instr, instr->map_set().Copy(phase_->zone()), - instr->is_stable()); + Insert(object, instr, instr->map_set().Copy(phase_->zone())); } } @@ -422,8 +415,7 @@ class HCheckTable : public ZoneObject { } } else { // No prior information. - // TODO(verwaest): Tag map constants with stability. - Insert(object, instr, map, false); + Insert(object, instr, map); } } @@ -440,14 +432,12 @@ 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()), - instr->is_stable()); + Insert(object, NULL, MapConstant(instr->transition())); } else if (IsMapAccess(instr->access())) { // This is a store directly to the map field of the object. Kill(object); if (!instr->value()->IsConstant()) return; - // TODO(verwaest): Tag with stability. - Insert(object, NULL, MapConstant(instr->value()), false); + Insert(object, NULL, MapConstant(instr->value())); } else { // If the instruction changes maps, it should be handled above. CHECK(!instr->CheckChangesFlag(kMaps)); @@ -497,26 +487,12 @@ class HCheckTable : public ZoneObject { } } - // Reset the table. - void Reset() { + // Kill everything in the table. + void Kill() { 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; @@ -598,24 +574,17 @@ class HCheckTable : public ZoneObject { return entry == NULL ? NULL : entry->maps_; } - void Insert(HValue* object, - HInstruction* check, - Unique map, - bool is_stable) { + void Insert(HValue* object, HInstruction* check, Unique map) { MapSet list = new(phase_->zone()) UniqueSet(); list->Add(map, phase_->zone()); - Insert(object, check, list, is_stable); + Insert(object, check, list); } - void Insert(HValue* object, - HInstruction* check, - MapSet maps, - bool is_stable) { + void Insert(HValue* object, HInstruction* check, MapSet maps) { 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_++; @@ -645,7 +614,8 @@ class HCheckTable : public ZoneObject { class HCheckMapsEffects : public ZoneObject { public: explicit HCheckMapsEffects(Zone* zone) - : stores_(5, zone) { } + : maps_stored_(false), + stores_(5, zone) { } inline bool Disabled() { return false; // Effects are _not_ disabled. @@ -653,22 +623,27 @@ class HCheckMapsEffects : public ZoneObject { // Process a possibly side-effecting instruction. void Process(HInstruction* instr, Zone* zone) { - if (instr->IsStoreNamedField()) { - stores_.Add(HStoreNamedField::cast(instr), zone); - } else { - flags_.Add(instr->ChangesFlags()); + 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)); + } } } // Apply these effects to the given check elimination table. void Apply(HCheckTable* table) { - if (flags_.Contains(kOsrEntries)) { - table->Reset(); - return; - } - if (flags_.Contains(kMaps) || flags_.Contains(kElementsKind)) { + if (maps_stored_) { // Uncontrollable map modifications; kill everything. - table->KillUnstableEntries(); + table->Kill(); return; } @@ -683,14 +658,14 @@ class HCheckMapsEffects : public ZoneObject { // Union these effects with the other effects. void Union(HCheckMapsEffects* that, Zone* zone) { - flags_.Add(that->flags_); + maps_stored_ |= that->maps_stored_; for (int i = 0; i < that->stores_.length(); i++) { stores_.Add(that->stores_[i], zone); } } private: - GVNFlagSet flags_; + bool maps_stored_ : 1; ZoneList stores_; }; @@ -707,7 +682,7 @@ void HCheckEliminationPhase::Run() { } else { // Perform only local analysis. for (int i = 0; i < graph()->blocks()->length(); i++) { - table->Reset(); + table->Kill(); engine.AnalyzeOneBlock(graph()->blocks()->at(i), table); } } diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 5b1d345..29d4e73 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -3120,7 +3120,7 @@ HCheckMaps* HCheckMaps::New(Zone* zone, CompilationInfo* info, HValue* typecheck) { HCheckMaps* check_map = new(zone) HCheckMaps(value, zone, typecheck); - check_map->Add(map, info, zone); + check_map->Add(map, zone); if (map->CanOmitMapChecks() && value->IsConstant() && HConstant::cast(value)->HasMap(map)) { diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index baf64fc..f7a3554 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -1564,10 +1564,8 @@ class HBranch V8_FINAL : public HUnaryControlInstruction { class HCompareMap V8_FINAL : public HUnaryControlInstruction { public: - DECLARE_INSTRUCTION_FACTORY_P3(HCompareMap, HValue*, Handle, - CompilationInfo*); - DECLARE_INSTRUCTION_FACTORY_P5(HCompareMap, HValue*, Handle, - CompilationInfo*, + DECLARE_INSTRUCTION_FACTORY_P2(HCompareMap, HValue*, Handle); + DECLARE_INSTRUCTION_FACTORY_P4(HCompareMap, HValue*, Handle, HBasicBlock*, HBasicBlock*); virtual bool KnownSuccessorBlock(HBasicBlock** block) V8_OVERRIDE { @@ -1593,10 +1591,6 @@ class HCompareMap V8_FINAL : public HUnaryControlInstruction { return Representation::Tagged(); } - bool is_stable() const { - return is_stable_; - } - DECLARE_CONCRETE_INSTRUCTION(CompareMap) protected: @@ -1605,23 +1599,15 @@ class HCompareMap V8_FINAL : public HUnaryControlInstruction { private: HCompareMap(HValue* value, Handle map, - CompilationInfo* info, HBasicBlock* true_target = NULL, HBasicBlock* false_target = NULL) : HUnaryControlInstruction(value, true_target, false_target), known_successor_index_(kNoKnownSuccessorIndex), map_(Unique(map)) { ASSERT(!map.is_null()); - is_stable_ = map->is_stable(); - - if (is_stable_) { - map->AddDependentCompilationInfo( - DependentCode::kPrototypeCheckGroup, info); - } set_representation(Representation::Tagged()); } int known_successor_index_; - bool is_stable_; Unique map_; }; @@ -2707,11 +2693,10 @@ 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), info, zone); + check_map->Add(maps->at(i), zone); } return check_map; } @@ -2743,10 +2728,6 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> { return has_migration_target_; } - bool is_stable() const { - return is_stable_; - } - DECLARE_CONCRETE_INSTRUCTION(CheckMaps) protected: @@ -2757,16 +2738,10 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> { virtual int RedefinedOperandIndex() { return 0; } private: - void Add(Handle map, CompilationInfo* info, Zone* zone) { + void Add(Handle map, Zone* zone) { map_set_.Add(Unique(map), zone); - is_stable_ = is_stable_ && map->is_stable(); - if (is_stable_) { - map->AddDependentCompilationInfo( - DependentCode::kPrototypeCheckGroup, info); - } else { - SetDependsOnFlag(kMaps); - SetDependsOnFlag(kElementsKind); - } + SetDependsOnFlag(kMaps); + SetDependsOnFlag(kElementsKind); if (!has_migration_target_ && map->is_migration_target()) { has_migration_target_ = true; @@ -2777,7 +2752,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), is_stable_(true) { + omit_(false), has_migration_target_(false) { SetOperandAt(0, value); // Use the object value for the dependency if NULL is passed. SetOperandAt(1, typecheck != NULL ? typecheck : value); @@ -2788,7 +2763,6 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> { bool omit_; bool has_migration_target_; - bool is_stable_; UniqueSet map_set_; }; @@ -6569,16 +6543,6 @@ class HStoreNamedField V8_FINAL : public HTemplateInstruction<3> { } SetOperandAt(2, map_constant); has_transition_ = true; - is_stable_ = map->is_stable(); - - if (is_stable_) { - map->AddDependentCompilationInfo( - DependentCode::kPrototypeCheckGroup, info); - } - } - - bool is_stable() const { - return is_stable_; } bool NeedsWriteBarrier() { @@ -6617,7 +6581,6 @@ 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). @@ -6633,7 +6596,6 @@ 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; }; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 1d437e7..6bc7f69 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -1364,8 +1364,7 @@ HValue* HGraphBuilder::BuildCopyElementsOnWrite(HValue* object, IfBuilder cow_checker(this); - cow_checker.If( - elements, factory->fixed_cow_array_map(), top_info()); + cow_checker.If(elements, factory->fixed_cow_array_map()); cow_checker.Then(); HValue* capacity = AddLoadFixedArrayLength(elements); @@ -1682,7 +1681,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( - object, isolate()->factory()->heap_number_map(), top_info()); + object, isolate()->factory()->heap_number_map()); if_objectisnumber.Then(); { // Compute hash for heap number similar to double_get_hash(). @@ -5745,15 +5744,13 @@ void HOptimizedGraphBuilder::HandlePolymorphicNamedFieldAccess( HValue* dependency; if (info.type()->Is(Type::Number())) { Handle heap_number_map = isolate()->factory()->heap_number_map(); - compare = New(object, heap_number_map, top_info(), - if_true, if_false); + compare = New(object, heap_number_map, if_true, if_false); dependency = smi_check; } else if (info.type()->Is(Type::String())) { compare = New(object, if_true, if_false); dependency = compare; } else { - compare = New(object, info.map(), top_info(), - if_true, if_false); + compare = New(object, info.map(), if_true, if_false); dependency = compare; } FinishCurrentBlock(compare); @@ -6371,7 +6368,7 @@ HInstruction* HOptimizedGraphBuilder::TryBuildConsolidatedElementLoad( } if (!has_double_maps && !has_smi_or_object_maps) return NULL; - HCheckMaps* checked_object = Add(object, maps, top_info()); + HCheckMaps* checked_object = Add(object, maps); // 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 @@ -6469,7 +6466,7 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess( HBasicBlock* this_map = graph()->CreateBasicBlock(); HBasicBlock* other_map = graph()->CreateBasicBlock(); HCompareMap* mapcompare = - New(object, map, top_info(), this_map, other_map); + New(object, map, this_map, other_map); FinishCurrentBlock(mapcompare); set_current_block(this_map); @@ -6681,7 +6678,7 @@ HInstruction* HOptimizedGraphBuilder::BuildNamedAccess( checked_object = Add(object, HCheckInstanceType::IS_STRING); } else { - checked_object = Add(object, types, top_info()); + checked_object = Add(object, types); } return BuildMonomorphicAccess( &info, object, checked_object, value, ast_id, return_id); @@ -6933,13 +6930,11 @@ void HOptimizedGraphBuilder::HandlePolymorphicCallNamed( Handle map = info.map(); if (info.type()->Is(Type::Number())) { Handle heap_number_map = isolate()->factory()->heap_number_map(); - compare = New(receiver, heap_number_map, top_info(), - if_true, if_false); + compare = New(receiver, heap_number_map, if_true, if_false); } else if (info.type()->Is(Type::String())) { compare = New(receiver, if_true, if_false); } else { - compare = New(receiver, map, top_info(), - if_true, if_false); + compare = New(receiver, map, if_true, if_false); } FinishCurrentBlock(compare); @@ -7789,7 +7784,7 @@ bool HOptimizedGraphBuilder::TryInlineApiCall(Handle function, case kCallApiFunction: case kCallApiMethod: // Need to check that none of the receiver maps could have changed. - Add(receiver, receiver_maps, top_info()); + Add(receiver, receiver_maps); // Need to ensure the chain between receiver and api_holder is intact. if (holder_lookup == CallOptimization::kHolderFound) { AddCheckPrototypeMaps(api_holder, receiver_maps->first()); diff --git a/test/mjsunit/regress/regress-map-invalidation-2.js b/test/mjsunit/regress/regress-map-invalidation-2.js index 7674e42..1f896a4 100644 --- a/test/mjsunit/regress/regress-map-invalidation-2.js +++ b/test/mjsunit/regress/regress-map-invalidation-2.js @@ -47,9 +47,7 @@ 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);