Introduce HObjectAccess, which is used by LoadNamedField and StoreNamedField to denote what parts

src/hydrogen-instructions.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 5203 matching lines @@
   virtual void PrintDataTo(StringStream* stream);
 
   DECLARE_CONCRETE_INSTRUCTION(StoreContextSlot)
 
  private:
   int slot_index_;
   Mode mode_;
 };
 
 
+// Represents an access to a portion of an object, such as the map pointer,
+// array elements pointer, etc, but not accesses to array elements themselves.
+class HObjectAccess: public ZoneObject {
+ public:
+  // internal use only; different parts of an object or array

[danno, 2013/05/06 15:53:06]

Can you please make the following enum protected/private?

[titzer, 2013/05/07 17:51:03]

Haha. Tried that. Can't do that because I can't make initialized static members
of the class that are not primitives. I wanted to make some global shared
HObjectAccess instances, so they have to be outside the class, which means they
need access to the enum. Love it.

[titzer, 2013/05/07 17:54:30]

Nevermind, moving it to a value object made this possible. Done.

+  enum Portion {
+    kMaps,             // map of an object
+    kArrayLengths,     // the length of an array
+    kElementsPointer,  // elements pointer
+    kBackingStore,     // some field in the backing store
+    kInobject          // some other in-object field
+  };
+
+  HObjectAccess(Portion portion, int offset,

[danno, 2013/05/06 15:53:06]

This should be private.

[titzer, 2013/05/07 17:51:03]

Wish I could do that, but I can't, for the same reason. Aggh.

[titzer, 2013/05/07 17:54:30]

Spoke too soon. Done.

+      Handle<String> name = Handle<String>::null())
+      : portion_(portion), offset_(offset), name_(name) { }
+
+  inline bool IsInobject() {

[danno, 2013/05/06 15:53:06]

Does IsInobject or offset have to be public?

[titzer, 2013/05/07 17:51:03]

They are used in HandlePolymorphicNamedField to check that the accesses are
equal in terms of inobject-ness and offset.

+    return portion_ != kBackingStore;
+  }
+
+  inline int offset() {
+    return offset_;
+  }
+
+  inline Handle<String> name() {
+    return name_;
+  }
+
+  static HObjectAccess* ForElementsPointer();

[danno, 2013/05/06 15:53:06]

OK, you've convinced me. There is precidence elsewhere to return objects by
value rather than object by pointer, and after talking to Jakob about zone
usage, I think allocating a new object for each field is probably a bad idea.
Sorry, can you got back to the non-pointer version. :-) 

+  static HObjectAccess* ForArrayLength();
+  static HObjectAccess* ForFixedArrayLength();
+  static HObjectAccess* ForMap();
+
+  // Create an access to a property in an object
+  static HObjectAccess* ForInobjectOffset(Zone *zone, int offset,
+      Handle<String> name = Handle<String>::null());
+
+  // Create a direct access for a given offset, not in an object
+  static HObjectAccess* ForOffset(Zone *zone, int offset,

[danno, 2013/05/06 15:53:06]

Roll ForInobjectOffset directly into this method. There should only be
ForOffset, and it should handle all cases. We should try to minimize the idea of
"inobject/outofobject" in the API, since it will be going away when we make all
out-of-properties two-step anyway.

+      Handle<String> name = Handle<String>::null());
+
+  // Create an access to a resolved field (in-object or backing store)
+  static HObjectAccess* ForField(Zone *zone, Handle<Map> map,
+      LookupResult *lookup, Handle<String> name = Handle<String>::null());
+
+ protected:
+  void SetGVNFlags(HValue *instr, bool is_store);
+
+ private:
+  Portion portion_;

[danno, 2013/05/06 15:53:06]

Please put these together into a bit field,
int portion_: 30;
int offset_ : 1;

[titzer, 2013/05/07 17:51:03]

Done.

