From 46d39cabd602095dafbdb54ea1198dcdf4b0ff23 Mon Sep 17 00:00:00 2001 From: "verwaest@chromium.org" Date: Tue, 7 May 2013 10:32:23 +0000 Subject: [PATCH] Fix polymorphic to monomorphic load to take representation into account. Review URL: https://chromiumcodereview.appspot.com/14966005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14565 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/code-stubs-hydrogen.cc | 4 +- src/hydrogen.cc | 92 ++++++++++++++++++++++++++------------------ src/hydrogen.h | 5 +++ src/property-details.h | 5 +++ test/mjsunit/track-fields.js | 19 +++++++++ 5 files changed, 86 insertions(+), 39 deletions(-) diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index dbe41cb..16cfd12 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -413,7 +413,7 @@ Handle KeyedLoadFastElementStub::GenerateCode() { template<> HValue* CodeStubGraphBuilder::BuildCodeStub() { Representation representation = casted_stub()->representation(); - HInstruction* load = AddInstruction(new(zone()) HLoadNamedField( + HInstruction* load = AddInstruction(DoBuildLoadNamedField( GetParameter(0), casted_stub()->is_inobject(), representation, casted_stub()->offset())); return load; @@ -428,7 +428,7 @@ Handle LoadFieldStub::GenerateCode() { template<> HValue* CodeStubGraphBuilder::BuildCodeStub() { Representation representation = casted_stub()->representation(); - HInstruction* load = AddInstruction(new(zone()) HLoadNamedField( + HInstruction* load = AddInstruction(DoBuildLoadNamedField( GetParameter(0), casted_stub()->is_inobject(), representation, casted_stub()->offset())); return load; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 58a9b78..3c03b15 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -7131,22 +7131,31 @@ void HOptimizedGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr, HValue* object, SmallMapList* types, Handle name) { - int count = 0; - int previous_field_offset = 0; - bool previous_field_is_in_object = false; - bool is_monomorphic_field = true; if (HandlePolymorphicArrayLengthLoad(expr, object, types, name)) return; - Handle map; - LookupResult lookup(isolate()); - for (int i = 0; i < types->length() && count < kMaxLoadPolymorphism; ++i) { - map = types->at(i); - if (ComputeLoadStoreField(map, name, &lookup, false)) { + AddInstruction(new(zone()) HCheckNonSmi(object)); + + // Use monomorphic load if property lookup results in the same field index + // for all maps. Requires special map check on the set of all handled maps. + HInstruction* instr = NULL; + if (types->length() > 0 && types->length() <= kMaxLoadPolymorphism) { + LookupResult lookup(isolate()); + int previous_field_offset = 0; + bool previous_field_is_in_object = false; + Representation representation = Representation::None(); + int count; + for (count = 0; count < types->length(); ++count) { + Handle map = types->at(count); + if (!ComputeLoadStoreField(map, name, &lookup, false)) break; + int index = ComputeLoadStoreFieldIndex(map, &lookup); + Representation new_representation = + ComputeLoadStoreRepresentation(map, &lookup); bool is_in_object = index < 0; int offset = index * kPointerSize; + if (index < 0) { // Negative property indices are in-object properties, indexed // from the end of the fixed part of the object. @@ -7154,31 +7163,33 @@ void HOptimizedGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr, } else { offset += FixedArray::kHeaderSize; } + if (count == 0) { previous_field_offset = offset; previous_field_is_in_object = is_in_object; - } else if (is_monomorphic_field) { - is_monomorphic_field = (offset == previous_field_offset) && - (is_in_object == previous_field_is_in_object); + representation = new_representation; + } else if (offset != previous_field_offset || + is_in_object != previous_field_is_in_object || + (FLAG_track_fields && + !representation.IsCompatibleForLoad(new_representation))) { + break; } - ++count; + + representation = representation.generalize(new_representation); + } + + if (count == types->length()) { + AddInstruction(HCheckMaps::New(object, types, zone())); + instr = DoBuildLoadNamedField( + object, previous_field_is_in_object, + representation, previous_field_offset); } } - // Use monomorphic load if property lookup results in the same field index - // for all maps. Requires special map check on the set of all handled maps. - AddInstruction(new(zone()) HCheckNonSmi(object)); - HInstruction* instr; - if (count == types->length() && is_monomorphic_field) { - AddInstruction(HCheckMaps::New(object, types, zone())); - instr = BuildLoadNamedField(object, map, &lookup); - } else { + if (instr == NULL) { HValue* context = environment()->LookupContext(); - instr = new(zone()) HLoadNamedFieldPolymorphic(context, - object, - types, - name, - zone()); + instr = new(zone()) HLoadNamedFieldPolymorphic( + context, object, types, name, zone()); } instr->set_position(expr->position()); @@ -7735,18 +7746,25 @@ HLoadNamedField* HOptimizedGraphBuilder::BuildLoadNamedField( HValue* object, Handle map, LookupResult* lookup) { - Representation representation = lookup->representation(); int index = lookup->GetLocalFieldIndexFromMap(*map); - 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 new(zone()) HLoadNamedField(object, true, representation, offset); - } else { - // Non-negative property indices are in the properties array. - int offset = (index * kPointerSize) + FixedArray::kHeaderSize; - return new(zone()) HLoadNamedField(object, false, representation, offset); - } + // 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 inobject = index < 0; + Representation representation = lookup->representation(); + int offset = inobject + ? index * kPointerSize + map->instance_size() + : index * kPointerSize + FixedArray::kHeaderSize; + return DoBuildLoadNamedField(object, inobject, representation, offset); +} + + +HLoadNamedField* HGraphBuilder::DoBuildLoadNamedField( + HValue* object, + bool inobject, + Representation representation, + int offset) { + return new(zone()) HLoadNamedField(object, inobject, representation, offset); } diff --git a/src/hydrogen.h b/src/hydrogen.h index 8df7a29..b61ab3b 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -982,6 +982,11 @@ class HGraphBuilder { HValue* BuildCheckMap(HValue* obj, Handle map); // Building common constructs + HLoadNamedField* DoBuildLoadNamedField(HValue* object, + bool inobject, + Representation representation, + int offset); + HInstruction* BuildExternalArrayElementAccess( HValue* external_elements, HValue* checked_key, diff --git a/src/property-details.h b/src/property-details.h index f025f15..dd8a777 100644 --- a/src/property-details.h +++ b/src/property-details.h @@ -103,6 +103,11 @@ class Representation { return kind_ == other.kind_; } + bool IsCompatibleForLoad(const Representation& other) const { + return (IsDouble() && other.IsDouble()) || + (!IsDouble() && !other.IsDouble()); + } + bool is_more_general_than(const Representation& other) const { ASSERT(kind_ != kExternal); ASSERT(other.kind_ != kExternal); diff --git a/test/mjsunit/track-fields.js b/test/mjsunit/track-fields.js index 3176a5a..ced2bb3 100644 --- a/test/mjsunit/track-fields.js +++ b/test/mjsunit/track-fields.js @@ -99,3 +99,22 @@ assertFalse(%HaveSameMap(o6, o7)); // Smi, double, object. o6.c = {}; assertTrue(%HaveSameMap(o6, o7)); + +function poly_load(o, b) { + var v = o.field; + if (b) { + return v + 10; + } + return o; +} + +var of1 = {a:0}; +of1.field = {}; +var of2 = {b:0}; +of2.field = 10; + +poly_load(of1, false); +poly_load(of1, false); +poly_load(of2, true); +%OptimizeFunctionOnNextCall(poly_load); +assertEquals("[object Object]10", poly_load(of1, true)); -- 2.7.4