Move representation into HObjectAccess and remove from HLoadNamedField and HStoreNamedField.

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 5313 matching lines @@
 class HObjectAccess {
  public:
   inline bool IsInobject() const {
     return portion() != kBackingStore;
   }
 
   inline int offset() const {
     return OffsetField::decode(value_);
   }
 
+  inline Representation representation() const {
+    return Representation::FromKind(RepresentationField::decode(value_));
+  }
+
   inline Handle<String> name() const {
     return name_;
   }
 
+  inline HObjectAccess WithRepresentation(Representation representation) {

[danno, 2013/07/16 11:51:58]

Ouch. I really wish we didn't need this version. If I understand correctly, this
is currently only this way because of the implicit unboxing of fields in
HeapNumbers and for polymorphic access. Is there a way to make if more specific,
e.g. ForHeapNumberBox() without the extra representation parameter? And do we
really need it for polymorphic access? You can always use the access of the most
generic type.

[titzer, 2013/07/23 15:04:10]

Not really, because Representation::generalize() can return either one of the
inputs or Representation::Tagged().

+    return HObjectAccess(portion(), offset(), representation, name());
+  }
+
   static HObjectAccess ForHeapNumberValue() {
-    return HObjectAccess(kDouble, HeapNumber::kValueOffset);
+    return HObjectAccess(
+        kDouble, HeapNumber::kValueOffset, Representation::Double());
   }
 
   static HObjectAccess ForElementsPointer() {
     return HObjectAccess(kElementsPointer, JSObject::kElementsOffset);
   }
 
-  static HObjectAccess ForArrayLength() {
-    return HObjectAccess(kArrayLengths, JSArray::kLengthOffset);
+  static HObjectAccess ForArrayLength(bool is_fast_elements = false) {

[danno, 2013/07/16 11:51:58]

Can you pass in the ElementsKind here? The boolean parameter doesn't give much
context,

[titzer, 2013/07/23 15:04:10]

What would be the default elements kind? We have 3 kinds of uses: ones where we
don't know (assume tagged), ones where we know (assume smi), and ones where we
have an elements kind.

[danno, 2013/07/23 16:15:43]

I would prefer that you use ElementsKind and not have a default at all, the
information about which elements you have is always available.  See below.
On 2013/07/23 15:04:10, titzer wrote:

+    return HObjectAccess(
+        kArrayLengths, JSArray::kLengthOffset,
+        is_fast_elements && FLAG_track_fields ?
+        Representation::Smi() : Representation::Tagged());
   }
 
   static HObjectAccess ForAllocationSiteTransitionInfo() {
     return HObjectAccess(kInobject, AllocationSite::kTransitionInfoOffset);
   }
 
   static HObjectAccess ForFixedArrayLength() {
-    return HObjectAccess(kArrayLengths, FixedArray::kLengthOffset);
+    return HObjectAccess(
+        kArrayLengths, FixedArray::kLengthOffset,
+        FLAG_track_fields ?
+        Representation::Smi() : Representation::Tagged());
   }
 
   static HObjectAccess ForPropertiesPointer() {
     return HObjectAccess(kInobject, JSObject::kPropertiesOffset);
   }
 
   static HObjectAccess ForPrototypeOrInitialMap() {
     return HObjectAccess(kInobject, JSFunction::kPrototypeOrInitialMapOffset);
   }
 
@@ skipping 10 matching lines @@
   }
 
   static HObjectAccess ForAllocationSiteInfoSite() {
     return HObjectAccess(kInobject, AllocationSiteInfo::kAllocationSiteOffset);
   }
 
   // Create an access to an offset in a fixed array header.
   static HObjectAccess ForFixedArrayHeader(int offset);
 
   // Create an access to an in-object property in a JSObject.
-  static HObjectAccess ForJSObjectOffset(int offset);
+  static HObjectAccess ForJSObjectOffset(int offset,
+      Representation representation = Representation::Tagged());
 
   // Create an access to an in-object property in a JSArray.
   static HObjectAccess ForJSArrayOffset(int offset);
 
   // Create an access to the backing store of an object.
-  static HObjectAccess ForBackingStoreOffset(int offset);
+  static HObjectAccess ForBackingStoreOffset(int offset,
+      Representation representation = Representation::Tagged());
 
   // Create an access to a resolved field (in-object or backing store).
   static HObjectAccess ForField(Handle<Map> map,
       LookupResult *lookup, Handle<String> name = Handle<String>::null());
 
   // Create an access for the payload of a Cell or JSGlobalPropertyCell.
   static HObjectAccess ForCellPayload(Isolate* isolate);
 
   void PrintTo(StringStream* stream);
 
   inline bool Equals(HObjectAccess that) const {
     return value_ == that.value_;  // portion and offset must match
   }
 
  protected:
   void SetGVNFlags(HValue *instr, bool is_store);
 
  private:
   // internal use only; different parts of an object or array
   enum Portion {
     kMaps,             // map of an object
     kArrayLengths,     // the length of an array
     kElementsPointer,  // elements pointer
     kBackingStore,     // some field in the backing store
     kDouble,           // some double field
     kInobject          // some other in-object field
   };
 
   HObjectAccess(Portion portion, int offset,
-      Handle<String> name = Handle<String>::null())
-    : value_(PortionField::encode(portion) | OffsetField::encode(offset)),
+                Representation representation = Representation::Tagged(),
+                Handle<String> name = Handle<String>::null())
+    : value_(PortionField::encode(portion) |
+             RepresentationField::encode(representation.kind()) |
+             OffsetField::encode(offset)),
       name_(name) {
-    ASSERT(this->offset() == offset);    // offset should decode correctly
-    ASSERT(this->portion() == portion);  // portion should decode correctly
+    // assert that the fields decode correctly
+    ASSERT(this->offset() == offset);
+    ASSERT(this->portion() == portion);
+    ASSERT(RepresentationField::decode(value_) == representation.kind());
   }
 
   class PortionField : public BitField<Portion, 0, 3> {};
