Reviewers: titzer,
https://chromiumcodereview.appspot.com/23241027/diff/1/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (left):
https://chromiumcodereview.appspot.com/23241027/diff/1/src/hydrogen-instructions.cc#oldcode3932
src/hydrogen-instructions.cc:3932: instr->SetGVNFlag(kDependsOnMaps);
No. We are transitioning because that property didn't exist yet for that
object. Hence it can't change a value of a property loaded from that
object.
Maps do change however, so DeopendsOnMaps values can't be hoisted over
it. HCheckMaps eg does depend on maps. And HLoadNamedField depend on
HCheckMaps.
For some reason kChangesMaps is still set manually in
BuildStoreNamedField.
On 2013/08/26 13:06:45, titzer wrote:
Can a map transition have side effects other than just changing that
one word in
the header of that object, e.g. change the layout of an object, thus
invalidating a previously loaded value?
https://chromiumcodereview.appspot.com/23241027/diff/1/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://chromiumcodereview.appspot.com/23241027/diff/1/src/hydrogen-instructions.cc#newcode3907
src/hydrogen-instructions.cc:3907: // Negative property indices are
in-object properties, indexed
Ok, I'll revert to it.
On 2013/08/26 13:06:45, titzer wrote:
I think I prefer the if; the logic gets too twisted otherwise.
https://chromiumcodereview.appspot.com/23241027/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://chromiumcodereview.appspot.com/23241027/diff/1/src/hydrogen-instructions.h#newcode5592
src/hydrogen-instructions.h:5592: class OffsetField : public
BitField<int, 7, 26> {};
Woops. 25 should be more than enough, given that we can only have 2^11
fast-mode properties.
On 2013/08/26 13:06:45, titzer wrote:
Not gonna fit in 32 anymore, unless you shrink the offset yet more :(
Description:
Avoid setting / depending on flags when transitioning, and when fields are
known
to be constant.
Please review this at https://chromiumcodereview.appspot.com/23241027/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/hydrogen-instructions.h
M src/hydrogen-instructions.cc
M src/hydrogen.cc
Index: src/hydrogen-instructions.cc
diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index
b0045b8751b06258f0a241416e54615924ea910f..4e71fabe39afdeee04e76681984b8e684582ab12
100644
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -3884,13 +3884,16 @@ HObjectAccess
HObjectAccess::ForBackingStoreOffset(int offset,
HObjectAccess HObjectAccess::ForField(Handle<Map> map,
- LookupResult *lookup, Handle<String> name) {
+ LookupResult *lookup,
+ Handle<String> name) {
ASSERT(lookup->IsField() || lookup->IsTransitionToField(*map));
int index;
Representation representation;
+ FieldType field_type;
if (lookup->IsField()) {
index = lookup->GetLocalFieldIndexFromMap(*map);
representation = lookup->representation();
+ field_type = MUTABLE;
} else {
Map* transition = lookup->GetTransitionMapFromMap(*map);
int descriptor = transition->LastAdded();
@@ -3899,17 +3902,15 @@ HObjectAccess HObjectAccess::ForField(Handle<Map>
map,
PropertyDetails details =
transition->instance_descriptors()->GetDetails(descriptor);
representation = details.representation();
+ field_type = CONSTANT;
}
- if (index < 0) {
- // Negative property indices are in-object properties, indexed
- // from the end of the fixed part of the object.
- int offset = (index * kPointerSize) + map->instance_size();
- return HObjectAccess(kInobject, offset, representation);
- } else {
- // Non-negative property indices are in the properties array.
- int offset = (index * kPointerSize) + FixedArray::kHeaderSize;
- return HObjectAccess(kBackingStore, offset, representation, name);
- }
+ // Negative property indices are in-object properties, indexed
+ // from the end of the fixed part of the object.
+ // Non-negative property indices are in the properties array.
+ int offset = index * kPointerSize;
+ offset += index < 0 ? map->instance_size() : FixedArray::kHeaderSize;
+ Portion portion = index < 0 ? kInobject : kBackingStore;
+ return HObjectAccess(portion, offset, representation, name, field_type);
}
@@ -3927,11 +3928,11 @@ void HObjectAccess::SetGVNFlags(HValue *instr, bool
is_store) {
instr->SetGVNFlag(kDependsOnNewSpacePromotion);
instr->SetFlag(HValue::kTrackSideEffectDominators);
} else {
- // try to GVN loads, but don't hoist above map changes
instr->SetFlag(HValue::kUseGVN);
- instr->SetGVNFlag(kDependsOnMaps);
}
+ if (FieldTypeField::decode(value_) == CONSTANT) return;
+
switch (portion()) {
case kArrayLengths:
instr->SetGVNFlag(is_store
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index
d5b1c75ebf8b7f87bffd705e7c3a2dce999229ed..ae5655bd7a1fc602b99323d5d1f0e69ad3d4221d
100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -5420,6 +5420,11 @@ class HStoreContextSlot V8_FINAL : public
HTemplateInstruction<2> {
// array elements pointer, etc, but not accesses to array elements
themselves.
class HObjectAccess V8_FINAL {
public:
+ // Indicates whether the field is initialized while transitioning, or
also
+ // mutable afterwards. Transitioning stores only initialize new fields
rather
+ // than changing existing fields, so they are marked as CONSTANT.
+ enum FieldType { MUTABLE, CONSTANT };
+
inline bool IsInobject() const {
return portion() != kBackingStore && portion() != kExternalMemory;
}
@@ -5444,8 +5449,10 @@ class HObjectAccess V8_FINAL {
return name_;
}
- inline HObjectAccess WithRepresentation(Representation representation) {
- return HObjectAccess(portion(), offset(), representation, name());
+ inline HObjectAccess WithRepresentation(Representation representation,
+ FieldType field_type = MUTABLE) {
+ return HObjectAccess(
+ portion(), offset(), representation, name(), field_type);
}
static HObjectAccess ForHeapNumberValue() {
@@ -5566,10 +5573,12 @@ class HObjectAccess V8_FINAL {
HObjectAccess(Portion portion, int offset,
Representation representation = Representation::Tagged(),
- Handle<String> name = Handle<String>::null())
+ Handle<String> name = Handle<String>::null(),
+ FieldType field_type = MUTABLE)
: value_(PortionField::encode(portion) |
- RepresentationField::encode(representation.kind()) |
- OffsetField::encode(offset)),
+ RepresentationField::encode(representation.kind()) |
+ FieldTypeField::encode(field_type) |
+ OffsetField::encode(offset)),
name_(name) {
// assert that the fields decode correctly
ASSERT(this->offset() == offset);
@@ -5579,7 +5588,8 @@ class HObjectAccess V8_FINAL {
class PortionField : public BitField<Portion, 0, 3> {};
class RepresentationField : public BitField<Representation::Kind, 3, 3>
{};
- class OffsetField : public BitField<int, 6, 26> {};
+ class FieldTypeField : public BitField<FieldType, 6, 1> {};
+ class OffsetField : public BitField<int, 7, 26> {};
uint32_t value_; // encodes portion, representation, and offset
Handle<String> name_;
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
fd4cc351af8b07d7fad9c547bfce99f04586b708..46e9b54ee08a9b2168ccf3ed3ae26c9ae581cba8
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4483,8 +4483,8 @@ HInstruction*
HOptimizedGraphBuilder::BuildStoreNamedField(
HStoreNamedField *instr;
if (FLAG_track_double_fields &&
field_access.representation().IsDouble()) {
- HObjectAccess heap_number_access =
- field_access.WithRepresentation(Representation::Tagged());
+ HObjectAccess heap_number_access = field_access.WithRepresentation(
+ Representation::Tagged(), HObjectAccess::CONSTANT);
if (transition_to_field) {
// The store requires a mutable HeapNumber to be allocated.
NoObservableSideEffectsScope no_side_effects(this);
@@ -5388,7 +5388,8 @@ HLoadNamedField*
HGraphBuilder::BuildLoadNamedField(HValue* object,
if (FLAG_track_double_fields && access.representation().IsDouble()) {
// load the heap number
HLoadNamedField* heap_number = Add<HLoadNamedField>(
- object, access.WithRepresentation(Representation::Tagged()));
+ object, access.WithRepresentation(
+ Representation::Tagged(), HObjectAccess::CONSTANT));
heap_number->set_type(HType::HeapNumber());
// load the double value from it
return New<HLoadNamedField>(
--
--
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.
