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

@@ -413,7 +413,7 @@ Handle<Code> KeyedLoadFastElementStub::GenerateCode() {
 template<>
 HValue* CodeStubGraphBuilder<LoadFieldStub>::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<Code> LoadFieldStub::GenerateCode() {
 template<>
 HValue* CodeStubGraphBuilder<KeyedLoadFieldStub>::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;

src/hydrogen.cc

@@ -7131,22 +7131,31 @@ void HOptimizedGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr,
     HValue* object,
     SmallMapList* types,
     Handle<String> 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> map;
+  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());
-  for (int i = 0; i < types->length() && count < kMaxLoadPolymorphism; ++i) {
-    map = types->at(i);
-    if (ComputeLoadStoreField(map, name, &lookup, false)) {
+    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> 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);
-      }
-      ++count;
+        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;
       }
+
+      representation = representation.generalize(new_representation);
     }
 
-  // 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) {
+    if (count == types->length()) {
       AddInstruction(HCheckMaps::New(object, types, zone()));
-    instr = BuildLoadNamedField(object, map, &lookup);
-  } else {
+      instr = DoBuildLoadNamedField(
+          object, previous_field_is_in_object,
+          representation, previous_field_offset);
+    }
+  }
+
+  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> 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);
 }
 
 

src/hydrogen.h

@@ -982,6 +982,11 @@ class HGraphBuilder {
   HValue* BuildCheckMap(HValue* obj, Handle<Map> map);
 
   // Building common constructs
+  HLoadNamedField* DoBuildLoadNamedField(HValue* object,
+                                         bool inobject,
+                                         Representation representation,
+                                         int offset);
+
   HInstruction* BuildExternalArrayElementAccess(
       HValue* external_elements,
       HValue* checked_key,

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);

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));