- Remove a few jumps from store barrier code.

runtime/vm/assembler_ia32.cc

Index: runtime/vm/assembler_ia32.cc
===================================================================
--- runtime/vm/assembler_ia32.cc	(revision 8953)
+++ runtime/vm/assembler_ia32.cc	(working copy)
@@ -1387,15 +1387,12 @@
 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.

 }
 
 
@@ -1404,13 +1401,15 @@
                                 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);
 }
 
 
@@ -1420,9 +1419,11 @@
   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.
 }