Consolidated all the key store/load classes in the Hydrogen and Lithium

src/hydrogen-instructions.h

Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 9e6344cd5f5e08035f65ae5e702558629090238b..6492ab188d58990f091d189f7e498640747aa6ea 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -133,10 +133,8 @@ class LChunkBuilder;
   V(LoadFunctionPrototype)                     \
   V(LoadGlobalCell)                            \
   V(LoadGlobalGeneric)                         \
-  V(LoadKeyedFastDoubleElement)                \
-  V(LoadKeyedFastElement)                      \
+  V(LoadKeyed)                                 \
   V(LoadKeyedGeneric)                          \
-  V(LoadKeyedSpecializedArrayElement)          \
   V(LoadNamedField)                            \
   V(LoadNamedFieldPolymorphic)                 \
   V(LoadNamedGeneric)                          \
@@ -163,10 +161,8 @@ class LChunkBuilder;
   V(StoreContextSlot)                          \
   V(StoreGlobalCell)                           \
   V(StoreGlobalGeneric)                        \
-  V(StoreKeyedFastDoubleElement)               \
-  V(StoreKeyedFastElement)                     \
+  V(StoreKeyed)                                \
   V(StoreKeyedGeneric)                         \
-  V(StoreKeyedSpecializedArrayElement)         \
   V(StoreNamedField)                           \
   V(StoreNamedGeneric)                         \
   V(StringAdd)                                 \
@@ -4243,28 +4239,57 @@ class ArrayInstructionInterface {
   virtual ~ArrayInstructionInterface() { };
 };
 
-class HLoadKeyedFastElement
+
+class HLoadKeyed
     : public HTemplateInstruction<3>, public ArrayInstructionInterface {
  public:
-  HLoadKeyedFastElement(HValue* obj,
-                        HValue* key,
-                        HValue* dependency,
-                        ElementsKind elements_kind = FAST_ELEMENTS)
+  HLoadKeyed(HValue* obj,
+             HValue* key,
+             HValue* dependency,
+             ElementsKind elements_kind)
       : bit_field_(0) {
-    ASSERT(IsFastSmiOrObjectElementsKind(elements_kind));
     bit_field_ = ElementsKindField::encode(elements_kind);
-    if (IsFastSmiElementsKind(elements_kind) &&
-        IsFastPackedElementsKind(elements_kind)) {
-      set_type(HType::Smi());
-    }
+
     SetOperandAt(0, obj);
     SetOperandAt(1, key);
     SetOperandAt(2, dependency);
-    set_representation(Representation::Tagged());
-    SetGVNFlag(kDependsOnArrayElements);
+
+    if (!is_external()) {
+      // I can detect the case between storing double (holey and fast) and
+      // smi/object by looking at elements_kind_.
+      ASSERT(IsFastSmiOrObjectElementsKind(elements_kind) ||
+             IsFastDoubleElementsKind(elements_kind));
+
+      if (IsFastSmiOrObjectElementsKind(elements_kind)) {
+        if (IsFastSmiElementsKind(elements_kind) &&
+            IsFastPackedElementsKind(elements_kind)) {
+          set_type(HType::Smi());
+        }
+
+        set_representation(Representation::Tagged());
+        SetGVNFlag(kDependsOnArrayElements);
+      } else {
+        set_representation(Representation::Double());
+        SetGVNFlag(kDependsOnDoubleArrayElements);
+      }
+    } else {
+      if (elements_kind == EXTERNAL_FLOAT_ELEMENTS ||
+          elements_kind == EXTERNAL_DOUBLE_ELEMENTS) {
+        set_representation(Representation::Double());
+      } else {
+        set_representation(Representation::Integer32());
+      }
+
+      SetGVNFlag(kDependsOnSpecializedArrayElements);
+      // Native code could change the specialized array.
+      SetGVNFlag(kDependsOnCalls);

[danno, 2012/10/24 11:42:43]

Technically, I think the call to SetGVNFlag(kDependsOnCalls) isn't necessary,
but that's a deeper discussion outside the scope of this CL. Ask it about me
when you get back, it might be a second cleanup CL.

[mvstanton, 2012/10/24 18:24:21]

okay, will do.

+    }
+
     SetFlag(kUseGVN);
   }
 
+  bool is_external() const { return
+        IsExternalArrayElementsKind(elements_kind()); }
   HValue* object() { return OperandAt(0); }
   HValue* key() { return OperandAt(1); }
   HValue* dependency() { return OperandAt(2); }
@@ -4274,6 +4299,7 @@ class HLoadKeyedFastElement
   }
   HValue* GetKey() { return key(); }
   void SetKey(HValue* key) { SetOperandAt(1, key); }
