Reviewers: Vitaly,

Description:
Optimize array-length and fast element loads.

1. Separating out the instance-type check from the array-length operation.

2. I also changed the bounds-check on keyed loads to use the length property
for JS arrays (like we do for array stores).

The new pattern should use less registers and allow more checks to be
eliminated.


Please review this at http://codereview.chromium.org/5961016/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
  M     src/hydrogen.cc
  M     src/ia32/lithium-codegen-ia32.cc
  M     src/ia32/lithium-ia32.h
  M     src/ia32/lithium-ia32.cc


Index: src/hydrogen.cc
===================================================================
--- src/hydrogen.cc     (revision 6124)
+++ src/hydrogen.cc     (working copy)
@@ -3643,9 +3643,18 @@
   Handle<Map> map = expr->GetMonomorphicReceiverType();
   ASSERT(map->has_fast_elements());
   AddInstruction(new HCheckMap(object, map));
-  HInstruction* elements = AddInstruction(new HLoadElements(object));
-  HInstruction* length = AddInstruction(new HArrayLength(elements));
-  AddInstruction(new HBoundsCheck(key, length));
+  bool is_array = (map->instance_type() == JS_ARRAY_TYPE);
+  HLoadElements* elements = new HLoadElements(object);
+  HInstruction* length = NULL;
+  if (is_array) {
+    length = AddInstruction(new HArrayLength(object));
+    AddInstruction(new HBoundsCheck(key, length));
+    AddInstruction(elements);
+  } else {
+    AddInstruction(elements);
+    length = AddInstruction(new HArrayLength(elements));
+    AddInstruction(new HBoundsCheck(key, length));
+  }
   return new HLoadKeyedFastElement(elements, key);
 }

@@ -3720,6 +3729,7 @@
   if (expr->IsArrayLength()) {
     HValue* array = Pop();
     AddInstruction(new HCheckNonSmi(array));
+ AddInstruction(new HCheckInstanceType(array, JS_ARRAY_TYPE, JS_ARRAY_TYPE));
     instr = new HArrayLength(array);

   } else if (expr->IsFunctionPrototype()) {
@@ -4859,7 +4869,9 @@
     switch (op) {
       case Token::EQ:
       case Token::EQ_STRICT: {
+        AddInstruction(new HCheckNonSmi(left));
AddInstruction(HCheckInstanceType::NewIsJSObjectOrJSFunction(left));
+        AddInstruction(new HCheckNonSmi(right));
AddInstruction(HCheckInstanceType::NewIsJSObjectOrJSFunction(right));
         instr = new HCompareJSObjectEq(left, right);
         break;
Index: src/ia32/lithium-codegen-ia32.cc
===================================================================
--- src/ia32/lithium-codegen-ia32.cc    (revision 6124)
+++ src/ia32/lithium-codegen-ia32.cc    (working copy)
@@ -979,19 +979,13 @@

 void LCodeGen::DoArrayLength(LArrayLength* instr) {
   Register result = ToRegister(instr->result());
+  Register array = ToRegister(instr->input());

   if (instr->hydrogen()->value()->IsLoadElements()) {
-    // We load the length directly from the elements array.
-    Register elements = ToRegister(instr->input());
-    __ mov(result, FieldOperand(elements, FixedArray::kLengthOffset));
+    // We load the length directly from an elements array.
+    __ mov(result, FieldOperand(array, FixedArray::kLengthOffset));
   } else {
-    // Check that the receiver really is an array.
-    Register array = ToRegister(instr->input());
-    Register temporary = ToRegister(instr->temporary());
-    __ CmpObjectType(array, JS_ARRAY_TYPE, temporary);
-    DeoptimizeIf(not_equal, instr->environment());
-
-    // Load length directly from the array.
+    // Load length from a JSArray.
     __ mov(result, FieldOperand(array, JSArray::kLengthOffset));
   }
 }
@@ -2933,9 +2927,6 @@
   InstanceType first = instr->hydrogen()->first();
   InstanceType last = instr->hydrogen()->last();

-  __ test(input, Immediate(kSmiTagMask));
-  DeoptimizeIf(zero, instr->environment());
-
   __ mov(temp, FieldOperand(input, HeapObject::kMapOffset));
   __ cmpb(FieldOperand(temp, Map::kInstanceTypeOffset),
           static_cast<int8_t>(first));
Index: src/ia32/lithium-ia32.cc
===================================================================
--- src/ia32/lithium-ia32.cc    (revision 6124)
+++ src/ia32/lithium-ia32.cc    (working copy)
@@ -1678,18 +1678,8 @@


 LInstruction* LChunkBuilder::DoArrayLength(HArrayLength* instr) {
-  LOperand* array = NULL;
-  LOperand* temporary = NULL;
-
-  if (instr->value()->IsLoadElements()) {
-    array = UseRegisterAtStart(instr->value());
-  } else {
-    array = UseRegister(instr->value());
-    temporary = TempRegister();
-  }
-
-  LInstruction* result = new LArrayLength(array, temporary);
-  return AssignEnvironment(DefineAsRegister(result));
+  LOperand* array = UseRegisterAtStart(instr->value());
+  return DefineAsRegister(new LArrayLength(array));
 }


Index: src/ia32/lithium-ia32.h
===================================================================
--- src/ia32/lithium-ia32.h     (revision 6124)
+++ src/ia32/lithium-ia32.h     (working copy)
@@ -1150,16 +1150,10 @@

 class LArrayLength: public LUnaryOperation {
  public:
-  LArrayLength(LOperand* input, LOperand* temporary)
-      : LUnaryOperation(input), temporary_(temporary) { }
+  explicit LArrayLength(LOperand* input) : LUnaryOperation(input) { }

-  LOperand* temporary() const { return temporary_; }
-
   DECLARE_CONCRETE_INSTRUCTION(ArrayLength, "array-length")
   DECLARE_HYDROGEN_ACCESSOR(ArrayLength)
-
- private:
-  LOperand* temporary_;
 };




--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to