[turbofan] Introduce MachineRepresentation to PropertyAccessInfo.

src/compiler/access-info.cc

Index: src/compiler/access-info.cc
diff --git a/src/compiler/access-info.cc b/src/compiler/access-info.cc
index 1172bfbc835960b166098bfeccd1479bf21e0e2c..67a72661c230e6c3a7c6e0ef67149d9df109a046 100644
--- a/src/compiler/access-info.cc
+++ b/src/compiler/access-info.cc
@@ -79,10 +79,11 @@ PropertyAccessInfo PropertyAccessInfo::DataConstant(
 
 // static
 PropertyAccessInfo PropertyAccessInfo::DataField(
-    MapList const& receiver_maps, FieldIndex field_index, Type* field_type,
+    MapList const& receiver_maps, FieldIndex field_index,
+    MachineRepresentation representation, Type* field_type,
     MaybeHandle<JSObject> holder, MaybeHandle<Map> transition_map) {
-  return PropertyAccessInfo(holder, transition_map, field_index, field_type,
-                            receiver_maps);
+  return PropertyAccessInfo(holder, transition_map, field_index, representation,
+                            field_type, receiver_maps);
 }
 
 // static
@@ -93,13 +94,17 @@ PropertyAccessInfo PropertyAccessInfo::AccessorConstant(
 }
 
 PropertyAccessInfo::PropertyAccessInfo()
-    : kind_(kInvalid), field_type_(Type::None()) {}
+    : kind_(kInvalid),
+      representation_(MachineRepresentation::kNone),
+      field_type_(Type::None()) {}
 
 PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
                                        MapList const& receiver_maps)
     : kind_(kNotFound),
       receiver_maps_(receiver_maps),
       holder_(holder),
+      // TODO(mvstanton): we probably need a representation here!
+      representation_(MachineRepresentation::kNone),
       field_type_(Type::None()) {}
 
 PropertyAccessInfo::PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
