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

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 329 matching lines @@
   bool IsNone() const { return kind_ == kNone; }
   bool IsTagged() const { return kind_ == kTagged; }
   bool IsInteger32() const { return kind_ == kInteger32; }
   bool IsDouble() const { return kind_ == kDouble; }
   bool IsExternal() const { return kind_ == kExternal; }
   bool IsSpecialization() const {
     return kind_ == kInteger32 || kind_ == kDouble;
   }
   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) { }
 
   // Make sure kind fits in int8.
   STATIC_ASSERT(kNumRepresentations <= (1 << kBitsPerByte));
 
   int8_t kind_;
 };
 
 
@@ skipping 2459 matching lines @@
 
   HValue* receiver() { return OperandAt(0); }
   HValue* function() { return OperandAt(1); }
 
   virtual HValue* Canonicalize();
 
   DECLARE_CONCRETE_INSTRUCTION(WrapReceiver)
 };
 
 
+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,
                   HValue* receiver,
                   HValue* length,
                   HValue* elements) {
     set_representation(Representation::Tagged());
     SetOperandAt(0, function);
     SetOperandAt(1, receiver);
     SetOperandAt(2, length);
@@ skipping 58 matching lines @@
   DECLARE_CONCRETE_INSTRUCTION(ArgumentsLength)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
  private:
   virtual bool IsDeletable() const { return true; }
 };
 
 
-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);
 
   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)
 
   virtual bool DataEquals(HValue* other) { return true; }
 };
 
 
-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)
       : HBinaryOperation(context, left, right) {
     SetFlag(kFlexibleRepresentation);
     SetFlag(kTruncatingToInt32);
     SetAllSideEffects();
   }
 
   virtual Representation RequiredInputRepresentation(int index) {
@@ skipping 1334 matching lines @@
   virtual HValue* GetKey() = 0;
   virtual void SetKey(HValue* key) = 0;
   virtual void SetIndexOffset(uint32_t index_offset) = 0;
   virtual bool IsDehoisted() = 0;
   virtual void SetDehoisted(bool is_dehoisted) = 0;
   virtual ~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
       // 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)) {
@@ skipping 21 matching lines @@
 
     SetFlag(kUseGVN);
   }
 
   bool is_external() const {
     return IsExternalArrayElementsKind(elements_kind());
   }
   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);
   }
   HValue* GetKey() { return key(); }
   void SetKey(HValue* key) { SetOperandAt(1, key); }
   bool IsDehoisted() { return IsDehoistedField::decode(bit_field_); }
   void SetDehoisted(bool is_dehoisted) {
     bit_field_ = IsDehoistedField::update(bit_field_, is_dehoisted);
   }
   ElementsKind elements_kind() const {
     return ElementsKindField::decode(bit_field_);
   }
 
   virtual Representation RequiredInputRepresentation(int index) {
     // 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();
+    if (index == 1) {
+      return OperandAt(1)->representation().KeyedAccessIndexRequirement();
+    }
     return Representation::None();
   }
 
   virtual Representation observed_input_representation(int index) {
     return RequiredInputRepresentation(index);
   }
 
   virtual void PrintDataTo(StringStream* stream);
 
   bool RequiresHoleCheck() const;
@@ skipping 161 matching lines @@
 
   DECLARE_CONCRETE_INSTRUCTION(StoreNamedGeneric)
 
  private:
   Handle<String> name_;
   StrictModeFlag strict_mode_flag_;
 };
 
 
 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);
     } else if (IsFastDoubleElementsKind(elements_kind)) {
       SetGVNFlag(kChangesDoubleArrayElements);
       SetFlag(kDeoptimizeOnUndefined);
     } else {
       SetGVNFlag(kChangesArrayElements);
     }
 
     // EXTERNAL_{UNSIGNED_,}{BYTE,SHORT,INT}_ELEMENTS are truncating.
     if (elements_kind >= EXTERNAL_BYTE_ELEMENTS &&
         elements_kind <= EXTERNAL_UNSIGNED_INT_ELEMENTS) {
       SetFlag(kTruncatingToInt32);
     }
   }
 
   virtual Representation RequiredInputRepresentation(int index) {
     // 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();
+      return OperandAt(1)->representation().KeyedAccessIndexRequirement();
+    } else if (index == 3) {
+      return Representation::None();
     }
 
     ASSERT_EQ(index, 2);
     if (IsDoubleOrFloatElementsKind(elements_kind())) {
       return Representation::Double();
     }
 
     return is_external() ? Representation::Integer32()
                          : Representation::Tagged();
   }
@@ skipping 10 matching lines @@
     if (is_external()) {
       return Representation::Integer32();
     }
     // For fast object elements kinds, don't assume anything.
     return Representation::None();
   }
 
   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_);
   }
   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; }
@@ skipping 122 matching lines @@
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
   // TODO(svenpanne) Might be safe, but leave it out until we know for sure.
   // private:
   //  virtual bool IsDeletable() const { return true; }
 };
 
 
-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);
     SetGVNFlag(kChangesNewSpacePromotion);
   }
 
   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)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
   virtual Range* InferRange(Zone* zone) {
     return new(zone) Range(0, String::kMaxUtf16CodeUnit);
   }
 
@@ skipping 550 matching lines @@
   virtual bool IsDeletable() const { return true; }
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_