-  class OffsetField : public BitField<int, 3, 29> {};
+  class RepresentationField : public BitField<Representation::Kind, 3, 3> {};
+  class OffsetField : public BitField<int, 6, 26> {};
 
-  uint32_t value_;  // encodes both portion and offset
+  uint32_t value_;  // encodes portion, representation, and offset
   Handle<String> name_;
 
   friend class HLoadNamedField;
   friend class HStoreNamedField;
 
   inline Portion portion() const {
     return PortionField::decode(value_);
   }
 };
 
 
 class HLoadNamedField: public HTemplateInstruction<2> {
  public:
   HLoadNamedField(HValue* object,
                   HObjectAccess access,
-                  HValue* typecheck = NULL,
-                  Representation field_representation
-                      = Representation::Tagged())
-      : access_(access),
-        field_representation_(field_representation) {
+                  HValue* typecheck = NULL)
+      : access_(access) {
     ASSERT(object != NULL);
     SetOperandAt(0, object);
     SetOperandAt(1, typecheck != NULL ? typecheck : object);
 
-    if (FLAG_track_fields && field_representation.IsSmi()) {
+    Representation representation = access.representation();
+    if (representation.IsSmi()) {
       set_type(HType::Smi());
-      set_representation(field_representation);
-    } else if (FLAG_track_double_fields && field_representation.IsDouble()) {
-      set_representation(field_representation);
+      set_representation(representation);
+    } else if (representation.IsDouble()) {
+      set_representation(representation);
     } else if (FLAG_track_heap_object_fields &&
-               field_representation.IsHeapObject()) {
+               representation.IsHeapObject()) {
       set_type(HType::NonPrimitive());
       set_representation(Representation::Tagged());
     } else {
       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); }
   HObjectAccess access() const { return access_; }
-  Representation field_representation() const { return representation_; }
+  Representation field_representation() const {
+      return access_.representation();
+  }
 
   virtual bool HasEscapingOperandAt(int index) { return false; }
   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 access_.Equals(b->access_);
   }
 
  private:
   virtual bool IsDeletable() const { return true; }
 
   HObjectAccess access_;
-  Representation field_representation_;
 };
 
 
 class HLoadNamedFieldPolymorphic: public HTemplateInstruction<2> {
  public:
   HLoadNamedFieldPolymorphic(HValue* context,
                              HValue* object,
                              SmallMapList* types,
                              Handle<String> name,
                              Zone* zone);
@@ skipping 279 matching lines @@
   virtual HValue* Canonicalize();
 
   DECLARE_CONCRETE_INSTRUCTION(LoadKeyedGeneric)
 };
 
 
 class HStoreNamedField: public HTemplateInstruction<2> {
  public:
   HStoreNamedField(HValue* obj,
                    HObjectAccess access,
-                   HValue* val,
-                   Representation field_representation
-                       = Representation::Tagged())

[danno, 2013/07/16 11:51:58]

nit: start the first parameter "obj" on a new line, that prevents the funky
indentation.

[titzer, 2013/07/23 15:04:10]

Because I removed this parameter, the funky indentation is now gone.

+                   HValue* val)
       : access_(access),
