- Remove a few jumps from store barrier code.

runtime/vm/assembler_ia32.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/assembler.h"
 #include "vm/heap.h"
 #include "vm/memory_region.h"
@@ skipping 1369 matching lines @@
       EmitOperand(7, Operand(reg));
       buffer_.EmitObject(object);
     }
   }
 }
 
 
 void Assembler::StoreIntoObjectFilter(Register object,
                                       Register value,
                                       Label* no_update) {
-  // Check that 'value' is a new object.  Store buffer updates are not
-  // required when storing a smi or an old object.
-  testl(value, Immediate(kNewObjectAlignmentOffset | kHeapObjectTag));
-  j(PARITY_ODD, no_update, Assembler::kNearJump);
-  j(ZERO, no_update, Assembler::kNearJump);
-  // Check that 'object' is an old object.  A store buffer update is
-  // not required when storing into a new object.
-  testl(object, Immediate(kNewObjectAlignmentOffset));
-  j(NOT_ZERO, no_update, Assembler::kNearJump);
+  andl(value, Immediate(0x5));

[kasperl, 2012/06/21 05:05:47]

I think you should add a comment here explaning that this is teh awesome. 

[Ivan Posva, 2012/06/22 06:56:22]

Thanks! Half of the credit goes to Slava...

I will also add a comment here explaining how it works and what properties need
to be preserved. This is tricky code because throwing in Smis does end up with
all kinds of weird unexpected bit patterns.

+  shrl(value, Immediate(1));
+  adcl(value, object);
+  andl(value, Immediate(0x7));
+  cmpl(value, Immediate(0x4));
+  j(NOT_ZERO, no_update);

[sra1, 2012/06/22 05:17:44]

and value,7
lea value,[object*8 + value]
and value,0b111111
cmp value,0b001101
jnz no_update

might not be faster if there is extra cycles on the alu -> address generator ->
alu path.

Is reducing the number of branches enough of a win to cover the extra push/pop
execution time and the ~5 bytes extra code (counting the push/pop)?
Branch predictors often ignore the low k bits, in effect predicting adjacent
and/or nearby branches together.

[Ivan Posva, 2012/06/22 06:56:22]

Thanks for the suggestion to use lea to separate the bits, it is similar to a
suggestion we received from Lasse. I will measure the impact once we have
convinced ourselves that this approach is valid.

 }
 
 
 void Assembler::StoreIntoObject(Register object,
                                 const FieldAddress& dest,
                                 Register value) {
   movl(dest, value);
   Label done;
+  pushl(value);
   StoreIntoObjectFilter(object, value, &done);
   // A store buffer update is required.
-  pushl(EAX);  // Preserve EAX.
+  if (value != EAX) pushl(EAX);  // Preserve EAX.
   leal(EAX, dest);
   call(&StubCode::UpdateStoreBufferLabel());
-  popl(EAX);  // Restore EAX.
+  if (value != EAX) popl(EAX);  // Restore EAX.
   Bind(&done);
+  popl(value);
 }
 
 
 void Assembler::StoreIntoObjectNoBarrier(Register object,
                                          const FieldAddress& dest,
                                          Register value) {
   movl(dest, value);
 #if defined(DEBUG)
   Label done;
+  pushl(value);
   StoreIntoObjectFilter(object, value, &done);
   Stop("Store buffer update is required");
   Bind(&done);
+  popl(value);
 #endif  // defined(DEBUG)
   // No store buffer update.
 }
 
 
 void Assembler::StoreIntoObjectNoBarrier(Register object,
                                          const FieldAddress& dest,
                                          const Object& value) {
   if (value.IsSmi()) {
     movl(dest, Immediate(reinterpret_cast<int32_t>(value.raw())));
@@ skipping 294 matching lines @@
     comments.SetCommentAt(i, comments_[i]->comment());
   }
 
   return comments;
 }
 
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_IA32