Revision: 4547
Author: [email protected]
Date: Thu Apr 29 05:52:09 2010
Log: Improve the success rate for inline keyed store on x64

Added a simple new space check on the elements fixed array which can allow
updating with other values than smis without updating the remembered set.

Also combined the positive smi and range check so that a separate smi check can be avoided when the key is known to be a smi.

This is a port of r4543.
Review URL: http://codereview.chromium.org/1702013
http://code.google.com/p/v8/source/detail?r=4547

Modified:
 /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc
 /branches/bleeding_edge/src/x64/codegen-x64.cc
 /branches/bleeding_edge/src/x64/macro-assembler-x64.cc
 /branches/bleeding_edge/src/x64/macro-assembler-x64.h

=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Thu Apr 15 07:43:32 2010 +++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Thu Apr 29 05:52:09 2010
@@ -100,6 +100,7 @@
                                 Register scratch,
                                 Condition cc,
                                 Label* branch) {
+  ASSERT(cc == equal || cc == not_equal);
   if (Serializer::enabled()) {
// Can't do arithmetic on external references if it might get serialized.
     mov(scratch, Operand(object));
=======================================
--- /branches/bleeding_edge/src/x64/codegen-x64.cc      Thu Apr 29 04:44:17 2010
+++ /branches/bleeding_edge/src/x64/codegen-x64.cc      Thu Apr 29 05:52:09 2010
@@ -6997,6 +6997,8 @@

         Result tmp = cgen_->allocator_->Allocate();
         ASSERT(tmp.is_valid());
+        Result tmp2 = cgen_->allocator_->Allocate();
+        ASSERT(tmp2.is_valid());

         // Determine whether the value is a constant before putting it
         // in a register.
@@ -7012,32 +7014,44 @@
                                                key.reg(),
                                                receiver.reg());

-        // Check that the value is a smi if it is not a constant.
-        // We can skip the write barrier for smis and constants.
-        if (!value_is_constant) {
-          __ JumpIfNotSmi(value.reg(), deferred->entry_label());
-        }
-
-        // Check that the key is a non-negative smi.
-        __ JumpIfNotPositiveSmi(key.reg(), deferred->entry_label());
-
         // Check that the receiver is not a smi.
         __ JumpIfSmi(receiver.reg(), deferred->entry_label());
+
+        // Check that the key is a smi.
+        if (!key.is_smi()) {
+          __ JumpIfNotSmi(key.reg(), deferred->entry_label());
+        } else {
+          if (FLAG_debug_code) {
+ __ AbortIfNotSmi(key.reg(), "Non-smi value in smi-typed value.");
+          }
+        }

         // Check that the receiver is a JSArray.
         __ CmpObjectType(receiver.reg(), JS_ARRAY_TYPE, kScratchRegister);
         deferred->Branch(not_equal);

         // Check that the key is within bounds.  Both the key and the
-        // length of the JSArray are smis.
+ // length of the JSArray are smis. Use unsigned comparison to handle
+        // negative keys.
         __ SmiCompare(FieldOperand(receiver.reg(), JSArray::kLengthOffset),
                       key.reg());
-        deferred->Branch(less_equal);
+        deferred->Branch(below_equal);

         // Get the elements array from the receiver and check that it
         // is a flat array (not a dictionary).
         __ movq(tmp.reg(),
                 FieldOperand(receiver.reg(), JSObject::kElementsOffset));
+
+        // Check whether it is possible to omit the write barrier. If the
+ // elements array is in new space or the value written is a smi we can + // safely update the elements array without updating the remembered set.
+        Label in_new_space;
+        __ InNewSpace(tmp.reg(), tmp2.reg(), equal, &in_new_space);
+        if (!value_is_constant) {
+          __ JumpIfNotSmi(value.reg(), deferred->entry_label());
+        }
+
+        __ bind(&in_new_space);
         // Bind the deferred code patch site to be able to locate the
         // fixed array map comparison.  When debugging, we patch this
         // comparison to always fail so that we will hit the IC call
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Wed Apr 28 07:06:35 2010 +++ /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Thu Apr 29 05:52:09 2010
@@ -165,6 +165,22 @@
   RecordWriteHelper(masm, object_, addr_, scratch_);
   masm->ret(0);
 }
+
+
+void MacroAssembler::InNewSpace(Register object,
+                                Register scratch,
+                                Condition cc,
+                                Label* branch) {
+  ASSERT(cc == equal || cc == not_equal);
+  if (!scratch.is(object)) {
+    movq(scratch, object);
+  }
+  ASSERT(is_int32(static_cast<int64_t>(Heap::NewSpaceMask())));
+  and_(scratch, Immediate(static_cast<int32_t>(Heap::NewSpaceMask())));
+  movq(kScratchRegister, ExternalReference::new_space_start());
+  cmpq(scratch, kScratchRegister);
+  j(cc, branch);
+}


 // Set the remembered set bit for [object+offset].
@@ -219,12 +235,7 @@

   // Test that the object address is not in the new space.  We cannot
   // set remembered set bits in the new space.
-  movq(scratch, object);
-  ASSERT(is_int32(static_cast<int64_t>(Heap::NewSpaceMask())));
-  and_(scratch, Immediate(static_cast<int32_t>(Heap::NewSpaceMask())));
-  movq(kScratchRegister, ExternalReference::new_space_start());
-  cmpq(scratch, kScratchRegister);
-  j(equal, &done);
+  InNewSpace(object, scratch, equal, &done);

   // The offset is relative to a tagged or untagged HeapObject pointer,
   // so either offset or offset + kHeapObjectTag must be a
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.h Wed Apr 28 07:06:35 2010 +++ /branches/bleeding_edge/src/x64/macro-assembler-x64.h Thu Apr 29 05:52:09 2010
@@ -66,6 +66,14 @@
// ---------------------------------------------------------------------------
   // GC Support

+  // Check if object is in new space. The condition cc can be equal or
+  // not_equal. If it is equal a jump will be done if the object is on new
+ // space. The register scratch can be object itself, but it will be clobbered.
+  void InNewSpace(Register object,
+                  Register scratch,
+                  Condition cc,
+                  Label* branch);
+
   // Set the remembered set bit for [object+offset].
// object is the object being stored into, value is the object being stored. // If offset is zero, then the scratch register contains the array index into

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

Reply via email to