@@ -109,17 +114,21 @@ PropertyAccessInfo::PropertyAccessInfo(Kind kind, MaybeHandle<JSObject> holder,
       receiver_maps_(receiver_maps),
       constant_(constant),
       holder_(holder),
+      representation_(MachineRepresentation::kNone),
       field_type_(Type::Any()) {}
 
 PropertyAccessInfo::PropertyAccessInfo(MaybeHandle<JSObject> holder,
                                        MaybeHandle<Map> transition_map,
-                                       FieldIndex field_index, Type* field_type,
+                                       FieldIndex field_index,
+                                       MachineRepresentation representation,
+                                       Type* field_type,
                                        MapList const& receiver_maps)
     : kind_(kDataField),
       receiver_maps_(receiver_maps),
       transition_map_(transition_map),
       holder_(holder),
       field_index_(field_index),
+      representation_(representation),
       field_type_(field_type) {}
 
 bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that) {
@@ -138,7 +147,8 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that) {
       if (this->transition_map_.address() == that->transition_map_.address() &&
           this->field_index_ == that->field_index_ &&
           this->field_type_->Is(that->field_type_) &&
-          that->field_type_->Is(this->field_type_)) {
+          that->field_type_->Is(this->field_type_) &&
+          this->representation_ == that->representation_) {
         this->receiver_maps_.insert(this->receiver_maps_.end(),
                                     that->receiver_maps_.begin(),
                                     that->receiver_maps_.end());
@@ -287,16 +297,18 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
           FieldIndex field_index = FieldIndex::ForPropertyIndex(
               *map, index, field_representation.IsDouble());
           Type* field_type = Type::Tagged();
+          MachineRepresentation rep = MachineRepresentation::kTagged;

[Benedikt Meurer, 2016/08/30 17:14:49]

Nit: field_representation

[mvstanton, 2016/08/30 18:59:34]

Done, but I had to rename the existing field_representation to
details_representation (since we are translating here from the Representation
type).

           if (field_representation.IsSmi()) {
             field_type = type_cache_.kSmi;
+            rep = MachineRepresentation::kTaggedSigned;
           } else if (field_representation.IsDouble()) {
             field_type = type_cache_.kFloat64;
+            rep = MachineRepresentation::kFloat64;
           } else if (field_representation.IsHeapObject()) {
             // Extract the field type from the property details (make sure its
             // representation is TaggedPointer to reflect the heap object case).
-            field_type = Type::Intersect(
-                descriptors->GetFieldType(number)->Convert(zone()),
-                Type::TaggedPointer(), zone());
+            rep = MachineRepresentation::kTaggedPointer;
+            field_type = descriptors->GetFieldType(number)->Convert(zone());
             if (field_type->Is(Type::None())) {
               // Store is not safe if the field type was cleared.
               if (access_mode == AccessMode::kStore) return false;
@@ -305,17 +317,19 @@ bool AccessInfoFactory::ComputePropertyAccessInfo(
               // about the contents now.
               // TODO(bmeurer): It would be awesome to make this saner in the
               // runtime/GC interaction.
-              field_type = Type::TaggedPointer();
+              // TODO(mvstanton): is type Any appropriate here?
+              field_type = Type::Any();
             } else if (!Type::Any()->Is(field_type)) {
               // Add proper code dependencies in case of stable field map(s).
+              // TODO(mvstanton): is this rep setting correct?
+              rep = MachineRepresentation::kTaggedPointer;
               Handle<Map> field_owner_map(map->FindFieldOwner(number),
                                           isolate());
               dependencies()->AssumeFieldType(field_owner_map);
             }
-            DCHECK(field_type->Is(Type::TaggedPointer()));
           }
           *access_info = PropertyAccessInfo::DataField(
-              MapList{receiver_map}, field_index, field_type, holder);
+              MapList{receiver_map}, field_index, rep, field_type, holder);
           return true;
         }
         case ACCESSOR_CONSTANT: {
@@ -422,11 +436,13 @@ bool AccessInfoFactory::LookupSpecialFieldAccessor(
   if (Accessors::IsJSObjectFieldAccessor(map, name, &offset)) {
     FieldIndex field_index = FieldIndex::ForInObjectOffset(offset);
     Type* field_type = Type::Tagged();
+    MachineRepresentation rep = MachineRepresentation::kTagged;

[Benedikt Meurer, 2016/08/30 17:14:49]

Nit: field_representation

[mvstanton, 2016/08/30 18:59:34]

Done.

     if (map->IsStringMap()) {
       DCHECK(Name::Equals(factory()->length_string(), name));
       // The String::length property is always a smi in the range
       // [0, String::kMaxLength].
       field_type = type_cache_.kStringLengthType;
+      rep = MachineRepresentation::kTaggedSigned;
     } else if (map->IsJSArrayMap()) {
       DCHECK(Name::Equals(factory()->length_string(), name));
       // The JSArray::length property is a smi in the range
@@ -436,14 +452,16 @@ bool AccessInfoFactory::LookupSpecialFieldAccessor(
       // case of other arrays.
       if (IsFastDoubleElementsKind(map->elements_kind())) {
         field_type = type_cache_.kFixedDoubleArrayLengthType;
+        rep = MachineRepresentation::kTaggedSigned;
       } else if (IsFastElementsKind(map->elements_kind())) {
         field_type = type_cache_.kFixedArrayLengthType;
+        rep = MachineRepresentation::kTaggedSigned;
       } else {
         field_type = type_cache_.kJSArrayLengthType;
       }
     }
-    *access_info =
-        PropertyAccessInfo::DataField(MapList{map}, field_index, field_type);
+    *access_info = PropertyAccessInfo::DataField(MapList{map}, field_index, rep,
+                                                 field_type);
     return true;
   }
   return false;
@@ -470,17 +488,20 @@ bool AccessInfoFactory::LookupTransition(Handle<Map> map, Handle<Name> name,
     FieldIndex field_index = FieldIndex::ForPropertyIndex(
         *transition_map, index, field_representation.IsDouble());
     Type* field_type = Type::Tagged();
+    MachineRepresentation rep = MachineRepresentation::kTagged;

[Benedikt Meurer, 2016/08/30 17:14:49]

Nit: field_representation

[mvstanton, 2016/08/30 18:59:34]

Done, but I had to rename the existing field_representation to
details_representation (since we are translating here from the Representation
type).

     if (field_representation.IsSmi()) {
       field_type = type_cache_.kSmi;
+      rep = MachineRepresentation::kTaggedSigned;
     } else if (field_representation.IsDouble()) {
       field_type = type_cache_.kFloat64;
+      rep = MachineRepresentation::kFloat64;
     } else if (field_representation.IsHeapObject()) {
       // Extract the field type from the property details (make sure its
       // representation is TaggedPointer to reflect the heap object case).
-      field_type = Type::Intersect(
+      rep = MachineRepresentation::kTaggedPointer;
+      field_type =
           transition_map->instance_descriptors()->GetFieldType(number)->Convert(
-              zone()),
-          Type::TaggedPointer(), zone());
+              zone());
       if (field_type->Is(Type::None())) {
         // Store is not safe if the field type was cleared.
         return false;
@@ -490,11 +511,10 @@ bool AccessInfoFactory::LookupTransition(Handle<Map> map, Handle<Name> name,
                                     isolate());
         dependencies()->AssumeFieldType(field_owner_map);
       }
-      DCHECK(field_type->Is(Type::TaggedPointer()));
     }
     dependencies()->AssumeMapNotDeprecated(transition_map);
     *access_info = PropertyAccessInfo::DataField(
-        MapList{map}, field_index, field_type, holder, transition_map);
+        MapList{map}, field_index, rep, field_type, holder, transition_map);
     return true;
   }
   return false;