Revision: 21186
Author: [email protected]
Date: Wed May 7 08:41:35 2014 UTC
Log: Fix constructors for HLoadNamedField.
Also try to determine an appropriate HType for tracked fields,
instead of (incorrectly) using HType::NonPrimitive() everywhere.
[email protected]
Review URL: https://codereview.chromium.org/269353003
http://code.google.com/p/v8/source/detail?r=21186
Modified:
/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-load-elimination.cc
/branches/bleeding_edge/src/hydrogen.cc
/branches/bleeding_edge/src/hydrogen.h
=======================================
--- /branches/bleeding_edge/src/hydrogen-check-elimination.cc Tue May 6
07:05:07 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-check-elimination.cc Wed May 7
08:41:35 2014 UTC
@@ -340,8 +340,10 @@
// Reduce a load of the map field when it is known to be a constant.
if (!instr->access().IsMap()) {
// Check if we introduce field maps here.
- if (instr->maps()->size() != 0) {
- Insert(instr, instr, instr->maps());
+ MapSet maps = instr->maps();
+ if (maps != NULL) {
+ ASSERT_NE(0, maps->size());
+ Insert(instr, NULL, maps);
}
return;
}
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Tue May 6
07:05:07 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-instructions.cc Wed May 7
08:41:35 2014 UTC
@@ -3418,7 +3418,7 @@
object()->PrintNameTo(stream);
access_.PrintTo(stream);
- if (maps()->size() != 0) {
+ if (maps() != NULL) {
stream->Add(" [%p", *maps()->at(0).handle());
for (int i = 1; i < maps()->size(); ++i) {
stream->Add(",%p", *maps()->at(i).handle());
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Wed May 7 06:24:29
2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-instructions.h Wed May 7 08:41:35
2014 UTC
@@ -6212,28 +6212,10 @@
class HLoadNamedField V8_FINAL : public HTemplateInstruction<2> {
public:
- static HLoadNamedField* New(Zone* zone, HValue* context,
- HValue* object, HValue* dependency,
- HObjectAccess access) {
- return new(zone) HLoadNamedField(
- object, dependency, access, new(zone) UniqueSet<Map>());
- }
- static HLoadNamedField* New(Zone* zone, HValue* context,
- HValue* object, HValue* dependency,
- HObjectAccess access, SmallMapList* map_list,
- CompilationInfo* info) {
- UniqueSet<Map>* maps = new(zone) UniqueSet<Map>(map_list->length(),
zone);
- for (int i = 0; i < map_list->length(); ++i) {
- Handle<Map> map = map_list->at(i);
- maps->Add(Unique<Map>::CreateImmovable(map), zone);
- // TODO(bmeurer): Get rid of this shit!
- if (map->CanTransition()) {
- Map::AddDependentCompilationInfo(
- map, DependentCode::kPrototypeCheckGroup, info);
- }
- }
- return new(zone) HLoadNamedField(object, dependency, access, maps);
- }
+ DECLARE_INSTRUCTION_FACTORY_P3(HLoadNamedField, HValue*,
+ HValue*, HObjectAccess);
+ DECLARE_INSTRUCTION_FACTORY_P5(HLoadNamedField, HValue*, HValue*,
+ HObjectAccess, const UniqueSet<Map>*,
HType);
HValue* object() { return OperandAt(0); }
HValue* dependency() {
@@ -6262,23 +6244,36 @@
virtual Range* InferRange(Zone* zone) V8_OVERRIDE;
virtual void PrintDataTo(StringStream* stream) V8_OVERRIDE;
+ bool CanBeReplacedWith(HValue* other) const {
+ if (!type().Equals(other->type())) return false;
+ if (!representation().Equals(other->representation())) return false;
+ if (!other->IsLoadNamedField()) return true;
+ HLoadNamedField* that = HLoadNamedField::cast(other);
+ if (this->maps_ == that->maps_) return true;
+ if (this->maps_ == NULL || that->maps_ == NULL) return false;
+ return this->maps_->IsSubset(that->maps_);
+ }
+
DECLARE_CONCRETE_INSTRUCTION(LoadNamedField)
protected:
virtual bool DataEquals(HValue* other) V8_OVERRIDE {
- HLoadNamedField* b = HLoadNamedField::cast(other);
- return access_.Equals(b->access_) && this->maps()->Equals(b->maps());
+ HLoadNamedField* that = HLoadNamedField::cast(other);
+ if (!this->access_.Equals(that->access_)) return false;
+ if (this->maps_ == that->maps_) return true;
+ return (this->maps_ != NULL &&
+ that->maps_ != NULL &&
+ this->maps_->Equals(that->maps_));
}
private:
HLoadNamedField(HValue* object,
HValue* dependency,
- HObjectAccess access,
- const UniqueSet<Map>* maps)
- : access_(access), maps_(maps) {
- ASSERT(object != NULL);
+ HObjectAccess access)
+ : access_(access), maps_(NULL) {
+ ASSERT_NOT_NULL(object);
SetOperandAt(0, object);
- SetOperandAt(1, dependency != NULL ? dependency : object);
+ SetOperandAt(1, dependency ? dependency : object);
Representation representation = access.representation();
if (representation.IsInteger8() ||
@@ -6298,6 +6293,8 @@
representation.IsInteger32()) {
set_representation(representation);
} else if (representation.IsHeapObject()) {
+ // TODO(bmeurer): This is probably broken. What we actually want to
to
+ // instead is set_representation(Representation::HeapObject()).
set_type(HType::NonPrimitive());
set_representation(Representation::Tagged());
} else {
@@ -6305,6 +6302,28 @@
}
access.SetGVNFlags(this, LOAD);
}
+
+ HLoadNamedField(HValue* object,
+ HValue* dependency,
+ HObjectAccess access,
+ const UniqueSet<Map>* maps,
+ HType type)
+ : HTemplateInstruction<2>(type), access_(access), maps_(maps) {
+ ASSERT_NOT_NULL(maps);
+ ASSERT_NE(0, maps->size());
+
+ ASSERT_NOT_NULL(object);
+ SetOperandAt(0, object);
+ SetOperandAt(1, dependency ? dependency : object);
+
+ ASSERT(access.representation().IsHeapObject());
+ // TODO(bmeurer): This is probably broken. What we actually want to to
+ // instead is set_representation(Representation::HeapObject()).
+ if (!type.IsHeapObject()) set_type(HType::NonPrimitive());
+ set_representation(Representation::Tagged());
+
+ access.SetGVNFlags(this, LOAD);
+ }
virtual bool IsDeletable() const V8_OVERRIDE { return true; }
=======================================
--- /branches/bleeding_edge/src/hydrogen-load-elimination.cc Mon May 5
06:53:19 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen-load-elimination.cc Wed May 7
08:41:35 2014 UTC
@@ -53,12 +53,7 @@
FieldOf(l->access()),
l->object()->ActualValue()->id()));
HValue* result = load(l);
- if (result != instr &&
- result->type().Equals(instr->type()) &&
- result->representation().Equals(instr->representation()) &&
- (!result->IsLoadNamedField() ||
- HLoadNamedField::cast(instr)->maps()->IsSubset(
- HLoadNamedField::cast(result)->maps()))) {
+ if (result != instr && l->CanBeReplacedWith(result)) {
// The load can be replaced with a previous load or a value.
TRACE((" replace L%d -> v%d\n", instr->id(), result->id()));
instr->DeleteAndReplaceWith(result);
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc Tue May 6 11:05:52 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen.cc Wed May 7 08:41:35 2014 UTC
@@ -5348,8 +5348,24 @@
// Load the double value from it.
access = HObjectAccess::ForHeapNumberValue();
}
+
+ SmallMapList* map_list = info->field_maps();
+ if (map_list->length() == 0) {
+ return New<HLoadNamedField>(checked_object, checked_object, access);
+ }
+
+ UniqueSet<Map>* maps = new(zone()) UniqueSet<Map>(map_list->length(),
zone());
+ for (int i = 0; i < map_list->length(); ++i) {
+ Handle<Map> map = map_list->at(i);
+ maps->Add(Unique<Map>::CreateImmovable(map), zone());
+ // TODO(bmeurer): Get rid of this shit!
+ if (map->CanTransition()) {
+ Map::AddDependentCompilationInfo(
+ map, DependentCode::kPrototypeCheckGroup, top_info());
+ }
+ }
return New<HLoadNamedField>(
- checked_object, checked_object, access, info->field_maps(),
top_info());
+ checked_object, checked_object, access, maps, info->field_type());
}
@@ -5488,6 +5504,7 @@
}
}
info->GeneralizeRepresentation(r);
+ info->field_type_ = info->field_type_.Combine(field_type_);
return true;
}
@@ -5539,8 +5556,9 @@
void HOptimizedGraphBuilder::PropertyAccessInfo::LoadFieldMaps(
Handle<Map> map) {
- // Clear any previously collected field maps.
+ // Clear any previously collected field maps/type.
field_maps_.Clear();
+ field_type_ = HType::Tagged();
// Figure out the field type from the accessor map.
Handle<HeapType> field_type(lookup_.GetFieldTypeFromMap(*map),
isolate());
@@ -5562,6 +5580,22 @@
}
field_maps_.Sort();
ASSERT_EQ(num_field_maps, field_maps_.length());
+
+ // Determine field HType from field HeapType.
+ if (field_type->Is(HeapType::Number())) {
+ field_type_ = HType::HeapNumber();
+ } else if (field_type->Is(HeapType::String())) {
+ field_type_ = HType::String();
+ } else if (field_type->Is(HeapType::Boolean())) {
+ field_type_ = HType::Boolean();
+ } else if (field_type->Is(HeapType::Array())) {
+ field_type_ = HType::JSArray();
+ } else if (field_type->Is(HeapType::Object())) {
+ field_type_ = HType::JSObject();
+ } else if (field_type->Is(HeapType::Null()) ||
+ field_type->Is(HeapType::Undefined())) {
+ field_type_ = HType::NonPrimitive();
+ }
// Add dependency on the map that introduced the field.
Map::AddDependentCompilationInfo(
=======================================
--- /branches/bleeding_edge/src/hydrogen.h Mon May 5 11:03:14 2014 UTC
+++ /branches/bleeding_edge/src/hydrogen.h Wed May 7 08:41:35 2014 UTC
@@ -2326,6 +2326,7 @@
access_type_(access_type),
type_(type),
name_(name),
+ field_type_(HType::Tagged()),
access_(HObjectAccess::ForMap()) { }
// Checkes whether this PropertyAccessInfo can be handled as a
monomorphic
@@ -2392,6 +2393,7 @@
Handle<Object> constant() { return constant_; }
Handle<Map> transition() { return
handle(lookup_.GetTransitionTarget()); }
SmallMapList* field_maps() { return &field_maps_; }
+ HType field_type() const { return field_type_; }
HObjectAccess access() { return access_; }
private:
@@ -2422,6 +2424,7 @@
Handle<JSObject> api_holder_;
Handle<Object> constant_;
SmallMapList field_maps_;
+ HType field_type_;
HObjectAccess access_;
};
--
--
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/d/optout.
