Revision: 19331
Author: [email protected]
Date: Wed Feb 12 15:38:42 2014 UTC
Log: Revert "Use stability to only conditionally flush information
from the CheckMaps table."
[email protected]
BUG=
Review URL: https://codereview.chromium.org/137863005
http://code.google.com/p/v8/source/detail?r=19331
Modified:
/branches/bleeding_edge/src/code-stubs-hydrogen.cc
/branches/bleeding_edge/src/compiler.cc
/branches/bleeding_edge/src/hydrogen-check-elimination.cc
/branches/bleeding_edge/src/hydrogen-instructions.cc
/branches/bleeding_edge/src/hydrogen-instructions.h
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/test/cctest/test-deoptimization.cc
/branches/bleeding_edge/test/mjsunit/regress/regress-map-invalidation-2.js
=======================================
--- /branches/bleeding_edge/src/code-stubs-hydrogen.cc Wed Feb 12 15:07:41
2014 UTC
+++ /branches/bleeding_edge/src/code-stubs-hydrogen.cc Wed Feb 12 15:38:42
2014 UTC
@@ -295,8 +295,7 @@
// 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(),
- top_info());
+ if_number.OrIf<HCompareMap>(value,
isolate()->factory()->heap_number_map());
if_number.Then();
// Return the number.
@@ -359,8 +358,7 @@
HValue* elements = AddLoadElements(boilerplate);
IfBuilder if_fixed_cow(this);
- if_fixed_cow.If<HCompareMap>(elements, factory->fixed_cow_array_map(),
- top_info());
+ if_fixed_cow.If<HCompareMap>(elements, factory->fixed_cow_array_map());
if_fixed_cow.Then();
push_value = BuildCloneShallowArray(boilerplate,
allocation_site,
@@ -371,7 +369,7 @@
if_fixed_cow.Else();
IfBuilder if_fixed(this);
- if_fixed.If<HCompareMap>(elements, factory->fixed_array_map(),
top_info());
+ if_fixed.If<HCompareMap>(elements, factory->fixed_array_map());
if_fixed.Then();
push_value = BuildCloneShallowArray(boilerplate,
allocation_site,
=======================================
--- /branches/bleeding_edge/src/compiler.cc Wed Feb 12 15:07:41 2014 UTC
+++ /branches/bleeding_edge/src/compiler.cc Wed Feb 12 15:38:42 2014 UTC
@@ -1185,7 +1185,7 @@
if (FLAG_trace_opt) {
PrintF("[failed to optimize ");
function->PrintName();
- PrintF(": %s]\n", GetBailoutReason(info->bailout_reason()));
+ PrintF("]\n");
}
if (isolate->has_pending_exception()) isolate->clear_pending_exception();
=======================================
--- /branches/bleeding_edge/src/hydrogen-check-elimination.cc Wed Feb 12
15:07:41 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-check-elimination.cc Wed Feb 12
15:38:42 2014 UTC
@@ -50,7 +50,6 @@
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 @@
}
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:
@@ -156,7 +154,6 @@
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.
@@ -178,8 +175,7 @@
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 @@
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<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)).
@@ -222,10 +217,10 @@
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 @@
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 @@
} 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;
}
@@ -357,8 +351,7 @@
}
} 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()));
}
}
@@ -420,8 +413,7 @@
}
} else {
// No prior information.
- // TODO(verwaest): Tag map constants with stability.
- Insert(object, instr, map, false);
+ Insert(object, instr, map);
}
}
@@ -438,14 +430,12 @@
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));
@@ -495,25 +485,11 @@
}
}
- // 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) {
@@ -596,24 +572,17 @@
return entry == NULL ? NULL : entry->maps_;
}
- void Insert(HValue* object,
- HInstruction* check,
- Unique<Map> map,
- bool is_stable) {
+ void Insert(HValue* object, HInstruction* check, Unique<Map> map) {
MapSet list = new(phase_->zone()) UniqueSet<Map>();
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_++;
@@ -643,7 +612,8 @@
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.
@@ -651,22 +621,27 @@
// 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;
}
@@ -681,14 +656,14 @@
// 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<HStoreNamedField*> stores_;
};
@@ -705,7 +680,7 @@
} 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);
}
}
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Wed Feb 12
15:07:41 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Wed Feb 12
15:38:42 2014 UTC
@@ -3107,7 +3107,7 @@
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)) {
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Wed Feb 12 15:07:41
2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-instructions.h Wed Feb 12 15:38:42
2014 UTC
@@ -1508,10 +1508,8 @@
class HCompareMap V8_FINAL : public HUnaryControlInstruction {
public:
- DECLARE_INSTRUCTION_FACTORY_P3(HCompareMap, HValue*, Handle<Map>,
- CompilationInfo*);
- DECLARE_INSTRUCTION_FACTORY_P5(HCompareMap, HValue*, Handle<Map>,
- CompilationInfo*,
+ DECLARE_INSTRUCTION_FACTORY_P2(HCompareMap, HValue*, Handle<Map>);
+ DECLARE_INSTRUCTION_FACTORY_P4(HCompareMap, HValue*, Handle<Map>,
HBasicBlock*, HBasicBlock*);
virtual bool KnownSuccessorBlock(HBasicBlock** block) V8_OVERRIDE {
@@ -1536,10 +1534,6 @@
virtual Representation RequiredInputRepresentation(int index)
V8_OVERRIDE {
return Representation::Tagged();
}
-
- bool is_stable() const {
- return is_stable_;
- }
DECLARE_CONCRETE_INSTRUCTION(CompareMap)
@@ -1549,22 +1543,14 @@
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_;
};
@@ -2647,11 +2633,10 @@
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;
}
@@ -2682,10 +2667,6 @@
bool has_migration_target() const {
return has_migration_target_;
}
-
- bool is_stable() const {
- return is_stable_;
- }
DECLARE_CONCRETE_INSTRUCTION(CheckMaps)
@@ -2697,15 +2678,8 @@
virtual int RedefinedOperandIndex() { return 0; }
private:
- void Add(Handle<Map> map, CompilationInfo* info, Zone* zone) {
+ void Add(Handle<Map> map, 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);
@@ -2715,7 +2689,7 @@
// 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);
@@ -2728,7 +2702,6 @@
bool omit_;
bool has_migration_target_;
- bool is_stable_;
UniqueSet<Map> map_set_;
};
@@ -6504,16 +6477,6 @@
}
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() {
@@ -6552,7 +6515,6 @@
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).
@@ -6568,7 +6530,6 @@
HValue* new_space_dominator_;
WriteBarrierMode write_barrier_mode_ : 1;
bool has_transition_ : 1;
- bool is_stable_ : 1;
StoreFieldOrKeyedMode store_mode_ : 1;
};
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Wed Feb 12 15:07:41 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen.cc Wed Feb 12 15:38:42 2014 UTC
@@ -1390,8 +1390,7 @@
IfBuilder cow_checker(this);
- cow_checker.If<HCompareMap>(
- elements, factory->fixed_cow_array_map(), top_info());
+ cow_checker.If<HCompareMap>(elements, factory->fixed_cow_array_map());
cow_checker.Then();
HValue* capacity = AddLoadFixedArrayLength(elements);
@@ -1708,7 +1707,7 @@
// Check if the object is a heap number.
IfBuilder if_objectisnumber(this);
HValue* objectisnumber = if_objectisnumber.If<HCompareMap>(
- 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().
@@ -5652,15 +5651,13 @@
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, top_info(),
- if_true, if_false);
+ compare = New<HCompareMap>(object, heap_number_map, 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(), top_info(),
- if_true, if_false);
+ compare = New<HCompareMap>(object, info.map(), if_true, if_false);
dependency = compare;
}
FinishCurrentBlock(compare);
@@ -6300,7 +6297,7 @@
}
if (!has_double_maps && !has_smi_or_object_maps) return NULL;
- HCheckMaps* checked_object = Add<HCheckMaps>(object, maps, top_info());
+ HCheckMaps* checked_object = Add<HCheckMaps>(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
@@ -6398,7 +6395,7 @@
HBasicBlock* this_map = graph()->CreateBasicBlock();
HBasicBlock* other_map = graph()->CreateBasicBlock();
HCompareMap* mapcompare =
- New<HCompareMap>(object, map, top_info(), this_map, other_map);
+ New<HCompareMap>(object, map, this_map, other_map);
FinishCurrentBlock(mapcompare);
set_current_block(this_map);
@@ -6606,7 +6603,7 @@
checked_object =
Add<HCheckInstanceType>(object, HCheckInstanceType::IS_STRING);
} else {
- checked_object = Add<HCheckMaps>(object, types, top_info());
+ checked_object = Add<HCheckMaps>(object, types);
}
return BuildMonomorphicAccess(
&info, object, checked_object, value, ast_id, return_id);
@@ -6858,13 +6855,11 @@
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, top_info(),
- if_true, if_false);
+ compare = New<HCompareMap>(receiver, heap_number_map, 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, top_info(),
- if_true, if_false);
+ compare = New<HCompareMap>(receiver, map, if_true, if_false);
}
FinishCurrentBlock(compare);
@@ -7709,7 +7704,7 @@
case kCallApiFunction:
case kCallApiMethod:
// Need to check that none of the receiver maps could have changed.
- Add<HCheckMaps>(receiver, receiver_maps, top_info());
+ Add<HCheckMaps>(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());
=======================================
--- /branches/bleeding_edge/test/cctest/test-deoptimization.cc Wed Feb 12
15:07:41 2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-deoptimization.cc Wed Feb 12
15:38:42 2014 UTC
@@ -538,7 +538,6 @@
TEST(DeoptimizeLoadICStoreIC) {
i::FLAG_concurrent_recompilation = false;
- i::FLAG_check_elimination = false;
LocalContext env;
v8::HandleScope scope(env->GetIsolate());
@@ -620,7 +619,6 @@
TEST(DeoptimizeLoadICStoreICNested) {
i::FLAG_concurrent_recompilation = false;
- i::FLAG_check_elimination = false;
LocalContext env;
v8::HandleScope scope(env->GetIsolate());
=======================================
---
/branches/bleeding_edge/test/mjsunit/regress/regress-map-invalidation-2.js
Wed Feb 12 15:07:41 2014 UTC
+++
/branches/bleeding_edge/test/mjsunit/regress/regress-map-invalidation-2.js
Wed Feb 12 15:38:42 2014 UTC
@@ -47,9 +47,7 @@
var fun = g();
fun(false, c);
fun(false, c);
-%OptimizeFunctionOnNextCall(fun);
fun(false, c);
-%TryMigrateInstance(c);
%OptimizeFunctionOnNextCall(fun);
fun(false, c);
fun(true, c);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