+

[danno, 2012/10/24 11:42:43]

nit: remove whitespace change

   bool IsDehoisted() { return IsDehoistedField::decode(bit_field_); }
   void SetDehoisted(bool is_dehoisted) {
     bit_field_ = IsDehoistedField::update(bit_field_, is_dehoisted);
@@ -4283,8 +4309,13 @@ class HLoadKeyedFastElement
   }
 
   virtual Representation RequiredInputRepresentation(int index) {
-    // The key is supposed to be Integer32.
-    if (index == 0) return Representation::Tagged();
+    // kind_fast:       tagged[int32] (none)
+    // kind_double:     tagged[int32] (none)
+    // kind_external: external[int32] (none)
+    if (index == 0) {
+      return is_external() ? Representation::External()
+          : Representation::Tagged();
+    }
     if (index == 1) return Representation::Integer32();
     return Representation::None();
   }
@@ -4293,157 +4324,36 @@ class HLoadKeyedFastElement
 
   bool RequiresHoleCheck() const;
 
-  DECLARE_CONCRETE_INSTRUCTION(LoadKeyedFastElement)
+  virtual Range* InferRange(Zone* zone);
+
+  DECLARE_CONCRETE_INSTRUCTION(LoadKeyed)
 
  protected:
   virtual bool DataEquals(HValue* other) {
-    if (!other->IsLoadKeyedFastElement()) return false;
-    HLoadKeyedFastElement* other_load = HLoadKeyedFastElement::cast(other);
+    if (!other->IsLoadKeyed()) return false;
+    HLoadKeyed* other_load = HLoadKeyed::cast(other);
+
     if (IsDehoisted() && index_offset() != other_load->index_offset())
       return false;
     return elements_kind() == other_load->elements_kind();
   }
 
  private:
-  virtual bool IsDeletable() const { return !RequiresHoleCheck(); }
+  virtual bool IsDeletable() const {
+    return !RequiresHoleCheck();
+  }
 
-  class ElementsKindField:  public BitField<ElementsKind, 0, 4> {};
-  class IndexOffsetField:   public BitField<uint32_t, 4, 27> {};
+  // TODO(mvstanton) - to deal with some of the external element
+  // kinds (PIXEL_ARRAY) I gave the ElementsKindField another bit,
+  // taking it from the index offset.
+  // Is this okay?

[danno, 2012/10/24 11:42:43]

That's fine. But crate a constant for the number of bits you use and add a
static assert here that makes sure that it is enough to hold ElementsKind, e.g.
STATIC_ASSERT(kElementsKindCount <= (1 << kBitsForElementsKind)) 

[mvstanton, 2012/10/24 18:24:21]

Done.

+  class ElementsKindField:  public BitField<ElementsKind, 0, 5> {};
+  class IndexOffsetField:   public BitField<uint32_t, 5, 26> {};
   class IsDehoistedField:   public BitField<bool, 31, 1> {};
   uint32_t bit_field_;
 };
 
 