-        field_representation_(field_representation),
         transition_(),
         transition_unique_id_(),
         new_space_dominator_(NULL),
         write_barrier_mode_(UPDATE_WRITE_BARRIER) {
     SetOperandAt(0, obj);
     SetOperandAt(1, val);
     access.SetGVNFlags(this, true);
   }
 
   DECLARE_CONCRETE_INSTRUCTION(StoreNamedField)
 
   virtual bool HasEscapingOperandAt(int index) { return index == 1; }
   virtual Representation RequiredInputRepresentation(int index) {
-    if (FLAG_track_double_fields &&
-        index == 1 && field_representation_.IsDouble()) {
-      return field_representation_;
-    } else if (FLAG_track_fields &&
-               index == 1 && field_representation_.IsSmi()) {
-      return field_representation_;
+    if (index == 1 && field_representation().IsDouble()) {
+      return field_representation();
+    } else if (index == 1 && field_representation().IsSmi()) {
+      return field_representation();
     }
     return Representation::Tagged();
   }
   virtual void SetSideEffectDominator(GVNFlag side_effect, HValue* dominator) {
     ASSERT(side_effect == kChangesNewSpacePromotion);
     new_space_dominator_ = dominator;
   }
   virtual void PrintDataTo(StringStream* stream);
 
   void SkipWriteBarrier() { write_barrier_mode_ = SKIP_WRITE_BARRIER; }
@@ skipping 10 matching lines @@
   void SetTransition(Handle<Map> map, CompilationInfo* info) {
     ASSERT(transition_.is_null());  // Only set once.
     if (map->CanBeDeprecated()) {
       map->AddDependentCompilationInfo(DependentCode::kTransitionGroup, info);
     }
     transition_ = map;
   }
   HValue* new_space_dominator() const { return new_space_dominator_; }
 
   bool NeedsWriteBarrier() {
-    ASSERT(!(FLAG_track_double_fields && field_representation_.IsDouble()) ||
+    ASSERT(!(FLAG_track_double_fields && field_representation().IsDouble()) ||
            transition_.is_null());
     if (IsSkipWriteBarrier()) return false;
-    return (!FLAG_track_fields || !field_representation_.IsSmi()) &&
-        // If there is a transition, a new storage object needs to be allocated.
-        !(FLAG_track_double_fields && field_representation_.IsDouble()) &&
-        StoringValueNeedsWriteBarrier(value()) &&
+    if (field_representation().IsDouble()) return false;
+    if (field_representation().IsSmi()) return false;
+    return StoringValueNeedsWriteBarrier(value()) &&
         ReceiverObjectNeedsWriteBarrier(object(), new_space_dominator());
   }
 
   bool NeedsWriteBarrierForMap() {
     if (IsSkipWriteBarrier()) return false;
     return ReceiverObjectNeedsWriteBarrier(object(), new_space_dominator());
   }
 
   virtual void FinalizeUniqueValueId() {
     transition_unique_id_ = UniqueValueId(transition_);
   }
 
   Representation field_representation() const {
-    return field_representation_;
+    return access_.representation();
   }
 
  private:
   HObjectAccess access_;
-  Representation field_representation_;
   Handle<Map> transition_;
   UniqueValueId transition_unique_id_;
   HValue* new_space_dominator_;
   WriteBarrierMode write_barrier_mode_;
 };
 
 
 class HStoreNamedGeneric: public HTemplateInstruction<3> {
  public:
   HStoreNamedGeneric(HValue* context,
@@ skipping 820 matching lines @@
   virtual bool IsDeletable() const { return true; }
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_