Make keyed operations use the unchecked index but still depend on the checked one.

src/hydrogen-instructions.h

Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 47a9da3b45deb917e7b4a5c213b4db57a3625d1d..8d303485ba7071ce1c68764b812644578d37bb2f 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -347,6 +347,10 @@ class Representation {
   }
   const char* Mnemonic() const;
 
+  Representation KeyedAccessIndexRequirement() {
+    return IsInteger32() ? Integer32() : Tagged();

[Sven Panne, 2012/12/06 14:59:02]

I think we should add an assertion here about the non-Integer32 case (it has to
be Tagged, i guess). I think we rely on the fact that things get monotonically
more specialized.

[Massi, 2012/12/07 10:11:11]

Well, this representation is meant to be used as return value by
RequiredInputRepresentation for the index in keyed loads and stores.
The required representation was int32 and this change is relaxing it to be int32
or tagged.
Generally RequiredInputRepresentation implementations do not assert on anything
about the input, they just state what they require.

[Sven Panne, 2012/12/07 10:24:58]

Well, in general RequiredInputRepresentation doesn't require this cruel hack.
;-) What actually is tried to be modeled is Integer32 \union Tagged (Smi, to be
even more exact), so I'd like to make this hack as explicit as possible. I don't
really care if it's done here or at all call sites, but we need an assertion.

+  }
+
  private:
   explicit Representation(Kind k) : kind_(k) { }
 
@@ -2826,6 +2830,65 @@ class HWrapReceiver: public HTemplateInstruction<2> {
 };
 
 