-enum HoleCheckMode { PERFORM_HOLE_CHECK, OMIT_HOLE_CHECK };
-
-
-class HLoadKeyedFastDoubleElement
-    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
- public:
-  HLoadKeyedFastDoubleElement(
-    HValue* elements,
-    HValue* key,
-    HValue* dependency,
-    HoleCheckMode hole_check_mode = PERFORM_HOLE_CHECK)
-      : index_offset_(0),
-        is_dehoisted_(false),
-        hole_check_mode_(hole_check_mode) {
-    SetOperandAt(0, elements);
-    SetOperandAt(1, key);
-    SetOperandAt(2, dependency);
-    set_representation(Representation::Double());
-    SetGVNFlag(kDependsOnDoubleArrayElements);
-    SetFlag(kUseGVN);
-  }
-
-  HValue* elements() { return OperandAt(0); }
-  HValue* key() { return OperandAt(1); }
-  HValue* dependency() { return OperandAt(2); }
-  uint32_t index_offset() { return index_offset_; }
-  void SetIndexOffset(uint32_t index_offset) { index_offset_ = index_offset; }
-  HValue* GetKey() { return key(); }
-  void SetKey(HValue* key) { SetOperandAt(1, key); }
-  bool IsDehoisted() { return is_dehoisted_; }
-  void SetDehoisted(bool is_dehoisted) { is_dehoisted_ = is_dehoisted; }
-
-  virtual Representation RequiredInputRepresentation(int index) {
-    // The key is supposed to be Integer32.
-    if (index == 0) return Representation::Tagged();
-    if (index == 1) return Representation::Integer32();
-    return Representation::None();
-  }
-
-  bool RequiresHoleCheck() const {
-    return hole_check_mode_ == PERFORM_HOLE_CHECK;
-  }
-
-  virtual void PrintDataTo(StringStream* stream);
-
-  DECLARE_CONCRETE_INSTRUCTION(LoadKeyedFastDoubleElement)
-
- protected:
-  virtual bool DataEquals(HValue* other) {
-    if (!other->IsLoadKeyedFastDoubleElement()) return false;
-    HLoadKeyedFastDoubleElement* other_load =
-        HLoadKeyedFastDoubleElement::cast(other);
-    return hole_check_mode_ == other_load->hole_check_mode_;
-  }
-
- private:
-  virtual bool IsDeletable() const { return !RequiresHoleCheck(); }
-
-  uint32_t index_offset_;
-  bool is_dehoisted_;
-  HoleCheckMode hole_check_mode_;
-};
-
-
-class HLoadKeyedSpecializedArrayElement
-    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
- public:
-  HLoadKeyedSpecializedArrayElement(HValue* external_elements,
-                                    HValue* key,
-                                    HValue* dependency,
-                                    ElementsKind elements_kind)
-      :  elements_kind_(elements_kind),
-         index_offset_(0),
-         is_dehoisted_(false) {
-    SetOperandAt(0, external_elements);
-    SetOperandAt(1, key);
-    SetOperandAt(2, dependency);
-    if (elements_kind == EXTERNAL_FLOAT_ELEMENTS ||
-        elements_kind == EXTERNAL_DOUBLE_ELEMENTS) {
-      set_representation(Representation::Double());
-    } else {
-      set_representation(Representation::Integer32());
-    }
-    SetGVNFlag(kDependsOnSpecializedArrayElements);
-    // Native code could change the specialized array.
-    SetGVNFlag(kDependsOnCalls);
-    SetFlag(kUseGVN);
-  }
-
-  virtual void PrintDataTo(StringStream* stream);
-
-  virtual Representation RequiredInputRepresentation(int index) {
-    // The key is supposed to be Integer32.
-    if (index == 0) return Representation::External();
-    if (index == 1) return Representation::Integer32();
-    return Representation::None();
-  }
-
-  HValue* external_pointer() { return OperandAt(0); }
-  HValue* key() { return OperandAt(1); }
-  HValue* dependency() { return OperandAt(2); }
-  ElementsKind elements_kind() const { return elements_kind_; }
-  uint32_t index_offset() { return index_offset_; }
-  void SetIndexOffset(uint32_t index_offset) { index_offset_ = index_offset; }
-  HValue* GetKey() { return key(); }
-  void SetKey(HValue* key) { SetOperandAt(1, key); }
-  bool IsDehoisted() { return is_dehoisted_; }
-  void SetDehoisted(bool is_dehoisted) { is_dehoisted_ = is_dehoisted; }
-
-  virtual Range* InferRange(Zone* zone);
-
-  DECLARE_CONCRETE_INSTRUCTION(LoadKeyedSpecializedArrayElement)
-
- protected:
-  virtual bool DataEquals(HValue* other) {
-    if (!other->IsLoadKeyedSpecializedArrayElement()) return false;
-    HLoadKeyedSpecializedArrayElement* cast_other =
-        HLoadKeyedSpecializedArrayElement::cast(other);
-    return elements_kind_ == cast_other->elements_kind();
-  }
-
- private:
-  virtual bool IsDeletable() const { return true; }
-
-  ElementsKind elements_kind_;
-  uint32_t index_offset_;
-  bool is_dehoisted_;
-};
-
-
 class HLoadKeyedGeneric: public HTemplateInstruction<3> {
  public:
   HLoadKeyedGeneric(HValue* context, HValue* obj, HValue* key) {
@@ -4461,6 +4371,7 @@ class HLoadKeyedGeneric: public HTemplateInstruction<3> {
   virtual void PrintDataTo(StringStream* stream);
 
   virtual Representation RequiredInputRepresentation(int index) {
+    // tagged[tagged]
     return Representation::Tagged();
   }
 
@@ -4566,31 +4477,55 @@ class HStoreNamedGeneric: public HTemplateInstruction<3> {
 };
 
 
-class HStoreKeyedFastElement
+class HStoreKeyed
     : public HTemplateInstruction<3>, public ArrayInstructionInterface {
  public:
-  HStoreKeyedFastElement(HValue* obj, HValue* key, HValue* val,
-                         ElementsKind elements_kind = FAST_ELEMENTS)
+  HStoreKeyed(HValue* obj, HValue* key, HValue* val,
+              ElementsKind elements_kind)
       : elements_kind_(elements_kind), index_offset_(0), is_dehoisted_(false) {
     SetOperandAt(0, obj);
     SetOperandAt(1, key);
     SetOperandAt(2, val);
-    SetGVNFlag(kChangesArrayElements);
+
+    if (is_external()) {
+      SetGVNFlag(kChangesSpecializedArrayElements);
+    } else if (IsFastDoubleElementsKind(elements_kind)) {
+      SetGVNFlag(kChangesDoubleArrayElements);
+      SetFlag(kDeoptimizeOnUndefined);
+    } else {
+      SetGVNFlag(kChangesArrayElements);
+    }
   }
 
   virtual Representation RequiredInputRepresentation(int index) {
-    // The key is supposed to be Integer32.
-    return index == 1
-        ? Representation::Integer32()
-        : Representation::Tagged();
+    // kind_fast:       tagged[int32] = tagged
+    // kind_double:     tagged[int32] = double
+    // kind_external: external[int32] = (double | int32)
+    if (index == 0) {
+      return is_external() ? Representation::External()
+                           : Representation::Tagged();
+    } else if (index == 1) {
+      return Representation::Integer32();
+    }
+
+    ASSERT_EQ(index, 2);
+    if (IsDoubleOrFloatElementsKind(elements_kind())) {
+      return Representation::Double();
+    }
+
+    return is_external() ? Representation::Integer32()
+                         : Representation::Tagged();
   }
 
+  bool is_external() const { return
+        IsExternalArrayElementsKind(elements_kind()); }
   HValue* object() { return OperandAt(0); }
   HValue* key() { return OperandAt(1); }
   HValue* value() { return OperandAt(2); }
-  bool value_is_smi() {
+  bool value_is_smi() const {
     return IsFastSmiElementsKind(elements_kind_);
   }
+  ElementsKind elements_kind() const { return elements_kind_; }
   uint32_t index_offset() { return index_offset_; }
   void SetIndexOffset(uint32_t index_offset) { index_offset_ = index_offset; }
   HValue* GetKey() { return key(); }
@@ -4606,110 +4541,11 @@ class HStoreKeyedFastElement
     }
   }
 
-  virtual void PrintDataTo(StringStream* stream);
-
-  DECLARE_CONCRETE_INSTRUCTION(StoreKeyedFastElement)
-
- private:
-  ElementsKind elements_kind_;
-  uint32_t index_offset_;
-  bool is_dehoisted_;
-};
-
-
-class HStoreKeyedFastDoubleElement
-    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
- public:
-  HStoreKeyedFastDoubleElement(HValue* elements,
-                               HValue* key,
-                               HValue* val)
-      : index_offset_(0), is_dehoisted_(false) {
-    SetOperandAt(0, elements);
-    SetOperandAt(1, key);
-    SetOperandAt(2, val);
-    SetFlag(kDeoptimizeOnUndefined);
-    SetGVNFlag(kChangesDoubleArrayElements);
-  }
-
-  virtual Representation RequiredInputRepresentation(int index) {
-    if (index == 1) {
-      return Representation::Integer32();
-    } else if (index == 2) {
-      return Representation::Double();
-    } else {
-      return Representation::Tagged();
-    }
-  }
-
-  HValue* elements() { return OperandAt(0); }
-  HValue* key() { return OperandAt(1); }
-  HValue* value() { return OperandAt(2); }
-  uint32_t index_offset() { return index_offset_; }
-  void SetIndexOffset(uint32_t index_offset) { index_offset_ = index_offset; }
-  HValue* GetKey() { return key(); }
-  void SetKey(HValue* key) { SetOperandAt(1, key); }
-  bool IsDehoisted() { return is_dehoisted_; }
-  void SetDehoisted(bool is_dehoisted) { is_dehoisted_ = is_dehoisted; }
-
-  bool NeedsWriteBarrier() {
-    return StoringValueNeedsWriteBarrier(value());
-  }
-
   bool NeedsCanonicalization();
 
   virtual void PrintDataTo(StringStream* stream);
 
-  DECLARE_CONCRETE_INSTRUCTION(StoreKeyedFastDoubleElement)
-
- private:
-  uint32_t index_offset_;
-  bool is_dehoisted_;
-};
-
-
-class HStoreKeyedSpecializedArrayElement
-    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
- public:
-  HStoreKeyedSpecializedArrayElement(HValue* external_elements,
-                                     HValue* key,
-                                     HValue* val,
-                                     ElementsKind elements_kind)
-      : elements_kind_(elements_kind), index_offset_(0), is_dehoisted_(false) {
-    SetGVNFlag(kChangesSpecializedArrayElements);
-    SetOperandAt(0, external_elements);
-    SetOperandAt(1, key);
-    SetOperandAt(2, val);
-  }
-
-  virtual void PrintDataTo(StringStream* stream);
-
-  virtual Representation RequiredInputRepresentation(int index) {
-    if (index == 0) {
-      return Representation::External();
-    } else {
-      bool float_or_double_elements =
-          elements_kind() == EXTERNAL_FLOAT_ELEMENTS ||
-          elements_kind() == EXTERNAL_DOUBLE_ELEMENTS;
-      if (index == 2 && float_or_double_elements) {
-        return Representation::Double();
-      } else {
-        return Representation::Integer32();
-      }
-    }
-  }
-
-  HValue* external_pointer() { return OperandAt(0); }
-  HValue* key() { return OperandAt(1); }
-  HValue* value() { return OperandAt(2); }
-  ElementsKind elements_kind() const { return elements_kind_; }
-  uint32_t index_offset() { return index_offset_; }
-  void SetIndexOffset(uint32_t index_offset) { index_offset_ = index_offset; }
-  HValue* GetKey() { return key(); }
-  void SetKey(HValue* key) { SetOperandAt(1, key); }
-  bool IsDehoisted() { return is_dehoisted_; }
-  void SetDehoisted(bool is_dehoisted) { is_dehoisted_ = is_dehoisted; }
-
-  DECLARE_CONCRETE_INSTRUCTION(StoreKeyedSpecializedArrayElement)
+  DECLARE_CONCRETE_INSTRUCTION(StoreKeyed)
 
  private:
   ElementsKind elements_kind_;
@@ -4740,6 +4576,7 @@ class HStoreKeyedGeneric: public HTemplateInstruction<4> {
   StrictModeFlag strict_mode_flag() { return strict_mode_flag_; }
 
   virtual Representation RequiredInputRepresentation(int index) {
+    // tagged[tagged] = tagged
     return Representation::Tagged();
   }