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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4547 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc
index 7a9bcf0ea30b97399070e4e1fc5de3e32fcb2624..c44cbbfa30e726f5a6a2ca1161f2e5ad6f590cd0 100644 (file)
--- a/src/ia32/macro-assembler-ia32.cc
+++ b/src/ia32/macro-assembler-ia32.cc
@@ -100,6 +100,7 @@ void MacroAssembler::InNewSpace(Register object,
                                 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));

diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc
index 8eee785b0ccf8dcb7b95eb4a142931210cdd9e16..e15a5fc8582c1c5a00f5b15c3fedcfd5ef82c4c2 100644 (file)
--- a/src/x64/codegen-x64.cc
+++ b/src/x64/codegen-x64.cc
@@ -6997,6 +6997,8 @@ void Reference::SetValue(InitState init_state) {
 
         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 @@ void Reference::SetValue(InitState init_state) {
                                                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

diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc
index d706fc1acd230f37e940c50a4e0b44dcb689123f..e2b54cd6ec8c9a982d45c9a84eff9bdd19325de6 100644 (file)
--- a/src/x64/macro-assembler-x64.cc
+++ b/src/x64/macro-assembler-x64.cc
@@ -167,6 +167,22 @@ void RecordWriteStub::Generate(MacroAssembler* masm) {
 }
 
 
+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].
 // object is the object being stored into, value is the object being stored.
 // If offset is zero, then the smi_index register contains the array index into
@@ -219,12 +235,7 @@ void MacroAssembler::RecordWriteNonSmi(Register object,
 
   // 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

diff --git a/src/x64/macro-assembler-x64.h b/src/x64/macro-assembler-x64.h
index 59b19fdadaa25aa38dd4c63f62ddfa3565e968d6..95cd1cfacd3c35b40f45cec23d5ffbbcbdba5822 100644 (file)
--- a/src/x64/macro-assembler-x64.h
+++ b/src/x64/macro-assembler-x64.h
@@ -66,6 +66,14 @@ class MacroAssembler: public Assembler {
   // ---------------------------------------------------------------------------
   // 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