+enum BoundsCheckKeyMode {
+  DONT_ALLOW_SMI_KEY,
+  ALLOW_SMI_KEY
+};
+
+
+class HBoundsCheck: public HTemplateInstruction<2> {
+ public:
+  HBoundsCheck(HValue* index, HValue* length,
+               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY)
+      : key_mode_(key_mode) {
+    SetOperandAt(0, index);
+    SetOperandAt(1, length);
+    set_representation(Representation::Integer32());
+    SetFlag(kUseGVN);
+  }
+
+  virtual Representation RequiredInputRepresentation(int arg_index) {
+    if (key_mode_ == DONT_ALLOW_SMI_KEY ||
+        !length()->representation().IsTagged()) {
+      return Representation::Integer32();
+    }
+    // If the index is tagged and isn't constant, then allow the length
+    // to be tagged, since it is usually already tagged from loading it out of
+    // the length field of a JSArray. This allows for direct comparison without
+    // untagging.
+    if (index()->representation().IsTagged() && !index()->IsConstant()) {
+      return Representation::Tagged();
+    }
+    // Also allow the length to be tagged if the index is constant, because
+    // it can be tagged to allow direct comparison.
+    if (index()->IsConstant() &&
+        index()->representation().IsInteger32() &&
+        arg_index == 1) {
+      return Representation::Tagged();
+    }
+    return Representation::Integer32();
+  }
+  virtual Representation observed_input_representation(int index) {
+    return Representation::Integer32();
+  }
+
+  virtual void PrintDataTo(StringStream* stream);
+
+  HValue* index() { return OperandAt(0); }
+  HValue* length() { return OperandAt(1); }
+
+  DECLARE_CONCRETE_INSTRUCTION(BoundsCheck)
+
+  static HValue* ExtractUncheckedIndex(HValue* index) {
+    return index->IsBoundsCheck() ? HBoundsCheck::cast(index)->index() : index;
+  }
+
+ protected:
+  virtual bool DataEquals(HValue* other) { return true; }
+  BoundsCheckKeyMode key_mode_;
+};
+
+
 class HApplyArguments: public HTemplateInstruction<4> {
  public:
   HApplyArguments(HValue* function,
@@ -2904,14 +2967,17 @@ class HArgumentsLength: public HUnaryOperation {
 };
 
 
-class HAccessArgumentsAt: public HTemplateInstruction<3> {
+class HAccessArgumentsAt: public HTemplateInstruction<4> {
  public:
-  HAccessArgumentsAt(HValue* arguments, HValue* length, HValue* index) {
+  HAccessArgumentsAt(HValue* arguments,
+                     HValue* length,
+                     HValue* checked_index) {
     set_representation(Representation::Tagged());
     SetFlag(kUseGVN);
     SetOperandAt(0, arguments);
     SetOperandAt(1, length);
-    SetOperandAt(2, index);
+    SetOperandAt(2, HBoundsCheck::ExtractUncheckedIndex(checked_index));
+    SetOperandAt(3, checked_index);
   }
 
   virtual void PrintDataTo(StringStream* stream);
@@ -2919,13 +2985,15 @@ class HAccessArgumentsAt: public HTemplateInstruction<3> {
   virtual Representation RequiredInputRepresentation(int index) {
     // The arguments elements is considered tagged.
     return index == 0
-        ? Representation::Tagged()
-        : Representation::Integer32();
+        ? Representation::Tagged() :

[Sven Panne, 2012/12/06 14:59:02]

Formatting nit: Consistently indent the ternary operator like

   condition
       ? trueValue
       : falseValue

if it doesn't fit into a line. I know it's not done consistently in our code,
but we shouldn't make it worse. :-) Alternatively, one might consider using a
switch, which is even more readable IMHO.

[Massi, 2012/12/07 10:11:11]

Done.

+            (index == 3 ? Representation::None() :

[Sven Panne, 2012/12/06 14:59:02]

Hmmm, I would have expected

 checked_index()->representation().KeyedAccessIndexRequirement()

instead of Representation::None(). If this is really intended, please add a
comment why this is different, it is not obvious when reading the CL.

[Massi, 2012/12/07 10:11:11]

Done.

+                Representation::Integer32());
   }
 
   HValue* arguments() { return OperandAt(0); }
   HValue* length() { return OperandAt(1); }
   HValue* index() { return OperandAt(2); }
+  HValue* checked_index() { return OperandAt(3); }
 
   DECLARE_CONCRETE_INSTRUCTION(AccessArgumentsAt)
 
@@ -2933,61 +3001,6 @@ class HAccessArgumentsAt: public HTemplateInstruction<3> {
 };
 
 
-enum BoundsCheckKeyMode {
-  DONT_ALLOW_SMI_KEY,
-  ALLOW_SMI_KEY
-};
-
-
-class HBoundsCheck: public HTemplateInstruction<2> {
- public:
-  HBoundsCheck(HValue* index, HValue* length,
-               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY)
-      : key_mode_(key_mode) {
-    SetOperandAt(0, index);
-    SetOperandAt(1, length);
-    set_representation(Representation::Integer32());
-    SetFlag(kUseGVN);
-  }
-
-  virtual Representation RequiredInputRepresentation(int arg_index) {
-    if (key_mode_ == DONT_ALLOW_SMI_KEY ||
-        !length()->representation().IsTagged()) {
-      return Representation::Integer32();
-    }
-    // If the index is tagged and isn't constant, then allow the length
-    // to be tagged, since it is usually already tagged from loading it out of
-    // the length field of a JSArray. This allows for direct comparison without
-    // untagging.
-    if (index()->representation().IsTagged() && !index()->IsConstant()) {
-      return Representation::Tagged();
-    }
-    // Also allow the length to be tagged if the index is constant, because
-    // it can be tagged to allow direct comparison.
-    if (index()->IsConstant() &&
-        index()->representation().IsInteger32() &&
-        arg_index == 1) {
-      return Representation::Tagged();
-    }
-    return Representation::Integer32();
-  }
-  virtual Representation observed_input_representation(int index) {
-    return Representation::Integer32();
-  }
-
-  virtual void PrintDataTo(StringStream* stream);
-
-  HValue* index() { return OperandAt(0); }
-  HValue* length() { return OperandAt(1); }
-
-  DECLARE_CONCRETE_INSTRUCTION(BoundsCheck)
-
- protected:
-  virtual bool DataEquals(HValue* other) { return true; }
-  BoundsCheckKeyMode key_mode_;
-};
-
-
 class HBitwiseBinaryOperation: public HBinaryOperation {
  public:
   HBitwiseBinaryOperation(HValue* context, HValue* left, HValue* right)
@@ -4342,18 +4355,18 @@ class ArrayInstructionInterface {
 
 
 class HLoadKeyed
-    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
+    : public HTemplateInstruction<4>, public ArrayInstructionInterface {
  public:
   HLoadKeyed(HValue* obj,
-             HValue* key,
+             HValue* checked_key,
              HValue* dependency,
              ElementsKind elements_kind)
       : bit_field_(0) {
     bit_field_ = ElementsKindField::encode(elements_kind);
-
     SetOperandAt(0, obj);
-    SetOperandAt(1, key);
+    SetOperandAt(1, HBoundsCheck::ExtractUncheckedIndex(checked_key));
     SetOperandAt(2, dependency);
+    SetOperandAt(3, checked_key);
 
     if (!is_external()) {
       // I can detect the case between storing double (holey and fast) and
@@ -4395,6 +4408,7 @@ class HLoadKeyed
   HValue* elements() { return OperandAt(0); }
   HValue* key() { return OperandAt(1); }
   HValue* dependency() { return OperandAt(2); }
+  HValue* checked_key() { return OperandAt(3); }
   uint32_t index_offset() { return IndexOffsetField::decode(bit_field_); }
   void SetIndexOffset(uint32_t index_offset) {
     bit_field_ = IndexOffsetField::update(bit_field_, index_offset);
@@ -4417,7 +4431,9 @@ class HLoadKeyed
       return is_external() ? Representation::External()
           : Representation::Tagged();
     }
-    if (index == 1) return Representation::Integer32();
+    if (index == 1) {
+      return OperandAt(1)->representation().KeyedAccessIndexRequirement();
+    }
     return Representation::None();
   }
 
@@ -4599,14 +4615,15 @@ class HStoreNamedGeneric: public HTemplateInstruction<3> {
 
 
 class HStoreKeyed
-    : public HTemplateInstruction<3>, public ArrayInstructionInterface {
+    : public HTemplateInstruction<4>, public ArrayInstructionInterface {
  public:
-  HStoreKeyed(HValue* obj, HValue* key, HValue* val,
+  HStoreKeyed(HValue* obj, HValue* checked_key, HValue* val,
               ElementsKind elements_kind)
       : elements_kind_(elements_kind), index_offset_(0), is_dehoisted_(false) {
     SetOperandAt(0, obj);
-    SetOperandAt(1, key);
+    SetOperandAt(1, HBoundsCheck::ExtractUncheckedIndex(checked_key));
     SetOperandAt(2, val);
+    SetOperandAt(3, checked_key);
 
     if (is_external()) {
       SetGVNFlag(kChangesSpecializedArrayElements);
@@ -4632,7 +4649,9 @@ class HStoreKeyed
       return is_external() ? Representation::External()
                            : Representation::Tagged();
     } else if (index == 1) {
-      return Representation::Integer32();
+      return OperandAt(1)->representation().KeyedAccessIndexRequirement();
+    } else if (index == 3) {
+      return Representation::None();
     }
 
     ASSERT_EQ(index, 2);
@@ -4663,6 +4682,7 @@ class HStoreKeyed
   HValue* elements() { return OperandAt(0); }
   HValue* key() { return OperandAt(1); }
   HValue* value() { return OperandAt(2); }
+  HValue* checked_key() { return OperandAt(3); }
   bool value_is_smi() const {
     return IsFastSmiElementsKind(elements_kind_);
   }
@@ -4805,12 +4825,13 @@ class HStringAdd: public HBinaryOperation {
 };
 
 
-class HStringCharCodeAt: public HTemplateInstruction<3> {
+class HStringCharCodeAt: public HTemplateInstruction<4> {
  public:
-  HStringCharCodeAt(HValue* context, HValue* string, HValue* index) {
+  HStringCharCodeAt(HValue* context, HValue* string, HValue* checked_index) {
     SetOperandAt(0, context);
     SetOperandAt(1, string);
-    SetOperandAt(2, index);
+    SetOperandAt(2, HBoundsCheck::ExtractUncheckedIndex(checked_index));
+    SetOperandAt(3, checked_index);
     set_representation(Representation::Integer32());
     SetFlag(kUseGVN);
     SetGVNFlag(kDependsOnMaps);
@@ -4819,14 +4840,15 @@ class HStringCharCodeAt: public HTemplateInstruction<3> {
 
   virtual Representation RequiredInputRepresentation(int index) {
     // The index is supposed to be Integer32.
-    return index == 2
-        ? Representation::Integer32()
-        : Representation::Tagged();
+    return index == 2 ?
+        Representation::Integer32() :
+        (index == 3 ? Representation::None() : Representation::Tagged());

[Sven Panne, 2012/12/06 14:59:02]

See comments regarding indentation/adding a comment above.

[Massi, 2012/12/07 10:11:11]

Done.

   }
 
   HValue* context() { return OperandAt(0); }
   HValue* string() { return OperandAt(1); }
   HValue* index() { return OperandAt(2); }
+  HValue* checked_index() { return OperandAt(3); }
 
   DECLARE_CONCRETE_INSTRUCTION(StringCharCodeAt)