+  int offset_;
+  Handle<String> name_;
+
+  friend class HLoadNamedField;
+  friend class HStoreNamedField;
+};
+
+
 class HLoadNamedField: public HTemplateInstruction<2> {
  public:
-  HLoadNamedField(HValue* object, bool is_in_object,
-                  Representation field_representation,
-                  int offset, HValue* typecheck = NULL)
-      : is_in_object_(is_in_object),
-        field_representation_(field_representation),
-        offset_(offset) {
+  HLoadNamedField(HValue* object, HObjectAccess *access,
+      HValue* typecheck = NULL)
+      : access_(access),
+        field_representation_(Representation::Tagged()) {
     ASSERT(object != NULL);
     SetOperandAt(0, object);
     SetOperandAt(1, typecheck != NULL ? typecheck : object);
 
-    if (FLAG_track_fields && field_representation.IsSmi()) {
-      set_type(HType::Smi());
-      set_representation(Representation::Tagged());
-    } else if (FLAG_track_double_fields && field_representation.IsDouble()) {
-      set_representation(field_representation);
-    } else {
-      set_representation(Representation::Tagged());
-    }
-    SetFlag(kUseGVN);
-    SetGVNFlag(kDependsOnMaps);
-    if (is_in_object) {
-      SetGVNFlag(kDependsOnInobjectFields);
-    } else {
-      SetGVNFlag(kDependsOnBackingStoreFields);
-    }
-  }
-
-  static HLoadNamedField* NewArrayLength(Zone* zone, HValue* object,
-                                         HValue* typecheck,
-                                         HType type = HType::Tagged()) {
-    Representation representation =
-        type.IsSmi() ? Representation::Smi() : Representation::Tagged();
-    HLoadNamedField* result = new(zone) HLoadNamedField(
-        object, true, representation, JSArray::kLengthOffset, typecheck);
-    result->set_type(type);
-    result->SetGVNFlag(kDependsOnArrayLengths);
-    result->ClearGVNFlag(kDependsOnInobjectFields);
-    return result;
+    set_representation(Representation::Tagged());
+    access->SetGVNFlags(this, false);
   }
 
   HValue* object() { return OperandAt(0); }
   HValue* typecheck() {
     ASSERT(HasTypeCheck());
     return OperandAt(1);
   }
 
   bool HasTypeCheck() const { return OperandAt(0) != OperandAt(1); }
-  bool is_in_object() const { return is_in_object_; }
+  bool is_in_object() const { return access_->IsInobject(); }
   Representation field_representation() const { return representation_; }
-  int offset() const { return offset_; }
+  int offset() const { return access_->offset(); }
+
+  void set_field_representation(Representation representation) {
+    field_representation_ = representation;
+  }
 
   virtual Representation RequiredInputRepresentation(int index) {
     return Representation::Tagged();
   }
   virtual void PrintDataTo(StringStream* stream);
 
   DECLARE_CONCRETE_INSTRUCTION(LoadNamedField)
 
  protected:
   virtual bool DataEquals(HValue* other) {
     HLoadNamedField* b = HLoadNamedField::cast(other);
-    return is_in_object_ == b->is_in_object_ && offset_ == b->offset_;
+    return is_in_object() == b->is_in_object() && offset() == b->offset();
   }
 
  private:
   virtual bool IsDeletable() const { return true; }
 
-  bool is_in_object_;
+  HObjectAccess* access_;
   Representation field_representation_;
-  int offset_;
 };
 
 
 class HLoadNamedFieldPolymorphic: public HTemplateInstruction<2> {
  public:
   HLoadNamedFieldPolymorphic(HValue* context,
                              HValue* object,
                              SmallMapList* types,
                              Handle<String> name,
                              Zone* zone);
@@ skipping 273 matching lines @@
 
   virtual HValue* Canonicalize();
 
   DECLARE_CONCRETE_INSTRUCTION(LoadKeyedGeneric)
 };
 
 
 class HStoreNamedField: public HTemplateInstruction<2> {
  public:
   HStoreNamedField(HValue* obj,
-                   Handle<String> name,
-                   HValue* val,
-                   bool in_object,
-                   Representation field_representation,
-                   int offset)
-      : name_(name),
-        is_in_object_(in_object),
-        field_representation_(field_representation),
-        offset_(offset),
+                   HObjectAccess* access,
+                   HValue* val)
+      : access_(access),
+        field_representation_(Representation::Tagged()),
         transition_unique_id_(),
         new_space_dominator_(NULL) {
     SetOperandAt(0, obj);
     SetOperandAt(1, val);
-    SetFlag(kTrackSideEffectDominators);
-    SetGVNFlag(kDependsOnNewSpacePromotion);
-    if (is_in_object_) {
-      SetGVNFlag(kChangesInobjectFields);
-    } else {
-      SetGVNFlag(kChangesBackingStoreFields);
-    }
+    access->SetGVNFlags(this, true);
   }
 
   DECLARE_CONCRETE_INSTRUCTION(StoreNamedField)
 
   virtual Representation RequiredInputRepresentation(int index) {
     if (FLAG_track_fields && index == 1 && field_representation_.IsSmi()) {
       return Representation::Integer32();
     }
     return Representation::Tagged();
   }
   virtual void SetSideEffectDominator(GVNFlag side_effect, HValue* dominator) {
     ASSERT(side_effect == kChangesNewSpacePromotion);
     new_space_dominator_ = dominator;
   }
   virtual void PrintDataTo(StringStream* stream);
 
   HValue* object() { return OperandAt(0); }
   HValue* value() { return OperandAt(1); }
 
-  Handle<String> name() const { return name_; }
-  bool is_in_object() const { return is_in_object_; }
-  int offset() const { return offset_; }
+  Handle<String> name() const { return access_->name(); }
+  bool is_in_object() const { return access_->IsInobject(); }
+  int offset() const { return access_->offset(); }
   Handle<Map> transition() const { return transition_; }
   UniqueValueId transition_unique_id() const { return transition_unique_id_; }
   void set_transition(Handle<Map> map) { transition_ = map; }
   HValue* new_space_dominator() const { return new_space_dominator_; }
 
   bool NeedsWriteBarrier() {
     return (!FLAG_track_fields || !field_representation_.IsSmi()) &&
         StoringValueNeedsWriteBarrier(value()) &&
         ReceiverObjectNeedsWriteBarrier(object(), new_space_dominator());
   }
 
   bool NeedsWriteBarrierForMap() {
     return ReceiverObjectNeedsWriteBarrier(object(), new_space_dominator());
   }
 
   virtual void FinalizeUniqueValueId() {
     transition_unique_id_ = UniqueValueId(transition_);
   }
 
+  void set_field_representation(Representation representation) {
+    field_representation_ = representation;
+  }
+
   Representation field_representation() const {
     return field_representation_;
   }
 
  private:
-  Handle<String> name_;
-  bool is_in_object_;
+  HObjectAccess* access_;
   Representation field_representation_;
-  int offset_;
   Handle<Map> transition_;
   UniqueValueId transition_unique_id_;
   HValue* new_space_dominator_;
 };
 
 
 class HStoreNamedGeneric: public HTemplateInstruction<3> {
  public:
   HStoreNamedGeneric(HValue* context,
                      HValue* object,
@@ skipping 888 matching lines @@
   virtual bool IsDeletable() const { return true; }
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_